Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- Hidden files and directories are scanned by default. Being hidden is no longer a reason to skip anything: the ignore files and the `exclude` config key decide alone, which is what "everything git would track" already claimed. A tag in `.github/workflows` (a pinned action version, a step commented out until a fix lands) was silently never checked, which is the exact failure this tool exists to prevent. Repositories that keep tags in dotfiles will see findings they did not see before, and `exclude` is how to quiet a large dotted directory that `.gitignore` does not cover.
- `--hidden` asks for what now always happens, so it does nothing. It is still accepted, and still silent, so a CI job already passing it keeps working; it no longer appears in `--help`.

### Fixed

- `.git` is never walked, with or without `--hidden`. It accounted for 97% of the files a `--hidden` scan read, and worse, `.git/COMMIT_EDITMSG` and `.git/logs` hold commit messages: a commit that merely discussed a tag read to the scanner exactly like the tag itself. The directory is matched by name, so the `.git` of a submodule or a nested checkout goes too, as does the `.git` file a worktree gets in place of a directory. Naming `.git` as a path argument does not reach it either, nor does running from inside it.
Comment thread
alies-dev marked this conversation as resolved.
Outdated

## [0.4.0] - 2026-08-26

### Added
Expand Down
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ todo-by --current-version 2.1.0 # override the project's current version, for ve
todo-by --warn 14 # also report tags due within 14 days, as warnings
todo-by --exit-zero # always exit 0 on findings (still 2 on errors)
todo-by --color always # auto, always, never (default: auto)
todo-by --hidden # also scan hidden files and directories
todo-by --files # list files that would be scanned, then exit
todo-by --dump-config # print effective config, then exit
```
Expand Down Expand Up @@ -148,7 +147,7 @@ Full workflow, checksum pinning, and how to phase it in on a codebase that alrea

## What gets scanned

Everything git would track. `todo-by` uses ripgrep's directory walker, so `.gitignore` is honored with full git semantics (nested files, negation, `**` globs, `.git/info/exclude`), even outside a repository. Hidden, binary and symlinked files are skipped; `--hidden` includes hidden ones. A file named on the command line is always scanned.
Everything git would track. `todo-by` uses ripgrep's directory walker, so `.gitignore` is honored with full git semantics (nested files, negation, `**` globs, `.git/info/exclude`), even outside a repository. Dotfiles and dotted directories are scanned like any other, `.github/workflows` among them, because git tracks them. `.git` is never walked, at no depth and not when named directly. Binary and symlinked files are skipped. Any other file named on the command line is always scanned, and the `exclude` config key covers whatever `.gitignore` does not.

## Configuration

Expand Down
263 changes: 229 additions & 34 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ Options:
--offline Never check issue triggers, overriding config
--exit-zero Always exit 0 on findings (still 2 on errors)
--color <WHEN> Color: auto, always, never [default: auto]
--hidden Also scan hidden files and directories
--files List files that would be scanned, then exit
--dump-config Print effective config, then exit
-h, --help Print help
Expand Down Expand Up @@ -74,7 +73,6 @@ struct Cli {
online: Option<bool>,
exit_zero: bool,
color: ColorWhen,
hidden: bool,
files: bool,
dump_config: bool,
}
Expand All @@ -89,7 +87,6 @@ fn parse_args(args: impl Iterator<Item = String>) -> Result<Cli, String> {
online: None,
exit_zero: false,
color: ColorWhen::Auto,
hidden: false,
files: false,
dump_config: false,
};
Expand Down Expand Up @@ -157,7 +154,15 @@ fn parse_args(args: impl Iterator<Item = String>) -> Result<Cli, String> {
}
}
}
"--hidden" => cli.hidden = true,
// Hidden files are scanned unconditionally now, so this flag
// asks for what already happens. It stays accepted, and stays
// silent, because the CLI surface is frozen after a release and
// a CI job passing it must neither fail nor start printing on
// every run. It is out of --help: there is nothing to choose.
// todo-by v1.0 delete this arm and its entry in VALUELESS.
// A major bump is the point at which dropping an accepted flag
// stops being a breaking change made by surprise.
"--hidden" => {}
"--files" => cli.files = true,
"--dump-config" => cli.dump_config = true,
"-h" | "--help" => {
Expand Down Expand Up @@ -563,28 +568,61 @@ fn build_overrides(root: &Path, patterns: &[String]) -> Result<Option<Override>,
.map_err(|err| format!("invalid exclude patterns: {err}"))
}

/// True when `path` is a `.git` directory or sits inside one. Resolved
/// first, so that running from within `.git` is caught as well as naming
/// it: a relative root carries no `.git` component of its own.
fn inside_git_dir(path: &Path) -> bool {
let resolved = std::fs::canonicalize(path);
let probe = resolved.as_deref().unwrap_or(path);
probe.components().any(|c| c.as_os_str() == ".git")
}

/// The walker configuration both scanning modes share, so `--files` and
/// the scan can never walk a different tree. The sets they report still
/// differ by the binary files the scan drops after the walk yields them.
///
/// `.gitignore` decides what is scanned, hidden entries included, with one
/// exception: `.git` is never walked. Nothing in it is tracked content,
/// and `.git/COMMIT_EDITMSG` and `.git/logs` hold commit messages, so a
/// commit that merely *discusses* a tag would otherwise read as carrying
/// one.
///
/// The exclusion matches on the entry name rather than on a path, so it
/// catches the `.git` of a submodule or a nested checkout as well, and the
/// `.git` *file* a worktree gets in place of a directory. Roots are
/// filtered separately because `ignore` applies `filter_entry` only from
/// depth 1 down, which would leave `todo-by .git` walking it after all.
///
/// Returns `None` when no root survives, which `ignore` cannot build from.
fn walk_builder(roots: &[PathBuf], overrides: Option<Override>) -> Option<WalkBuilder> {
let mut kept = roots.iter().filter(|root| !inside_git_dir(root));
let mut builder = WalkBuilder::new(kept.next()?);
for root in kept {
builder.add(root);
}
builder
.hidden(false)
.require_git(false)
.filter_entry(|entry| entry.file_name() != ".git");
if let Some(ov) = overrides {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
builder.overrides(ov);
}
Some(builder)
}

/// Walks `roots` (already filtered to existing, non-stdin paths) in
/// parallel, scanning every file. Returns findings and whether any I/O
/// error occurred.
fn scan_roots(
roots: &[PathBuf],
hidden: bool,
overrides: Option<Override>,
today: Date,
warn_until: Option<Date>,
tags: &[String],
) -> (Vec<Finding>, bool) {
let Some((first, rest)) = roots.split_first() else {
let Some(builder) = walk_builder(roots, overrides) else {
return (Vec::new(), false);
};
let mut builder = WalkBuilder::new(first);
for root in rest {
builder.add(root);
}
builder.hidden(!hidden).require_git(false);
if let Some(ov) = overrides {
builder.overrides(ov);
}

let io_error = AtomicBool::new(false);
let (tx, rx) = mpsc::channel::<Finding>();
Expand Down Expand Up @@ -624,22 +662,10 @@ fn scan_roots(
}

/// Walks `roots` single-threaded, collecting file paths for `--files`.
fn list_file_paths(
roots: &[PathBuf],
hidden: bool,
overrides: Option<Override>,
) -> (Vec<String>, bool) {
let Some((first, rest)) = roots.split_first() else {
fn list_file_paths(roots: &[PathBuf], overrides: Option<Override>) -> (Vec<String>, bool) {
let Some(builder) = walk_builder(roots, overrides) else {
return (Vec::new(), false);
};
let mut builder = WalkBuilder::new(first);
for root in rest {
builder.add(root);
}
builder.hidden(!hidden).require_git(false);
if let Some(ov) = overrides {
builder.overrides(ov);
}

let mut had_error = false;
let mut paths = Vec::new();
Expand Down Expand Up @@ -768,7 +794,7 @@ fn main() -> ExitCode {
}

if cli.files {
let (paths, walk_error) = list_file_paths(&fs_paths, cli.hidden, overrides);
let (paths, walk_error) = list_file_paths(&fs_paths, overrides);
for p in &paths {
println!("{p}");
}
Expand All @@ -779,9 +805,7 @@ fn main() -> ExitCode {
};
}

let (mut findings, walk_error) = scan_roots(
&fs_paths, cli.hidden, overrides, today, warn_until, &cfg.tags,
);
let (mut findings, walk_error) = scan_roots(&fs_paths, overrides, today, warn_until, &cfg.tags);
had_error = had_error || walk_error;

if has_stdin {
Expand Down Expand Up @@ -1404,10 +1428,13 @@ mod tests {
let err = parse_args(args(&[arg])).expect_err("rejected");
assert!(err.contains("takes no value"), "{arg}: {err}");
}
// The bare forms still work.
// The bare forms still work. `--hidden` no longer sets anything,
// but it must still parse: an existing CI invocation carrying it
// has to keep running, and it has to keep being a valueless flag.
let cli = parse_args(args(&["--online", "--exit-zero", "--hidden"])).expect("valid");
assert_eq!(cli.online, Some(true));
assert!(cli.exit_zero && cli.hidden);
assert!(cli.exit_zero);
assert_eq!(cli.paths, vec![PathBuf::from(".")]);
}

#[test]
Expand Down Expand Up @@ -1485,4 +1512,172 @@ mod tests {
scanner::Kind::IssueClosed { written, .. } if written == "#1"
));
}

/// Every file the walk should reach carries a tag, so a findings set
/// and a file list can be compared directly. Two shapes of `.git`, per
/// `walk_builder`.
fn walk_fixture(tag: &str) -> PathBuf {
let root = unique_temp_dir(tag);
let write = |rel: &str, body: &str| {
let path = root.join(rel);
std::fs::create_dir_all(path.parent().unwrap()).unwrap();
std::fs::write(path, body).unwrap();
};
write("visible.txt", "// todo-by 2998-01-01 plain file\n");
write(
".github/workflows/ci.yml",
"# todo-by 2998-01-01 unpin the action\n",
);
write(
".gitignore",
"# todo-by 2998-01-01 drop this\nignored.txt\n",
);
write(
"worktree/kept.txt",
"// todo-by 2998-01-01 beside a .git file\n",
);
write("worktree/.git", "gitdir: ../.git/worktrees/wt\n");
write("ignored.txt", "# todo-by 2998-01-01 never read\n");
write(
".git/COMMIT_EDITMSG",
"fix: drop the todo-by 2998-01-01 tag\n",
);
write(".git/logs/HEAD", "0000 1111 commit: todo-by 2998-01-01\n");
root
}

/// The four files above that are neither gitignored nor inside `.git`.
const WALK_FIXTURE_FILES: [&str; 4] = [
".github/workflows/ci.yml",
".gitignore",
"visible.txt",
"worktree/kept.txt",
];

fn relative_to(root: &Path, paths: impl IntoIterator<Item = String>) -> Vec<String> {
let mut out: Vec<String> = paths
.into_iter()
.map(|p| {
Path::new(&p)
.strip_prefix(root)
.unwrap_or(Path::new(&p))
.to_string_lossy()
.replace('\\', "/")
})
.collect();
out.sort();
out
}

fn walked(roots: &[PathBuf], root: &Path, overrides: Option<Override>) -> Vec<String> {
let (paths, had_error) = list_file_paths(roots, overrides);
assert!(!had_error, "walk reported an I/O error");
relative_to(root, paths)
}

fn scanned(roots: &[PathBuf], root: &Path, overrides: Option<Override>) -> Vec<String> {
let today = Date::parse_full("2999-01-01").unwrap();
let tags = vec!["todo-by".to_string()];
let (findings, had_error) = scan_roots(roots, overrides, today, None, &tags);
assert!(!had_error, "scan reported an I/O error");
relative_to(root, findings.into_iter().map(|f| f.file))
}

#[test]
fn the_walk_covers_hidden_files_and_never_enters_a_git_dir() {
let root = walk_fixture("walk");
let roots = vec![root.clone()];
assert_eq!(
walked(&roots, &root, None),
WALK_FIXTURE_FILES,
"unexpected file set"
);
std::fs::remove_dir_all(&root).ok();
}

#[test]
fn scanning_reaches_exactly_the_files_that_files_advertises() {
// `--files` exists to answer "what will be scanned", so the two
// walks have to be the same walk. They were separate builders
// once, and a filter added to one would not reach the other. Every
// fixture file carries a tag, so the findings name every file the
// scan actually read.
let root = walk_fixture("scan");
let roots = vec![root.clone()];
assert_eq!(scanned(&roots, &root, None), walked(&roots, &root, None));
std::fs::remove_dir_all(&root).ok();
}

#[test]
fn a_git_dir_named_as_a_root_is_dropped_rather_than_walked() {
// `ignore` applies filter_entry only from depth 1 down, so without
// the separate root filter this is how .git gets walked anyway.
let root = walk_fixture("git-root");
let git = root.join(".git");
assert!(walked(std::slice::from_ref(&git), &root, None).is_empty());
assert!(walked(&[git.join("logs")], &root, None).is_empty());
// Alongside a real root, the good root still produces its files
// and the .git one contributes nothing.
let both = vec![root.clone(), git];
assert_eq!(walked(&both, &root, None), WALK_FIXTURE_FILES);
std::fs::remove_dir_all(&root).ok();
}

#[test]
fn every_root_past_the_first_is_walked_too() {
// The roots after the first go through `WalkBuilder::add`, which a
// single-root test leaves entirely unexercised.
let root = walk_fixture("roots");
let roots = vec![root.join("worktree"), root.join(".github")];
assert_eq!(
walked(&roots, &root, None),
[".github/workflows/ci.yml", "worktree/kept.txt"]
);
std::fs::remove_dir_all(&root).ok();
}

#[test]
fn config_excludes_reach_the_walker_and_apply_to_hidden_files() {
// Building the matcher is not the same as handing it to the
// walker, and a hidden path is the case the exclude globs never
// had to cover before.
let root = walk_fixture("excludes");
let roots = vec![root.clone()];
let overrides = build_overrides(&root, &[".github/**".to_string()])
.expect("valid pattern")
.expect("some overrides");
assert_eq!(
walked(&roots, &root, Some(overrides)),
[".gitignore", "visible.txt", "worktree/kept.txt"]
);
std::fs::remove_dir_all(&root).ok();
}

#[test]
fn gitignore_applies_outside_a_repository() {
// `require_git(false)` is what makes the walker usable on an
// extracted tarball or a vendored tree. The fixture above has a
// real `.git`, so it would pass either way.
let root = unique_temp_dir("no-repo");
std::fs::write(root.join(".gitignore"), "ignored.txt\n").unwrap();
std::fs::write(root.join("ignored.txt"), "x\n").unwrap();
std::fs::write(root.join("kept.txt"), "x\n").unwrap();
assert_eq!(
walked(std::slice::from_ref(&root), &root, None),
[".gitignore", "kept.txt"]
);
std::fs::remove_dir_all(&root).ok();
}

#[test]
fn a_walk_with_no_usable_root_yields_nothing_instead_of_panicking() {
assert_eq!(list_file_paths(&[], None), (Vec::new(), false));
}

#[test]
fn hidden_is_absent_from_the_help_text() {
// The flag is accepted but deliberately undocumented. Re-adding
// the line would advertise a switch that changes nothing.
assert!(!USAGE.contains("--hidden"));
}
}