From 0138fbc96850f6ba03ac75daae4f1b953aef3b61 Mon Sep 17 00:00:00 2001 From: hubcio Date: Tue, 23 Jun 2026 11:09:23 +0200 Subject: [PATCH] feat!: per-language [[headers]] rules with existingStrategy Replace the style-keyed [mapping.STYLE] config and useDefaultMapping with a per-language [[headers]] rule list carrying styles (preferred first) and an existingStrategy (replace|skip|error). Fixes #210: format no longer stacks a duplicate header when a file already has a header in a different comment style. An existing header is detected style-agnostically by a whole-word, case-insensitive scan for the configured keywords. A foreign-but-listed header (e.g. /* */ under a // rule) is migrated to the preferred style, not duplicated. One in an unlisted style, or a stray keyword that cannot be normalized, is left unchanged and reported as "foreign": under existingStrategy=replace this fails the run (exit non-zero) rather than silently passing un-normalized; skip leaves it and exits zero. check adds a "foreign" category; --output JSON gains foreign/skipped/conflict. format and remove now write files atomically (temp, fsync, rename); a symlinked path is replaced by a regular file, and source permissions are preserved on a best-effort basis. BREAKING CHANGE: [mapping.STYLE] blocks and useDefaultMapping are removed. Replace each [mapping.FOO] { extensions = [...] } with a [[headers]] rule carrying styles = ["FOO"], and rename useDefaultMapping to useDefaultHeaders. A 6.x config is rejected with a migration hint. format now exits non-zero when it finds an existing license-looking header it cannot normalize to the preferred style; hawkeye remove's --output "removed" entries are now bare paths (previously path=removed). Tested on downstream hawkeye users by migrating their configs: apache/opendal, datafuselabs/databend and apache/fory match 6.5.1 with no new diagnostics; apache/iggy needs one gitignored file excluded. Closes #210 --- CHANGELOG.md | 42 ++ Cargo.lock | 4 +- Cargo.toml | 4 +- README.md | 51 +- cli/src/subcommand.rs | 156 ++++- fmt/src/config/mod.rs | 121 +--- fmt/src/document/factory.rs | 153 ++++- fmt/src/document/mod.rs | 585 +++++++++++++++++- fmt/src/document/model.rs | 58 +- fmt/src/header/parser.rs | 15 +- fmt/src/processor.rs | 51 +- fmt/src/selection.rs | 4 +- fmt/tests/content/foreign_style.rs | 7 + fmt/tests/content/no_header.rs | 3 + fmt/tests/tests.rs | 54 ++ tests/atomic_inplace/licenserc.toml | 16 + tests/atomic_inplace/main.rs | 3 + tests/atomic_inplace/main.rs.expected | 17 + tests/bom_issue/licenserc.toml | 3 +- .../licenserc.toml | 15 + tests/existing_foreign_style_error/main.rs | 9 + .../licenserc.toml | 16 + tests/existing_foreign_style_replace/main.rs | 9 + .../main.rs.expected | 19 + .../licenserc.toml | 16 + .../main.rs | 11 + .../main.rs.expected | 19 + .../licenserc.toml | 18 + .../main.rs | 9 + .../licenserc.toml | 15 + tests/existing_foreign_style_skip/main.rs | 9 + .../licenserc.toml | 17 + tests/existing_foreign_style_unlisted/main.rs | 9 + tests/it.py | 79 +++ .../licenserc.toml | 16 + tests/keyword_identifier_no_header/main.rs | 3 + .../main.rs.expected | 17 + tests/keyword_in_code/licenserc.toml | 17 + tests/keyword_in_code/main.rs | 3 + tests/legacy_mapping_config/licenserc.toml | 16 + tests/legacy_mapping_config/main.rs | 3 + 41 files changed, 1501 insertions(+), 191 deletions(-) create mode 100644 fmt/tests/content/foreign_style.rs create mode 100644 fmt/tests/content/no_header.rs create mode 100644 tests/atomic_inplace/licenserc.toml create mode 100644 tests/atomic_inplace/main.rs create mode 100644 tests/atomic_inplace/main.rs.expected create mode 100644 tests/existing_foreign_style_error/licenserc.toml create mode 100644 tests/existing_foreign_style_error/main.rs create mode 100644 tests/existing_foreign_style_replace/licenserc.toml create mode 100644 tests/existing_foreign_style_replace/main.rs create mode 100644 tests/existing_foreign_style_replace/main.rs.expected create mode 100644 tests/existing_foreign_style_replace_mixed/licenserc.toml create mode 100644 tests/existing_foreign_style_replace_mixed/main.rs create mode 100644 tests/existing_foreign_style_replace_mixed/main.rs.expected create mode 100644 tests/existing_foreign_style_replace_mixed_unlisted/licenserc.toml create mode 100644 tests/existing_foreign_style_replace_mixed_unlisted/main.rs create mode 100644 tests/existing_foreign_style_skip/licenserc.toml create mode 100644 tests/existing_foreign_style_skip/main.rs create mode 100644 tests/existing_foreign_style_unlisted/licenserc.toml create mode 100644 tests/existing_foreign_style_unlisted/main.rs create mode 100644 tests/keyword_identifier_no_header/licenserc.toml create mode 100644 tests/keyword_identifier_no_header/main.rs create mode 100644 tests/keyword_identifier_no_header/main.rs.expected create mode 100644 tests/keyword_in_code/licenserc.toml create mode 100644 tests/keyword_in_code/main.rs create mode 100644 tests/legacy_mapping_config/licenserc.toml create mode 100644 tests/legacy_mapping_config/main.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 88709964..8a7efb7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,48 @@ All notable changes to this project will be documented in this file. ## Unreleased +### Breaking changes + +The license-header configuration model has changed. The style-keyed `[mapping.STYLE_NAME]` blocks and the `useDefaultMapping` option are removed; configure file handling with a per-language `[[headers]]` rule list instead: + +* Replace each `[mapping.STYLE] { extensions = ["x"], filenames = ["y"] }` with: + ```toml + [[headers]] + extensions = ["x"] + filenames = ["y"] + styles = ["STYLE"] + ``` +* Default language-to-style rules stay active; declare `[[headers]]` only to override or extend them. `useDefaultMapping` is renamed to `useDefaultHeaders`. + +`hawkeye format` no longer stacks a duplicate header when a file already has one written in a comment style different from the configured one. An existing header is detected by a style-agnostic scan for the configured `keywords`. Each `[[headers]]` rule chooses what happens to such a file via `existingStrategy`: + +* `"replace"` (default): remove the existing header and write the preferred style. When a rule lists more than one style, a header in any listed style is removed, so it migrates to the preferred (first) style instead of being duplicated. If the file looks licensed but the command cannot normalize it to the preferred style (the header is in an unlisted comment style, or only a stray keyword is present), the file is left unchanged and the run fails (see the breaking note below). +* `"skip"`: leave the file untouched and report it; the run still succeeds (exit 0). +* `"error"`: fail the run. + +Detection is a keyword scan, so it has a blind spot in the other direction too: a real notice that contains none of the configured `keywords` (an MIT/BSD notice with no `copyright` line, or the keyword-less tail of a header split by an inserted blank line) is not detected and is left below the rewritten header. Tune `keywords` if this or the stray-keyword case bites. + +The `keywords` scan is now case-insensitive as documented: configured keywords are lowercased before matching, so a custom `keywords = ["Copyright"]` now matches a `copyright` notice. In 6.x a capitalized custom keyword silently never matched. The default (`copyright`) is unaffected. + +`existingStrategy` governs `hawkeye format` only; `hawkeye check` and `hawkeye remove` ignore it. + +Under `existingStrategy = "replace"`, a license-looking header that cannot be normalized to the preferred style now fails the run with a non-zero exit code. In 6.x the file was left unchanged and the run still exited 0, silently passing an un-normalized file through CI. The file is still left untouched; only the exit code changed. `"skip"` is unaffected and still exits 0. + +`hawkeye check` now reports files that carry a non-matching existing header (a stale header in a recognized style, or a notice in a style the rule does not list) as a distinct `foreign` category, separate from files missing a header entirely. Because detection includes the same keyword scan, a file that has no header but contains a stray keyword near the top is reported as `foreign` rather than `missing`. + +The `--output` JSON uses one grammar across `check`, `format`, and `remove`: every result field that lists files is a list of file paths categorized by the field name (`unknown`, `missing`, `foreign`, `skipped`, `conflict`, `removed`). `format`'s `updated` is the same list shape but its entries are suffixed `path=added` or `path=replaced`. `remove`'s `removed` entries are now bare file paths (previously `path=removed`), so scripts that split on `=` must be updated. `format` and `remove` additionally carry a boolean `dry_run` field (not a path list). + +The `foreign` field means different things per command. `check.foreign` is any non-matching existing header (stale, listed-foreign-style, or unlisted), found by the structural parse or the keyword scan. `format.foreign` and `remove.foreign` are narrower: only what the command could not handle - an unlisted comment style, or a stray keyword (`format` could not normalize to the preferred style; `remove` could not locate a removable block). A stale or listed-foreign-style header is migrated/removed instead, so it never appears in their `foreign` field. + +`hawkeye format` and `hawkeye remove` now write files atomically (write a sibling temp file, fsync, then rename over the target) instead of editing in place. Consequences: + +* If a target file is a symlink, it is replaced by a regular file (the 6.x in-place write followed the link). The replacement carries the link target's permission bits. +* Writing now hinges on the parent directory rather than the file. Creating the temp file and renaming it over the target need write permission on the directory, not on the file, so a read-only source file can now be rewritten (the rename replaces the directory entry), while a writable file in a read-only directory now fails the run instead of being rewritten. + +File permissions are preserved on a best-effort basis; ownership and ACLs are not. + +`hawkeye` now processes a file matched by a `.gitignore` rule when that file is tracked in the Git index (for example, force-added with `git add -f`); 6.x skipped every gitignore-matched file (the #209 fix - a force-added file should still get a header). On upgrade such a file is checked for the first time and can newly appear in `missing` or `foreign`, failing a previously-green run. List the newly-processed set with `git ls-files | git check-ignore --stdin --no-index` and add any you do not want checked to `excludes`. + ## [6.5.1] 2026-02-14 ### Bug fixes diff --git a/Cargo.lock b/Cargo.lock index f077293e..8661db94 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1264,7 +1264,7 @@ dependencies = [ [[package]] name = "hawkeye" -version = "6.5.1" +version = "7.0.0" dependencies = [ "build-data", "clap", @@ -1281,7 +1281,7 @@ dependencies = [ [[package]] name = "hawkeye-fmt" -version = "6.5.1" +version = "7.0.0" dependencies = [ "exn", "gix", diff --git a/Cargo.toml b/Cargo.toml index 6df5dbc8..50537763 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ members = ["cli", "fmt"] resolver = "2" [workspace.package] -version = "6.5.1" +version = "7.0.0" edition = "2021" authors = ["tison "] readme = "README.md" @@ -26,7 +26,7 @@ repository = "https://github.com/korandoru/hawkeye/" rust-version = "1.90.0" [workspace.dependencies] -hawkeye-fmt = { version = "=6.5.1", path = "fmt" } +hawkeye-fmt = { version = "=7.0.0", path = "fmt" } build-data = { version = "0.3.0" } clap = { version = "4.5.23", features = ["derive"] } diff --git a/README.md b/README.md index 81069aea..6386ed46 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ To check license headers in GitHub Actions, add a step in your GitHub workflow: ```yaml - name: Check License Header - uses: korandoru/hawkeye@v6 + uses: korandoru/hawkeye@v7 ``` ### Docker @@ -77,7 +77,7 @@ cargo install hawkeye Instead of `cargo install`, you can install `hawkeye` as a prebuilt binary by: ```shell -export VERSION=v6.0.0 +export VERSION=v7.0.0 curl --proto '=https' --tlsv1.2 -LsSf https://github.com/korandoru/hawkeye/releases/download/$VERSION/hawkeye-installer.sh | sh ``` @@ -101,6 +101,12 @@ docker build . -t hawkeye ### Config file +> [!IMPORTANT] +> HawkEye 7.0 replaced the `[mapping.STYLE]` blocks and `useDefaultMapping` with a +> per-language `[[headers]]` rule list (see `headers` below). Replace each +> `[mapping.FOO] { extensions = [...] }` with a `[[headers]]` rule carrying +> `styles = ["FOO"]`, and rename `useDefaultMapping` to `useDefaultHeaders`. + ```toml # Base directory for the whole execution. # All relative paths is based on this path. @@ -145,25 +151,42 @@ excludes = ["..."] # default: ["copyright"] keywords = ["copyright", "..."] -# Whether you use the default mapping. Check DocumentType.defaultMapping() for the completed list. +# Whether to use the built-in language-to-style rules. Check DocumentType defaults for the completed list. # default: true -useDefaultMapping = true +useDefaultHeaders = true # Paths to additional header style files. The model of user-defined header style can be found below. # default: empty additionalHeaders = ["..."] -# Mapping rules (repeated). +# Header rules (repeated). Each rule binds file patterns to one or more comment styles and +# decides what to do when a file already has a header. # -# The key of a mapping rule is a header style type (case-insensitive). +# A style name references a built-in HeaderType or one defined in `additionalHeaders` +# (case-insensitive). styles[0] is the preferred style that `format` writes; any further +# styles are also recognized for removal, so a header already written in one of them is +# migrated to the preferred style instead of being duplicated. # -# Available header style types consist of those defined at `HeaderType` and user-defined ones in `additionalHeaders`. -# The name of header style type is case-insensitive. +# User rules take precedence over the built-in defaults (matched first; exact filename +# before extension). +[[headers]] +extensions = ["..."] # e.g. "cc" +filenames = ["..."] # e.g. "Dockerfile.native" +styles = ["STYLE_NAME"] # preferred first + +# What to do when a file already has a header that is not an exact match for the preferred +# style (an existing header is detected by a style-agnostic scan for `keywords`). This applies +# to `format` only; `check` and `remove` ignore it. +# "replace" - remove the existing header (any listed style) and write the preferred one (default) +# "skip" - leave the file untouched and report it +# "error" - fail the run # -# If useDefaultMapping is true, the mapping rules defined here can override the default one. -[mapping.STYLE_NAME] -filenames = ["..."] # e.g. "Dockerfile.native" -extensions = ["..."] # e.g. "cc" +# Because detection is a keyword scan, a stray occurrence of a keyword (default "copyright") near +# the top of an otherwise unheadered file looks like an existing header: "skip" then silently +# leaves it without a header, and "error" fails the run. The reverse also holds: a real notice +# that contains none of the `keywords` is not detected, so "replace" adds a second header on top +# of it. Tune `keywords` if either case bites. +existingStrategy = "replace" # Properties to fulfill the template. # For a defined key-value pair, you can use {{props["key"]}} in the header template, which will be @@ -177,7 +200,9 @@ inceptionYear = 2023 # Options to configure Git features. [git] -# If enabled, do not process files that are ignored by Git; possible value: ['auto', 'enable', 'disable'] +# If enabled, do not process files that are ignored by Git AND untracked; a file matched by a +# .gitignore rule but tracked in the index (e.g. force-added with `git add -f`) is still processed. +# possible value: ['auto', 'enable', 'disable'] # 'auto' means this feature tries to be enabled with: # * gix - if `basedir` is in a Git repository. # * ignore crate's gitignore rules - if `basedir` is not in a Git repository. diff --git a/cli/src/subcommand.rs b/cli/src/subcommand.rs index 823b15fa..bfb24a69 100644 --- a/cli/src/subcommand.rs +++ b/cli/src/subcommand.rs @@ -19,6 +19,7 @@ use std::path::PathBuf; use clap::Args; use clap::Parser; use exn::Result; +use hawkeye_fmt::config::ExistingStrategy; use hawkeye_fmt::document::Document; use hawkeye_fmt::error::Error; use hawkeye_fmt::header::matcher::HeaderMatcher; @@ -79,7 +80,7 @@ pub struct CommandCheck { shared: SharedOptions, #[arg( long, - help = "whether to exit with non-zero code if missing headers", + help = "whether to exit with non-zero code if headers are missing or non-matching (foreign)", action = clap::ArgAction::Set, default_value_t = true )] @@ -90,6 +91,7 @@ pub struct CommandCheck { struct CheckContext { unknown: Vec, missing: Vec, + foreign: Vec, } impl Callback for CheckContext { @@ -102,7 +104,15 @@ impl Callback for CheckContext { } fn on_not_matched(&mut self, _: &HeaderMatcher, document: Document) -> Result<(), Error> { - self.missing.push(document.filepath.display().to_string()); + let path = document.filepath.display().to_string(); + // A file that already carries a non-matching header (a stale one in a recognized + // style, or a notice in a style this rule does not list) is reported distinctly from + // a file that has no header at all. + if document.has_existing_header() { + self.foreign.push(path); + } else { + self.missing.push(path); + } Ok(()) } } @@ -113,14 +123,22 @@ impl CommandCheck { let mut context = CheckContext { unknown: vec![], missing: vec![], + foreign: vec![], }; - check_license_header(config, &mut context).unwrap(); + run_processor(config, &mut context); let mut failed = check_unknown_files(&context.unknown, self.shared.fail_if_unknown); if !context.missing.is_empty() { log::error!("Found header missing in files: {:?}", context.missing); failed |= self.fail_if_missing; } + if !context.foreign.is_empty() { + log::error!( + "Found files with a non-matching existing header (different style or outdated content): {:?}", + context.foreign + ); + failed |= self.fail_if_missing; + } if let Some(f) = self.shared.output_file { write_to_file(&f, &context); } @@ -144,6 +162,9 @@ struct FormatContext { dry_run: bool, unknown: Vec, updated: Vec, + skipped: Vec, + foreign: Vec, + conflict: Vec, } impl Callback for FormatContext { @@ -156,16 +177,54 @@ impl Callback for FormatContext { } fn on_not_matched(&mut self, header: &HeaderMatcher, mut doc: Document) -> Result<(), Error> { - if doc.header_detected() { - doc.remove_header(); - doc.update_header(header)?; - self.updated - .push(format!("{}=replaced", doc.filepath.display())); - } else { - doc.update_header(header)?; - self.updated - .push(format!("{}=added", doc.filepath.display())); - } + let path = doc.filepath.display().to_string(); + + // `kind` is set only when the file is actually rewritten (added/replaced); skip and + // error outcomes leave it untouched. `error` must not return Err: `run_processor` + // aggregates conflicts and exits(1), like the missing/updated paths. + let strategy = doc.existing_strategy(); + let kind = match strategy { + ExistingStrategy::Replace => { + // Strip ALL recognized header blocks (preferred style + every listed foreign + // style, in either stacking order), then insert exactly one preferred header. + // Re-check `looks_licensed` AFTER stripping: a notice in an UNLISTED style is not + // recognized, so it survives the strip; inserting on top of it would re-stack the + // #210 duplicate over a survivor. If anything still looks licensed (a surviving + // unlisted notice, or a stray keyword), leave the file for a human - a controlled + // failure beats a silent duplicate. + let removed = doc.remove_existing_headers(); + if doc.looks_licensed() { + log::warn!( + "{path}: looks licensed (unlisted comment style or a stray keyword); left unchanged" + ); + self.foreign.push(path); + return Ok(()); + } + doc.update_header(header)?; + if removed { + "replaced" + } else { + "added" + } + } + // Skip and Error differ only in the bucket: Error -> conflict (exits 1 in + // CommandFormat::run), Skip -> skipped (warn only). + ExistingStrategy::Skip | ExistingStrategy::Error => { + if doc.has_existing_header() { + let bucket = if strategy == ExistingStrategy::Error { + &mut self.conflict + } else { + &mut self.skipped + }; + bucket.push(path); + return Ok(()); + } + doc.update_header(header)?; + "added" + } + }; + + self.updated.push(format!("{path}={kind}")); if self.dry_run { let mut extension = doc.filepath.extension().unwrap_or_default().to_os_string(); @@ -185,8 +244,11 @@ impl CommandFormat { dry_run: self.shared_edit.dry_run, unknown: vec![], updated: vec![], + skipped: vec![], + foreign: vec![], + conflict: vec![], }; - check_license_header(config, &mut context).unwrap(); + run_processor(config, &mut context); let mut failed = check_unknown_files(&context.unknown, self.shared.fail_if_unknown); if !context.updated.is_empty() { @@ -197,6 +259,30 @@ impl CommandFormat { ); failed |= self.shared_edit.fail_if_updated; } + if !context.skipped.is_empty() { + log::warn!( + "Skipped files with an existing header: {:?}", + context.skipped + ); + } + if !context.foreign.is_empty() { + // existingStrategy=replace found a license-looking header it could not normalize to + // the preferred style (an unlisted comment style, or a stray keyword). Leaving it + // would silently exit 0 with the file un-normalized, so this is a controlled failure. + // Unlike `skipped` (the user's explicit opt-out), `foreign` is not a chosen outcome. + log::warn!( + "Files that look licensed (unlisted comment style or a stray keyword); left unchanged: {:?}", + context.foreign + ); + failed = true; + } + if !context.conflict.is_empty() { + log::error!( + "Existing headers conflict with existingStrategy=error: {:?}", + context.conflict + ); + failed = true; + } if let Some(f) = self.shared.output_file { write_to_file(&f, &context); } @@ -220,16 +306,31 @@ struct RemoveContext { dry_run: bool, unknown: Vec, removed: Vec, + foreign: Vec, } impl RemoveContext { fn remove(&mut self, doc: &mut Document) -> Result<(), Error> { - if !doc.header_detected() { + let path = doc.filepath.display().to_string(); + // Remove a header in the preferred style, or failing that any other listed style. + let removed = if doc.header_detected() { + doc.remove_header(); + true + } else { + doc.remove_foreign_header() + }; + + if !removed { + if doc.looks_licensed() { + log::warn!( + "{path}: looks licensed (unlisted comment style or a stray keyword); not removed" + ); + self.foreign.push(path); + } return Ok(()); } - doc.remove_header(); - self.removed - .push(format!("{}=removed", doc.filepath.display())); + + self.removed.push(path); if self.dry_run { let mut extension = doc.filepath.extension().unwrap_or_default().to_os_string(); extension.push(".removed"); @@ -262,8 +363,9 @@ impl CommandRemove { dry_run: self.shared_edit.dry_run, unknown: vec![], removed: vec![], + foreign: vec![], }; - check_license_header(config, &mut context).unwrap(); + run_processor(config, &mut context); let mut failed = check_unknown_files(&context.unknown, self.shared.fail_if_unknown); if !context.removed.is_empty() { @@ -274,6 +376,12 @@ impl CommandRemove { ); failed |= self.shared_edit.fail_if_updated; } + if !context.foreign.is_empty() { + log::warn!( + "Files that look licensed (unlisted comment style or a stray keyword); not removed: {:?}", + context.foreign + ); + } if let Some(f) = self.shared.output_file { write_to_file(&f, &context); } @@ -284,6 +392,16 @@ impl CommandRemove { } } +/// Run the processor, surfacing a config/IO error as a clean stderr message + `exit(1)` +/// rather than an `unwrap` panic that buries it (e.g. the 6.x migration hint) under a +/// backtrace. Matches the missing/conflict paths, which also log and `exit(1)`. +fn run_processor(config: PathBuf, callback: &mut C) { + if let Err(err) = check_license_header(config, callback) { + log::error!("{err:?}"); + std::process::exit(1); + } +} + fn write_to_file(path: &Path, result: impl Serialize) { fn do_write_to_file(path: &Path, result: impl Serialize) -> std::io::Result<()> { let mut file = std::fs::File::create(path)?; diff --git a/fmt/src/config/mod.rs b/fmt/src/config/mod.rs index 4afacfc2..13424718 100644 --- a/fmt/src/config/mod.rs +++ b/fmt/src/config/mod.rs @@ -13,9 +13,6 @@ // limitations under the License. use std::collections::HashMap; -use std::collections::HashSet; -use std::hash::Hash; -use std::hash::Hasher; use std::path::PathBuf; use serde::de::Error; @@ -39,7 +36,7 @@ pub struct Config { #[serde(default = "default_true")] pub use_default_excludes: bool, #[serde(default = "default_true")] - pub use_default_mapping: bool, + pub use_default_headers: bool, #[serde(default = "default_keywords")] pub keywords: Vec, @@ -48,8 +45,7 @@ pub struct Config { #[serde(deserialize_with = "de_properties")] pub properties: HashMap, - #[serde(deserialize_with = "de_mapping")] - pub mapping: HashSet, + pub headers: Vec, pub git: Git, @@ -108,63 +104,36 @@ impl FeatureGate { } } -#[derive(Debug, Clone)] -pub enum Mapping { - Filename { - pattern: String, - header_type: String, - }, - Extension { - pattern: String, - header_type: String, - }, -} - -impl PartialEq for Mapping { - fn eq(&self, other: &Self) -> bool { - match (self, other) { - (Mapping::Filename { pattern: p1, .. }, Mapping::Filename { pattern: p2, .. }) => { - p1 == p2 - } - (Mapping::Extension { pattern: p1, .. }, Mapping::Extension { pattern: p2, .. }) => { - p1 == p2 - } - _ => false, - } - } -} - -impl Eq for Mapping {} - -impl Hash for Mapping { - fn hash(&self, state: &mut H) { - state.write(match self { - Mapping::Filename { pattern, .. } => pattern.as_bytes(), - Mapping::Extension { pattern, .. } => pattern.as_bytes(), - }); - } +/// What `format` does when a file already has a header that is not an exact match for +/// the rule's preferred style. An exact match is always left alone; a file with no +/// header at all always gets the preferred style inserted. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ExistingStrategy { + /// Remove the existing header (in any listed style) and write the preferred one. + #[default] + Replace, + /// Leave the file untouched and report it. + Skip, + /// Fail the run. + Error, } -impl Mapping { - pub fn header_type(&self, filename: &str) -> Option { - let filename = filename.to_lowercase(); - match self { - Mapping::Filename { - header_type, - pattern, - } => { - let pattern = pattern.to_lowercase(); - (filename == pattern).then(|| header_type.clone()) - } - Mapping::Extension { - header_type, - pattern, - } => { - let pattern = format!(".{pattern}").to_lowercase(); - filename.ends_with(&pattern).then(|| header_type.clone()) - } - } - } +/// Per-language rule binding file patterns to comment style(s). Replaces `[mapping.STYLE]`. +/// +/// `styles[0]` is the preferred style (what `format` writes). Any further styles are also +/// recognized for removal, so a header written in one of them can be migrated to the +/// preferred style instead of being duplicated. +#[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +pub struct HeaderRule { + #[serde(default)] + pub extensions: Vec, + #[serde(default)] + pub filenames: Vec, + pub styles: Vec, + #[serde(default)] + pub existing_strategy: ExistingStrategy, } fn default_cwd() -> PathBuf { @@ -195,33 +164,3 @@ where }) .collect() } - -fn de_mapping<'de, D>(de: D) -> Result, D::Error> -where - D: Deserializer<'de>, -{ - #[derive(Debug, Default, Clone, Deserialize)] - #[serde(default)] - struct MappingModel { - extensions: Vec, - filenames: Vec, - } - - let mappings = HashMap::::deserialize(de)?; - let mut set = HashSet::new(); - for (header_type, model) in mappings { - for pattern in model.extensions { - set.insert(Mapping::Extension { - pattern, - header_type: header_type.clone(), - }); - } - for pattern in model.filenames { - set.insert(Mapping::Filename { - pattern, - header_type: header_type.clone(), - }); - } - } - Ok(set) -} diff --git a/fmt/src/document/factory.rs b/fmt/src/document/factory.rs index f5105d06..3aca8a89 100644 --- a/fmt/src/document/factory.rs +++ b/fmt/src/document/factory.rs @@ -13,25 +13,35 @@ // limitations under the License. use std::collections::HashMap; -use std::collections::HashSet; use std::fs; use std::path::Path; use std::path::PathBuf; use std::time::SystemTime; +use exn::bail; use exn::OptionExt; use exn::Result; -use crate::config::Mapping; +use crate::config::ExistingStrategy; +use crate::config::HeaderRule; use crate::document::Attributes; use crate::document::Document; use crate::error::Error; use crate::git::GitFileAttrs; use crate::header::model::HeaderDef; +/// A [`HeaderRule`] resolved to concrete [`HeaderDef`]s; `defs[0]` is preferred, +/// the rest are recognized for removal only. +struct ResolvedRule { + extensions: Vec, // lowercased, leading dot, e.g. ".rs" + filenames: Vec, // lowercased + defs: Vec, + existing_strategy: ExistingStrategy, +} + pub struct DocumentFactory { - mapping: HashSet, - definitions: HashMap, + rules: Vec, + unknown_def: HeaderDef, properties: HashMap, keywords: Vec, @@ -39,20 +49,73 @@ pub struct DocumentFactory { } impl DocumentFactory { + /// Resolve rule style names up front so per-file lookup is cheap and typos fail at startup. pub fn new( - mapping: HashSet, + header_rules: Vec, definitions: HashMap, properties: HashMap, keywords: Vec, git_file_attrs: HashMap, - ) -> Self { - Self { - mapping, - definitions, + ) -> Result { + let unknown_def = definitions + .get("unknown") + .cloned() + .ok_or_raise(|| Error::new("missing built-in 'unknown' header definition"))?; + + let mut rules = Vec::with_capacity(header_rules.len()); + for rule in header_rules { + if rule.styles.is_empty() { + bail!(Error::new(format!( + "header rule for extensions={:?} filenames={:?} must list at least one style", + rule.extensions, rule.filenames + ))); + } + let mut defs = Vec::with_capacity(rule.styles.len()); + for style in &rule.styles { + let def = definitions.get(&style.to_lowercase()).ok_or_raise(|| { + Error::new(format!("header rule references unknown style: {style}")) + })?; + defs.push(def.clone()); + } + rules.push(ResolvedRule { + extensions: rule + .extensions + .iter() + .map(|e| format!(".{}", e.to_lowercase())) + .collect(), + filenames: rule.filenames.iter().map(|f| f.to_lowercase()).collect(), + defs, + existing_strategy: rule.existing_strategy, + }); + } + + Ok(Self { + rules, + unknown_def, properties, - keywords, + // lowercase once for case-insensitive matching against the lowercased haystack + keywords: keywords.into_iter().map(|k| k.to_lowercase()).collect(), git_file_attrs, + }) + } + + /// Resolve a file name to its styles (preferred first) and strategy. Filename rules + /// win over extension rules; first match wins per tier; falls back to `unknown`. + fn resolve(&self, lower_file_name: &str) -> (&[HeaderDef], ExistingStrategy) { + for rule in &self.rules { + if rule.filenames.iter().any(|f| f == lower_file_name) { + return (&rule.defs, rule.existing_strategy); + } + } + for rule in &self.rules { + if rule.extensions.iter().any(|e| lower_file_name.ends_with(e)) { + return (&rule.defs, rule.existing_strategy); + } } + ( + std::slice::from_ref(&self.unknown_def), + ExistingStrategy::default(), + ) } pub fn create_document(&self, filepath: &Path) -> Result, Error> { @@ -60,19 +123,13 @@ impl DocumentFactory { .file_name() .map(|n| n.to_string_lossy().to_lowercase()) .unwrap_or_default(); - let header_type = self - .mapping - .iter() - .find_map(|m| m.header_type(&lower_file_name)) - .unwrap_or_else(|| "unknown".to_string()) - .to_lowercase(); - let header_def = self.definitions.get(&header_type).ok_or_raise(|| { - Error::new(format!( - "cannot create document: {}, header type {} not found", - filepath.display(), - header_type - )) - })?; + let (defs, existing_strategy) = self.resolve(&lower_file_name); + // `resolve` always returns a non-empty slice (empty `styles` rejected in `new`). + let (preferred, rest) = defs + .split_first() + .expect("resolve guarantees at least one style"); + let header_def = preferred.clone(); + let removal_candidates = rest.to_vec(); let props = self.properties.clone(); @@ -102,8 +159,10 @@ impl DocumentFactory { Document::new( filepath.to_path_buf(), - header_def.clone(), - &self.keywords, + header_def, + removal_candidates, + self.keywords.clone(), + existing_strategy, props, attrs, ) @@ -122,3 +181,47 @@ fn git_time_to_year(t: gix::date::Time) -> Option { .to_zoned(offset.to_time_zone()); Some(zoned.year()) } + +#[cfg(test)] +mod tests { + use crate::header::model::default_headers; + + use super::*; + + fn rule(styles: &[&str]) -> HeaderRule { + HeaderRule { + extensions: vec!["rs".to_string()], + filenames: vec![], + styles: styles.iter().map(|s| s.to_string()).collect(), + existing_strategy: ExistingStrategy::Replace, + } + } + + fn factory(rules: Vec) -> Result { + DocumentFactory::new( + rules, + default_headers(), + HashMap::new(), + vec!["copyright".to_string()], + HashMap::new(), + ) + } + + /// A rule with no styles can never insert a header; reject it at startup, not per file. + #[test] + fn empty_styles_is_rejected() { + assert!(factory(vec![rule(&[])]).is_err()); + } + + /// A style name absent from the definitions is almost certainly a typo; fail fast. + #[test] + fn unknown_style_is_rejected() { + assert!(factory(vec![rule(&["NO_SUCH_STYLE"])]).is_err()); + } + + /// Happy path resolves: pins that the rejections above are not over-eager. + #[test] + fn valid_rule_is_accepted() { + assert!(factory(vec![rule(&["DOUBLESLASH_STYLE", "SLASHSTAR_STYLE"])]).is_ok()); + } +} diff --git a/fmt/src/document/mod.rs b/fmt/src/document/mod.rs index de2d817e..943f7356 100644 --- a/fmt/src/document/mod.rs +++ b/fmt/src/document/mod.rs @@ -28,6 +28,7 @@ use minijinja::Environment; use serde::Deserialize; use serde::Serialize; +use crate::config::ExistingStrategy; use crate::error::Error; use crate::header::matcher::HeaderMatcher; use crate::header::model::HeaderDef; @@ -52,6 +53,9 @@ pub struct Document { pub filepath: PathBuf, header_def: HeaderDef, + removal_candidates: Vec, + keywords: Vec, + existing_strategy: ExistingStrategy, props: HashMap, attrs: Attributes, parser: HeaderParser, @@ -61,15 +65,20 @@ impl Document { pub fn new( filepath: PathBuf, header_def: HeaderDef, - keywords: &[String], + removal_candidates: Vec, + keywords: Vec, + existing_strategy: ExistingStrategy, props: HashMap, attrs: Attributes, ) -> Result, Error> { match FileContent::new(&filepath) { Ok(content) => Ok(Some(Self { - parser: parse_header(content, &header_def, keywords), + parser: parse_header(content, &header_def, &keywords), filepath, header_def, + removal_candidates, + keywords, + existing_strategy, props, attrs, })), @@ -96,6 +105,124 @@ impl Document { self.parser.end_pos.is_some() } + pub fn existing_strategy(&self) -> ExistingStrategy { + self.existing_strategy + } + + /// Style-agnostic probe: does the file's prefix carry a license keyword as a whole word, + /// in any comment style? Looser than [`Self::header_detected`] so `format` reports an + /// unlisted-style header instead of stacking a duplicate. A hit in a string literal or + /// prose still classifies as foreign on purpose: controlled failure over a silent miss. + pub fn looks_licensed(&self) -> bool { + // Char cap, not line cap: still catches a notice pushed down by a long preamble yet + // stays cheap on a minified single-line file. `chars().take` is panic-safe (no UTF-8 + // boundary slicing). `keywords` are lowercased at construction; lowercase the prefix too. + const SCAN_CHARS: usize = 16 * 1024; + let prefix: String = self + .parser + .file_content + .content() + .chars() + .take(SCAN_CHARS) + .flat_map(char::to_lowercase) + .collect(); + self.keywords + .iter() + .any(|kw| contains_whole_word(&prefix, kw)) + } + + /// True if the file already carries a header: one the preferred parser recognized + /// structurally, or a license notice in another style. + pub fn has_existing_header(&self) -> bool { + self.header_detected() || self.looks_licensed() + } + + /// Strip ALL header blocks in any listed non-preferred style, then move the insertion point + /// to where the preferred style wants its header. Returns whether anything was removed. + /// Unlike [`Self::remove_header`] (topmost-only), this drains every foreign block: a file + /// may carry several, and a leftover would re-emit the preferred header on top of it. + pub fn remove_foreign_header(&mut self) -> bool { + // No removal candidates -> nothing foreign to strip. Return before cloning content. + if self.removal_candidates.is_empty() { + return false; + } + + let filepath = self.filepath.to_string_lossy().to_string(); + let mut removed_any = false; + + // Re-snapshot after each deletion so the next probe's offsets stay valid. Each probe + // removes a non-empty span, so the buffer shrinks and the loop terminates. + loop { + let content = self.parser.file_content.content().to_string(); + let mut removed_this_pass = false; + for def in &self.removal_candidates { + let probe = parse_header( + FileContent::from_content(content.clone(), filepath.clone()), + def, + &self.keywords, + ); + if let Some(end) = probe.end_pos { + self.parser.file_content.delete(probe.begin_pos, end); + removed_any = true; + removed_this_pass = true; + break; // re-snapshot before probing again + } + } + if !removed_this_pass { + break; + } + } + + if removed_any { + // Foreign skip handling may differ from the preferred style. Re-parse with the + // preferred def so the insertion point lands where the preferred style wants it (not + // at a foreign begin_pos) and any block exposed beneath the stripped one shows in + // `end_pos` for the caller's strip loop to remove instead of stacking a duplicate. + self.reparse_preferred(&filepath); + } + + removed_any + } + + /// Strip every header block (preferred plus all listed foreign styles) so exactly one + /// preferred header can be inserted afterward. Returns whether anything was removed. The + /// `format` replace path uses this to migrate a file stacking mixed styles, in either order. + pub fn remove_existing_headers(&mut self) -> bool { + let filepath = self.filepath.to_string_lossy().to_string(); + let mut removed_any = false; + + // Terminates: each continuing iteration deletes a non-empty span, so the buffer shrinks; + // the final iteration removes nothing and breaks. + loop { + if self.header_detected() { + self.remove_header(); + removed_any = true; + // `remove_header` leaves `parser.end_pos` set, so re-parse to refresh detection + // (and expose any block beneath the one just removed); else this loops forever. + self.reparse_preferred(&filepath); + continue; + } + if self.remove_foreign_header() { + removed_any = true; + continue; + } + break; + } + + removed_any + } + + /// Replace `self.parser` with a fresh parse of the current content against the preferred + /// header def, so `begin_pos`/`end_pos` reflect what is left after edits. + fn reparse_preferred(&mut self, filepath: &str) { + let content = self.parser.file_content.content().to_string(); + self.parser = parse_header( + FileContent::from_content(content, filepath.to_string()), + &self.header_def, + &self.keywords, + ); + } + /// Detected and valid header pub fn header_matched( &self, @@ -167,10 +294,12 @@ impl Document { pub fn save(&mut self) -> Result<(), Error> { let filepath = self.filepath.as_path(); - fs::write(filepath, self.parser.file_content.content()) + atomic_write(filepath, self.parser.file_content.content()) .or_raise(|| Error::new(format!("cannot save document {}", filepath.display()))) } + /// Write to a throwaway `--dry-run` debug artifact (`.formatted`/`.removed`). Never the + /// user's source, so a plain write is enough; crash-safe [`atomic_write`] is for [`Self::save`]. pub fn save_to(&mut self, filepath: impl AsRef) -> Result<(), Error> { let filepath = filepath.as_ref(); fs::write(filepath, self.parser.file_content.content()) @@ -193,3 +322,453 @@ impl Document { Ok(result) } } + +/// Whole-word search: `needle` matches only when the chars bracketing each occurrence are word +/// boundaries (not alphanumeric, not `_`; string start/end count). Keeps `copyright` from +/// matching inside `copyrightHolder`. Both args are expected lowercased by the caller. +fn contains_whole_word(haystack: &str, needle: &str) -> bool { + if needle.is_empty() { + return false; + } + // `match_indices` yields offsets at char boundaries, so the surrounding slices stay UTF-8 + // valid and `chars().next_back()`/`.next()` are safe. + let is_word = |c: char| c.is_alphanumeric() || c == '_'; + haystack.match_indices(needle).any(|(start, m)| { + let before_ok = haystack[..start] + .chars() + .next_back() + .is_none_or(|c| !is_word(c)); + let after_ok = haystack[start + m.len()..] + .chars() + .next() + .is_none_or(|c| !is_word(c)); + before_ok && after_ok + }) +} + +/// Write `content` to `path` atomically: temp file, flush, rename over the target. A crash or +/// `ENOSPC` mid-write leaves the original intact (the rename never runs). Permissions are +/// preserved; ownership/ACLs are not. A symlink at `path` is replaced by a regular file +/// carrying the link TARGET's mode (`fs::metadata` follows the link, `fs::rename` replaces it). +fn atomic_write(path: &Path, content: &str) -> std::io::Result<()> { + use std::io::Write; + + let dir = match path.parent() { + Some(parent) if !parent.as_os_str().is_empty() => parent, + _ => Path::new("."), + }; + let file_name = path.file_name().and_then(|n| n.to_str()).unwrap_or("out"); + // Same directory as the target so the rename stays on one filesystem (atomic, no EXDEV). + let tmp = dir.join(format!(".{file_name}.hawkeye-{}.tmp", std::process::id())); + + let result = (|| { + let mut file = File::create(&tmp)?; + file.write_all(content.as_bytes())?; + file.sync_all()?; + match fs::metadata(path) { + // Best-effort: a perm-copy failure must not abort an otherwise-successful write. + Ok(meta) => { + if let Err(e) = fs::set_permissions(&tmp, meta.permissions()) { + log::warn!( + "{}: cannot copy existing permissions ({e}); new file uses default mode", + path.display() + ); + } + } + // Absent target is normal (umask perms). A metadata error on an existing path means + // we cannot preserve its mode; warn rather than silently downgrade. + Err(e) if e.kind() != std::io::ErrorKind::NotFound => { + log::warn!( + "{}: cannot read existing permissions ({e}); new file uses default mode", + path.display() + ); + } + Err(_) => {} + } + fs::rename(&tmp, path)?; + // Best-effort fsync of the parent dir so the rename survives a crash, not just the data. + // Not cfg-gated: opening a dir as a file just fails where unsupported and we skip it. + if let Ok(dir_file) = File::open(dir) { + let _ = dir_file.sync_all(); + } + Ok(()) + })(); + + if result.is_err() { + let _ = fs::remove_file(&tmp); + } + result +} + +#[cfg(test)] +mod tests { + use std::sync::atomic::AtomicU64; + use std::sync::atomic::Ordering; + + use crate::header::model::default_headers; + + use super::*; + + /// After removing a header in a foreign style whose skip handling differs from the + /// preferred style, the insertion point must be re-derived with the preferred def. Here the + /// foreign `javapkg_style` skips the leading `package ...;` line, but the preferred + /// `doubleslash_style` has no skip pattern, so the new header belongs at the top of the + /// file. Pins the bug where `begin_pos` kept the foreign (post-package) offset. + #[test] + fn foreign_skip_pattern_does_not_offset_preferred_insertion() { + let defs = default_headers(); + let preferred = defs.get("doubleslash_style").unwrap().clone(); + let javapkg = defs.get("javapkg_style").unwrap().clone(); + + let content = + "package com.foo;\n\n/*\n * Copyright 2099 Old Authors.\n */\n\npub fn answer() -> i32 { 42 }\n"; + + // Unique on-disk path: `Document::new` -> `FileContent::new` reads the file. + static COUNTER: AtomicU64 = AtomicU64::new(0); + let fixture = std::env::temp_dir().join(format!( + "hawkeye-foreign-skip-{}-{}.rs", + std::process::id(), + COUNTER.fetch_add(1, Ordering::Relaxed) + )); + fs::write(&fixture, content).unwrap(); + + let mut doc = Document::new( + fixture.clone(), + preferred, + vec![javapkg], // removal-only style that skips the `package` line + vec!["copyright".to_string()], + ExistingStrategy::Replace, + HashMap::new(), + Attributes { + filename: fixture.file_name().map(|s| s.to_string_lossy().to_string()), + disk_file_created_year: None, + git_file_created_year: None, + git_file_modified_year: None, + git_authors: Default::default(), + }, + ) + .unwrap() + .unwrap(); + + // The /* */ header is not in the preferred // style, so structural detection misses it. + assert!(!doc.header_detected()); + assert!(doc.remove_foreign_header()); + // Preferred doubleslash has no skip pattern, so the header goes to the top of the file, + // not after `package com.foo;` where the foreign javapkg probe stopped. + assert_eq!(doc.parser.begin_pos, 0); + + let _ = fs::remove_file(&fixture); + } + + /// Two stacked foreign-style blocks must BOTH be stripped, not just the first: leaving the + /// second behind would re-emit the preferred header on top of a surviving notice (the #210 + /// stacking regression). Pins the loop in [`Document::remove_foreign_header`]. + #[test] + fn remove_foreign_header_strips_all_stacked_blocks() { + let defs = default_headers(); + let preferred = defs.get("doubleslash_style").unwrap().clone(); + let slashstar = defs.get("slashstar_style").unwrap().clone(); + + let content = "/*\n * Copyright 2099 Vendor Authors.\n */\n\n/*\n * Copyright 2015 Older Project Authors.\n */\n\npub fn answer() -> i32 { 42 }\n"; + + static COUNTER: AtomicU64 = AtomicU64::new(0); + let fixture = std::env::temp_dir().join(format!( + "hawkeye-stacked-foreign-{}-{}.rs", + std::process::id(), + COUNTER.fetch_add(1, Ordering::Relaxed) + )); + fs::write(&fixture, content).unwrap(); + + let mut doc = Document::new( + fixture.clone(), + preferred, + vec![slashstar], // /* */ accepted for removal only + vec!["copyright".to_string()], + ExistingStrategy::Replace, + HashMap::new(), + Attributes { + filename: fixture.file_name().map(|s| s.to_string_lossy().to_string()), + disk_file_created_year: None, + git_file_created_year: None, + git_file_modified_year: None, + git_authors: Default::default(), + }, + ) + .unwrap() + .unwrap(); + + assert!(!doc.header_detected()); + assert!(doc.remove_foreign_header()); + // Both blocks gone: no keyword left and no /* */ comment survives. + assert!(!doc.looks_licensed()); + assert!(!doc.parser.file_content.content().contains("/*")); + + let _ = fs::remove_file(&fixture); + } + + /// A notice pushed down by many leading lines must still be detected: [`Document::looks_licensed`] + /// scans a bounded character prefix, not a fixed line count, so a notice deep in the prefix + /// (here after 60 filler lines) is still found. + #[test] + fn looks_licensed_detects_notice_after_many_leading_lines() { + let defs = default_headers(); + let preferred = defs.get("doubleslash_style").unwrap().clone(); + + let mut content = String::new(); + for i in 0..60 { + content.push_str(&format!("// filler line {i}\n")); + } + content.push_str("// Copyright 2099 Late Authors.\n\npub fn answer() -> i32 { 42 }\n"); + + static COUNTER: AtomicU64 = AtomicU64::new(0); + let fixture = std::env::temp_dir().join(format!( + "hawkeye-late-notice-{}-{}.rs", + std::process::id(), + COUNTER.fetch_add(1, Ordering::Relaxed) + )); + fs::write(&fixture, &content).unwrap(); + + let doc = Document::new( + fixture.clone(), + preferred, + vec![], + vec!["copyright".to_string()], + ExistingStrategy::Replace, + HashMap::new(), + Attributes { + filename: fixture.file_name().map(|s| s.to_string_lossy().to_string()), + disk_file_created_year: None, + git_file_created_year: None, + git_file_modified_year: None, + git_authors: Default::default(), + }, + ) + .unwrap() + .unwrap(); + + assert!(doc.looks_licensed()); + + let _ = fs::remove_file(&fixture); + } + + /// Build a `Document` over `content` (written to a unique on-disk path) with `doubleslash` + /// preferred and `slashstar` accepted for removal only. + fn replace_document(tag: &str, content: &str) -> (Document, PathBuf) { + let defs = default_headers(); + let preferred = defs.get("doubleslash_style").unwrap().clone(); + let slashstar = defs.get("slashstar_style").unwrap().clone(); + + static COUNTER: AtomicU64 = AtomicU64::new(0); + let fixture = std::env::temp_dir().join(format!( + "hawkeye-{tag}-{}-{}.rs", + std::process::id(), + COUNTER.fetch_add(1, Ordering::Relaxed) + )); + fs::write(&fixture, content).unwrap(); + + let doc = Document::new( + fixture.clone(), + preferred, + vec![slashstar], + vec!["copyright".to_string()], + ExistingStrategy::Replace, + HashMap::new(), + Attributes { + filename: fixture.file_name().map(|s| s.to_string_lossy().to_string()), + disk_file_created_year: None, + git_file_created_year: None, + git_file_modified_year: None, + git_authors: Default::default(), + }, + ) + .unwrap() + .unwrap(); + (doc, fixture) + } + + /// Run the `format` replace path (strip-all then insert one preferred header) and assert + /// exactly one resulting header and no surviving old block, regardless of stacking order. + /// Mixing a preferred-style block with a listed-foreign-style block must not double-notice + /// (the #210 regression for mixed-style files, both orders). + fn assert_single_header_after_replace(doc: &mut Document) { + let header = HeaderMatcher::new("Copyright 2024 New Authors.\n".to_string()); + assert!(doc.remove_existing_headers()); + doc.update_header(&header).unwrap(); + + let result = doc.parser.file_content.content(); + assert_eq!( + result.matches("Copyright").count(), + 1, + "exactly one header must remain, got: {result:?}" + ); + // No old notice and no foreign /* */ block survives. + assert!( + !result.contains("Old Authors"), + "stale block survived: {result:?}" + ); + assert!(!result.contains("/*"), "foreign block survived: {result:?}"); + } + + #[test] + fn replace_strips_foreign_block_above_preferred_block() { + let content = "/*\n * Copyright 2099 Old Authors (foreign style).\n */\n\n// Copyright 2015 Old Authors (preferred style).\n\npub fn answer() -> i32 { 42 }\n"; + let (mut doc, fixture) = replace_document("d2-foreign-above", content); + assert_single_header_after_replace(&mut doc); + let _ = fs::remove_file(&fixture); + } + + #[test] + fn replace_strips_preferred_block_above_foreign_block() { + let content = "// Copyright 2015 Old Authors (preferred style).\n\n/*\n * Copyright 2099 Old Authors (foreign style).\n */\n\npub fn answer() -> i32 { 42 }\n"; + let (mut doc, fixture) = replace_document("d2-preferred-above", content); + assert_single_header_after_replace(&mut doc); + let _ = fs::remove_file(&fixture); + } + + /// Form-1 residual-duplicate edge (#210): a listed-style block ABOVE a notice in an UNLISTED + /// style. Stripping the listed block leaves the unlisted notice behind, so the file STILL + /// looks licensed afterward. The `format` replace path relies on this post-strip signal to + /// route the file to `foreign` instead of inserting a preferred header on top of the + /// survivor (which would re-stack the duplicate and never self-heal). + #[test] + fn replace_residual_unlisted_notice_still_looks_licensed_after_strip() { + // slashstar (listed, removed) on top; a hash-comment notice (unlisted) below. + let content = "/*\n * Copyright 2099 Vendor (listed slashstar style).\n */\n\n# Copyright 2015 Project (unlisted hash style).\n\npub fn answer() -> i32 { 42 }\n"; + let (mut doc, fixture) = replace_document("residual-unlisted", content); + + assert!( + doc.remove_existing_headers(), + "the listed slashstar block must be removed" + ); + // The unlisted hash notice cannot be stripped, so it survives and the file still looks + // licensed: the replace path must re-check this and route to foreign, not insert on top. + assert!(doc.looks_licensed()); + assert!(doc.parser.file_content.content().contains('#')); + + let _ = fs::remove_file(&fixture); + } + + /// Build a single-style (`doubleslash`) `Document` over `content`, so `looks_licensed` is + /// the only existing-header signal (no removal candidates to confuse the probe). + fn single_style_document(tag: &str, content: &str) -> (Document, PathBuf) { + let defs = default_headers(); + let preferred = defs.get("doubleslash_style").unwrap().clone(); + + static COUNTER: AtomicU64 = AtomicU64::new(0); + let fixture = std::env::temp_dir().join(format!( + "hawkeye-{tag}-{}-{}.rs", + std::process::id(), + COUNTER.fetch_add(1, Ordering::Relaxed) + )); + fs::write(&fixture, content).unwrap(); + + let doc = Document::new( + fixture.clone(), + preferred, + vec![], + vec!["copyright".to_string()], + ExistingStrategy::Replace, + HashMap::new(), + Attributes { + filename: fixture.file_name().map(|s| s.to_string_lossy().to_string()), + disk_file_created_year: None, + git_file_created_year: None, + git_file_modified_year: None, + git_authors: Default::default(), + }, + ) + .unwrap() + .unwrap(); + (doc, fixture) + } + + /// Word-boundary fix: `copyright` embedded in the identifier `copyrightHolder` must NOT mark + /// the file as licensed, or `format` would wrongly refuse to insert a header. + #[test] + fn looks_licensed_ignores_keyword_inside_identifier() { + let content = "struct Config { copyrightHolder: String }\n"; + let (doc, fixture) = single_style_document("word-ident", content); + assert!(!doc.looks_licensed()); + let _ = fs::remove_file(&fixture); + } + + /// A standalone `Copyright` word (here in a `/* */` block) is a real notice and must match, + /// even though the surrounding comment style is not the preferred one. + #[test] + fn looks_licensed_matches_standalone_word_in_comment() { + let content = "/*\n * Copyright 2099 Real Authors.\n */\n\npub fn answer() -> i32 { 42 }\n"; + let (doc, fixture) = single_style_document("word-comment", content); + assert!(doc.looks_licensed()); + let _ = fs::remove_file(&fixture); + } + + /// A whole-word keyword inside a string literal still classifies as foreign by design: we + /// prefer a controlled failure over silently inserting a second notice. + #[test] + fn looks_licensed_matches_standalone_word_in_string_literal() { + let content = "let s = \"copyright 2020\";\n"; + let (doc, fixture) = single_style_document("word-string", content); + assert!(doc.looks_licensed()); + let _ = fs::remove_file(&fixture); + } + + /// Unit coverage for the boundary logic in isolation, including start/end-of-string and the + /// `_` word-char rule. + #[test] + fn contains_whole_word_boundaries() { + assert!(contains_whole_word("copyright 2024", "copyright")); + assert!(contains_whole_word("a copyright.", "copyright")); + assert!(contains_whole_word("copyright", "copyright")); // whole string + assert!(!contains_whole_word("copyrightholder", "copyright")); + assert!(!contains_whole_word("my_copyright", "copyright")); // `_` is a word char + assert!(!contains_whole_word("copyright_owner", "copyright")); + assert!(!contains_whole_word("anything", "")); + } + + /// [`atomic_write`] over a SYMLINK must replace the link with a regular file carrying the + /// pre-existing target's permission bits, leave the original target untouched, and leave no + /// `.tmp` behind. Pins the symlink-replacement and permission-preservation contract. + #[cfg(unix)] + #[test] + fn atomic_write_replaces_symlink_with_regular_file() { + use std::os::unix::fs::PermissionsExt; + + static COUNTER: AtomicU64 = AtomicU64::new(0); + let dir = std::env::temp_dir().join(format!( + "hawkeye-atomic-{}-{}", + std::process::id(), + COUNTER.fetch_add(1, Ordering::Relaxed) + )); + fs::create_dir_all(&dir).unwrap(); + + let target = dir.join("target.rs"); + let link = dir.join("link.rs"); + fs::write(&target, "old content\n").unwrap(); + // A non-default mode so the assertion is meaningful (not whatever umask produces). + fs::set_permissions(&target, fs::Permissions::from_mode(0o640)).unwrap(); + std::os::unix::fs::symlink(&target, &link).unwrap(); + + atomic_write(&link, "new content\n").unwrap(); + + // The link path is now a regular file (rename replaced the link, not its target). + let link_meta = fs::symlink_metadata(&link).unwrap(); + assert!(link_meta.file_type().is_file()); + assert!(!link_meta.file_type().is_symlink()); + assert_eq!(fs::read_to_string(&link).unwrap(), "new content\n"); + + // Original target is untouched: rename replaced the link, not what it pointed at. + assert_eq!(fs::read_to_string(&target).unwrap(), "old content\n"); + + // New file inherited the target's mode bits (metadata followed the link to read them). + assert_eq!(link_meta.permissions().mode() & 0o777, 0o640); + + // No leftover temp file in the directory. + let leftover = fs::read_dir(&dir) + .unwrap() + .filter_map(|e| e.ok()) + .any(|e| e.file_name().to_string_lossy().contains(".hawkeye-")); + assert!(!leftover, "a .hawkeye-*.tmp file was left behind"); + + let _ = fs::remove_dir_all(&dir); + } +} diff --git a/fmt/src/document/model.rs b/fmt/src/document/model.rs index 732a3121..7177221a 100644 --- a/fmt/src/document/model.rs +++ b/fmt/src/document/model.rs @@ -17,7 +17,8 @@ use std::collections::HashMap; use serde::Deserialize; use serde::Serialize; -use crate::config::Mapping; +use crate::config::ExistingStrategy; +use crate::config::HeaderRule; #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[serde(default, rename_all = "camelCase")] @@ -28,28 +29,59 @@ pub struct DocumentType { pub filename: bool, } -pub fn default_mapping() -> Vec { +pub fn default_header_rules() -> Vec { let defaults = include_str!("defaults.toml"); let mapping: HashMap = toml::from_str(defaults).expect("default mapping must be valid"); + // Order is nondeterministic and resolve is first-match-wins; defaults must stay + // collision-free (see test below). mapping .into_values() - .flat_map(|doctype| { - let mut ms = vec![]; + // Drop doctypes matched by neither extension nor filename; they could never match. + .filter(|doctype| doctype.extension || doctype.filename) + .map(|doctype| { + let mut extensions = vec![]; + let mut filenames = vec![]; if doctype.extension { - ms.push(Mapping::Extension { - pattern: doctype.pattern.clone(), - header_type: doctype.header_type.clone(), - }) + extensions.push(doctype.pattern.clone()); } if doctype.filename { - ms.push(Mapping::Filename { - pattern: doctype.pattern, - header_type: doctype.header_type, - }) + filenames.push(doctype.pattern.clone()); + } + HeaderRule { + extensions, + filenames, + styles: vec![doctype.header_type], + existing_strategy: ExistingStrategy::Replace, } - ms }) .collect() } + +#[cfg(test)] +mod tests { + use super::*; + + /// Guards that no two default rules claim the same extension or filename. Resolution is + /// order-nondeterministic and first-match-wins, so a clash makes a file's style vary run to + /// run. Compared case-insensitively because `resolve` lowercases. + #[test] + fn default_rules_have_no_colliding_patterns() { + let mut ext_owner: HashMap = HashMap::new(); + let mut name_owner: HashMap = HashMap::new(); + for rule in default_header_rules() { + let style = rule.styles[0].clone(); + for ext in rule.extensions { + if let Some(prev) = ext_owner.insert(ext.to_lowercase(), style.clone()) { + panic!("default extension `{ext}` is claimed by both `{prev}` and `{style}`"); + } + } + for name in rule.filenames { + if let Some(prev) = name_owner.insert(name.to_lowercase(), style.clone()) { + panic!("default filename `{name}` is claimed by both `{prev}` and `{style}`"); + } + } + } + } +} diff --git a/fmt/src/header/parser.rs b/fmt/src/header/parser.rs index 31cb54c1..98701e49 100644 --- a/fmt/src/header/parser.rs +++ b/fmt/src/header/parser.rs @@ -303,6 +303,17 @@ impl FileContent { }) } + /// Build a `FileContent` over an already line-normalized in-memory buffer. Used to + /// re-probe the current content with alternative header styles without touching disk. + pub fn from_content(content: String, filepath: String) -> Self { + Self { + pos: 0, + old_pos: 0, + content, + filepath, + } + } + pub fn reset_to(&mut self, pos: usize) { self.old_pos = pos; self.pos = pos; @@ -339,8 +350,8 @@ impl FileContent { Some(result) } - pub fn content(&self) -> String { - self.content.clone() + pub fn content(&self) -> &str { + &self.content } pub fn insert(&mut self, index: usize, s: &str) { diff --git a/fmt/src/processor.rs b/fmt/src/processor.rs index 71890b7b..a8ce764d 100644 --- a/fmt/src/processor.rs +++ b/fmt/src/processor.rs @@ -26,7 +26,7 @@ use exn::ResultExt; use crate::config::Config; use crate::document::factory::DocumentFactory; -use crate::document::model::default_mapping; +use crate::document::model::default_header_rules; use crate::document::Document; use crate::error::Error; use crate::git; @@ -56,9 +56,10 @@ pub fn check_license_header( ) -> Result<(), Error> { let config = { let name = run_config.display().to_string(); - let config = fs::read_to_string(&run_config) + let raw = fs::read_to_string(&run_config) .or_raise(|| Error::new(format!("cannot load config: {name}")))?; - toml::from_str::(&config) + reject_legacy_config(&raw, &name)?; + toml::from_str::(&raw) .or_raise(|| Error::new(format!("cannot parse config file: {name}")))? }; @@ -89,19 +90,13 @@ pub fn check_license_header( selection.select()? }; - let mapping = { - let mut mapping = config.mapping.clone(); - if config.use_default_mapping { - let default_mapping = default_mapping(); - for m in default_mapping { - if let Some(o) = mapping.get(&m) { - log::warn!("default mapping {m:?} is override by {o:?}"); - continue; - } - mapping.insert(m); - } + let header_rules = { + // User rules take precedence; defaults are appended so first-match wins downstream. + let mut rules = config.headers.clone(); + if config.use_default_headers { + rules.extend(default_header_rules()); } - mapping + rules }; let definitions = { @@ -109,7 +104,7 @@ pub fn check_license_header( for (k, v) in default_headers() { match defs.entry(k) { Entry::Occupied(mut ent) => { - log::warn!("Default header {} is override", ent.key()); + log::warn!("Default header {} is overridden", ent.key()); ent.insert(v); } Entry::Vacant(ent) => { @@ -123,7 +118,7 @@ pub fn check_license_header( for (k, v) in additional_defs { match defs.entry(k) { Entry::Occupied(mut ent) => { - log::warn!("Additional header {} is override", ent.key()); + log::warn!("Additional header {} is overridden", ent.key()); ent.insert(v); } Entry::Vacant(ent) => { @@ -144,12 +139,12 @@ pub fn check_license_header( let git_file_attrs = git::resolve_file_attrs(git_context)?; let document_factory = DocumentFactory::new( - mapping, + header_rules, definitions, config.properties, config.keywords, git_file_attrs, - ); + )?; for file in selected_files { let document = match document_factory.create_document(&file)? { @@ -172,6 +167,24 @@ pub fn check_license_header( Ok(()) } +/// HawkEye 7.0 replaced the `[mapping.STYLE]` / `useDefaultMapping` config with the +/// `[[headers]]` rule list. Detect the removed keys and fail with a migration hint instead +/// of the opaque `unknown field` error that `deny_unknown_fields` would otherwise produce. +fn reject_legacy_config(raw: &str, name: &str) -> Result<(), Error> { + let Ok(toml::Value::Table(table)) = toml::from_str::(raw) else { + return Ok(()); // let the typed parse below surface the real syntax error + }; + if table.contains_key("mapping") || table.contains_key("useDefaultMapping") { + bail!(Error::new(format!( + "config {name} uses the 6.x `[mapping]` / `useDefaultMapping` format, removed in 7.0.\n\ + Replace each `[mapping.STYLE] {{ extensions = [...], filenames = [...] }}` with:\n\ + \n [[headers]]\n extensions = [...]\n filenames = [...]\n styles = [\"STYLE\"]\n existingStrategy = \"replace\"\n\ + \nand replace `useDefaultMapping` with `useDefaultHeaders`. See the README configuration section." + ))); + } + Ok(()) +} + fn load_additional_headers( additional_header: impl AsRef, config: &Config, diff --git a/fmt/src/selection.rs b/fmt/src/selection.rs index c2d5ed0f..ab48c360 100644 --- a/fmt/src/selection.rs +++ b/fmt/src/selection.rs @@ -274,7 +274,7 @@ fn select_files_with_git( } pub const INCLUDES: [&str; 1] = ["**"]; -pub const EXCLUDES: [&str; 140] = [ +pub const EXCLUDES: [&str; 141] = [ // Miscellaneous typical temporary files "**/*~", "**/#*#", @@ -283,6 +283,8 @@ pub const EXCLUDES: [&str; 140] = [ "**/._*", "**/.repository/**", "**/*.lck", + // HawkEye's own atomic-write temp files, leaked on SIGKILL/power loss; skip on re-walk. + "**/.*.hawkeye-*.tmp", // CVS "**/CVS", "**/CVS/**", diff --git a/fmt/tests/content/foreign_style.rs b/fmt/tests/content/foreign_style.rs new file mode 100644 index 00000000..eb78a580 --- /dev/null +++ b/fmt/tests/content/foreign_style.rs @@ -0,0 +1,7 @@ +/* + * Copyright 2099 Foreign Authors. + */ + +pub fn answer() -> i32 { + 42 +} diff --git a/fmt/tests/content/no_header.rs b/fmt/tests/content/no_header.rs new file mode 100644 index 00000000..565b9f1c --- /dev/null +++ b/fmt/tests/content/no_header.rs @@ -0,0 +1,3 @@ +pub fn answer() -> i32 { + 42 +} diff --git a/fmt/tests/tests.rs b/fmt/tests/tests.rs index c23bb369..0ad0b36d 100644 --- a/fmt/tests/tests.rs +++ b/fmt/tests/tests.rs @@ -12,8 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::HashMap; use std::path::Path; +use hawkeye_fmt::config::ExistingStrategy; +use hawkeye_fmt::document::Attributes; +use hawkeye_fmt::document::Document; use hawkeye_fmt::header::model::default_headers; use hawkeye_fmt::header::parser::parse_header; use hawkeye_fmt::header::parser::FileContent; @@ -45,3 +49,53 @@ fn test_two_headers_should_only_remove_the_first() { let content = document.file_content.content(); assert!(content[end_pos..].contains("Copyright 2015 The Prometheus Authors")); } + +fn attributes(name: &str) -> Attributes { + Attributes { + filename: Some(name.to_string()), + disk_file_created_year: None, + git_file_created_year: None, + git_file_modified_year: None, + git_authors: Default::default(), + } +} + +fn rust_document(fixture: &str) -> Document { + let defs = default_headers(); + let preferred = defs.get("doubleslash_style").unwrap().clone(); + let slashstar = defs.get("slashstar_style").unwrap().clone(); + Document::new( + Path::new(fixture).to_path_buf(), + preferred, + vec![slashstar], // SLASHSTAR accepted for removal, so /* */ headers can migrate + vec!["copyright".to_string()], + ExistingStrategy::Replace, + HashMap::new(), + attributes(fixture), + ) + .unwrap() + .unwrap() +} + +#[test] +fn foreign_style_header_detected_and_removed() { + let mut doc = rust_document("tests/content/foreign_style.rs"); + + // The /* */ header is not in the preferred // style, so structural detection misses it, + // but the keyword scan still recognizes the file as already licensed. + assert!(!doc.header_detected()); + assert!(doc.looks_licensed()); + + // Multi-style removal locates the header via the listed SLASHSTAR style and strips it. + assert!(doc.remove_foreign_header()); + assert!(!doc.looks_licensed()); +} + +#[test] +fn unlicensed_file_is_not_detected() { + let mut doc = rust_document("tests/content/no_header.rs"); + + assert!(!doc.header_detected()); + assert!(!doc.looks_licensed()); + assert!(!doc.remove_foreign_header()); +} diff --git a/tests/atomic_inplace/licenserc.toml b/tests/atomic_inplace/licenserc.toml new file mode 100644 index 00000000..172e5b52 --- /dev/null +++ b/tests/atomic_inplace/licenserc.toml @@ -0,0 +1,16 @@ +baseDir = "." +headerPath = "Apache-2.0.txt" + +includes = ["*.rs"] +excludes = ["*.expected"] + +# Exercises the in-place atomic save path (format WITHOUT --dry-run): the header is inserted +# and the file rewritten via temp+rename, not the .formatted sibling the dry-run path writes. +[[headers]] +extensions = ["rs"] +styles = ["DOUBLESLASH_STYLE"] +existingStrategy = "replace" + +[properties] +inceptionYear = 2023 +copyrightOwner = "The CopyrightOwner" diff --git a/tests/atomic_inplace/main.rs b/tests/atomic_inplace/main.rs new file mode 100644 index 00000000..565b9f1c --- /dev/null +++ b/tests/atomic_inplace/main.rs @@ -0,0 +1,3 @@ +pub fn answer() -> i32 { + 42 +} diff --git a/tests/atomic_inplace/main.rs.expected b/tests/atomic_inplace/main.rs.expected new file mode 100644 index 00000000..3724257e --- /dev/null +++ b/tests/atomic_inplace/main.rs.expected @@ -0,0 +1,17 @@ +// Copyright 2023 The CopyrightOwner +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +pub fn answer() -> i32 { + 42 +} diff --git a/tests/bom_issue/licenserc.toml b/tests/bom_issue/licenserc.toml index 84a220c9..0c2ae7a6 100644 --- a/tests/bom_issue/licenserc.toml +++ b/tests/bom_issue/licenserc.toml @@ -5,5 +5,6 @@ excludes = ["*.toml", "*.expected"] additionalHeaders = ["style.toml"] -[mapping.CS_TEST_STYLE] +[[headers]] extensions = ["cs"] +styles = ["CS_TEST_STYLE"] diff --git a/tests/existing_foreign_style_error/licenserc.toml b/tests/existing_foreign_style_error/licenserc.toml new file mode 100644 index 00000000..e32c835c --- /dev/null +++ b/tests/existing_foreign_style_error/licenserc.toml @@ -0,0 +1,15 @@ +baseDir = "." +headerPath = "Apache-2.0.txt" + +includes = ["*.rs"] +excludes = ["*.expected"] + +# An existing header (any style) makes the run fail instead of being touched. +[[headers]] +extensions = ["rs"] +styles = ["DOUBLESLASH_STYLE", "SLASHSTAR_STYLE"] +existingStrategy = "error" + +[properties] +inceptionYear = 2023 +copyrightOwner = "The CopyrightOwner" diff --git a/tests/existing_foreign_style_error/main.rs b/tests/existing_foreign_style_error/main.rs new file mode 100644 index 00000000..f52ba0d5 --- /dev/null +++ b/tests/existing_foreign_style_error/main.rs @@ -0,0 +1,9 @@ +/* + * Copyright 2099 The Old Authors, in a non-configured comment style. + */ + +//! crate docs + +pub fn answer() -> i32 { + 42 +} diff --git a/tests/existing_foreign_style_replace/licenserc.toml b/tests/existing_foreign_style_replace/licenserc.toml new file mode 100644 index 00000000..b1228828 --- /dev/null +++ b/tests/existing_foreign_style_replace/licenserc.toml @@ -0,0 +1,16 @@ +baseDir = "." +headerPath = "Apache-2.0.txt" + +includes = ["*.rs"] +excludes = ["*.expected"] + +# `.rs` prefers DOUBLESLASH; a SLASHSTAR header is accepted for removal so it migrates +# instead of being duplicated. +[[headers]] +extensions = ["rs"] +styles = ["DOUBLESLASH_STYLE", "SLASHSTAR_STYLE"] +existingStrategy = "replace" + +[properties] +inceptionYear = 2023 +copyrightOwner = "The CopyrightOwner" diff --git a/tests/existing_foreign_style_replace/main.rs b/tests/existing_foreign_style_replace/main.rs new file mode 100644 index 00000000..f52ba0d5 --- /dev/null +++ b/tests/existing_foreign_style_replace/main.rs @@ -0,0 +1,9 @@ +/* + * Copyright 2099 The Old Authors, in a non-configured comment style. + */ + +//! crate docs + +pub fn answer() -> i32 { + 42 +} diff --git a/tests/existing_foreign_style_replace/main.rs.expected b/tests/existing_foreign_style_replace/main.rs.expected new file mode 100644 index 00000000..7c70189f --- /dev/null +++ b/tests/existing_foreign_style_replace/main.rs.expected @@ -0,0 +1,19 @@ +// Copyright 2023 The CopyrightOwner +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! crate docs + +pub fn answer() -> i32 { + 42 +} diff --git a/tests/existing_foreign_style_replace_mixed/licenserc.toml b/tests/existing_foreign_style_replace_mixed/licenserc.toml new file mode 100644 index 00000000..145c13bc --- /dev/null +++ b/tests/existing_foreign_style_replace_mixed/licenserc.toml @@ -0,0 +1,16 @@ +baseDir = "." +headerPath = "Apache-2.0.txt" + +includes = ["*.rs"] +excludes = ["*.expected"] + +# `.rs` prefers DOUBLESLASH; a SLASHSTAR header is accepted for removal. A file may stack a +# block in each style; format must strip BOTH and leave exactly one preferred header (#210). +[[headers]] +extensions = ["rs"] +styles = ["DOUBLESLASH_STYLE", "SLASHSTAR_STYLE"] +existingStrategy = "replace" + +[properties] +inceptionYear = 2023 +copyrightOwner = "The CopyrightOwner" diff --git a/tests/existing_foreign_style_replace_mixed/main.rs b/tests/existing_foreign_style_replace_mixed/main.rs new file mode 100644 index 00000000..a03d93d4 --- /dev/null +++ b/tests/existing_foreign_style_replace_mixed/main.rs @@ -0,0 +1,11 @@ +/* + * Copyright 2099 The Old Authors, in a non-configured comment style. + */ + +// Copyright 2015 The Older Authors, in the preferred comment style. + +//! crate docs + +pub fn answer() -> i32 { + 42 +} diff --git a/tests/existing_foreign_style_replace_mixed/main.rs.expected b/tests/existing_foreign_style_replace_mixed/main.rs.expected new file mode 100644 index 00000000..7c70189f --- /dev/null +++ b/tests/existing_foreign_style_replace_mixed/main.rs.expected @@ -0,0 +1,19 @@ +// Copyright 2023 The CopyrightOwner +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! crate docs + +pub fn answer() -> i32 { + 42 +} diff --git a/tests/existing_foreign_style_replace_mixed_unlisted/licenserc.toml b/tests/existing_foreign_style_replace_mixed_unlisted/licenserc.toml new file mode 100644 index 00000000..29498edc --- /dev/null +++ b/tests/existing_foreign_style_replace_mixed_unlisted/licenserc.toml @@ -0,0 +1,18 @@ +baseDir = "." +headerPath = "Apache-2.0.txt" + +includes = ["*.rs"] +excludes = ["*.expected"] + +# `.rs` lists DOUBLESLASH (preferred) + SLASHSTAR (removable). The SLASHSTAR block at the top +# is stripped on replace, but the hash-comment notice below is in an UNLISTED style and +# survives. format must NOT insert a // header on top of the survivor (the #210 residual-dup +# edge); it reports a controlled failure (foreign) and writes nothing. +[[headers]] +extensions = ["rs"] +styles = ["DOUBLESLASH_STYLE", "SLASHSTAR_STYLE"] +existingStrategy = "replace" + +[properties] +inceptionYear = 2023 +copyrightOwner = "The CopyrightOwner" diff --git a/tests/existing_foreign_style_replace_mixed_unlisted/main.rs b/tests/existing_foreign_style_replace_mixed_unlisted/main.rs new file mode 100644 index 00000000..8ec8af9d --- /dev/null +++ b/tests/existing_foreign_style_replace_mixed_unlisted/main.rs @@ -0,0 +1,9 @@ +/* + * Copyright 2099 Vendor, in the listed SLASHSTAR style (removed on replace). + */ + +# Copyright 2015 Project, in an unlisted hash-comment style. + +pub fn answer() -> i32 { + 42 +} diff --git a/tests/existing_foreign_style_skip/licenserc.toml b/tests/existing_foreign_style_skip/licenserc.toml new file mode 100644 index 00000000..6e38d5d3 --- /dev/null +++ b/tests/existing_foreign_style_skip/licenserc.toml @@ -0,0 +1,15 @@ +baseDir = "." +headerPath = "Apache-2.0.txt" + +includes = ["*.rs"] +excludes = ["*.expected"] + +# An existing header (any style) is left untouched and reported, never duplicated. +[[headers]] +extensions = ["rs"] +styles = ["DOUBLESLASH_STYLE", "SLASHSTAR_STYLE"] +existingStrategy = "skip" + +[properties] +inceptionYear = 2023 +copyrightOwner = "The CopyrightOwner" diff --git a/tests/existing_foreign_style_skip/main.rs b/tests/existing_foreign_style_skip/main.rs new file mode 100644 index 00000000..f52ba0d5 --- /dev/null +++ b/tests/existing_foreign_style_skip/main.rs @@ -0,0 +1,9 @@ +/* + * Copyright 2099 The Old Authors, in a non-configured comment style. + */ + +//! crate docs + +pub fn answer() -> i32 { + 42 +} diff --git a/tests/existing_foreign_style_unlisted/licenserc.toml b/tests/existing_foreign_style_unlisted/licenserc.toml new file mode 100644 index 00000000..c99beed0 --- /dev/null +++ b/tests/existing_foreign_style_unlisted/licenserc.toml @@ -0,0 +1,17 @@ +baseDir = "." +headerPath = "Apache-2.0.txt" + +includes = ["*.rs"] +excludes = ["*.expected"] + +# `.rs` lists ONLY DOUBLESLASH. A header in the unlisted SLASHSTAR style cannot be migrated, +# so under existingStrategy=replace it lands in the `foreign` bucket: format must report a +# controlled failure and must NOT stack a second // header on top (#210). +[[headers]] +extensions = ["rs"] +styles = ["DOUBLESLASH_STYLE"] +existingStrategy = "replace" + +[properties] +inceptionYear = 2023 +copyrightOwner = "The CopyrightOwner" diff --git a/tests/existing_foreign_style_unlisted/main.rs b/tests/existing_foreign_style_unlisted/main.rs new file mode 100644 index 00000000..49f04d07 --- /dev/null +++ b/tests/existing_foreign_style_unlisted/main.rs @@ -0,0 +1,9 @@ +/* + * Copyright 2099 The Old Authors, in a comment style this rule does not list. + */ + +//! crate docs + +pub fn answer() -> i32 { + 42 +} diff --git a/tests/it.py b/tests/it.py index 22e5be2d..8b4e9a4b 100755 --- a/tests/it.py +++ b/tests/it.py @@ -19,6 +19,7 @@ import subprocess import os import shutil +import tempfile import datetime def diff_files(file1, file2): @@ -74,9 +75,87 @@ def drive(name, files, create_temp_copy=False): if os.path.exists(case_dir / expected_file): os.remove(case_dir / expected_file) +def drive_no_change(name, files): + """Run format --dry-run and assert the files are left untouched (no .formatted written).""" + case_dir = basedir / name + subprocess.run([hawkeye, "format", "--fail-if-unknown", "--fail-if-updated=false", "--dry-run"], cwd=case_dir, check=True) + for file in files: + formatted = case_dir / f"{file}.formatted" + if formatted.exists(): + print(f"{name}: expected no change for {file}, but {formatted} was produced") + formatted.unlink() + exit(1) + + +def drive_expect_failure(name, args, expect_stderr=None): + """Run hawkeye with args and assert a non-zero exit (e.g. existingStrategy = error). + If expect_stderr is given, also assert that text appears on stderr.""" + case_dir = basedir / name + result = subprocess.run([hawkeye, *args], cwd=case_dir, check=False, capture_output=expect_stderr is not None, text=True) + if result.returncode == 0: + print(f"{name}: expected non-zero exit from {args}, got 0") + exit(1) + if expect_stderr is not None and expect_stderr not in result.stderr: + print(f"{name}: expected stderr to contain {expect_stderr!r}, got:\n{result.stderr}") + exit(1) + + +def drive_expect_failure_no_change(name, files): + """Run format --dry-run and assert BOTH a non-zero exit AND that no .formatted is written + (a controlled failure that must not stack a duplicate header). Covers the unlisted-style + foreign path and the stray-keyword path.""" + case_dir = basedir / name + result = subprocess.run([hawkeye, "format", "--fail-if-unknown", "--dry-run"], cwd=case_dir, check=False) + failed = False + if result.returncode == 0: + print(f"{name}: expected non-zero exit from format, got 0") + failed = True + for file in files: + formatted = case_dir / f"{file}.formatted" + if formatted.exists(): + print(f"{name}: expected no change for {file}, but {formatted} was produced (duplicate header?)") + formatted.unlink() + failed = True + if failed: + exit(1) + + +def drive_in_place(name, files): + """Exercise the non-dry-run atomic save path: copy the fixture (source + config) into a + throwaway temp dir, run `format` WITHOUT --dry-run so the file is rewritten in place via + temp+rename, then diff the rewritten file against its .expected. Hermetic: never dirties + the repo tree.""" + case_dir = basedir / name + with tempfile.TemporaryDirectory(prefix=f"hawkeye-it-{name}-") as tmp: + tmp = Path(tmp) + shutil.copy2(case_dir / "licenserc.toml", tmp / "licenserc.toml") + for file in files: + shutil.copy2(case_dir / file, tmp / file) + subprocess.run([hawkeye, "format", "--fail-if-unknown", "--fail-if-updated=false"], cwd=tmp, check=True) + for file in files: + diff_files(case_dir / f"{file}.expected", tmp / file) + + drive("attrs_and_props", ["main.rs"]) drive("load_header_path", ["main.rs"]) drive("bom_issue", ["headless_bom.cs"]) drive("regression_blank_line", ["main.rs"]) drive("regression_no_blank_lines", ["repro.py"]) drive("disk_file_created_year", ["main.rs"], True) +drive("existing_foreign_style_replace", ["main.rs"]) +drive("existing_foreign_style_replace_mixed", ["main.rs"]) +drive_no_change("existing_foreign_style_skip", ["main.rs"]) +drive_expect_failure("existing_foreign_style_error", ["format", "--dry-run"]) +# A header in an unlisted comment style cannot be normalized: controlled failure, no duplicate. +drive_expect_failure_no_change("existing_foreign_style_unlisted", ["main.rs"]) +# Form-1 residual dup (#210): a listed-style block ABOVE an unlisted-style notice. Stripping the +# listed block leaves the unlisted notice; format must re-detect it and fail, not stack on top. +drive_expect_failure_no_change("existing_foreign_style_replace_mixed_unlisted", ["main.rs"]) +# A stray whole-word keyword in code looks licensed: controlled failure, no header inserted. +drive_expect_failure_no_change("keyword_in_code", ["main.rs"]) +# Word-boundary fix: `copyrightHolder` is not a notice, so the header is inserted normally. +drive("keyword_identifier_no_header", ["main.rs"]) +# 6.x [mapping]/useDefaultMapping config is rejected with a migration hint. +drive_expect_failure("legacy_mapping_config", ["format", "--dry-run"], expect_stderr="useDefaultHeaders") +# Non-dry-run in-place atomic write path. +drive_in_place("atomic_inplace", ["main.rs"]) diff --git a/tests/keyword_identifier_no_header/licenserc.toml b/tests/keyword_identifier_no_header/licenserc.toml new file mode 100644 index 00000000..2be360d7 --- /dev/null +++ b/tests/keyword_identifier_no_header/licenserc.toml @@ -0,0 +1,16 @@ +baseDir = "." +headerPath = "Apache-2.0.txt" + +includes = ["*.rs"] +excludes = ["*.expected"] + +# The word-boundary fix: `copyright` appears only inside the identifier `copyrightHolder`, which +# is NOT a license notice, so the file is treated as header-less and gets the preferred header. +[[headers]] +extensions = ["rs"] +styles = ["DOUBLESLASH_STYLE"] +existingStrategy = "replace" + +[properties] +inceptionYear = 2023 +copyrightOwner = "The CopyrightOwner" diff --git a/tests/keyword_identifier_no_header/main.rs b/tests/keyword_identifier_no_header/main.rs new file mode 100644 index 00000000..002e8358 --- /dev/null +++ b/tests/keyword_identifier_no_header/main.rs @@ -0,0 +1,3 @@ +pub struct Config { + copyrightHolder: String, +} diff --git a/tests/keyword_identifier_no_header/main.rs.expected b/tests/keyword_identifier_no_header/main.rs.expected new file mode 100644 index 00000000..bb150a10 --- /dev/null +++ b/tests/keyword_identifier_no_header/main.rs.expected @@ -0,0 +1,17 @@ +// Copyright 2023 The CopyrightOwner +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +pub struct Config { + copyrightHolder: String, +} diff --git a/tests/keyword_in_code/licenserc.toml b/tests/keyword_in_code/licenserc.toml new file mode 100644 index 00000000..1624a783 --- /dev/null +++ b/tests/keyword_in_code/licenserc.toml @@ -0,0 +1,17 @@ +baseDir = "." +headerPath = "Apache-2.0.txt" + +includes = ["*.rs"] +excludes = ["*.expected"] + +# Single-style rule. The source has no header but uses the word `copyright` as a whole word in +# a string literal, so the keyword scan classifies it foreign. Under replace this is a +# controlled failure (nonzero exit), and no header is inserted. +[[headers]] +extensions = ["rs"] +styles = ["DOUBLESLASH_STYLE"] +existingStrategy = "replace" + +[properties] +inceptionYear = 2023 +copyrightOwner = "The CopyrightOwner" diff --git a/tests/keyword_in_code/main.rs b/tests/keyword_in_code/main.rs new file mode 100644 index 00000000..e2780d7f --- /dev/null +++ b/tests/keyword_in_code/main.rs @@ -0,0 +1,3 @@ +pub fn banner() -> &'static str { + "copyright 2020 Example Corp" +} diff --git a/tests/legacy_mapping_config/licenserc.toml b/tests/legacy_mapping_config/licenserc.toml new file mode 100644 index 00000000..c94a92cf --- /dev/null +++ b/tests/legacy_mapping_config/licenserc.toml @@ -0,0 +1,16 @@ +baseDir = "." +headerPath = "Apache-2.0.txt" + +includes = ["*.rs"] +excludes = ["*.expected"] + +# 6.x-era config: [mapping.STYLE] + useDefaultMapping were removed in 7.0. The run must reject +# this with a migration hint instead of an opaque parse error. +useDefaultMapping = true + +[mapping.DOUBLESLASH_STYLE] +extensions = ["rs"] + +[properties] +inceptionYear = 2023 +copyrightOwner = "The CopyrightOwner" diff --git a/tests/legacy_mapping_config/main.rs b/tests/legacy_mapping_config/main.rs new file mode 100644 index 00000000..565b9f1c --- /dev/null +++ b/tests/legacy_mapping_config/main.rs @@ -0,0 +1,3 @@ +pub fn answer() -> i32 { + 42 +}