diff --git a/CHANGELOG.md b/CHANGELOG.md index 02ca706..f00df4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,15 @@ While the major version is 0, minor version bumps may contain breaking changes. ### Fixed +- `W_INDEX_ORPHAN` no longer fires for files the root index does reference. A + link destination ended at the last `)` on the line, so an entry annotated + with a parenthetical — `- [Alpha](tasks/alpha.md) (superseded)` — recorded a + path matching no file, and only the first link on a line was read at all, so + two links on one line reported both files as orphans. Destinations now end at + the parenthesis that closes their own link, parentheses inside a path are + balanced, angle-bracketed and titled destinations are understood, and every + link on a line is collected (#60). + - `.lashignore` is reachable from where users hit it. The `W_INDEX_ORPHAN` warning names it in the message text, `lash lint --help` and `lash --help` describe file discovery, and the README, user guide and error-code reference diff --git a/crates/lash-cli/tests/lint_output_tests.rs b/crates/lash-cli/tests/lint_output_tests.rs index 7a48f27..51108b2 100644 --- a/crates/lash-cli/tests/lint_output_tests.rs +++ b/crates/lash-cli/tests/lint_output_tests.rs @@ -1673,3 +1673,98 @@ fn test_mut000491_494_exact_severity_counts_warning_file() { "hints must be 0 (mut-000494); summary={summary}" ); } + +// --------------------------------------------------------------------------- +// W_INDEX_ORPHAN: index entries annotated with parentheses (GitHub issue #60) +// +// The index link parser used to end a destination at the last `)` on the line, +// so `- [Alpha](tasks/alpha.md) (note)` recorded a path no file matched and the +// linked file was reported as an orphan. Two links on one line broke both. +// --------------------------------------------------------------------------- + +/// Run `lash --json lint ` and return the codes of every diagnostic. +fn lint_json_codes(path: &std::path::Path) -> Vec { + let output = lash() + .arg("--json") + .arg("lint") + .arg(path) + .output() + .expect("lash must run"); + let stdout = String::from_utf8_lossy(&output.stdout); + let json: serde_json::Value = serde_json::from_str(&stdout).unwrap_or_else(|e| { + panic!("stdout was not valid JSON: {e}\nstdout={stdout}"); + }); + json["diagnostics"] + .as_array() + .map(|diags| { + diags + .iter() + .filter_map(|d| d["code"].as_str().map(str::to_string)) + .collect() + }) + .unwrap_or_default() +} + +#[test] +fn test_annotated_index_links_are_not_orphans() { + let td = TempDir::new().unwrap(); + fs::create_dir(td.path().join("tasks")).unwrap(); + + write_md( + &td, + "lash.index.md", + "# Demo Project\n\ + \n\ + @id: index\n\ + \n\ + ## Epic Task Files\n\ + \n\ + - [Alpha](tasks/alpha.md) (trailing parenthetical)\n\ + - [Beta](tasks/beta.md) — trailing em dash\n\ + - [Gamma](tasks/gamma.md) plain trailing words\n\ + - [Delta](tasks/delta.md) mentions (a parenthetical) mid-sentence\n\ + - [Epsilon](tasks/epsilon.md)(immediately adjacent)\n\ + - [Zeta](tasks/zeta.md) and [Eta](tasks/eta.md) on one line\n", + ); + + for name in ["alpha", "beta", "gamma", "delta", "epsilon", "zeta", "eta"] { + write_md( + &td, + &format!("tasks/{name}.md"), + &format!("# {name}\n\n@id: {name}\n@created: 2024-01-15\n\n## Tasks\n\n- [ ] A task\n"), + ); + } + + let codes = lint_json_codes(td.path()); + assert!( + !codes.iter().any(|code| code == "W_INDEX_ORPHAN"), + "every task file is linked from the index; got diagnostics: {codes:?}" + ); +} + +#[test] +fn test_unreferenced_file_is_still_an_orphan() { + let td = TempDir::new().unwrap(); + fs::create_dir(td.path().join("tasks")).unwrap(); + + write_md( + &td, + "lash.index.md", + "# Demo Project\n\n@id: index\n\n## Epic Task Files\n\n\ + - [Alpha](tasks/alpha.md) (trailing parenthetical)\n", + ); + for name in ["alpha", "stray"] { + write_md( + &td, + &format!("tasks/{name}.md"), + &format!("# {name}\n\n@id: {name}\n@created: 2024-01-15\n\n## Tasks\n\n- [ ] A task\n"), + ); + } + + let codes = lint_json_codes(td.path()); + assert_eq!( + codes.iter().filter(|c| *c == "W_INDEX_ORPHAN").count(), + 1, + "only tasks/stray.md is unreferenced; got diagnostics: {codes:?}" + ); +} diff --git a/crates/lash-core/src/display.rs b/crates/lash-core/src/display.rs index 7f13991..5fa80f6 100644 --- a/crates/lash-core/src/display.rs +++ b/crates/lash-core/src/display.rs @@ -21,23 +21,83 @@ use std::path::Path; /// ``` #[must_use] pub fn extract_link_path(text: &str) -> Option { - // Try to match [link text](path) pattern - let open_bracket = text.find('[')?; - let close_bracket = text.find("](")?; + extract_link_paths(text).into_iter().next() +} + +/// Extract every Markdown link destination on a line, in order +/// +/// Unlike [`extract_link_path`], this collects all links rather than just the +/// first, and each destination ends at the parenthesis that closes *its* link +/// rather than at the last parenthesis on the line. Parentheses nested inside a +/// destination are balanced; angle-bracketed destinations (`[a]()`) are +/// unwrapped. +/// +/// # Examples +/// +/// ``` +/// use lash_core::display::extract_link_paths; +/// +/// assert_eq!( +/// extract_link_paths("[A](a.md) and [B](b.md) (see also)"), +/// vec!["a.md".to_string(), "b.md".to_string()] +/// ); +/// assert_eq!(extract_link_paths("Plain text"), Vec::::new()); +/// ``` +#[must_use] +pub fn extract_link_paths(text: &str) -> Vec { + let mut paths = Vec::new(); + let mut cursor = 0; - if open_bracket >= close_bracket { - return None; + while let Some(offset) = text[cursor..].find("](") { + let close_bracket = cursor + offset; + let dest_start = close_bracket + 2; // Skip "](" + + // A destination only counts as a link if some `[` opens it. + if !text[cursor..close_bracket].contains('[') { + cursor = dest_start; + continue; + } + + let Some(dest_end) = find_dest_end(text, dest_start) else { + break; + }; + + let dest = text[dest_start..dest_end].trim(); + let dest = dest + .strip_prefix('<') + .and_then(|inner| inner.strip_suffix('>')) + .unwrap_or(dest); + if !dest.is_empty() { + paths.push(dest.to_string()); + } + + cursor = dest_end + 1; // Skip the closing ')' } - let path_start = close_bracket + 2; // Skip "](" - let close_paren = text[path_start..].find(')')?; - let path = &text[path_start..path_start + close_paren]; + paths +} + +/// Find the parenthesis that closes a link destination starting at `start` +/// +/// Parentheses inside the destination are balanced, so `[a](f(1).md)` ends at +/// the final `)` rather than the one after `1`. +fn find_dest_end(text: &str, start: usize) -> Option { + let mut depth = 0usize; - if path.is_empty() { - None - } else { - Some(path.to_string()) + for (offset, ch) in text[start..].char_indices() { + match ch { + '(' => depth += 1, + ')' => { + if depth == 0 { + return Some(start + offset); + } + depth -= 1; + } + _ => {} + } } + + None } /// Extract link text from Markdown links @@ -225,6 +285,54 @@ mod tests { assert_eq!(extract_link_text("`path/file.md`"), "`path/file.md`"); } + #[test] + fn test_extract_link_paths() { + // Every link on the line is collected, in order + assert_eq!( + extract_link_paths("[A](a.md) and [B](b.md)"), + vec!["a.md".to_string(), "b.md".to_string()] + ); + + // A later ')' does not extend the destination (GitHub issue #60) + assert_eq!( + extract_link_paths("[A](a.md) (see also)"), + vec!["a.md".to_string()] + ); + + // Parentheses inside the destination are balanced + assert_eq!( + extract_link_paths("[Copy](a(1).md)"), + vec!["a(1).md".to_string()] + ); + + // Angle brackets are unwrapped + assert_eq!( + extract_link_paths("[A]()"), + vec!["my file.md".to_string()] + ); + + // Empty destinations and non-links are skipped + assert!(extract_link_paths("[A]()").is_empty()); + assert!(extract_link_paths("Plain text").is_empty()); + assert!(extract_link_paths("not a link](a.md)").is_empty()); + + // Unterminated destination + assert!(extract_link_paths("[A](a.md").is_empty()); + } + + #[test] + fn test_extract_link_path_first_link_only() { + assert_eq!( + extract_link_path("[A](a.md) and [B](b.md)"), + Some("a.md".to_string()) + ); + assert_eq!( + extract_link_path("[A](a.md) (see also)"), + Some("a.md".to_string()) + ); + assert_eq!(extract_link_path("Plain text"), None); + } + #[test] fn test_format_index_annotations() { // Full annotation: strip @id and convert @labels to hashtags diff --git a/crates/lash-core/src/linter/rules/crossfile/orphaned_files.rs b/crates/lash-core/src/linter/rules/crossfile/orphaned_files.rs index ae16552..d32cde7 100644 --- a/crates/lash-core/src/linter/rules/crossfile/orphaned_files.rs +++ b/crates/lash-core/src/linter/rules/crossfile/orphaned_files.rs @@ -118,18 +118,18 @@ impl OrphanedFilesRule { for task in file.tasks.tasks() { let title = task.title.trim(); - // Try to extract path from markdown link [text](path) - if let Some(path) = Self::extract_markdown_link_path(title) { - references.push(path); + // Try to extract paths from markdown links [text](path) + let linked = Self::extract_markdown_link_paths(title); + if !linked.is_empty() { + for path in linked { + Self::push_reference(&mut references, path); + } continue; } // Fall back to checking if the title itself is a path - if std::path::Path::new(title) - .extension() - .is_some_and(|ext| ext.eq_ignore_ascii_case("md")) - { - references.push(title.to_string()); + if Self::is_markdown_path(title) { + Self::push_reference(&mut references, title.to_string()); } } @@ -145,11 +145,9 @@ impl OrphanedFilesRule { { continue; } - // Check for markdown link on this line - if let Some(path) = Self::extract_markdown_link_path(line) { - if !references.contains(&path) { - references.push(path); - } + // Check for markdown links on this line + for path in Self::extract_markdown_link_paths(line) { + Self::push_reference(&mut references, path); } } } @@ -157,26 +155,50 @@ impl OrphanedFilesRule { references } - /// Extract path from a markdown link format: `\[text\](path)` + /// Record a reference, skipping ones already collected + fn push_reference(references: &mut Vec, path: String) { + if !references.contains(&path) { + references.push(path); + } + } + + /// Extract every task-file path linked from a line of the index /// - /// Returns Some(path) if found, None otherwise - fn extract_markdown_link_path(text: &str) -> Option { - // Look for pattern [...](...) - let open_paren = text.find("](")?; - let close_paren = text.rfind(')')?; - - if open_paren + 2 < close_paren { - let path = &text[open_paren + 2..close_paren]; - // Accept paths ending in .md or directories ending in / - let is_md = std::path::Path::new(path) - .extension() - .is_some_and(|ext| ext.eq_ignore_ascii_case("md")); - if is_md || path.ends_with('/') { - return Some(path.to_string()); - } + /// A line may carry more than one link, and a link may be followed by a + /// parenthetical, so each destination is delimited by the parenthesis that + /// closes its own link (GitHub issue #60). Destinations that do not name a + /// Markdown file or a directory are ignored. + fn extract_markdown_link_paths(text: &str) -> Vec { + crate::display::extract_link_paths(text) + .iter() + .filter_map(|dest| Self::link_dest_as_file_reference(dest)) + .collect() + } + + /// Narrow a link destination to a file reference, if it is one + /// + /// A destination may carry a link title (`path.md "Title"`), so when the + /// destination as a whole does not name a file, its first token is tried. + /// Bare paths containing spaces still resolve, since they are checked first. + fn link_dest_as_file_reference(dest: &str) -> Option { + if Self::is_file_reference(dest) { + return Some(dest.to_string()); } - None + let first_token = dest.split_whitespace().next()?; + Self::is_file_reference(first_token).then(|| first_token.to_string()) + } + + /// Check whether a link destination names a Markdown file or a directory + fn is_file_reference(path: &str) -> bool { + Self::is_markdown_path(path) || path.ends_with('/') + } + + /// Check whether a path names a Markdown file + fn is_markdown_path(path: &str) -> bool { + Path::new(path) + .extension() + .is_some_and(|ext| ext.eq_ignore_ascii_case("md")) } /// Find the root index file in the context @@ -589,38 +611,137 @@ mod tests { } #[test] - fn test_extract_markdown_link_path() { + fn test_extract_markdown_link_paths() { // Standard markdown link assert_eq!( - OrphanedFilesRule::extract_markdown_link_path("[Title](path/to/file.md)"), - Some("path/to/file.md".to_string()) + OrphanedFilesRule::extract_markdown_link_paths("[Title](path/to/file.md)"), + vec!["path/to/file.md".to_string()] ); // Link with annotations after assert_eq!( - OrphanedFilesRule::extract_markdown_link_path( + OrphanedFilesRule::extract_markdown_link_paths( "[Physics](systems/physics.md) @id:`systems.physics`" ), - Some("systems/physics.md".to_string()) + vec!["systems/physics.md".to_string()] ); // Directory reference assert_eq!( - OrphanedFilesRule::extract_markdown_link_path("[World 1](worlds/forest/)"), - Some("worlds/forest/".to_string()) + OrphanedFilesRule::extract_markdown_link_paths("[World 1](worlds/forest/)"), + vec!["worlds/forest/".to_string()] ); // Plain text (no link) + assert!(OrphanedFilesRule::extract_markdown_link_paths("Just plain text").is_empty()); + + // Plain path (no markdown link format) + assert!(OrphanedFilesRule::extract_markdown_link_paths("path/to/file.md").is_empty()); + } + + // GitHub issue #60: the destination used to end at the last ')' on the + // line, so a trailing parenthetical swallowed the rest of the line into + // the path and the linked file was reported as an orphan. + #[test] + fn test_extract_link_paths_ignores_trailing_parentheses() { + for line in [ + "- [Alpha](tasks/alpha.md) (trailing parenthetical)", + "- [Alpha](tasks/alpha.md) mentions (a parenthetical) mid-sentence", + "- [Alpha](tasks/alpha.md)(immediately adjacent)", + ] { + assert_eq!( + OrphanedFilesRule::extract_markdown_link_paths(line), + vec!["tasks/alpha.md".to_string()], + "failed for line: {line}" + ); + } + } + + // GitHub issue #60: only the first link on a line was considered, and the + // over-long path it produced still ended in `.md`, so both files linked on + // a shared line were reported as orphans. + #[test] + fn test_extract_link_paths_collects_every_link_on_a_line() { assert_eq!( - OrphanedFilesRule::extract_markdown_link_path("Just plain text"), - None + OrphanedFilesRule::extract_markdown_link_paths( + "- [Alpha](tasks/alpha.md) and [Beta](tasks/beta.md) on one line" + ), + vec!["tasks/alpha.md".to_string(), "tasks/beta.md".to_string()] ); + } - // Plain path (no markdown link format) + #[test] + fn test_extract_link_paths_handles_titles_and_nesting() { + // CommonMark link title after the destination assert_eq!( - OrphanedFilesRule::extract_markdown_link_path("path/to/file.md"), - None + OrphanedFilesRule::extract_markdown_link_paths("[Alpha](tasks/alpha.md \"Alpha\")"), + vec!["tasks/alpha.md".to_string()] ); + + // Parentheses inside the path are balanced + assert_eq!( + OrphanedFilesRule::extract_markdown_link_paths("[Copy](tasks/alpha(1).md)"), + vec!["tasks/alpha(1).md".to_string()] + ); + + // Angle-bracketed destination with a space in the path + assert_eq!( + OrphanedFilesRule::extract_markdown_link_paths("[Alpha]()"), + vec!["tasks/my alpha.md".to_string()] + ); + + // Bare destination with a space still resolves + assert_eq!( + OrphanedFilesRule::extract_markdown_link_paths("[Alpha](tasks/my alpha.md)"), + vec!["tasks/my alpha.md".to_string()] + ); + } + + // GitHub issue #60, end to end: files linked from index lines that carry + // parentheticals or share a line are referenced, not orphaned. + #[test] + fn test_annotated_index_entries_are_not_orphans() { + let rule = OrphanedFilesRule::new(); + let config = LashConfig::default(); + + let mut files = HashMap::new(); + files.insert( + PathBuf::from("lash.index.md"), + make_index_file( + "lash.index.md", + &[ + "[Alpha](tasks/alpha.md) (trailing parenthetical)", + "[Delta](tasks/delta.md) mentions (a parenthetical) mid-sentence", + "[Epsilon](tasks/epsilon.md)(immediately adjacent)", + "[Zeta](tasks/zeta.md) and [Eta](tasks/eta.md) on one line", + ], + ), + ); + + for name in ["alpha", "delta", "epsilon", "zeta", "eta"] { + let path = format!("tasks/{name}.md"); + files.insert(PathBuf::from(&path), make_regular_file(&path, name)); + } + files.insert( + PathBuf::from("tasks/orphan.md"), + make_regular_file("tasks/orphan.md", "orphan"), + ); + + for name in ["alpha", "delta", "epsilon", "zeta", "eta"] { + let path = PathBuf::from(format!("tasks/{name}.md")); + let ctx = LintContext::new(&config, path.clone(), &files); + let diagnostics = rule.check_file(files.get(&path).unwrap(), &ctx); + assert_eq!( + diagnostics.len(), + 0, + "{name} is linked from the index but was flagged: {diagnostics:?}" + ); + } + + // A genuinely unreferenced file is still flagged + let path = PathBuf::from("tasks/orphan.md"); + let ctx = LintContext::new(&config, path.clone(), &files); + assert_eq!(rule.check_file(files.get(&path).unwrap(), &ctx).len(), 1); } #[test] diff --git a/devlog.md b/devlog.md index bc91650..cd2c6a8 100644 --- a/devlog.md +++ b/devlog.md @@ -2777,3 +2777,41 @@ runtime, creation), which is also the grouping `--list` prints. Finally, the lint summary now closes the loop it opens: it names one of the codes it just reported and the `lash explain` invocation for it. + +## A trailing parenthetical hid an index entry (#60, 2026-08-13) + +`W_INDEX_ORPHAN` reported files the index does reference. Annotating an entry +was enough to trigger it: + +```markdown +- [Alpha](tasks/alpha.md) (historical, superseded) +``` + +`extract_markdown_link_path` took the destination from the first `](` to +`rfind(')')` — the last parenthesis on the line, not the one closing the link. +For that entry the path became `tasks/alpha.md) (historical, superseded`, which +matches nothing on disk. The `.md` extension guard below usually dropped the +garbage silently, so the reference simply went missing and the file looked +orphaned. + +The two-link case was worse: `[Alpha](a.md) and [Beta](b.md)` yields +`a.md) and [Beta](b.md`, whose last component still ends in `.md`. The guard +passed, a nonsense path was recorded as a legitimate reference, and both real +files were reported as orphans. `find` for the opening delimiter also meant +only the first link on a line was ever considered. + +Destinations now end at the parenthesis that closes their own link, with nested +parentheses balanced, and the scan continues along the line so every link is +collected. The scanner lives in `display::extract_link_paths`, since +`display::extract_link_path` was already doing the forward scan for a single +link and now delegates to it; the orphan rule keeps only the destinations that +name a Markdown file or a directory. Angle-bracketed destinations are unwrapped +and a CommonMark link title is stripped, but only as a fallback — a bare path +containing spaces is tried whole first, so `[A](tasks/my file.md)` still +resolves. + +What made this expensive to diagnose is that the diagnostic names the orphaned +file, not the index line that failed to parse, so the obvious repair is the one +thing already done. Worth remembering the next time a cross-file rule reports +an absence: the report points at the symptom, and the parse that produced it is +never on screen.