diff --git a/Cargo.lock b/Cargo.lock index 80f353a..7246e94 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -82,6 +82,12 @@ dependencies = [ "wait-timeout", ] +[[package]] +name = "bitflags" +version = "2.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ded4057c258ba199e2d26386d3af3780957ecaee6c4ef4041c6b4b8b97c0b06" + [[package]] name = "bstr" version = "1.13.1" @@ -103,6 +109,12 @@ dependencies = [ "shlex", ] +[[package]] +name = "cfg-if" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" + [[package]] name = "clap" version = "4.6.7" @@ -159,6 +171,7 @@ dependencies = [ "compact-tree-sitter", "globset", "serde", + "tempfile", "thiserror", "toml", "tree-sitter", @@ -186,12 +199,39 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" +[[package]] +name = "errno" +version = "0.3.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" +dependencies = [ + "libc", + "windows-sys", +] + +[[package]] +name = "fastrand" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" + [[package]] name = "find-msvc-tools" version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3e0f1c7c3a72c66fd80abe965175f7523475c0489a87d3ff9d6e8c87d87a9d2d" +[[package]] +name = "getrandom" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "300e883d756b2e4ec94e02791f39b04b522276138852cfc41d9fb7e904106099" +dependencies = [ + "cfg-if", + "libc", + "r-efi", +] + [[package]] name = "globset" version = "0.4.20" @@ -245,6 +285,12 @@ version = "0.2.189" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" +[[package]] +name = "linux-raw-sys" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" + [[package]] name = "log" version = "0.4.34" @@ -257,6 +303,12 @@ version = "2.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" +[[package]] +name = "once_cell" +version = "1.21.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" + [[package]] name = "once_cell_polyfill" version = "1.70.2" @@ -308,6 +360,12 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "r-efi" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" + [[package]] name = "regex" version = "1.13.1" @@ -337,6 +395,19 @@ version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" +[[package]] +name = "rustix" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190" +dependencies = [ + "bitflags", + "errno", + "libc", + "linux-raw-sys", + "windows-sys", +] + [[package]] name = "same-file" version = "1.0.6" @@ -428,6 +499,19 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tempfile" +version = "3.27.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" +dependencies = [ + "fastrand", + "getrandom", + "once_cell", + "rustix", + "windows-sys", +] + [[package]] name = "termtree" version = "0.5.1" diff --git a/Cargo.toml b/Cargo.toml index 5e096be..753653f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,3 +25,4 @@ tree-sitter-language = "0.1" walkdir = "2.5" assert_cmd = "2.2" +tempfile = "3.27" diff --git a/crates/compact-lint/Cargo.toml b/crates/compact-lint/Cargo.toml index 0983475..384ce97 100644 --- a/crates/compact-lint/Cargo.toml +++ b/crates/compact-lint/Cargo.toml @@ -31,3 +31,4 @@ walkdir.workspace = true [dev-dependencies] assert_cmd.workspace = true +tempfile.workspace = true diff --git a/crates/compact-lint/README.md b/crates/compact-lint/README.md index 28bd628..5f2cfd6 100644 --- a/crates/compact-lint/README.md +++ b/crates/compact-lint/README.md @@ -2,12 +2,14 @@ Doc-comment linter for Compact sources. It parses `.compact` files with [`compact-tree-sitter`](../compact-tree-sitter), matches every declaration against the -per-kind template in `compact-lint.toml`, and prints one finding per line. +per-kind template in `compact-lint.toml`, and prints one finding per line. `fix` +rewrites the comments the repairable rules report. ## Usage ```sh compact-lint check [PATHS]... [--config ] [--strict] [--no-format] [--compact-bin ] +compact-lint fix [PATHS]... [--config ] [--dry-run] ``` - `PATHS` — files or directories, walked recursively for `.compact`. Skipped while @@ -32,6 +34,52 @@ summary goes to stderr, where `M` is the number of files checked. - `1` — findings. - `2` — usage, config, IO or parser error. +## Fix + +`fix` rewrites `.compact` files so a following `check` is clean for every rule it +covers. It runs the same detection pass as `check`, so the two never disagree. + +```sh +compact-lint fix [PATHS]... [--config ] [--dry-run] +``` + +- `PATHS` and `--config` resolve exactly as they do for `check`. +- `--dry-run` — report the edits and write nothing. Exit `1` when there are edits, so + CI gates on it the way it gates on `cargo fmt --check`. + +Edits go to stdout, one per line, in `path:line:col: fix: ` form. The +`N edits in M files` summary goes to stderr, where `M` is the number of files changed. + +Exit codes: + +- `0` — the files were written, including when there was nothing to do. +- `1` — `--dry-run` only: edits would be made. +- `2` — usage, config, IO or parser error. + +What each rule's fix does: + +| Rule | Fix | +| --- | --- | +| `missing-doc` | Inserts a doc skeleton above the declaration, indented to it. Body lines come from `kinds..tags` in config order; `@module` takes the declaration's name, every other tag takes `fix.placeholder`. Exported non-pure circuits also get the constraints line. A kind with neither gets a single placeholder line. | +| `missing-tag` | Inserts each missing tag after the block of the last tag that precedes it in `kinds..tags`, else at the top of the body. A missing `@description` adopts untagged prose that opens the body instead of writing a placeholder. A `/** … */` one-liner expands to the multi-line form first. | +| `missing-constraints` | Inserts ` k=?, rows=?` after the `@description` block, with a blank ` *` line before it and, where prose follows, one after. Without a `@description` it opens the body. | +| `forbidden-tag` | Renames the tag in place, and only where `tags.rename` maps it. | +| `module-name` | Replaces the first word of the `@module` value with the module's name, keeping what follows. | + +Not fixed: + +- `constraints-format` — a human wrote a measurement the rule cannot parse; guessing at + it would lose the value. +- `constraints-placeholder` — filling `k=?` needs a real measurement. +- `parse` — the file does not parse, so no rewrite is trustworthy. +- `format` — that is `compact format`'s job. +- A forbidden tag with no `tags.rename` entry stays a `check` finding. + +Existing lines are never reordered or reflowed. Edits are computed against the original +text and applied from the highest offset down; several edits on one comment merge into a +single replacement of it. Files are rewritten through a sibling `.tmp` file and renamed +into place, keeping the file's line ending and its trailing newline, or lack of one. + ## Rules | Rule | Meaning | @@ -79,6 +127,10 @@ tag = "@constraints" [tags] forbid = ["@return"] +rename = { "@return" = "@returns" } + +[fix] +placeholder = "TODO" [kinds.module] docs = "exported" @@ -94,6 +146,8 @@ Defaults when no config file is found: | `exclude` | `[]` | | `constraints.tag` | `"@constraints"` | | `tags.forbid` | `[]` | +| `tags.rename` | `{}` | +| `fix.placeholder` | `"TODO"` | | `kinds..docs` | `"exported"` | | `kinds..tags` | `[]` | @@ -105,6 +159,11 @@ Defaults when no config file is found: `"all"` or `"none"` there. - `exclude` filters files found by walking a directory. A file named on the command line is always checked. +- `tags.rename` maps a forbidden tag to the tag `fix` writes in its place. Every key + must appear in `tags.forbid` and no value may, or the config is rejected. Both + spellings are validated like every other tag. An unmapped forbidden tag is reported, + never rewritten. +- `fix.placeholder` is the text `fix` writes where it has no value of its own. `examples/compact-contracts.toml` is the config for OpenZeppelin/compact-contracts. @@ -123,6 +182,10 @@ Defaults when no config file is found: cargo test --workspace ``` -`tests/fixtures//` holds one case per rule plus a clean case: a +`tests/fixtures//` holds one `check` case per rule plus a clean case: a `compact-lint.toml`, the `.compact` sources, and `expected.txt`, the exact stdout with paths relative to the case directory. + +`tests/fixtures/fix-/` holds one `fix` case per rule: a `compact-lint.toml`, +`before/`, `after/` and `expected.txt`. The run copies `before/` into a temporary +directory, so the fixtures are never rewritten in place. diff --git a/crates/compact-lint/examples/compact-contracts.toml b/crates/compact-lint/examples/compact-contracts.toml index 402186d..d20c3a9 100644 --- a/crates/compact-lint/examples/compact-contracts.toml +++ b/crates/compact-lint/examples/compact-contracts.toml @@ -11,6 +11,7 @@ tag = "@constraints" [tags] forbid = ["@return"] +rename = { "@return" = "@returns" } [kinds.module] docs = "exported" diff --git a/crates/compact-lint/src/check.rs b/crates/compact-lint/src/check.rs index 76ee393..7fbc195 100644 --- a/crates/compact-lint/src/check.rs +++ b/crates/compact-lint/src/check.rs @@ -1,22 +1,19 @@ -//! The `check` subcommand: resolve config, collect files, run the rules. +//! The `check` subcommand: resolve the target, run the rules, add the format pass. use std::ffi::OsString; use std::path::{Path, PathBuf}; use thiserror::Error; -use crate::config::{Config, ConfigError, discover as discover_config}; -use crate::discover::{self, DiscoverError}; use crate::format::{self, FormatError}; use crate::report::Finding; use crate::rules::{LintError, Linter}; +use crate::target::{self, TargetError}; #[derive(Debug, Error)] pub enum CheckError { #[error(transparent)] - Config(#[from] ConfigError), - #[error(transparent)] - Discover(#[from] DiscoverError), + Target(#[from] TargetError), #[error(transparent)] Lint(#[from] LintError), #[error(transparent)] @@ -59,95 +56,25 @@ impl Outcome { /// # Errors /// Returns an error when the config, the file walk, the parser or the formatter fails. pub fn run(options: &Options, cwd: &Path) -> Result { - let (config, config_path, base) = resolve_config(options, cwd)?; - let source = config_path.as_deref().unwrap_or(Path::new("")); - - let exclude = config.exclude_set(source)?; - let files = if options.paths.is_empty() { - let include = config.include_set(source)?; - discover::from_globs(&base, &include, &exclude)? - } else { - let paths = absolute_paths(&options.paths, cwd); - discover::from_paths(&paths, &base, &exclude)? - }; - let files: Vec = files.iter().map(|path| display_path(path, cwd)).collect(); + let target = target::resolve(&options.paths, options.config_path.as_deref(), cwd)?; let mut linter = Linter::new()?; let mut findings = Vec::new(); - for path in &files { + for path in &target.files { let text = std::fs::read_to_string(path).map_err(|source| CheckError::Read { path: path.clone(), source, })?; - findings.extend(linter.check(path, &text, &config, options.strict)?); + findings.extend(linter.check(path, &text, &target.config, options.strict)?); } if !options.no_format { - findings.extend(format::check(&options.compact_bin, &files)?); + findings.extend(format::check(&options.compact_bin, &target.files)?); } findings.sort_by(|left, right| left.sort_key().cmp(&right.sort_key())); Ok(Outcome { findings, - files_checked: files.len(), + files_checked: target.files.len(), }) } - -/// An explicit `--config` is a shareable preset, so glob discovery stays anchored at -/// `cwd`; a config found by walking upward anchors discovery at its own directory. -fn resolve_config( - options: &Options, - cwd: &Path, -) -> Result<(Config, Option, PathBuf), ConfigError> { - if let Some(path) = options.config_path.as_deref() { - return Ok((Config::load(path)?, Some(path.to_owned()), cwd.to_owned())); - } - - match discover_config(cwd) { - Some(path) => { - let base = path.parent().unwrap_or(cwd).to_owned(); - Ok((Config::load(&path)?, Some(path), base)) - } - None => Ok((Config::default(), None, cwd.to_owned())), - } -} - -/// Discovery matches the exclude globs against paths relative to the config's -/// directory, so a CLI path has to be anchored at `cwd` first: a bare `archive` -/// given from a subdirectory would otherwise never match `contracts/archive/**`. -fn absolute_paths(paths: &[PathBuf], cwd: &Path) -> Vec { - paths - .iter() - .map(|path| { - if path.is_absolute() { - path.clone() - } else { - cwd.join(path) - } - }) - .collect() -} - -/// Paths under the working directory print relative to it; anything else prints as is. -fn display_path(path: &Path, cwd: &Path) -> PathBuf { - path.strip_prefix(cwd).unwrap_or(path).to_owned() -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn a_relative_path_is_anchored_at_the_working_directory() { - let resolved = absolute_paths(&[PathBuf::from("archive")], Path::new("/repo/contracts")); - - assert_eq!(resolved, vec![PathBuf::from("/repo/contracts/archive")]); - } - - #[test] - fn an_absolute_path_is_left_alone() { - let resolved = absolute_paths(&[PathBuf::from("/elsewhere/X.compact")], Path::new("/repo")); - - assert_eq!(resolved, vec![PathBuf::from("/elsewhere/X.compact")]); - } -} diff --git a/crates/compact-lint/src/config.rs b/crates/compact-lint/src/config.rs index 0d7ae2b..3bddff2 100644 --- a/crates/compact-lint/src/config.rs +++ b/crates/compact-lint/src/config.rs @@ -1,5 +1,6 @@ //! `compact-lint.toml`: discovery, parsing, and the built-in defaults. +use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use globset::{Glob, GlobSet, GlobSetBuilder}; @@ -37,6 +38,20 @@ pub enum ConfigError { "config {path} lists tag {tag:?}; tags are spelled with their `@`, like `@description`" )] Tag { path: PathBuf, tag: String }, + #[error("config {path} renames {tag}, which tags.forbid does not list")] + RenameUnforbidden { path: PathBuf, tag: String }, + #[error("config {path} renames {from} to {to}, which tags.forbid also lists")] + RenameToForbidden { + path: PathBuf, + from: String, + to: String, + }, + #[error("config {path} has an invalid fix.placeholder {placeholder:?}: {reason}")] + Placeholder { + path: PathBuf, + placeholder: String, + reason: &'static str, + }, #[error("config {path} has an invalid glob {pattern:?}")] Glob { path: PathBuf, @@ -117,11 +132,28 @@ impl Default for ConstraintsConfig { } } -/// Tags no doc comment may carry. +/// Tags no doc comment may carry, and the replacements `fix` writes for them. #[derive(Clone, Debug, Default, Deserialize, PartialEq, Eq)] #[serde(deny_unknown_fields, default)] pub struct TagsConfig { pub forbid: Vec, + /// Forbidden tag to its replacement; an unmapped forbidden tag stays a finding. + pub rename: BTreeMap, +} + +/// What `fix` writes where it has no value to write. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq)] +#[serde(deny_unknown_fields, default)] +pub struct FixConfig { + pub placeholder: String, +} + +impl Default for FixConfig { + fn default() -> Self { + Self { + placeholder: "TODO".to_owned(), + } + } } /// A parsed `compact-lint.toml`, or the built-in defaults when no file was found. @@ -133,6 +165,7 @@ pub struct Config { pub exclude: Vec, pub constraints: ConstraintsConfig, pub tags: TagsConfig, + pub fix: FixConfig, pub kinds: KindsConfig, } @@ -144,6 +177,7 @@ impl Default for Config { exclude: Vec::new(), constraints: ConstraintsConfig::default(), tags: TagsConfig::default(), + fix: FixConfig::default(), kinds: KindsConfig::default(), } } @@ -178,6 +212,30 @@ impl Config { }); } + for (from, to) in &config.tags.rename { + if !config.tags.forbid.contains(from) { + return Err(ConfigError::RenameUnforbidden { + path: path.to_owned(), + tag: from.to_string(), + }); + } + if config.tags.forbid.contains(to) { + return Err(ConfigError::RenameToForbidden { + path: path.to_owned(), + from: from.to_string(), + to: to.to_string(), + }); + } + } + + if let Some(reason) = placeholder_defect(&config.fix.placeholder) { + return Err(ConfigError::Placeholder { + path: path.to_owned(), + placeholder: config.fix.placeholder.clone(), + reason, + }); + } + Ok(config) } @@ -199,9 +257,17 @@ impl Config { per_kind .chain(self.tags.forbid.iter()) + .chain(self.tags.rename.keys()) + .chain(self.tags.rename.values()) .chain(std::iter::once(&self.constraints.tag)) } + /// The replacement `fix` writes for a forbidden tag, when the config names one. + #[must_use] + pub fn rename_of(&self, tag: &Tag) -> Option<&Tag> { + self.tags.rename.get(tag) + } + /// Compiles the `include` globs. /// # Errors /// Returns an error when a pattern is not a valid glob. @@ -217,6 +283,20 @@ impl Config { } } +/// Why a placeholder cannot go on a `*` line of a doc comment, or `None` when it can. +fn placeholder_defect(placeholder: &str) -> Option<&'static str> { + if placeholder.trim().is_empty() { + return Some("it is empty"); + } + if placeholder.contains(['\n', '\r']) { + return Some("it spans more than one line"); + } + if placeholder.contains("*/") { + return Some("`*/` would close the comment early"); + } + None +} + fn glob_set(patterns: &[String], path: &Path) -> Result { let mut builder = GlobSetBuilder::new(); for pattern in patterns { @@ -248,9 +328,21 @@ pub fn discover(start: &Path) -> Option { #[cfg(test)] mod tests { - use super::{Config, DocsPolicy, SUPPORTED_VERSION}; + use super::{CONFIG_FILE_NAME, Config, DocsPolicy, SUPPORTED_VERSION}; use crate::doc::Tag; use crate::model::DeclKind; + use tempfile::TempDir; + + /// Loads `snippet` as a config file and returns the rejection it produced. + fn rejection(snippet: &str) -> String { + let directory = TempDir::new().expect("a temporary directory is available"); + let path = directory.path().join(CONFIG_FILE_NAME); + std::fs::write(&path, snippet).expect("the config is writable"); + + Config::load(&path) + .expect_err("the config is invalid") + .to_string() + } #[test] fn defaults_require_docs_on_exported_declarations_only() { @@ -295,6 +387,81 @@ mod tests { std::fs::remove_dir_all(&dir).expect("the temp dir is removable"); } + #[test] + fn a_rename_maps_a_forbidden_tag_to_its_replacement() { + let config: Config = toml::from_str( + "[tags]\nforbid = [\"@return\"]\nrename = { \"@return\" = \"@returns\" }\n", + ) + .expect("the snippet is valid config"); + + assert_eq!( + config.rename_of(&Tag::new("@return")), + Some(&Tag::new("@returns")) + ); + assert_eq!(config.rename_of(&Tag::new("@notice")), None); + } + + #[test] + fn a_rename_target_without_its_at_sign_is_rejected() { + let dir = std::env::temp_dir().join(format!("compact-lint-rename-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("the temp dir is writable"); + let path = dir.join("compact-lint.toml"); + std::fs::write(&path, "[tags]\nrename = { \"@return\" = \"returns\" }\n") + .expect("the config is writable"); + + let error = Config::load(&path).expect_err("the replacement lacks its @"); + + assert!(error.to_string().contains("\"returns\""), "{error}"); + std::fs::remove_dir_all(&dir).expect("the temp dir is removable"); + } + + #[test] + fn the_fix_placeholder_defaults_to_todo() { + assert_eq!(Config::default().fix.placeholder, "TODO"); + + let config: Config = toml::from_str("[fix]\nplaceholder = \"FIXME\"\n") + .expect("the snippet is valid config"); + assert_eq!(config.fix.placeholder, "FIXME"); + } + + #[test] + fn a_rename_of_a_tag_that_is_not_forbidden_is_rejected() { + let error = rejection("[tags]\nrename = { \"@return\" = \"@returns\" }\n"); + + assert!(error.contains("@return"), "{error}"); + assert!(error.contains("tags.forbid does not list"), "{error}"); + } + + #[test] + fn a_rename_onto_a_forbidden_tag_is_rejected() { + let error = rejection( + "[tags]\nforbid = [\"@return\", \"@returns\"]\nrename = { \"@return\" = \"@returns\" }\n", + ); + + assert!(error.contains("tags.forbid also lists"), "{error}"); + } + + #[test] + fn an_empty_fix_placeholder_is_rejected() { + let error = rejection("[fix]\nplaceholder = \" \"\n"); + + assert!(error.contains("is empty"), "{error}"); + } + + #[test] + fn a_multi_line_fix_placeholder_is_rejected() { + let error = rejection("[fix]\nplaceholder = \"TO\\nDO\"\n"); + + assert!(error.contains("more than one line"), "{error}"); + } + + #[test] + fn a_fix_placeholder_closing_the_comment_is_rejected() { + let error = rejection("[fix]\nplaceholder = \"TODO */\"\n"); + + assert!(error.contains("close the comment"), "{error}"); + } + #[test] fn an_unknown_kind_is_rejected() { let error = diff --git a/crates/compact-lint/src/edit.rs b/crates/compact-lint/src/edit.rs new file mode 100644 index 0000000..281d859 --- /dev/null +++ b/crates/compact-lint/src/edit.rs @@ -0,0 +1,570 @@ +//! Byte-range edits and the doc-comment rewriting `fix` applies. +//! +//! Every edit is computed against the original text. They are applied from the highest +//! offset down, so positions never shift, and edits that target the same doc comment are +//! merged into one replacement of that comment. + +use std::ops::Range; + +use crate::report::Position; + +/// One repair, carrying the line `fix` prints for it. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Edit { + pub position: Position, + pub message: String, + pub kind: EditKind, +} + +/// What an edit rewrites. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum EditKind { + /// Text inserted at a byte offset, used for a whole doc-comment skeleton. + Insert { offset: usize, text: String }, + /// One rewrite of the doc comment spanning `range`. + Doc { + range: Range, + /// Column the `/**` starts on, the indentation of every line the op writes. + indent: usize, + op: DocOp, + }, +} + +/// One change to the lines of a doc comment. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum DocOp { + /// A `@tag value` line added after the last of `after` present, else at the top of the body. + InsertTag { + line: String, + /// Tags that precede this one in config order. + after: Vec, + /// Untagged prose opening the body becomes this tag's value instead of the placeholder. + adopts_prose: bool, + }, + /// A `@tag k=?, rows=?` line added after the `@description` block. + InsertConstraints(String), + /// The tag name on one line replaced in place. + RenameTag { + /// 0-based line index inside the original comment. + line: usize, + from: String, + to: String, + }, + /// The first word of `@module`'s value replaced with the module's name. + SetModuleName { + /// 0-based line index inside the original comment. + line: usize, + name: String, + }, +} + +/// The line ending the file already uses, taken from its first line. +#[must_use] +pub fn newline_of(source: &str) -> &'static str { + match source.find('\n') { + Some(index) if index > 0 && source.as_bytes()[index - 1] == b'\r' => "\r\n", + _ => "\n", + } +} + +/// Rewrites `source` with every edit, merging the edits that share a doc comment. +#[must_use] +pub fn apply(source: &str, edits: &[Edit], newline: &str) -> String { + let mut replacements: Vec<(Range, String)> = Vec::new(); + let mut docs: Vec<(Range, usize, Vec<&DocOp>)> = Vec::new(); + + for edit in edits { + match &edit.kind { + EditKind::Insert { offset, text } => { + replacements.push((*offset..*offset, text.clone())); + } + EditKind::Doc { range, indent, op } => { + match docs.iter_mut().find(|(known, _, _)| known == range) { + Some((_, _, ops)) => ops.push(op), + None => docs.push((range.clone(), *indent, vec![op])), + } + } + } + } + + for (range, indent, ops) in docs { + let original = source.get(range.clone()).unwrap_or_default(); + replacements.push((range, render_doc(original, indent, newline, &ops))); + } + + replacements.sort_by_key(|(range, _)| std::cmp::Reverse(range.start)); + + let mut out = source.to_owned(); + for (range, text) in replacements { + if range.end <= out.len() { + out.replace_range(range, &text); + } + } + out +} + +/// Applies every op to one doc comment and renders it back with `newline`. +#[must_use] +fn render_doc(doc: &str, indent: usize, newline: &str, ops: &[&DocOp]) -> String { + let mut lines: Vec = doc + .split('\n') + .map(|line| line.strip_suffix('\r').unwrap_or(line).to_owned()) + .collect(); + + let margin = " ".repeat(indent); + let gutter = format!("{margin} * "); + + // An insertion needs a body, so a `/** … */` one-liner expands first. + let expands = lines.len() == 1 + && ops + .iter() + .any(|op| matches!(op, DocOp::InsertTag { .. } | DocOp::InsertConstraints(_))); + let shift = usize::from(expands); + if expands { + lines = expand(&lines[0], &margin, &gutter); + } + + // In-place ops address the original lines, so they run before any insertion moves + // them; tags precede the constraints line, which anchors itself on `@description`. + for op in ops { + match op { + DocOp::RenameTag { line, from, to } => { + if let Some(target) = lines.get_mut(line + shift) + && let Some(renamed) = rename_tag(target, from, to) + { + *target = renamed; + } + } + DocOp::SetModuleName { line, name } => { + if let Some(target) = lines.get_mut(line + shift) + && let Some(renamed) = set_module_name(target, name) + { + *target = renamed; + } + } + _ => {} + } + } + + for op in ops { + if let DocOp::InsertTag { + line, + after, + adopts_prose, + } = op + { + insert_tag(&mut lines, &gutter, line, after, *adopts_prose); + } + } + + for op in ops { + if let DocOp::InsertConstraints(body) = op { + insert_constraints(&mut lines, &margin, &gutter, body); + } + } + + lines.join(newline) +} + +/// Splits `/** text */` into the three-line form, keeping the comment's indentation. +fn expand(single: &str, margin: &str, gutter: &str) -> Vec { + let inner = single + .strip_prefix("/**") + .and_then(|rest| rest.strip_suffix("*/")) + .unwrap_or(single) + .trim(); + + vec![ + "/**".to_owned(), + format!("{gutter}{inner}"), + format!("{margin} */"), + ] +} + +/// A tag line lands after the block of the last tag in `after` that is present, else at +/// the top of the body. With `adopts_prose`, untagged prose opening the body takes the +/// tag instead of a new line going in. +fn insert_tag( + lines: &mut Vec, + gutter: &str, + line: &str, + after: &[String], + adopts_prose: bool, +) { + let closer = lines.len().saturating_sub(1); + + if adopts_prose + && let Some(first) = (1..closer).find(|index| !is_blank(&lines[*index])) + && !starts_tag(content(&lines[first])) + && let Some((tag, _)) = line.split_once(' ') + { + lines[first] = format!("{gutter}{tag} {}", content(&lines[first])); + return; + } + + let at = tag_block_end(lines, closer, after).map_or(1.min(closer), |end| end + 1); + lines.insert(at, format!("{gutter}{line}")); +} + +/// The constraints line follows the `@description` block, or opens the body without one. +fn insert_constraints(lines: &mut Vec, margin: &str, gutter: &str, body: &str) { + let closer = lines.len().saturating_sub(1); + let blank = format!("{margin} *"); + + let at = match description_end(lines, closer) { + Some(end) => { + lines.insert(end + 1, blank.clone()); + end + 2 + } + None => 1.min(closer), + }; + + lines.insert(at, format!("{gutter}{body}")); + + let follows = at + 1; + if follows < lines.len() && follows != lines.len() - 1 && !is_blank(&lines[follows]) { + lines.insert(follows, blank); + } +} + +/// The last line of the `@description` block: its tag line plus every continuation. +fn description_end(lines: &[String], closer: usize) -> Option { + tag_block_end(lines, closer, &["@description".to_owned()]) +} + +/// The last line of the block opened by the last occurrence of any tag in `tags`. +fn tag_block_end(lines: &[String], closer: usize, tags: &[String]) -> Option { + let start = (1..closer).rev().find(|index| { + let text = content(&lines[*index]); + tags.iter().any(|tag| { + text.strip_prefix(tag.as_str()) + .is_some_and(|rest| rest.is_empty() || rest.starts_with(char::is_whitespace)) + }) + })?; + + let mut end = start; + while end + 1 < closer { + let next = content(&lines[end + 1]); + if next.is_empty() || starts_tag(next) { + break; + } + end += 1; + } + Some(end) +} + +/// A comment line without its indentation, its `*` and the space after it. +fn content(line: &str) -> &str { + let trimmed = line.trim_start(); + match trimmed.strip_prefix('*') { + Some(rest) => rest.trim(), + None => trimmed.trim(), + } +} + +fn is_blank(line: &str) -> bool { + content(line).is_empty() +} + +fn starts_tag(content: &str) -> bool { + content + .strip_prefix('@') + .is_some_and(|rest| rest.starts_with(|character: char| character.is_ascii_alphabetic())) +} + +/// Replaces the first whole-word occurrence of `from`, so `@return` never claims `@returns`. +fn rename_tag(line: &str, from: &str, to: &str) -> Option { + let bytes = line.as_bytes(); + let mut search = 0; + while let Some(offset) = line.get(search..)?.find(from) { + let start = search + offset; + let end = start + from.len(); + let before_is_word = start > 0 && bytes[start - 1].is_ascii_alphanumeric(); + let after_is_word = bytes.get(end).is_some_and(u8::is_ascii_alphabetic); + if !before_is_word && !after_is_word { + return Some(format!("{}{to}{}", &line[..start], &line[end..])); + } + search = end; + } + None +} + +/// Replaces the first word after `@module`, keeping whatever follows it. +fn set_module_name(line: &str, name: &str) -> Option { + let tag = "@module"; + let start = line.find(tag)?; + let after = start + tag.len(); + let rest = line.get(after..)?; + + let spaces = rest.len() - rest.trim_start().len(); + let value = rest.trim_start(); + let word = value + .find(char::is_whitespace) + .map_or(value.len(), |index| index); + + if value.is_empty() { + return Some(format!("{line} {name}")); + } + Some(format!( + "{}{}{name}{}", + &line[..after], + &rest[..spaces], + &value[word..] + )) +} + +#[cfg(test)] +mod tests { + use super::{ + DocOp, Edit, EditKind, apply, newline_of, rename_tag, render_doc, set_module_name, + }; + use crate::report::Position; + + fn doc_edit(range: std::ops::Range, indent: usize, op: DocOp) -> Edit { + Edit { + position: Position::file_start(), + message: String::new(), + kind: EditKind::Doc { range, indent, op }, + } + } + + fn tag(line: &str, after: &[&str]) -> DocOp { + DocOp::InsertTag { + line: line.to_owned(), + after: after.iter().map(ToString::to_string).collect(), + adopts_prose: line.starts_with("@description"), + } + } + + fn rendered(doc: &str, indent: usize, ops: &[DocOp]) -> String { + let borrowed: Vec<&DocOp> = ops.iter().collect(); + render_doc(doc, indent, "\n", &borrowed) + } + + #[test] + fn a_tag_goes_in_at_the_top_of_the_body() { + let doc = "/**\n * @constraints k=6, rows=28\n */"; + + assert_eq!( + rendered(doc, 2, &[tag("@description TODO", &[])]), + "/**\n * @description TODO\n * @constraints k=6, rows=28\n */" + ); + } + + #[test] + fn several_tags_keep_the_order_they_were_produced_in() { + let doc = "/**\n * @notice loose\n */"; + let ops = [ + tag("@module Ownable", &[]), + tag("@description TODO", &["@module"]), + ]; + + assert_eq!( + rendered(doc, 0, &ops), + "/**\n * @module Ownable\n * @description TODO\n * @notice loose\n */" + ); + } + + #[test] + fn a_tag_lands_after_the_tags_that_precede_it_in_config_order() { + let doc = "/**\n * @module Ownable\n * spanning two lines\n *\n * @notice loose\n */"; + + assert_eq!( + rendered(doc, 0, &[tag("@description TODO", &["@module"])]), + "/**\n * @module Ownable\n * spanning two lines\n * @description TODO\n *\n * @notice loose\n */" + ); + } + + #[test] + fn a_description_adopts_the_prose_that_opens_the_body() { + let doc = "/**\n * Approved accounts.\n * @param {T} a - A.\n */"; + + assert_eq!( + rendered(doc, 0, &[tag("@description TODO", &[])]), + "/**\n * @description Approved accounts.\n * @param {T} a - A.\n */" + ); + assert_eq!( + rendered( + "/** Approved accounts. */", + 2, + &[tag("@description TODO", &[])] + ), + "/**\n * @description Approved accounts.\n */" + ); + } + + #[test] + fn a_single_line_doc_expands_before_a_tag_goes_in() { + let doc = "/** @description Approved accounts. */"; + + assert_eq!( + rendered(doc, 2, &[tag("@module M", &[])]), + "/**\n * @module M\n * @description Approved accounts.\n */" + ); + } + + #[test] + fn constraints_follow_the_description_and_its_continuation_lines() { + let doc = "/**\n * @description Sends a coin\n * to the recipient.\n * @param {Coin} coin - The coin.\n */"; + + assert_eq!( + rendered( + doc, + 0, + &[DocOp::InsertConstraints( + "@constraints k=?, rows=?".to_owned() + )] + ), + "/**\n * @description Sends a coin\n * to the recipient.\n *\n * @constraints k=?, rows=?\n *\n * @param {Coin} coin - The coin.\n */" + ); + } + + #[test] + fn constraints_need_no_trailing_blank_before_the_closer() { + let doc = "/**\n * @description Bumps.\n */"; + + assert_eq!( + rendered( + doc, + 0, + &[DocOp::InsertConstraints( + "@constraints k=?, rows=?".to_owned() + )] + ), + "/**\n * @description Bumps.\n *\n * @constraints k=?, rows=?\n */" + ); + } + + #[test] + fn constraints_open_the_body_when_there_is_no_description() { + let doc = "/**\n * @notice loose prose.\n */"; + + assert_eq!( + rendered( + doc, + 0, + &[DocOp::InsertConstraints( + "@constraints k=?, rows=?".to_owned() + )] + ), + "/**\n * @constraints k=?, rows=?\n *\n * @notice loose prose.\n */" + ); + } + + #[test] + fn a_missing_description_and_missing_constraints_compose_on_one_comment() { + let doc = "/**\n * @param {Uint<8>} value - The value.\n */"; + let composed = "/**\n * @description TODO\n *\n * @constraints k=?, rows=?\n *\n * @param {Uint<8>} value - The value.\n */"; + let tag = tag("@description TODO", &[]); + let constraints = DocOp::InsertConstraints("@constraints k=?, rows=?".to_owned()); + + assert_eq!( + rendered(doc, 0, &[tag.clone(), constraints.clone()]), + composed + ); + // The rule that reports first must not decide where the annotation lands. + assert_eq!(rendered(doc, 0, &[constraints, tag]), composed); + } + + #[test] + fn two_edits_on_one_comment_merge_into_a_single_replacement() { + let source = "/**\n * @return nothing\n */\nexport circuit run(): [] { }\n"; + let edits = [ + doc_edit( + 0..26, + 0, + DocOp::RenameTag { + line: 1, + from: "@return".to_owned(), + to: "@returns".to_owned(), + }, + ), + doc_edit(0..26, 0, tag("@description TODO", &[])), + ]; + + assert_eq!( + apply(source, &edits, "\n"), + "/**\n * @description TODO\n * @returns nothing\n */\nexport circuit run(): [] { }\n" + ); + } + + #[test] + fn an_insertion_and_a_later_comment_edit_both_land() { + let source = + "export ledger a: Uint<8>;\n/**\n * @return x\n */\nexport circuit run(): [] { }\n"; + let edits = [ + Edit { + position: Position::file_start(), + message: String::new(), + kind: EditKind::Insert { + offset: 0, + text: "/**\n * TODO\n */\n".to_owned(), + }, + }, + doc_edit( + 26..45, + 0, + DocOp::RenameTag { + line: 1, + from: "@return".to_owned(), + to: "@returns".to_owned(), + }, + ), + ]; + + assert_eq!( + apply(source, &edits, "\n"), + "/**\n * TODO\n */\nexport ledger a: Uint<8>;\n/**\n * @returns x\n */\nexport circuit run(): [] { }\n" + ); + } + + #[test] + fn a_rename_leaves_the_rest_of_the_line_alone() { + assert_eq!( + rename_tag(" * @return {[]} - Empty tuple.", "@return", "@returns"), + Some(" * @returns {[]} - Empty tuple.".to_owned()) + ); + } + + #[test] + fn a_rename_does_not_match_a_longer_tag() { + assert_eq!(rename_tag(" * @returns x", "@return", "@returns"), None); + } + + #[test] + fn a_module_rewrite_keeps_what_follows_the_name() { + assert_eq!( + set_module_name(" * @module Stale (archived)", "Renamed"), + Some(" * @module Renamed (archived)".to_owned()) + ); + assert_eq!( + set_module_name(" * @module Stale", "Renamed"), + Some(" * @module Renamed".to_owned()) + ); + } + + #[test] + fn crlf_sources_keep_their_line_ending() { + let source = "/**\r\n * @description Bumps.\r\n */\r\n"; + let edits = [doc_edit( + 0..32, + 0, + DocOp::InsertConstraints("@constraints k=?, rows=?".to_owned()), + )]; + + assert_eq!(newline_of(source), "\r\n"); + assert_eq!( + apply(source, &edits, newline_of(source)), + "/**\r\n * @description Bumps.\r\n *\r\n * @constraints k=?, rows=?\r\n */\r\n" + ); + } + + #[test] + fn a_file_without_a_trailing_newline_keeps_it_missing() { + let source = "/**\n * @description Bumps.\n */"; + let edits = [doc_edit(0..30, 0, tag("@module M", &[]))]; + + let out = apply(source, &edits, "\n"); + assert!(!out.ends_with('\n'), "{out:?}"); + } +} diff --git a/crates/compact-lint/src/fix.rs b/crates/compact-lint/src/fix.rs new file mode 100644 index 0000000..4e44f5f --- /dev/null +++ b/crates/compact-lint/src/fix.rs @@ -0,0 +1,444 @@ +//! The `fix` subcommand: turn the fixable issues into edits and rewrite the files. + +use std::ffi::{OsStr, OsString}; +use std::fs::{File, OpenOptions}; +use std::io::{ErrorKind, Write}; +use std::path::{Path, PathBuf}; +use std::time::{SystemTime, UNIX_EPOCH}; + +use thiserror::Error; + +use crate::config::Config; +use crate::doc::Tag; +use crate::edit::{DocOp, Edit, EditKind, apply, newline_of}; +use crate::model::DeclKind; +use crate::report::Position; +use crate::rules::{Issue, LintError, Linter}; +use crate::target::{self, TargetError}; + +/// The value written into an inserted constraints annotation. +const UNMEASURED: &str = "k=?, rows=?"; + +/// The tag whose value is the declaration's own name. +const MODULE_TAG: &str = "@module"; + +/// The tag that adopts untagged prose opening a doc comment. +const DESCRIPTION_TAG: &str = "@description"; + +#[derive(Debug, Error)] +pub enum FixError { + #[error(transparent)] + Target(#[from] TargetError), + #[error(transparent)] + Lint(#[from] LintError), + #[error("cannot read {path}")] + Read { + path: PathBuf, + #[source] + source: std::io::Error, + }, + #[error("cannot write {path}")] + Write { + path: PathBuf, + #[source] + source: std::io::Error, + }, +} + +/// Everything the `fix` subcommand needs, already resolved from flags. +pub struct Options { + pub paths: Vec, + pub config_path: Option, + pub dry_run: bool, +} + +/// The edits one file took. +pub struct FileEdits { + pub path: PathBuf, + pub edits: Vec, +} + +/// Every file the run changed, in the order they were visited. +pub struct Outcome { + pub files: Vec, +} + +impl Outcome { + #[must_use] + pub fn edits(&self) -> usize { + self.files.iter().map(|file| file.edits.len()).sum() + } + + #[must_use] + pub fn summary(&self) -> String { + format!("{} edits in {} files", self.edits(), self.files.len()) + } +} + +/// Runs the subcommand with `cwd` as the working directory. +/// # Errors +/// Returns an error when the config, the file walk, the parser or a write fails. +pub fn run(options: &Options, cwd: &Path) -> Result { + let target = target::resolve(&options.paths, options.config_path.as_deref(), cwd)?; + + let mut linter = Linter::new()?; + let mut files = Vec::new(); + + for path in &target.files { + let source = std::fs::read_to_string(path).map_err(|source| FixError::Read { + path: path.clone(), + source, + })?; + + let newline = newline_of(&source); + let issues = linter.issues(path, &source, &target.config, false)?; + let edits: Vec = issues + .iter() + .filter(|issue| !supplied_by_a_rename(issue, &issues, &target.config)) + .filter_map(|issue| edit_for(issue, &target.config, newline)) + .collect(); + + if edits.is_empty() { + continue; + } + + if !options.dry_run { + write_atomically(path, &apply(&source, &edits, newline))?; + } + files.push(FileEdits { + path: path.clone(), + edits, + }); + } + + Ok(Outcome { files }) +} + +/// Whether a forbidden tag in the same doc comment is renamed to the tag this +/// `missing-tag` issue reports. +/// +/// Inserting it as well would leave the comment carrying the tag twice. +fn supplied_by_a_rename(issue: &Issue, issues: &[Issue], config: &Config) -> bool { + let Issue::MissingTag { tag, doc, .. } = issue else { + return false; + }; + + issues.iter().any(|other| { + matches!( + other, + Issue::ForbiddenTag { + tag: forbidden, + doc: other_doc, + .. + } if other_doc == doc && config.rename_of(forbidden) == Some(tag) + ) + }) +} + +/// The repair for one issue, or `None` where the rule has no safe fix. +/// +/// `constraints-format` and `constraints-placeholder` need a measurement a rewrite +/// cannot invent, and `parse` and `format` are not doc-comment issues at all. +#[must_use] +pub fn edit_for(issue: &Issue, config: &Config, newline: &str) -> Option { + match issue { + Issue::MissingDoc { + position, + subject, + kind, + name, + offset, + indent, + constraints, + } => Some(Edit { + position: *position, + message: format!("inserted doc skeleton for {subject}"), + kind: EditKind::Insert { + offset: *offset, + text: skeleton( + *kind, + name.as_deref(), + *constraints, + config, + *indent, + newline, + ), + }, + }), + + Issue::MissingTag { + position, + kind, + tag, + name, + doc, + .. + } => Some(Edit { + position: *position, + message: format!("inserted {tag}"), + kind: EditKind::Doc { + range: doc.range.clone(), + indent: doc.indent, + op: missing_tag_op(*kind, tag, name.as_deref(), config), + }, + }), + + Issue::MissingConstraints { + position, tag, doc, .. + } => Some(Edit { + position: *position, + message: format!("inserted {tag} {UNMEASURED}"), + kind: EditKind::Doc { + range: doc.range.clone(), + indent: doc.indent, + op: DocOp::InsertConstraints(format!("{tag} {UNMEASURED}")), + }, + }), + + Issue::ForbiddenTag { + position, + tag, + line_offset, + doc, + } => { + let replacement = config.rename_of(tag)?; + Some(Edit { + position: *position, + message: format!("renamed {tag} to {replacement}"), + kind: EditKind::Doc { + range: doc.range.clone(), + indent: doc.indent, + op: DocOp::RenameTag { + line: *line_offset, + from: tag.to_string(), + to: replacement.to_string(), + }, + }, + }) + } + + Issue::ModuleName { + position, + name, + line_offset, + doc, + .. + } => Some(Edit { + position: *position, + message: format!("set {MODULE_TAG} to `{name}`"), + kind: EditKind::Doc { + range: doc.range.clone(), + indent: doc.indent, + op: DocOp::SetModuleName { + line: *line_offset, + name: name.clone(), + }, + }, + }), + + Issue::Parse { .. } + | Issue::ConstraintsFormat { .. } + | Issue::ConstraintsPlaceholder { .. } => None, + } +} + +/// The tag line lands after the tags that precede it in config order; `@description` +/// adopts prose that opens the body. +fn missing_tag_op(kind: DeclKind, tag: &Tag, name: Option<&str>, config: &Config) -> DocOp { + DocOp::InsertTag { + line: tag_line(tag, name, config), + after: config + .kinds + .get(kind) + .tags + .iter() + .take_while(|required| *required != tag) + .map(ToString::to_string) + .collect(), + adopts_prose: tag.as_str() == DESCRIPTION_TAG, + } +} + +/// `@module` takes the declaration's own name; every other tag takes the placeholder. +fn tag_line(tag: &Tag, name: Option<&str>, config: &Config) -> String { + match name { + Some(name) if tag.as_str() == MODULE_TAG => format!("{tag} {name}"), + _ => format!("{tag} {}", config.fix.placeholder), + } +} + +/// The doc comment `fix` writes above an undocumented declaration. +/// +/// Body lines come from `kinds..tags` in config order, then the constraints +/// annotation where the declaration takes one. +fn skeleton( + kind: DeclKind, + name: Option<&str>, + constraints: bool, + config: &Config, + indent: usize, + newline: &str, +) -> String { + let mut body: Vec = config + .kinds + .get(kind) + .tags + .iter() + .map(|tag| tag_line(tag, name, config)) + .collect(); + + if constraints { + if !body.is_empty() { + body.push(String::new()); + } + body.push(format!("{} {UNMEASURED}", config.constraints.tag)); + } + if body.is_empty() { + body.push(config.fix.placeholder.clone()); + } + + let margin = " ".repeat(indent); + let mut lines = vec!["/**".to_owned()]; + for line in body { + if line.is_empty() { + lines.push(format!("{margin} *")); + } else { + lines.push(format!("{margin} * {line}")); + } + } + lines.push(format!("{margin} */")); + + // The declaration follows on its own line, back at its original indentation. + format!("{}{newline}{margin}", lines.join(newline)) +} + +/// How many unique names to try before giving up on the temporary file. +const TEMPORARY_ATTEMPTS: u32 = 8; + +/// Writes through a sibling temporary file, so a failed write never truncates the source. +fn write_atomically(path: &Path, text: &str) -> Result<(), FixError> { + let (temporary, mut handle) = create_temporary(path)?; + + let write = handle + .write_all(text.as_bytes()) + .and_then(|()| handle.sync_all()) + .and_then(|()| std::fs::metadata(path)) + .and_then(|metadata| std::fs::set_permissions(&temporary, metadata.permissions())) + .and_then(|()| std::fs::rename(&temporary, path)); + + write.map_err(|source| { + // The rename never happened, so the temporary file is ours to clean up. + let _ = std::fs::remove_file(&temporary); + FixError::Write { + path: path.to_owned(), + source, + } + }) +} + +/// Creates a sibling temporary file under a name nothing else holds. +/// +/// A predictable name lets another process plant a file the write would truncate, so +/// the name carries the pid and a timestamp and the file is created exclusively. +fn create_temporary(path: &Path) -> Result<(PathBuf, File), FixError> { + let directory = path.parent().unwrap_or(Path::new(".")); + let stem = path.file_name().unwrap_or(OsStr::new("source")); + let pid = std::process::id(); + + let mut last = None; + for attempt in 0..TEMPORARY_ATTEMPTS { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |since| since.subsec_nanos()); + let mut name = OsString::from("."); + name.push(stem); + name.push(format!(".{pid}.{nanos}.{attempt}.tmp")); + let candidate = directory.join(name); + + match OpenOptions::new() + .write(true) + .create_new(true) + .open(&candidate) + { + Ok(handle) => return Ok((candidate, handle)), + Err(error) if error.kind() == ErrorKind::AlreadyExists => last = Some(error), + Err(source) => { + return Err(FixError::Write { + path: path.to_owned(), + source, + }); + } + } + } + + Err(FixError::Write { + path: path.to_owned(), + source: last.unwrap_or_else(|| ErrorKind::AlreadyExists.into()), + }) +} + +/// The line `fix` prints for one edit. +#[must_use] +pub fn line(path: &Path, position: Position, message: &str) -> String { + format!( + "{}:{}:{}: fix: {message}", + path.display(), + position.line, + position.column + ) +} + +#[cfg(test)] +mod tests { + use super::skeleton; + use crate::config::Config; + use crate::doc::Tag; + use crate::model::DeclKind; + + fn config() -> Config { + let mut config = Config::default(); + config.kinds.module.tags = vec![Tag::new("@module"), Tag::new("@description")]; + config.kinds.circuit.tags = vec![Tag::new("@description")]; + config + } + + #[test] + fn a_module_skeleton_names_the_module() { + assert_eq!( + skeleton(DeclKind::Module, Some("Ownable"), false, &config(), 0, "\n"), + "/**\n * @module Ownable\n * @description TODO\n */\n" + ); + } + + #[test] + fn an_exported_circuit_skeleton_carries_the_constraints_line() { + assert_eq!( + skeleton(DeclKind::Circuit, Some("run"), true, &config(), 2, "\n"), + "/**\n * @description TODO\n *\n * @constraints k=?, rows=?\n */\n " + ); + } + + #[test] + fn a_pure_circuit_skeleton_stops_at_the_tag_lines() { + assert_eq!( + skeleton(DeclKind::Circuit, Some("double"), false, &config(), 2, "\n"), + "/**\n * @description TODO\n */\n " + ); + } + + #[test] + fn a_kind_with_no_required_tags_still_gets_a_placeholder_body() { + assert_eq!( + skeleton(DeclKind::Type, Some("RoleId"), false, &config(), 2, "\n"), + "/**\n * TODO\n */\n " + ); + } + + #[test] + fn a_crlf_skeleton_uses_the_file_line_ending() { + assert_eq!( + skeleton(DeclKind::Type, None, false, &config(), 0, "\r\n"), + "/**\r\n * TODO\r\n */\r\n" + ); + } +} diff --git a/crates/compact-lint/src/lib.rs b/crates/compact-lint/src/lib.rs index fdaf6e9..2eb94bb 100644 --- a/crates/compact-lint/src/lib.rs +++ b/crates/compact-lint/src/lib.rs @@ -2,7 +2,8 @@ //! //! The crate parses `.compact` files with the bundled tree-sitter grammar, matches each //! declaration against the per-kind doc-comment template in `compact-lint.toml`, and -//! reports one [`report::Finding`] per violation. +//! yields one [`rules::Issue`] per violation. `check` renders an issue as a +//! [`report::Finding`]; `fix` renders the repairable ones as an [`edit::Edit`]. #![forbid(unsafe_code)] @@ -10,7 +11,10 @@ pub mod check; pub mod config; pub mod discover; pub mod doc; +pub mod edit; +pub mod fix; pub mod format; pub mod model; pub mod report; pub mod rules; +pub mod target; diff --git a/crates/compact-lint/src/main.rs b/crates/compact-lint/src/main.rs index dc4d364..e47c785 100644 --- a/crates/compact-lint/src/main.rs +++ b/crates/compact-lint/src/main.rs @@ -10,10 +10,11 @@ use std::process::ExitCode; use anyhow::{Context, Result}; use clap::{Parser, Subcommand}; -use compact_lint::check::{self, Options, Outcome}; +use compact_lint::check::{self, Outcome}; +use compact_lint::fix; use compact_lint::format::{COMPACT_BIN_ENV, DEFAULT_COMPACT_BIN}; -/// Exit code for a run that produced findings. +/// Exit code for a run that produced findings, or a `--dry-run` that would edit. const EXIT_FINDINGS: u8 = 1; /// Exit code for a usage, config, IO or parser error. @@ -34,6 +35,8 @@ struct Cli { enum Command { /// Check doc comments against the per-kind templates in compact-lint.toml. Check(CheckArgs), + /// Rewrite doc comments so the rules `fix` covers stop reporting. + Fix(FixArgs), } #[derive(Debug, clap::Args)] @@ -58,6 +61,20 @@ struct CheckArgs { compact_bin: Option, } +#[derive(Debug, clap::Args)] +struct FixArgs { + /// Files or directories to fix; defaults to the config's include globs. + paths: Vec, + + /// Config file to use instead of searching upward for compact-lint.toml. + #[arg(long, value_name = "FILE")] + config: Option, + + /// Report the edits without writing them; exits 1 when there are any. + #[arg(long)] + dry_run: bool, +} + fn main() -> ExitCode { match run() { Ok(code) => code, @@ -70,10 +87,16 @@ fn main() -> ExitCode { fn run() -> Result { let cli = Cli::parse(); - let Command::Check(args) = cli.command; - let cwd = std::env::current_dir().context("reading the current directory")?; - let options = Options { + + match cli.command { + Command::Check(args) => run_check(args, &cwd), + Command::Fix(args) => run_fix(args, &cwd), + } +} + +fn run_check(args: CheckArgs, cwd: &std::path::Path) -> Result { + let options = check::Options { paths: args.paths, config_path: args.config, strict: args.strict, @@ -83,8 +106,8 @@ fn run() -> Result { .unwrap_or_else(|| OsString::from(DEFAULT_COMPACT_BIN)), }; - let outcome = check::run(&options, &cwd).context("running the check")?; - emit(&outcome).context("writing the report")?; + let outcome = check::run(&options, cwd).context("running the check")?; + emit_findings(&outcome).context("writing the report")?; Ok(if outcome.findings.is_empty() { ExitCode::SUCCESS @@ -93,7 +116,24 @@ fn run() -> Result { }) } -fn emit(outcome: &Outcome) -> std::io::Result<()> { +fn run_fix(args: FixArgs, cwd: &std::path::Path) -> Result { + let options = fix::Options { + paths: args.paths, + config_path: args.config, + dry_run: args.dry_run, + }; + + let outcome = fix::run(&options, cwd).context("running the fix")?; + emit_edits(&outcome).context("writing the report")?; + + Ok(if options.dry_run && outcome.edits() > 0 { + ExitCode::from(EXIT_FINDINGS) + } else { + ExitCode::SUCCESS + }) +} + +fn emit_findings(outcome: &Outcome) -> std::io::Result<()> { let stdout = std::io::stdout(); let mut out = stdout.lock(); for finding in &outcome.findings { @@ -105,6 +145,24 @@ fn emit(outcome: &Outcome) -> std::io::Result<()> { Ok(()) } +fn emit_edits(outcome: &fix::Outcome) -> std::io::Result<()> { + let stdout = std::io::stdout(); + let mut out = stdout.lock(); + for file in &outcome.files { + for edit in &file.edits { + writeln!( + out, + "{}", + fix::line(&file.path, edit.position, &edit.message) + )?; + } + } + out.flush()?; + + eprintln!("{}", outcome.summary()); + Ok(()) +} + /// Prints the error and every source under it, one cause per line. fn report_error(error: &anyhow::Error) { eprintln!("compact-lint: {error}"); diff --git a/crates/compact-lint/src/model.rs b/crates/compact-lint/src/model.rs index 968eee8..5634f9c 100644 --- a/crates/compact-lint/src/model.rs +++ b/crates/compact-lint/src/model.rs @@ -1,6 +1,7 @@ //! Declarations the linter checks, lifted out of the tree-sitter tree. use std::fmt; +use std::ops::Range; use tree_sitter::Node; @@ -75,6 +76,8 @@ pub struct Declaration { pub exported: bool, pub pure: bool, pub position: Position, + /// Byte offset of the declaration's first token, where `fix` inserts a skeleton. + pub offset: usize, /// The doc comment immediately preceding the declaration, if one attaches. pub doc: Option, } @@ -86,6 +89,8 @@ pub struct AttachedDoc { /// 0-based row of the `/**` line, for turning a tag's offset into a position. pub start_row: usize, pub start_column: usize, + /// Byte range of the `/** … */` node, the span `fix` rewrites. + pub range: Range, } impl AttachedDoc { @@ -152,6 +157,7 @@ fn declaration(kind: DeclKind, node: Node<'_>, source: &str) -> Declaration { exported, pure, position: Position::from_zero_based(start.row, start.column), + offset: node.start_byte(), doc: attached_doc(node, source), } } @@ -169,6 +175,7 @@ fn attached_doc(node: Node<'_>, source: &str) -> Option { comment: DocComment::parse(text(previous, source)?), start_row: start.row, start_column: start.column, + range: previous.start_byte()..previous.end_byte(), }) } diff --git a/crates/compact-lint/src/rules.rs b/crates/compact-lint/src/rules.rs index 4e8cefc..3c5ab94 100644 --- a/crates/compact-lint/src/rules.rs +++ b/crates/compact-lint/src/rules.rs @@ -1,5 +1,9 @@ //! The doc-comment rules, run over one parsed file at a time. +//! +//! Detection yields [`Issue`]s. `check` renders them as [`Finding`]s and `fix` renders +//! the fixable ones as edits, so both subcommands read the same detection pass. +use std::ops::Range; use std::path::{Path, PathBuf}; use thiserror::Error; @@ -18,6 +22,142 @@ pub enum LintError { NoTree(PathBuf), } +/// Where a doc comment sits in the file, for the rules that rewrite one. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct DocRef { + /// Byte range of the `/** … */` node. + pub range: Range, + /// Column the `/**` starts on, reused as the rewritten comment's indentation. + pub indent: usize, +} + +/// One rule violation, carrying what `check` prints and what `fix` needs to repair it. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Issue { + Parse { + position: Position, + node_kind: String, + missing: bool, + }, + MissingDoc { + position: Position, + subject: String, + kind: DeclKind, + name: Option, + /// Byte offset of the declaration's first token. + offset: usize, + /// The declaration's start column, reused as the skeleton's indentation. + indent: usize, + /// The declaration also carries the constraints annotation. + constraints: bool, + }, + MissingTag { + position: Position, + subject: String, + kind: DeclKind, + tag: Tag, + /// The declaration's name, which `@module` takes as its value. + name: Option, + doc: DocRef, + }, + ForbiddenTag { + position: Position, + tag: Tag, + /// 0-based line index of the tag inside the doc comment. + line_offset: usize, + doc: DocRef, + }, + ModuleName { + position: Position, + documented: String, + name: String, + /// 0-based line index of `@module` inside the doc comment. + line_offset: usize, + doc: DocRef, + }, + MissingConstraints { + position: Position, + subject: String, + tag: Tag, + doc: DocRef, + }, + ConstraintsFormat { + position: Position, + tag: Tag, + value: String, + }, + ConstraintsPlaceholder { + position: Position, + tag: Tag, + value: String, + }, +} + +impl Issue { + #[must_use] + pub const fn position(&self) -> Position { + match self { + Self::Parse { position, .. } + | Self::MissingDoc { position, .. } + | Self::MissingTag { position, .. } + | Self::ForbiddenTag { position, .. } + | Self::ModuleName { position, .. } + | Self::MissingConstraints { position, .. } + | Self::ConstraintsFormat { position, .. } + | Self::ConstraintsPlaceholder { position, .. } => *position, + } + } + + #[must_use] + pub const fn rule(&self) -> RuleId { + match self { + Self::Parse { .. } => RuleId::PARSE, + Self::MissingDoc { .. } => RuleId::MISSING_DOC, + Self::MissingTag { .. } => RuleId::MISSING_TAG, + Self::ForbiddenTag { .. } => RuleId::FORBIDDEN_TAG, + Self::ModuleName { .. } => RuleId::MODULE_NAME, + Self::MissingConstraints { .. } => RuleId::MISSING_CONSTRAINTS, + Self::ConstraintsFormat { .. } => RuleId::CONSTRAINTS_FORMAT, + Self::ConstraintsPlaceholder { .. } => RuleId::CONSTRAINTS_PLACEHOLDER, + } + } + + #[must_use] + pub fn message(&self) -> String { + match self { + Self::Parse { + node_kind, missing, .. + } => { + if *missing { + format!("parse error: missing {node_kind}") + } else { + format!("parse error: unexpected {node_kind}") + } + } + Self::MissingDoc { subject, .. } => format!("{subject} has no doc comment"), + Self::MissingTag { subject, tag, .. } + | Self::MissingConstraints { subject, tag, .. } => { + format!("{subject} doc comment has no {tag}") + } + Self::ForbiddenTag { tag, .. } => format!("forbidden tag {tag}"), + Self::ModuleName { + documented, name, .. + } => format!("@module names `{documented}`, but the module is `{name}`"), + Self::ConstraintsFormat { tag, value, .. } => { + format!("{tag} value `{value}` is not `k=, rows=`") + } + Self::ConstraintsPlaceholder { tag, value, .. } => { + format!("{tag} value `{value}` still holds a placeholder") + } + } + } + + #[must_use] + pub fn finding(&self, display: &Path) -> Finding { + Finding::new(display, self.position(), self.rule(), self.message()) + } +} + /// A reusable parser plus the rules that run over its trees. pub struct Linter { parser: Parser, @@ -34,19 +174,19 @@ impl Linter { Ok(Self { parser }) } - /// Checks one file. `display` is the path printed in findings. + /// Runs every rule over one file, in source order. /// - /// A tree holding an `ERROR` or `MISSING` node yields a single `parse` finding and - /// no doc findings, because declarations around the defect are not trustworthy. + /// A tree holding an `ERROR` or `MISSING` node yields a single `parse` issue and no + /// doc issues, because declarations around the defect are not trustworthy. /// # Errors /// Returns an error when tree-sitter produces no tree for `source`. - pub fn check( + pub fn issues( &mut self, display: &Path, source: &str, config: &Config, strict: bool, - ) -> Result, LintError> { + ) -> Result, LintError> { let tree = self .parser .parse(source, None) @@ -54,42 +194,42 @@ impl Linter { let root = tree.root_node(); if let Some(defect) = first_defect(root) { - return Ok(vec![defect.finding(display)]); + return Ok(vec![defect]); } - let mut findings = Vec::new(); + let mut issues = Vec::new(); for declaration in declarations(root, source) { - check_declaration(&declaration, display, config, strict, &mut findings); + check_declaration(&declaration, config, strict, &mut issues); } - findings.sort_by(|left, right| left.sort_key().cmp(&right.sort_key())); - Ok(findings) + issues.sort_by_key(|issue| (issue.position(), issue.rule())); + Ok(issues) } -} - -struct Defect { - position: Position, - kind: String, - missing: bool, -} -impl Defect { - fn finding(&self, display: &Path) -> Finding { - let message = if self.missing { - format!("parse error: missing {}", self.kind) - } else { - format!("parse error: unexpected {}", self.kind) - }; - Finding::new(display, self.position, RuleId::PARSE, message) + /// Checks one file. `display` is the path printed in findings. + /// # Errors + /// Returns an error when tree-sitter produces no tree for `source`. + pub fn check( + &mut self, + display: &Path, + source: &str, + config: &Config, + strict: bool, + ) -> Result, LintError> { + Ok(self + .issues(display, source, config, strict)? + .iter() + .map(|issue| issue.finding(display)) + .collect()) } } /// The first `ERROR` or `MISSING` node in document order. -fn first_defect(node: Node<'_>) -> Option { +fn first_defect(node: Node<'_>) -> Option { if node.is_error() || node.is_missing() { let start = node.start_position(); - return Some(Defect { + return Some(Issue::Parse { position: Position::from_zero_based(start.row, start.column), - kind: node.kind().to_owned(), + node_kind: node.kind().to_owned(), missing: node.is_missing(), }); } @@ -104,10 +244,9 @@ fn first_defect(node: Node<'_>) -> Option { fn check_declaration( declaration: &Declaration, - display: &Path, config: &Config, strict: bool, - findings: &mut Vec, + issues: &mut Vec, ) { let kind_config = config.kinds.get(declaration.kind); let requires_docs = match kind_config.docs { @@ -118,36 +257,44 @@ fn check_declaration( let Some(attached) = declaration.doc.as_ref() else { if requires_docs { - findings.push(Finding::new( - display, - declaration.position, - RuleId::MISSING_DOC, - format!("{} has no doc comment", subject(declaration)), - )); + issues.push(Issue::MissingDoc { + position: declaration.position, + subject: subject(declaration), + kind: declaration.kind, + name: declaration.name.clone(), + offset: declaration.offset, + indent: declaration.position.column - 1, + constraints: takes_constraints(declaration), + }); } return; }; - // A forbidden tag is a finding wherever it appears, documented or not. + let doc = DocRef { + range: attached.range.clone(), + indent: attached.start_column, + }; + + // A forbidden tag is an issue wherever it appears, documented or not. for forbidden in &config.tags.forbid { for occurrence in attached.comment.tags() { if &occurrence.tag == forbidden { - findings.push(Finding::new( - display, - attached.tag_position(occurrence), - RuleId::FORBIDDEN_TAG, - format!("forbidden tag {forbidden}"), - )); + issues.push(Issue::ForbiddenTag { + position: attached.tag_position(occurrence), + tag: forbidden.clone(), + line_offset: occurrence.line_offset, + doc: doc.clone(), + }); } } } if declaration.kind == DeclKind::Module { - check_module_name(declaration, display, findings); + check_module_name(declaration, &doc, issues); } // The constraints rule is fixed, so it stays outside the per-kind docs policy. - check_constraints(declaration, display, config, strict, findings); + check_constraints(declaration, &doc, config, strict, issues); if !requires_docs { return; @@ -155,18 +302,20 @@ fn check_declaration( for required in &kind_config.tags { if !attached.comment.has(required) { - findings.push(Finding::new( - display, - declaration.position, - RuleId::MISSING_TAG, - format!("{} doc comment has no {required}", subject(declaration)), - )); + issues.push(Issue::MissingTag { + position: declaration.position, + subject: subject(declaration), + kind: declaration.kind, + tag: required.clone(), + name: declaration.name.clone(), + doc: doc.clone(), + }); } } } /// `@module ` must name the module it documents. -fn check_module_name(declaration: &Declaration, display: &Path, findings: &mut Vec) { +fn check_module_name(declaration: &Declaration, doc: &DocRef, issues: &mut Vec) { let tag = Tag::new("@module"); let (Some(attached), Some(name)) = (declaration.doc.as_ref(), declaration.name.as_deref()) else { @@ -178,12 +327,13 @@ fn check_module_name(declaration: &Declaration, display: &Path, findings: &mut V let documented = module_name(&occurrence.value); if documented != name { - findings.push(Finding::new( - display, - attached.tag_position(occurrence), - RuleId::MODULE_NAME, - format!("@module names `{documented}`, but the module is `{name}`"), - )); + issues.push(Issue::ModuleName { + position: attached.tag_position(occurrence), + documented: documented.to_owned(), + name: name.to_owned(), + line_offset: occurrence.line_offset, + doc: doc.clone(), + }); } } @@ -195,14 +345,18 @@ fn module_name(value: &str) -> &str { } /// Exported non-pure circuits carry the constraints annotation. +const fn takes_constraints(declaration: &Declaration) -> bool { + matches!(declaration.kind, DeclKind::Circuit) && declaration.exported && !declaration.pure +} + fn check_constraints( declaration: &Declaration, - display: &Path, + doc: &DocRef, config: &Config, strict: bool, - findings: &mut Vec, + issues: &mut Vec, ) { - if declaration.kind != DeclKind::Circuit || !declaration.exported || declaration.pure { + if !takes_constraints(declaration) { return; } let Some(attached) = declaration.doc.as_ref() else { @@ -211,12 +365,12 @@ fn check_constraints( let tag = &config.constraints.tag; let Some(occurrence) = attached.comment.first(tag) else { - findings.push(Finding::new( - display, - declaration.position, - RuleId::MISSING_CONSTRAINTS, - format!("{} doc comment has no {tag}", subject(declaration)), - )); + issues.push(Issue::MissingConstraints { + position: declaration.position, + subject: subject(declaration), + tag: tag.clone(), + doc: doc.clone(), + }); return; }; @@ -225,22 +379,20 @@ fn check_constraints( let position = attached.tag_position(occurrence); let Some(placeholders) = parse_constraints(value) else { - findings.push(Finding::new( - display, + issues.push(Issue::ConstraintsFormat { position, - RuleId::CONSTRAINTS_FORMAT, - format!("{tag} value `{value}` is not `k=, rows=`"), - )); + tag: tag.clone(), + value: value.to_owned(), + }); return; }; if strict && placeholders { - findings.push(Finding::new( - display, + issues.push(Issue::ConstraintsPlaceholder { position, - RuleId::CONSTRAINTS_PLACEHOLDER, - format!("{tag} value `{value}` still holds a placeholder"), - )); + tag: tag.clone(), + value: value.to_owned(), + }); } } diff --git a/crates/compact-lint/src/target.rs b/crates/compact-lint/src/target.rs new file mode 100644 index 0000000..fe2f907 --- /dev/null +++ b/crates/compact-lint/src/target.rs @@ -0,0 +1,101 @@ +//! What a subcommand runs over: the resolved config plus the file list. + +use std::path::{Path, PathBuf}; + +use thiserror::Error; + +use crate::config::{Config, ConfigError, discover as discover_config}; +use crate::discover::{self, DiscoverError}; + +#[derive(Debug, Error)] +pub enum TargetError { + #[error(transparent)] + Config(#[from] ConfigError), + #[error(transparent)] + Discover(#[from] DiscoverError), +} + +/// The config and the files it selects, shared by `check` and `fix`. +pub struct Target { + pub config: Config, + pub files: Vec, +} + +/// Resolves the config and the file list for one run. +/// +/// An explicit `config_path` is a shareable preset, so glob discovery stays anchored at +/// `cwd`; a config found by walking upward anchors discovery at its own directory. +/// # Errors +/// Returns an error when the config is unreadable or a path cannot be walked. +pub fn resolve( + paths: &[PathBuf], + config_path: Option<&Path>, + cwd: &Path, +) -> Result { + let (config, source, base) = match config_path { + Some(path) => (Config::load(path)?, path.to_owned(), cwd.to_owned()), + None => match discover_config(cwd) { + Some(path) => { + let base = path.parent().unwrap_or(cwd).to_owned(); + (Config::load(&path)?, path, base) + } + None => ( + Config::default(), + PathBuf::from(""), + cwd.to_owned(), + ), + }, + }; + + let exclude = config.exclude_set(&source)?; + let files = if paths.is_empty() { + let include = config.include_set(&source)?; + discover::from_globs(&base, &include, &exclude)? + } else { + let paths = absolute_paths(paths, cwd); + discover::from_paths(&paths, &base, &exclude)? + }; + + let files = files.iter().map(|path| display_path(path, cwd)).collect(); + Ok(Target { config, files }) +} + +/// Discovery matches the exclude globs against paths relative to the config's +/// directory, so a CLI path has to be anchored at `cwd` first: a bare `archive` +/// given from a subdirectory would otherwise never match `contracts/archive/**`. +fn absolute_paths(paths: &[PathBuf], cwd: &Path) -> Vec { + paths + .iter() + .map(|path| { + if path.is_absolute() { + path.clone() + } else { + cwd.join(path) + } + }) + .collect() +} + +/// Paths under the working directory print relative to it; anything else prints as is. +fn display_path(path: &Path, cwd: &Path) -> PathBuf { + path.strip_prefix(cwd).unwrap_or(path).to_owned() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_relative_path_is_anchored_at_the_working_directory() { + let resolved = absolute_paths(&[PathBuf::from("archive")], Path::new("/repo/contracts")); + + assert_eq!(resolved, vec![PathBuf::from("/repo/contracts/archive")]); + } + + #[test] + fn an_absolute_path_is_left_alone() { + let resolved = absolute_paths(&[PathBuf::from("/elsewhere/X.compact")], Path::new("/repo")); + + assert_eq!(resolved, vec![PathBuf::from("/elsewhere/X.compact")]); + } +} diff --git a/crates/compact-lint/tests/fix.rs b/crates/compact-lint/tests/fix.rs new file mode 100644 index 0000000..a92aa1f --- /dev/null +++ b/crates/compact-lint/tests/fix.rs @@ -0,0 +1,250 @@ +//! `compact-lint fix` end to end, one fixture case per rule. +//! +//! A case holds `compact-lint.toml`, `before/`, `after/` and `expected.txt`. The run +//! works on a copy of `before/`, so the fixtures themselves are never rewritten. + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; +use std::process::Command; + +use assert_cmd::prelude::*; +use tempfile::TempDir; + +/// Exit code for a `--dry-run` that would edit. +const EXIT_EDITS: i32 = 1; + +fn case(name: &str) -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures") + .join(format!("fix-{name}")) + .canonicalize() + .expect("the fixture case exists") +} + +fn binary() -> Command { + Command::cargo_bin("compact-lint").expect("the binary is built alongside this test") +} + +/// Copies the case's config and `before/` tree into a fresh temporary directory. +fn work(name: &str) -> TempDir { + let directory = TempDir::new().expect("a temporary directory is available"); + let root = case(name); + + std::fs::copy( + root.join("compact-lint.toml"), + directory.path().join("compact-lint.toml"), + ) + .expect("the case has a config"); + copy_tree(&root.join("before"), directory.path()); + + directory +} + +fn copy_tree(from: &Path, to: &Path) { + for entry in std::fs::read_dir(from).expect("the tree is readable") { + let entry = entry.expect("the entry is readable"); + let target = to.join(entry.file_name()); + if entry.path().is_dir() { + std::fs::create_dir_all(&target).expect("the target is writable"); + copy_tree(&entry.path(), &target); + } else { + std::fs::copy(entry.path(), &target).expect("the file is copyable"); + } + } +} + +/// Every `.compact` file under `root`, keyed by its path relative to it. +fn sources(root: &Path) -> BTreeMap> { + let mut found = BTreeMap::new(); + collect(root, root, &mut found); + found +} + +fn collect(root: &Path, directory: &Path, out: &mut BTreeMap>) { + for entry in std::fs::read_dir(directory).expect("the tree is readable") { + let entry = entry.expect("the entry is readable"); + let path = entry.path(); + if path.is_dir() { + collect(root, &path, out); + } else if path + .extension() + .is_some_and(|extension| extension == "compact") + { + let relative = path.strip_prefix(root).expect("the path is under the root"); + out.insert( + relative.to_owned(), + std::fs::read(&path).expect("the source is readable"), + ); + } + } +} + +fn expected(name: &str) -> String { + std::fs::read_to_string(case(name).join("expected.txt")).expect("the case has an expectation") +} + +/// Runs a subcommand inside `directory` and returns its exit code and stdout. +fn run(directory: &Path, args: &[&str]) -> (i32, String) { + let output = binary() + .current_dir(directory) + .args(args) + .output() + .expect("the binary ran"); + + ( + output.status.code().expect("the binary was not signalled"), + String::from_utf8(output.stdout).expect("the report is UTF-8"), + ) +} + +/// The whole contract of a fixable case: it fixes, it lands, it settles, it previews. +fn assert_case(name: &str) { + let directory = work(name); + let root = directory.path(); + + let (code, stdout) = run(root, &["fix"]); + assert_eq!(stdout, expected(name), "stdout for case {name}"); + assert_eq!(code, 0, "exit code for case {name}"); + assert_eq!( + sources(root), + sources(&case(name).join("after")), + "rewritten sources for case {name}" + ); + + let (code, stdout) = run(root, &["check", "--no-format"]); + assert_eq!(stdout, "", "check is clean after fixing case {name}"); + assert_eq!(code, 0, "check exit code after fixing case {name}"); + + let (code, stdout) = run(root, &["fix"]); + assert_eq!(stdout, "", "a second fix on case {name} edits nothing"); + assert_eq!(code, 0, "second fix exit code for case {name}"); + + let preview = work(name); + let before = sources(preview.path()); + let (code, stdout) = run(preview.path(), &["fix", "--dry-run"]); + assert_eq!(stdout, expected(name), "dry-run stdout for case {name}"); + assert_eq!(code, EXIT_EDITS, "dry-run exit code for case {name}"); + assert_eq!( + sources(preview.path()), + before, + "dry-run wrote to case {name}" + ); +} + +#[test] +fn undocumented_declarations_of_every_kind_take_a_skeleton() { + assert_case("skeleton"); +} + +#[test] +fn a_required_tag_goes_into_the_comment_that_lacks_it() { + assert_case("missing-tag"); +} + +#[test] +fn the_constraints_annotation_follows_the_description_block() { + assert_case("constraints"); +} + +#[test] +fn a_forbidden_tag_with_a_replacement_is_renamed_in_place() { + assert_case("rename"); +} + +#[test] +fn a_rename_that_supplies_a_required_tag_suppresses_the_insert() { + assert_case("rename-supplies-tag"); + + let directory = work("rename-supplies-tag"); + run(directory.path(), &["fix"]); + let fixed = std::fs::read_to_string(directory.path().join("Supplied.compact")) + .expect("the fixed source is readable"); + + assert_eq!(fixed.matches("@returns").count(), 1, "{fixed}"); +} + +#[test] +fn a_stale_module_tag_takes_the_module_name() { + assert_case("module-name"); +} + +#[test] +fn a_crlf_source_keeps_its_line_endings() { + assert_case("crlf"); +} + +#[test] +fn nothing_fixable_writes_nothing_and_exits_zero() { + let directory = work("unfixable"); + let root = directory.path(); + let before = sources(root); + + let (code, stdout) = run(root, &["fix"]); + assert_eq!(stdout, ""); + assert_eq!(code, 0); + assert_eq!(sources(root), before); + + let (code, stdout) = run(root, &["fix", "--dry-run"]); + assert_eq!(stdout, ""); + assert_eq!(code, 0); +} + +#[test] +fn the_summary_goes_to_stderr_and_the_edits_to_stdout() { + let directory = work("rename"); + let output = binary() + .current_dir(directory.path()) + .arg("fix") + .output() + .expect("the binary ran"); + + let stderr = String::from_utf8(output.stderr).expect("the summary is UTF-8"); + assert_eq!(stderr.trim_end(), "2 edits in 1 files"); + assert!( + String::from_utf8_lossy(&output.stdout).contains(": fix: "), + "the edits belong on stdout" + ); +} + +#[test] +fn an_explicit_config_replaces_the_one_beside_the_sources() { + let directory = work("rename"); + std::fs::remove_file(directory.path().join("compact-lint.toml")) + .expect("the copied config is removable"); + + let config = case("rename").join("compact-lint.toml"); + let output = binary() + .current_dir(directory.path()) + .args(["fix", "--config"]) + .arg(&config) + .output() + .expect("the binary ran"); + + assert_eq!( + String::from_utf8(output.stdout).expect("the report is UTF-8"), + expected("rename") + ); +} + +#[test] +fn a_file_named_like_the_old_temporary_survives_a_fix() { + let directory = work("rename"); + let planted = directory.path().join("Returns.compact.tmp"); + std::fs::write(&planted, "not mine to truncate\n").expect("the plant is writable"); + + let (code, _) = run(directory.path(), &["fix"]); + + assert_eq!(code, 0); + assert_eq!( + std::fs::read_to_string(&planted).expect("the plant is still there"), + "not mine to truncate\n" + ); +} + +#[test] +fn a_missing_path_exits_two() { + let directory = work("rename"); + let (code, _) = run(directory.path(), &["fix", "Nope.compact"]); + + assert_eq!(code, 2); +} diff --git a/crates/compact-lint/tests/fixtures/fix-constraints/after/Unmeasured.compact b/crates/compact-lint/tests/fixtures/fix-constraints/after/Unmeasured.compact new file mode 100644 index 0000000..6c55524 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-constraints/after/Unmeasured.compact @@ -0,0 +1,37 @@ +/** + * @module Unmeasured + * @description A module whose exported circuits are unmeasured. + */ +export module Unmeasured { + /** + * @description Bumps the counter. + * + * @constraints k=?, rows=? + */ + export circuit bump(): [] { } + + /** + * @description Sends a coin + * to the recipient named below. + * + * @constraints k=?, rows=? + * + * @param {Bytes<32>} to - The recipient. + * @returns {[]} Empty tuple. + */ + export circuit send(to: Bytes<32>): [] { } + + /** + * @constraints k=?, rows=? + * + * @notice The annotation goes to the top, since there is no description. + */ + export circuit run(): [] { } + + /** + * @description Pure circuits are never annotated. + */ + export pure circuit double(value: Uint<8>): Uint<16> { + return value as Uint<16> * 2; + } +} diff --git a/crates/compact-lint/tests/fixtures/fix-constraints/before/Unmeasured.compact b/crates/compact-lint/tests/fixtures/fix-constraints/before/Unmeasured.compact new file mode 100644 index 0000000..ea9ffde --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-constraints/before/Unmeasured.compact @@ -0,0 +1,31 @@ +/** + * @module Unmeasured + * @description A module whose exported circuits are unmeasured. + */ +export module Unmeasured { + /** + * @description Bumps the counter. + */ + export circuit bump(): [] { } + + /** + * @description Sends a coin + * to the recipient named below. + * + * @param {Bytes<32>} to - The recipient. + * @returns {[]} Empty tuple. + */ + export circuit send(to: Bytes<32>): [] { } + + /** + * @notice The annotation goes to the top, since there is no description. + */ + export circuit run(): [] { } + + /** + * @description Pure circuits are never annotated. + */ + export pure circuit double(value: Uint<8>): Uint<16> { + return value as Uint<16> * 2; + } +} diff --git a/crates/compact-lint/tests/fixtures/fix-constraints/compact-lint.toml b/crates/compact-lint/tests/fixtures/fix-constraints/compact-lint.toml new file mode 100644 index 0000000..832f4ee --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-constraints/compact-lint.toml @@ -0,0 +1,13 @@ +version = 1 +include = ["**/*.compact"] + +[constraints] +tag = "@constraints" + +[kinds.module] +tags = ["@module", "@description"] + +[kinds.circuit] + +[kinds.constructor] +docs = "none" diff --git a/crates/compact-lint/tests/fixtures/fix-constraints/expected.txt b/crates/compact-lint/tests/fixtures/fix-constraints/expected.txt new file mode 100644 index 0000000..9b49823 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-constraints/expected.txt @@ -0,0 +1,3 @@ +Unmeasured.compact:9:3: fix: inserted @constraints k=?, rows=? +Unmeasured.compact:18:3: fix: inserted @constraints k=?, rows=? +Unmeasured.compact:23:3: fix: inserted @constraints k=?, rows=? diff --git a/crates/compact-lint/tests/fixtures/fix-crlf/after/Windows.compact b/crates/compact-lint/tests/fixtures/fix-crlf/after/Windows.compact new file mode 100644 index 0000000..d1f3118 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-crlf/after/Windows.compact @@ -0,0 +1,19 @@ +/** + * @module Windows + * @description A module written with Windows line endings. + */ +export module Windows { + /** + * @description TODO + */ + export ledger _count: Counter; + + /** + * @description Bumps the counter. + * + * @constraints k=?, rows=? + * + * @returns {[]} - Empty tuple. + */ + export circuit bump(): [] { } +} diff --git a/crates/compact-lint/tests/fixtures/fix-crlf/before/Windows.compact b/crates/compact-lint/tests/fixtures/fix-crlf/before/Windows.compact new file mode 100644 index 0000000..2ba9860 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-crlf/before/Windows.compact @@ -0,0 +1,13 @@ +/** + * @module Windows + * @description A module written with Windows line endings. + */ +export module Windows { + export ledger _count: Counter; + + /** + * @description Bumps the counter. + * @return {[]} - Empty tuple. + */ + export circuit bump(): [] { } +} diff --git a/crates/compact-lint/tests/fixtures/fix-crlf/compact-lint.toml b/crates/compact-lint/tests/fixtures/fix-crlf/compact-lint.toml new file mode 100644 index 0000000..5e3068c --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-crlf/compact-lint.toml @@ -0,0 +1,18 @@ +version = 1 +include = ["**/*.compact"] + +[tags] +forbid = ["@return"] +rename = { "@return" = "@returns" } + +[kinds.module] +tags = ["@module", "@description"] + +[kinds.circuit] +tags = ["@description"] + +[kinds.ledger] +tags = ["@description"] + +[kinds.constructor] +docs = "none" diff --git a/crates/compact-lint/tests/fixtures/fix-crlf/expected.txt b/crates/compact-lint/tests/fixtures/fix-crlf/expected.txt new file mode 100644 index 0000000..994f2b4 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-crlf/expected.txt @@ -0,0 +1,3 @@ +Windows.compact:6:3: fix: inserted doc skeleton for ledger `_count` +Windows.compact:10:6: fix: renamed @return to @returns +Windows.compact:12:3: fix: inserted @constraints k=?, rows=? diff --git a/crates/compact-lint/tests/fixtures/fix-missing-tag/after/Untagged.compact b/crates/compact-lint/tests/fixtures/fix-missing-tag/after/Untagged.compact new file mode 100644 index 0000000..5f112de --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-missing-tag/after/Untagged.compact @@ -0,0 +1,25 @@ +/** + * @module Untagged + * @description TODO + */ +export module Untagged { + /** + * @description Approved accounts. Absent means not allowed. + */ + export ledger _allowed: Set>; + + /** + * @description The circuit has no description either. + * @constraints k=6, rows=28 + */ + export circuit run(): [] { } + + /** + * @description TODO + * + * @constraints k=?, rows=? + * + * @param {Uint<8>} value - The value to store. + */ + export circuit store(value: Uint<8>): [] { } +} diff --git a/crates/compact-lint/tests/fixtures/fix-missing-tag/before/Untagged.compact b/crates/compact-lint/tests/fixtures/fix-missing-tag/before/Untagged.compact new file mode 100644 index 0000000..49ef079 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-missing-tag/before/Untagged.compact @@ -0,0 +1,18 @@ +/** + * @module Untagged + */ +export module Untagged { + /** Approved accounts. Absent means not allowed. */ + export ledger _allowed: Set>; + + /** + * The circuit has no description either. + * @constraints k=6, rows=28 + */ + export circuit run(): [] { } + + /** + * @param {Uint<8>} value - The value to store. + */ + export circuit store(value: Uint<8>): [] { } +} diff --git a/crates/compact-lint/tests/fixtures/fix-missing-tag/compact-lint.toml b/crates/compact-lint/tests/fixtures/fix-missing-tag/compact-lint.toml new file mode 100644 index 0000000..3af7b21 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-missing-tag/compact-lint.toml @@ -0,0 +1,14 @@ +version = 1 +include = ["**/*.compact"] + +[kinds.module] +tags = ["@module", "@description"] + +[kinds.circuit] +tags = ["@description"] + +[kinds.ledger] +tags = ["@description"] + +[kinds.constructor] +docs = "none" diff --git a/crates/compact-lint/tests/fixtures/fix-missing-tag/expected.txt b/crates/compact-lint/tests/fixtures/fix-missing-tag/expected.txt new file mode 100644 index 0000000..a33b4e2 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-missing-tag/expected.txt @@ -0,0 +1,5 @@ +Untagged.compact:4:1: fix: inserted @description +Untagged.compact:6:3: fix: inserted @description +Untagged.compact:12:3: fix: inserted @description +Untagged.compact:17:3: fix: inserted @constraints k=?, rows=? +Untagged.compact:17:3: fix: inserted @description diff --git a/crates/compact-lint/tests/fixtures/fix-module-name/after/Renamed.compact b/crates/compact-lint/tests/fixtures/fix-module-name/after/Renamed.compact new file mode 100644 index 0000000..4e467b1 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-module-name/after/Renamed.compact @@ -0,0 +1,12 @@ +/** + * @module Renamed (archived) + * @description The tag still names the module's old name. + */ +export module Renamed { + /** + * @module Nested + * @description The inner tag names nothing that exists. + */ + export module Nested { + } +} diff --git a/crates/compact-lint/tests/fixtures/fix-module-name/before/Renamed.compact b/crates/compact-lint/tests/fixtures/fix-module-name/before/Renamed.compact new file mode 100644 index 0000000..2ac3362 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-module-name/before/Renamed.compact @@ -0,0 +1,12 @@ +/** + * @module Stale (archived) + * @description The tag still names the module's old name. + */ +export module Renamed { + /** + * @module Inner. + * @description The inner tag names nothing that exists. + */ + export module Nested { + } +} diff --git a/crates/compact-lint/tests/fixtures/fix-module-name/compact-lint.toml b/crates/compact-lint/tests/fixtures/fix-module-name/compact-lint.toml new file mode 100644 index 0000000..d404f25 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-module-name/compact-lint.toml @@ -0,0 +1,8 @@ +version = 1 +include = ["**/*.compact"] + +[kinds.module] +tags = ["@module", "@description"] + +[kinds.constructor] +docs = "none" diff --git a/crates/compact-lint/tests/fixtures/fix-module-name/expected.txt b/crates/compact-lint/tests/fixtures/fix-module-name/expected.txt new file mode 100644 index 0000000..5e9811a --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-module-name/expected.txt @@ -0,0 +1,2 @@ +Renamed.compact:2:4: fix: set @module to `Renamed` +Renamed.compact:7:6: fix: set @module to `Nested` diff --git a/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/after/Supplied.compact b/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/after/Supplied.compact new file mode 100644 index 0000000..a405a3b --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/after/Supplied.compact @@ -0,0 +1,12 @@ +/** + * @module Supplied + * @description A module whose circuit still uses the old return tag. + */ +export module Supplied { + /** + * @description Does nothing. + * @constraints k=6, rows=28 + * @returns {[]} - Empty tuple. + */ + export circuit run(): [] { } +} diff --git a/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/before/Supplied.compact b/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/before/Supplied.compact new file mode 100644 index 0000000..fa377d5 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/before/Supplied.compact @@ -0,0 +1,12 @@ +/** + * @module Supplied + * @description A module whose circuit still uses the old return tag. + */ +export module Supplied { + /** + * @description Does nothing. + * @constraints k=6, rows=28 + * @return {[]} - Empty tuple. + */ + export circuit run(): [] { } +} diff --git a/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/compact-lint.toml b/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/compact-lint.toml new file mode 100644 index 0000000..815000c --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/compact-lint.toml @@ -0,0 +1,12 @@ +version = 1 +include = ["**/*.compact"] + +[tags] +forbid = ["@return"] +rename = { "@return" = "@returns" } + +[kinds.module] +tags = ["@module", "@description"] + +[kinds.circuit] +tags = ["@description", "@returns"] diff --git a/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/expected.txt b/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/expected.txt new file mode 100644 index 0000000..9a3fad0 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-rename-supplies-tag/expected.txt @@ -0,0 +1 @@ +Supplied.compact:9:6: fix: renamed @return to @returns diff --git a/crates/compact-lint/tests/fixtures/fix-rename/after/Returns.compact b/crates/compact-lint/tests/fixtures/fix-rename/after/Returns.compact new file mode 100644 index 0000000..21cbe5d --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-rename/after/Returns.compact @@ -0,0 +1,19 @@ +/** + * @module Returns + * @description A module that still uses the old return tag. + */ +export module Returns { + /** + * @description Does nothing. + * @constraints k=6, rows=28 + * @returns {[]} - Empty tuple. + */ + export circuit run(): [] { } + + /** + * @description Not exported, so its docs are optional, but the tag is not. + * @returns {[]} - Empty tuple. + * @returns {[]} - The new spelling is left alone. + */ + circuit helper(): [] { } +} diff --git a/crates/compact-lint/tests/fixtures/fix-rename/before/Returns.compact b/crates/compact-lint/tests/fixtures/fix-rename/before/Returns.compact new file mode 100644 index 0000000..8c8fc7a --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-rename/before/Returns.compact @@ -0,0 +1,19 @@ +/** + * @module Returns + * @description A module that still uses the old return tag. + */ +export module Returns { + /** + * @description Does nothing. + * @constraints k=6, rows=28 + * @return {[]} - Empty tuple. + */ + export circuit run(): [] { } + + /** + * @description Not exported, so its docs are optional, but the tag is not. + * @return {[]} - Empty tuple. + * @returns {[]} - The new spelling is left alone. + */ + circuit helper(): [] { } +} diff --git a/crates/compact-lint/tests/fixtures/fix-rename/compact-lint.toml b/crates/compact-lint/tests/fixtures/fix-rename/compact-lint.toml new file mode 100644 index 0000000..ad6ecc5 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-rename/compact-lint.toml @@ -0,0 +1,15 @@ +version = 1 +include = ["**/*.compact"] + +[tags] +forbid = ["@return"] +rename = { "@return" = "@returns" } + +[kinds.module] +tags = ["@module", "@description"] + +[kinds.circuit] +tags = ["@description"] + +[kinds.constructor] +docs = "none" diff --git a/crates/compact-lint/tests/fixtures/fix-rename/expected.txt b/crates/compact-lint/tests/fixtures/fix-rename/expected.txt new file mode 100644 index 0000000..50c5c2a --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-rename/expected.txt @@ -0,0 +1,2 @@ +Returns.compact:9:6: fix: renamed @return to @returns +Returns.compact:15:6: fix: renamed @return to @returns diff --git a/crates/compact-lint/tests/fixtures/fix-skeleton/after/Bare.compact b/crates/compact-lint/tests/fixtures/fix-skeleton/after/Bare.compact new file mode 100644 index 0000000..77abb21 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-skeleton/after/Bare.compact @@ -0,0 +1,59 @@ +pragma language_version >= 0.23.0; + +/** + * @module Bare + * @description TODO + */ +export module Bare { + import CompactStandardLibrary; + + /** + * @description TODO + */ + export ledger _count: Counter; + + /** + * @description TODO + */ + witness wit_Secret(): Bytes<32>; + + /** + * @description TODO + */ + export struct Reading { + value: Uint<64> + } + + /** + * @description TODO + */ + export enum Direction { + Up, + Down + } + + /** + * TODO + */ + export type Alias = Bytes<32>; + + constructor() { + _count.resetToDefault(); + } + + /** + * @description TODO + */ + export pure circuit double(value: Uint<8>): Uint<16> { + return value as Uint<16> * 2; + } + + /** + * @description TODO + * + * @constraints k=?, rows=? + */ + export circuit bump(): [] { + _count.increment(1); + } +} diff --git a/crates/compact-lint/tests/fixtures/fix-skeleton/before/Bare.compact b/crates/compact-lint/tests/fixtures/fix-skeleton/before/Bare.compact new file mode 100644 index 0000000..9a88c83 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-skeleton/before/Bare.compact @@ -0,0 +1,32 @@ +pragma language_version >= 0.23.0; + +export module Bare { + import CompactStandardLibrary; + + export ledger _count: Counter; + + witness wit_Secret(): Bytes<32>; + + export struct Reading { + value: Uint<64> + } + + export enum Direction { + Up, + Down + } + + export type Alias = Bytes<32>; + + constructor() { + _count.resetToDefault(); + } + + export pure circuit double(value: Uint<8>): Uint<16> { + return value as Uint<16> * 2; + } + + export circuit bump(): [] { + _count.increment(1); + } +} diff --git a/crates/compact-lint/tests/fixtures/fix-skeleton/compact-lint.toml b/crates/compact-lint/tests/fixtures/fix-skeleton/compact-lint.toml new file mode 100644 index 0000000..5fd3408 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-skeleton/compact-lint.toml @@ -0,0 +1,31 @@ +version = 1 +include = ["**/*.compact"] + +[fix] +placeholder = "TODO" + +[tags] +forbid = ["@return"] +rename = { "@return" = "@returns" } + +[kinds.module] +tags = ["@module", "@description"] + +[kinds.circuit] +tags = ["@description"] + +[kinds.ledger] +tags = ["@description"] + +[kinds.witness] +docs = "all" +tags = ["@description"] + +[kinds.struct] +tags = ["@description"] + +[kinds.enum] +tags = ["@description"] + +[kinds.constructor] +docs = "none" diff --git a/crates/compact-lint/tests/fixtures/fix-skeleton/expected.txt b/crates/compact-lint/tests/fixtures/fix-skeleton/expected.txt new file mode 100644 index 0000000..8dadb68 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-skeleton/expected.txt @@ -0,0 +1,8 @@ +Bare.compact:3:1: fix: inserted doc skeleton for module `Bare` +Bare.compact:6:3: fix: inserted doc skeleton for ledger `_count` +Bare.compact:8:3: fix: inserted doc skeleton for witness `wit_Secret` +Bare.compact:10:3: fix: inserted doc skeleton for struct `Reading` +Bare.compact:14:3: fix: inserted doc skeleton for enum `Direction` +Bare.compact:19:3: fix: inserted doc skeleton for type `Alias` +Bare.compact:25:3: fix: inserted doc skeleton for circuit `double` +Bare.compact:29:3: fix: inserted doc skeleton for circuit `bump` diff --git a/crates/compact-lint/tests/fixtures/fix-unfixable/after/Malformed.compact b/crates/compact-lint/tests/fixtures/fix-unfixable/after/Malformed.compact new file mode 100644 index 0000000..d7039c8 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-unfixable/after/Malformed.compact @@ -0,0 +1,12 @@ +/** + * @module Malformed + * @description A module whose annotations a rewrite cannot repair. + */ +export module Malformed { + /** + * @description The comma is missing, and nothing can guess the measurement. + * @constraints k=10 rows=626 + * @throws {Error} The forbidden tag has no replacement. + */ + export circuit first(): [] { } +} diff --git a/crates/compact-lint/tests/fixtures/fix-unfixable/before/Malformed.compact b/crates/compact-lint/tests/fixtures/fix-unfixable/before/Malformed.compact new file mode 100644 index 0000000..d7039c8 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-unfixable/before/Malformed.compact @@ -0,0 +1,12 @@ +/** + * @module Malformed + * @description A module whose annotations a rewrite cannot repair. + */ +export module Malformed { + /** + * @description The comma is missing, and nothing can guess the measurement. + * @constraints k=10 rows=626 + * @throws {Error} The forbidden tag has no replacement. + */ + export circuit first(): [] { } +} diff --git a/crates/compact-lint/tests/fixtures/fix-unfixable/compact-lint.toml b/crates/compact-lint/tests/fixtures/fix-unfixable/compact-lint.toml new file mode 100644 index 0000000..9bb3ab8 --- /dev/null +++ b/crates/compact-lint/tests/fixtures/fix-unfixable/compact-lint.toml @@ -0,0 +1,15 @@ +version = 1 +include = ["**/*.compact"] + +# `@throws` is forbidden but has no replacement, so `fix` leaves it where it is. +[tags] +forbid = ["@throws"] + +[kinds.module] +tags = ["@module", "@description"] + +[kinds.circuit] +tags = ["@description"] + +[kinds.constructor] +docs = "none" diff --git a/crates/compact-lint/tests/fixtures/fix-unfixable/expected.txt b/crates/compact-lint/tests/fixtures/fix-unfixable/expected.txt new file mode 100644 index 0000000..e69de29