[dduugg]

Summary

validate <files>, gv <files> and for-file now normalize each path they're given before matching it. ./app/x.rb, app//x.rb, app/./x.rb, app/../app/x.rb and an absolute path through a symlink to the project (macOS's /tmp) all get the answer their plain form gets. Results and messages still show each path as given.

Fixes #145.

Why

Paths were matched as typed. The only cleanup was stripping the project root from an absolute path, and the CLI canonicalizes the root. So, with the issue's project:

• With owned_globs: ["**/*.rb"], validate ./app/valid.rb (and //, /./, .., and the /tmp absolute path) reported an owned file as unowned. for-file --from-codeowners ./app/valid.rb gave Unowned.
• With {app,config}/**/*.rb, validate ./app/unowned.rb and the /tmp absolute path skipped a file with no owner, so it passed. for-file <tmp abs>/app/valid.rb ignored the annotation.
• With optionally_owned_globs, .. and symlinked paths were reported as unowned where the plain path was excused.

Changes

• path_utils::project_relative_path finds the project-relative path that a given path names, or None.
  • It joins the path onto the absolute root and cleans it lexically, as cli.rs does for --config-path. Most paths stop here, with no file system access.
  • Otherwise it resolves the root, since a library caller's RunConfig may give it unresolved. Then it resolves the path's ancestors, shortest first, until one is in the project, and keeps the rest of the path as written. Nothing inside the project is resolved, so:
    • A symlink inside the project keeps its own path, as CODEOWNERS and the project walk name it.
    • A deleted file, or one in a missing or unreadable directory, is still checked rather than skipped.
    • Every spelling of a path names what its plain form does.
  • The file itself is never resolved.
• Runner::validate_files uses it for the owned/unowned glob filter and the optionally_owned_globs check. That replaces Add optionally_owned_globs to permit files with no owner #135's plain_relative and its second CODEOWNERS lookup.
• codeowners_query::teams_for_files_from_codeowners looks up each path by its normalized form. That covers for-file --from-codeowners, validate <files>, and the gem's CodeOwnership.for_file and teams_for_files.
• find_file_owners (for-file and runner::file_owner_for_file) uses it too.
• A path that names nothing under the root is matched as on main: as given, with the root stripped if it's absolute. Paths outside the project behave exactly as before, including for for-file, which the gem uses for files in other gems.
• The public runner::teams_for_files_from_codeowners is now keyed by each path exactly as given.
  • Before, an absolute path came back under its stripped relative form, though the gem stores result[file_path] under the path it passed.
  • Inputs that name the same file stay separate entries.
  • Parser::teams_from_files_paths (crate-private) now returns teams in input order, and the query zips them with the given strings.
• README: validate and gv list [files...], and one line says paths may be relative to the project root or absolute.

Relation to #126

#126 attempts the same fix and is open, but it conflicts with main and hasn't moved since its review. This is a fresh implementation that avoids the problems that review found. Keys stay the caller's strings. An IO error while resolving steps up to the next ancestor rather than dropping the path. Nonexistent paths are still checked. ../../a can't be read as an in-project a. If this lands, #126 can probably be closed.

Test plan

• cargo fmt --all -- --check, cargo clippy --all-targets --all-features -- -D warnings and cargo test pass (273 passed, the 3 existing ignores).
• New tests/path_normalization_test.rs (13 tests, temp repos built with the tests/common helpers) covers:
  • both of the issue's tables, for validate, gv and for-file, including --from-codeowners
  • unowned_globs with ./ and //
  • a path outside the project, relative and absolute, or leaving through .., still reported
  • .. after a symlinked directory, resolved lexically
  • for-file on an annotated file outside the project
  • teams_for_files_from_codeowners keyed exactly by x, ./x, the absolute path and a symlinked absolute path
• project_relative_path unit tests cover:
  • ./, //, . and .. inside and outside the project, including ../../
  • relative roots and the root itself
  • a symlinked ancestor of the root, and the reverse case of an unresolved root
  • a symlinked file, and symlinked directories inside the project
  • a self-referential symlink (ELOOP), a missing directory and a deleted file
• optionally_owned_globs_test.rs: the .. test now expects the optionally owned heading, listing the path as given. New tests cover a symlinked path and a path outside the project, which is never excused.
• Symlink tests are #[cfg(unix)] and make their own links in temp dirs rather than relying on macOS's /tmp.
• Mutation-checked: reverting each piece fails at least one test. The pieces are the helper's use in each caller, the keyed-as-given map, each step of the symlink walk, and the outside-path fallbacks.
• Manually reran the issue's three tables with main's binary (reproduces) and this branch's (fixed).
• Compared validate, crosscheck-owners, generate and per-file for-file across every fixture against main. Nothing changed for plain or canonical absolute paths.

Follow-ups (not in this PR)

• The code_ownership gem's TeamFinder.for_file returns nil for any path starting with ./ when from_codeowners: false. That's in the gem.
• ** matches .., so a path that leaves the project with .. can still match owned_globs and CODEOWNERS globs, as on main.
• for-file . and for-file <root> pick up a .codeowner from above the root (pre-existing).
• validate_files still does one CODEOWNERS lookup per file. Batching them is a separate perf change.