From 7bf8808ead16ea16d703404152c4bf638ce0ef16 Mon Sep 17 00:00:00 2001 From: takeokunn Date: Mon, 3 Aug 2026 19:10:47 +0900 Subject: [PATCH] feat(lint): add a Carp rule, and record four Carp reader defects RULE_COUNT 320 -> 321. This completes dialect coverage in the strict sense: every one of the ten dialects now has a rule written for it specifically. (Carp was not previously absent from the catalogue -- `self-recursive-tail-call` and the macro-hygiene rules already list it -- but nothing targeted it.) Only one rule, and the reason is the interesting part: **the Carp compiler already rejects almost everything worth linting.** Ownership, `@`/`&` misuse, move-after-use and dangling references are each walked through in `docs/Memory.md`, and every one ends "the memory management system detects this and reports an error". `fmt` specifier/argument mismatch raises `macro-error` at expansion (`core/Format.carp:8-22`). Named holes `?x` generate a type error. A rule for any of them would duplicate the compiler. `carp-deprecated-thread-macro` survives precisely because it is the one that **builds silently**. `core/ControlMacros.carp:27,31` declares `=>` and `==>` deprecated, but `deprecated` (`core/Macros.carp:174`) expands to `meta-set!` and nothing else, and that key is read in exactly two places -- `src/Primitives.hs:355` for the REPL's `(info ...)` and `src/RenderDocs.hs:197-219` for HTML docs. No compilation path touches it. Carp's own `core/Binary.carp:68,77` still uses `==>`, which is the proof that nothing warns. Fixable, because `=>` and `->` are byte-identical macro bodies in Carp's stdlib -- the repair is a rename. The fix is withheld (finding still reported) when the file defines its own `->`/`-->`. Four candidate rules were dropped on zero or all-deliberate corpus occurrences rather than shipped and labelled unproven. **The batch's larger result is four Carp reader defects**, recorded in the package README: - `@` and `&` are not reader prefixes, though `docs/LanguageGuide.md` defines them under a literal "Reader Macros" heading. `@(f x)` splits into a bare `@` atom plus a sibling, **inflating the enclosing call's arity** -- 1493 such atoms across 116 of 248 files (47%). Byte spans survive, so round-trips are lossless; what breaks is structure, which makes any argument-counting analysis unsound for Carp. - `@"..."` silently splits string literals, because `@` glues to the next token and swallows the opening quote. 46 split atoms across 10 files that otherwise parse cleanly. - Character literals are unrecognized, so `\{ \} \[ \] \( \) \"` are read as real delimiters. - `#"..."` Pattern literals are unrecognized. Six of 248 files fail to parse outright, four of them in `core/`, each attributed by repairing one defect at a time. Same class as the Hy, LFE and Janet gaps; a repair belongs in `core/syntax`. Wiring hit the same trap PR #88 did: `fixable_rules_match_the_fix_engine` drives one fixture per dialect, so a `Fixable` rule with no fixture is a `--fix` that silently does nothing. Added a Carp fixture rather than relaxing the assertion; `fix plan` reports fix_count 1. --- Cargo.lock | 9 + Cargo.toml | 1 + packages/feature/lint-carp-idiom/Cargo.toml | 30 ++ packages/feature/lint-carp-idiom/README.md | 113 +++++++ .../lint-carp-idiom/src/corpus_tests.rs | 166 ++++++++++ .../src/deprecated_thread_macro/domain.rs | 291 ++++++++++++++++++ .../src/deprecated_thread_macro/mod.rs | 5 + .../src/deprecated_thread_macro/rule.rs | 118 +++++++ .../lint-carp-idiom/src/engine_pass_tests.rs | 276 +++++++++++++++++ packages/feature/lint-carp-idiom/src/lib.rs | 21 ++ .../feature/lint-carp-idiom/src/support.rs | 242 +++++++++++++++ src/lint/registry/catalog.rs | 14 +- src/lint/registry/mod.rs | 23 +- src/presentation/cli/lint_report/workflow.rs | 7 + tests/cli/lint_report.rs | 15 +- .../lint_golden/expected/broad.json.golden | 6 + .../lint_golden/expected/broad.sarif.golden | 16 + .../lint_golden/expected/broad.text.golden | 1 + .../expected/emacs-lisp.json.golden | 6 + .../expected/emacs-lisp.sarif.golden | 16 + .../expected/emacs-lisp.text.golden | 1 + .../lint_golden/expected/nested.json.golden | 6 + .../lint_golden/expected/nested.sarif.golden | 16 + .../lint_golden/expected/nested.text.golden | 1 + .../expected/suppressed.json.golden | 6 + .../expected/suppressed.sarif.golden | 16 + .../expected/suppressed.text.golden | 1 + 27 files changed, 1415 insertions(+), 8 deletions(-) create mode 100644 packages/feature/lint-carp-idiom/Cargo.toml create mode 100644 packages/feature/lint-carp-idiom/README.md create mode 100644 packages/feature/lint-carp-idiom/src/corpus_tests.rs create mode 100644 packages/feature/lint-carp-idiom/src/deprecated_thread_macro/domain.rs create mode 100644 packages/feature/lint-carp-idiom/src/deprecated_thread_macro/mod.rs create mode 100644 packages/feature/lint-carp-idiom/src/deprecated_thread_macro/rule.rs create mode 100644 packages/feature/lint-carp-idiom/src/engine_pass_tests.rs create mode 100644 packages/feature/lint-carp-idiom/src/lib.rs create mode 100644 packages/feature/lint-carp-idiom/src/support.rs diff --git a/Cargo.lock b/Cargo.lock index 6a2f95ed..e2778e3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -675,6 +675,7 @@ dependencies = [ "paredit-feature-inline", "paredit-feature-lint-build-system", "paredit-feature-lint-call-shape", + "paredit-feature-lint-carp-idiom", "paredit-feature-lint-clojure-idiom", "paredit-feature-lint-compile-time", "paredit-feature-lint-concurrency", @@ -1001,6 +1002,14 @@ dependencies = [ "serde_json", ] +[[package]] +name = "paredit-feature-lint-carp-idiom" +version = "1.4.0" +dependencies = [ + "paredit-core-lint-engine", + "paredit-core-syntax", +] + [[package]] name = "paredit-feature-lint-clojure-idiom" version = "1.4.0" diff --git a/Cargo.toml b/Cargo.toml index 0c2ad0b5..fc0ac29b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -146,6 +146,7 @@ paredit-feature-lint-fennel-janet-idiom = { path = "packages/feature/lint-fennel paredit-feature-lint-type-declaration = { path = "packages/feature/lint-type-declaration" } paredit-feature-lint-compile-time = { path = "packages/feature/lint-compile-time" } paredit-feature-lint-hy-lfe-idiom = { path = "packages/feature/lint-hy-lfe-idiom" } +paredit-feature-lint-carp-idiom = { path = "packages/feature/lint-carp-idiom" } paredit-feature-emacs-lisp = { path = "packages/feature/emacs-lisp" } paredit-feature-conditional-conversion = { path = "packages/feature/conditional-conversion" } paredit-feature-external-check = { path = "packages/feature/external-check" } diff --git a/packages/feature/lint-carp-idiom/Cargo.toml b/packages/feature/lint-carp-idiom/Cargo.toml new file mode 100644 index 00000000..f32b67e1 --- /dev/null +++ b/packages/feature/lint-carp-idiom/Cargo.toml @@ -0,0 +1,30 @@ +[package] +name = "paredit-feature-lint-carp-idiom" +description = "Lint rules for Carp idiom, where the defects worth reporting are the ones its ownership-tracking compiler accepts" +readme = "README.md" +publish = false +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +paredit-core-syntax = { path = "../../core/syntax" } +paredit-core-lint-engine = { path = "../../core/lint-engine" } + +# No feature-package dependency, dev or otherwise. The cost table in this +# package's report was taken with a temporary dev-dependency so a *shipped* +# rule could be timed in the same pass; it was removed again because a +# feature-to-feature edge needs an entry in the dependency allowlist contract, +# and a scratch benchmark does not earn one. +# +# Any such crate must stay unnamed here even in a comment: +# `tests/cli/feature_dependency_contract.rs` scans this file as whole text +# rather than parsing it, so a crate-name prefix appearing anywhere in it — +# a comment included — reads as a declared edge and fails the contract. + +# Mandatory: without it this package silently opts out of the workspace lint +# table, including `unsafe_code = "deny"`, with no error at all. +[lints] +workspace = true diff --git a/packages/feature/lint-carp-idiom/README.md b/packages/feature/lint-carp-idiom/README.md new file mode 100644 index 00000000..36e69177 --- /dev/null +++ b/packages/feature/lint-carp-idiom/README.md @@ -0,0 +1,113 @@ +# paredit-feature-lint-carp-idiom + +Lint rules for Carp, the statically typed, ownership-tracked Lisp that compiles +to C. + +Carp is the last of this tool's ten dialects to get rules of its own. It is +also the dialect where a linter has the least to say, and the reason is worth +stating up front: **Carp's compiler already rejects almost everything a lint +rule would want to report.** Its linear type system catches use-after-move, +invalid references and dangling references outright — `docs/Memory.md` walks +through each in turn, and each ends "the memory management system detects this +and reports an error". A rule that re-reports a compile error is worthless. + +So the rules here are confined to what the compiler has *no opinion about*: +spellings that build cleanly and silently while being wrong. That is a small +category in Carp, and this package is small accordingly. + +## Rules + +| Rule | Category | Severity | Fixability | Heads | +| --- | --- | --- | --- | --- | +| `carp-deprecated-thread-macro` | Portability | Warning | Fixable | `=>`, `==>` | + +`core/ControlMacros.carp:27,31` declares `=>` and `==>` deprecated in favour of +`->` and `-->`. The declaration expands to `meta-set!` and nothing else, and +the compiler reads that metadata key in exactly two places — `primitiveInfo` +(the REPL's `(info …)` command) and the HTML doc renderer. **No compilation +path reads it**, so the deprecated spelling builds with no diagnostic at all. +Carp's own `core/Binary.carp:68,77` still uses `==>`. + +The fix is a rename rather than a rewrite: `=>` and `->` are both defined +`(defmacro _ [:rest forms] (thread-first-internal forms))`, with identical +bodies. It is withheld when the file defines its own `->` or `-->`. + +## What this workspace's reader does with Carp + +Investigating the rules turned up four reader defects, all of them larger than +the rules. They are recorded here because they bound what any Carp rule can +do; fixing them belongs in `core/syntax`, not this package. + +Measured over `carp-lang/Carp` at 248 `.carp` files: + +**1. `@` and `&` are not reader prefixes.** Carp's guide +(`docs/LanguageGuide.md`, "Reader Macros") defines `&x` as `(ref x)` and `@x` +as `(copy x)`. `reader_policy.rs` routes Carp through `classify_legacy`, which +implements neither. So `@x` lexes as a single atom `"@x"`, and `@(f x)` lexes +as a **bare `@` atom followed by a sibling list** — which inflates the +enclosing call's arity by one: + +```text +(f @(g y)) Carp => 3 children: ["f", "@", "(g y)"] wrong +(f @(g y)) Clojure => 2 children: ["f", "@(g y)"] right +``` + +1493 such bare sigil atoms occur in 116 of the 248 files. Byte spans stay +intact, so a round trip is lossless — but **no arity or argument-position +analysis is trustworthy for Carp**, which is why the rule here keys on the head +symbol alone. + +**2. A string literal directly after `@` is not lexed as a string.** Because +`@` glues to the following token, `@"…"` is read as an atom that swallows the +opening quote. `(f @"a b")` silently becomes *two* atoms, `@"a` and `b"`, with +no error; `(f @"{")` fails to parse outright. 46 silently split string atoms +occur across 10 files that otherwise parse cleanly, and this is the cause of +three of the six outright parse failures (`core/Map.carp`, `core/Pattern.carp`, +`core/Test.carp`). + +**3. Character literals are not recognized.** Carp spells them `\a`, and +`character_literal_prefix_width` has arms for Scheme, Racket, Clojure and Emacs +Lisp but none for Carp. `\a` and `\space` survive by luck; `\{`, `\}`, `\[`, +`\]`, `\(`, `\)` and `\"` do not, because the delimiter is read as a real +delimiter. This is the cause of `core/Format.carp` (`\{`) and, with defect 2, +`examples/json_parser.carp` (`\]`, `\"`). + +**4. `#"…"` pattern literals are not recognized.** Carp's `Pattern` type has a +literal syntax, used 36 times in 2 files. It is the cause of +`test/pattern.carp`. + +Together these make **6 of 248 files fail to parse**, four of them in `core/` +— and a file that does not parse is one no command in this tool can say +anything about. + +A fifth, benign observation: Carp's unquote is `%` and its unquote-splicing is +`%@` (`docs/Quasiquotation.md`), neither of which the reader recognizes as a +prefix. Everything textually inside a `` ` `` template therefore reads as data, +which suppresses findings rather than inventing them. + +## Cost + +`HeadFilter::Heads` means the rule is never invoked at all on a file with no +`=>` or `==>` — 239 of the corpus's 248 files. When it does fire, a finding +costs one `SyntaxTree::root_view`, which materializes the document; on a +67140-byte fixture that measured 2.21 M ns/call against 4.50 M at double the +size, a ratio of 2.04 — linear in file size, as `root_view` is. The shipped +`self-recursive-tail-call`, timed in the same pass, ran 161 ns/call at ratio +1.01. The per-finding document cost is a `root_view` property this package +shares with shipped rules elsewhere in the workspace; fixing it belongs in +`core/syntax`. + +## Candidates that were investigated and rejected + +- **Ownership and `@`/`&` misuse.** Compiler-caught (`docs/Memory.md`), and in + any case unreachable given reader defect 1. +- **`fmt` specifier/argument mismatch.** `core/Format.carp` raises + `macro-error` at expansion time; the guide states the check explicitly. +- **`Array.unsafe-nth`.** Used ~20 times legitimately inside `core/Array.carp` + where the index is provably in bounds. A false-positive machine. +- **`Debug.sanitize-addresses`.** Four uses in the corpus, all deliberate, all + in `bench/`. +- **`Debug.trace`, `Debug.leak-array`, `Pointer.unsafe-alloc`, `Unsafe.*`.** + Defensible in principle but with zero or all-deliberate corpus occurrences, + so nothing could be demonstrated. Left unwritten rather than shipped on a + zero denominator. diff --git a/packages/feature/lint-carp-idiom/src/corpus_tests.rs b/packages/feature/lint-carp-idiom/src/corpus_tests.rs new file mode 100644 index 00000000..b2a947a5 --- /dev/null +++ b/packages/feature/lint-carp-idiom/src/corpus_tests.rs @@ -0,0 +1,166 @@ +//! A permanent corpus: realistic *correct* Carp that must stay silent, and a +//! dangerous twin that must fire every rule exactly once. +//! +//! The silent half is worthless on its own. A rule whose head is misspelled, +//! whose dialect scope is wrong, or that was deleted entirely also produces +//! zero findings here — so the corpus asserts a **candidate count** as well, +//! taken from the same `candidate_count` the audit used. A zero-finding sweep +//! over zero candidates says nothing; a zero-finding sweep over five threading +//! macro calls says the rule looked and declined. +//! +//! The idioms are taken from real code: the shapes below follow +//! `carp-lang/Carp`'s own `core/` and `examples/` — `defmodule` wrapping +//! `defn`, a `sig` beside its definition, `&` on borrowed arguments, `@` to +//! take an owned copy, `Array.reduce` with a `&(fn …)`, `let-do` for +//! sequencing, and `-> `/`-->` for threading. +//! +//! The correct corpus deliberately includes `@(…)` and `&(…)` forms, which +//! this workspace's reader mis-lexes into an extra sibling atom (see +//! `crate::support`). They belong here precisely because the rule must be +//! immune to that: it keys on the head symbol and never on arity. + +use paredit_core_syntax::dialect::Dialect; +use paredit_core_syntax::sexpr::SyntaxTree; + +use crate::deprecated_thread_macro; +use crate::engine_pass_tests::fired; + +/// Correct, idiomatic Carp. Every rule in this package must decline it. +const CARP_CORPUS: &str = r#";; A small module in the style of Carp's own core/. +(defmodule Stats + + (doc mean "the arithmetic mean of `xs`.") + (sig mean (Fn [&(Array Double)] Double)) + (defn mean [xs] + (let [total (Array.reduce &(fn [acc x] (+ acc @x)) 0.0 xs) + n (Array.length xs)] + (if (= n 0) + 0.0 + (/ total (from-int n))))) + + (doc normalize "scales `xs` into the unit range.") + (defn normalize [xs] + (let [top (Array.reduce &(fn [acc x] (Double.max acc @x)) 0.0 xs)] + (if (= top 0.0) + @xs + (Array.copy-map &(fn [x] (/ @x top)) xs)))) + + (doc summary "a human-readable one-line summary of `xs`.") + (defn summary [xs] + (-> (mean xs) + (Double.to-string) + (String.append " avg"))) + + (doc describe "the same, threaded the other way.") + (defn describe [xs] + (--> (mean xs) + (Double.to-string) + (String.append "average: "))) +) + +(deftype Point [x Double y Double]) + +(defmodule Point + (defn shifted [p dx] + (Point.set-x @p (+ @(Point.x p) dx))) + + (defn label [p] + (-> (Point.x p) + (Double.copy) + (Double.to-string))) + + (defn tagged [p] + (--> (Point.y p) + (Double.copy) + (Double.to-string) + (String.append "y="))) +) + +(defn main [] + (let-do [points [(Point.init 1.0 2.0) (Point.init 3.0 4.0)] + names (Array.copy-map &Point.label &points)] + (println* &(String.join ", " &names)) + (println* &(Stats.summary &[1.0 2.0 3.0])))) +"#; + +/// The same code with each rule's defect introduced exactly once. +const CARP_DANGEROUS: &str = r#"(defmodule Stats + (defn summary [xs] + (=> (mean xs) + (Double.to-string) + (String.append " avg"))) + + (defn describe [xs] + (==> (mean xs) + (Double.to-string) + (String.append "average: "))) +) +"#; + +#[test] +fn the_correct_corpus_parses() { + SyntaxTree::parse_with_dialect(CARP_CORPUS, Dialect::Carp) + .expect("the correct corpus must parse"); + SyntaxTree::parse_with_dialect(CARP_DANGEROUS, Dialect::Carp) + .expect("the dangerous corpus must parse"); +} + +#[test] +fn correct_carp_yields_no_findings() { + assert_eq!( + fired(CARP_CORPUS, Dialect::Carp), + Vec::::new(), + "idiomatic Carp must be silent" + ); +} + +/// The denominator, without which the assertion above is a false-clean. +#[test] +fn the_correct_corpus_actually_contains_candidates() { + let tree = SyntaxTree::parse_with_dialect(CARP_CORPUS, Dialect::Carp).expect("parse"); + let candidates = deprecated_thread_macro::domain::candidate_count(Dialect::Carp, &tree); + assert!( + candidates >= 4, + "the corpus must exercise the rule; got {candidates} threading macro calls" + ); + // And none of them is a deprecated spelling. + assert!( + deprecated_thread_macro::domain::collect(Dialect::Carp, &tree).is_empty(), + "the correct corpus must use only the supported spellings" + ); +} + +#[test] +fn the_dangerous_twin_fires_each_rule_exactly_once() { + let found = fired(CARP_DANGEROUS, Dialect::Carp); + assert_eq!( + found, + vec![ + "carp-deprecated-thread-macro".to_owned(), + "carp-deprecated-thread-macro".to_owned(), + ], + "each deprecated spelling must be reported once" + ); +} + +/// The correct corpus exercises the reader's `@(…)` / `&(…)` arity inflation. +/// Pinned so a later edit that "simplifies" the corpus does not quietly remove +/// the only coverage of the shape the rule had to be built around. +#[test] +fn the_correct_corpus_exercises_the_readers_arity_inflation() { + let tree = SyntaxTree::parse_with_dialect(CARP_CORPUS, Dialect::Carp).expect("parse"); + fn count_bare(view: &paredit_core_syntax::sexpr::ExpressionView, n: &mut usize) { + for child in &view.children { + if matches!(child.text.as_deref(), Some("@") | Some("&")) { + *n += 1; + } + count_bare(child, n); + } + } + let mut bare = 0; + count_bare(&tree.root_view(), &mut bare); + assert!( + bare > 0, + "the corpus should contain `@(…)`/`&(…)`, which the reader splits into a bare sigil atom" + ); +} diff --git a/packages/feature/lint-carp-idiom/src/deprecated_thread_macro/domain.rs b/packages/feature/lint-carp-idiom/src/deprecated_thread_macro/domain.rs new file mode 100644 index 00000000..e9999408 --- /dev/null +++ b/packages/feature/lint-carp-idiom/src/deprecated_thread_macro/domain.rs @@ -0,0 +1,291 @@ +//! `carp-deprecated-thread-macro` detection: a threading macro that Carp's own +//! standard library marks deprecated. +//! +//! This is not a judgement call, and it is not read off prose. `core/ControlMacros.carp` +//! declares it in Carp: +//! +//! ```text +//! core/ControlMacros.carp:27 (deprecated => "deprecated in favor of `->`.") +//! core/ControlMacros.carp:28 (defmacro => [:rest forms] (thread-first-internal forms)) +//! core/ControlMacros.carp:31 (deprecated ==> "deprecated in favor of `-->`.") +//! core/ControlMacros.carp:32 (defmacro ==> [:rest forms] (thread-last-internal forms)) +//! ``` +//! +//! and the replacements are defined in the same file from the *same* helper: +//! +//! ```text +//! core/ControlMacros.carp:45 (defmacro -> [:rest forms] (thread-first-internal forms)) +//! core/ControlMacros.carp:59 (defmacro --> [:rest forms] (thread-last-internal forms)) +//! ``` +//! +//! # Why the compiler will not tell you +//! +//! `(deprecated name "…")` is `core/Macros.carp:174`, and it expands to +//! `(meta-set! name "deprecated" v)` — it only writes metadata. The compiler +//! reads that key in exactly two places: +//! +//! - `src/Primitives.hs:355`, inside `primitiveInfo` — the REPL's `(info …)` +//! command, which a build never invokes. +//! - `src/RenderDocs.hs:197-219`, which renders a "deprecated" badge into +//! generated HTML documentation. +//! +//! There is no call from any compilation path. **Using `=>` compiles cleanly +//! with no diagnostic whatsoever**, which is precisely why the spelling +//! survives: Carp's own `core/Binary.carp:68,77` still uses `==>`. +//! +//! # Why the fix is safe +//! +//! `=>` and `->` have byte-identical macro bodies, as do `==>` and `-->`, so +//! the replacement is a rename and not a rewrite. The one way that could stop +//! being true is a file that defines its own `->`; the rule withholds the fix +//! in that case rather than assuming core's binding is the one in scope. + +use paredit_core_syntax::dialect::Dialect; +use paredit_core_syntax::sexpr::{ByteSpan, ExpressionView, SyntaxTree}; + +use crate::support::head_symbol; + +/// Carp only. `=>` is a live, non-deprecated operator elsewhere: it is +/// Clojure's `clojure.test` assertion arrow and a common user-defined macro, +/// and `==>` is an implication operator in several test libraries. Widening +/// the scope would report on code Carp's standard library says nothing about. +pub const DIALECTS: [Dialect; 1] = [Dialect::Carp]; + +/// One deprecated threading macro and the replacement core names for it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Deprecation { + pub head: &'static str, + pub replacement: &'static str, + /// Where in Carp's own sources the deprecation is declared. + pub citation: &'static str, +} + +/// Every binding in Carp's standard library carrying `deprecated` metadata. +/// +/// The list is exhaustive as of the upstream tree scanned: `(deprecated …)` +/// has exactly two call sites in all of `core/`, both in `ControlMacros.carp`. +/// It is a closed set because Carp's deprecation mechanism is an explicit +/// declaration rather than an inferred property — a new one would be a new +/// `(deprecated …)` line. +pub const DEPRECATIONS: [Deprecation; 2] = [ + Deprecation { + head: "=>", + replacement: "->", + citation: "core/ControlMacros.carp:27, `(deprecated => \"deprecated in favor of `->`.\")`", + }, + Deprecation { + head: "==>", + replacement: "-->", + citation: "core/ControlMacros.carp:31, `(deprecated ==> \"deprecated in favor of `-->`.\")`", + }, +]; + +/// The deprecation for `head`, if it names one. +#[must_use] +pub fn deprecation_for(head: &str) -> Option { + DEPRECATIONS + .iter() + .copied() + .find(|entry| entry.head == head) +} + +/// One use of a deprecated threading macro. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DeprecatedUse { + /// The whole call, which is what gets reported. + pub span: ByteSpan, + /// The head atom alone, which is the only part a fix rewrites. + pub head_span: ByteSpan, + pub deprecation: Deprecation, +} + +/// Examines one form. +/// +/// Reads the head and nothing else. It deliberately does **not** check the +/// call's arity: for Carp this workspace's reader inflates the child count of +/// any call containing `@(…)` or `&(…)`, so an arity guard here would silently +/// drop real findings — `(=> x @(f y))` reads as four children, not three. +#[must_use] +pub fn examine(dialect: Dialect, view: &ExpressionView) -> Option { + if !DIALECTS.contains(&dialect) { + return None; + } + let head = head_symbol(view)?; + let deprecation = deprecation_for(head)?; + let head_span = view.children.first()?.span; + Some(DeprecatedUse { + span: view.span, + head_span, + deprecation, + }) +} + +/// Every deprecated threading macro call in one file. +#[must_use] +pub fn collect(dialect: Dialect, tree: &SyntaxTree) -> Vec { + let root = tree.root_view(); + let mut found = Vec::new(); + let mut stack: Vec<&ExpressionView> = root.children.iter().collect(); + while let Some(view) = stack.pop() { + if let Some(item) = examine(dialect, view) { + found.push(item); + } + stack.extend(view.children.iter()); + } + found.sort_by_key(|item| item.span.start().get()); + found +} + +/// Every threading macro Carp defines, deprecated or not. +/// +/// This is the population [`candidate_count`] measures, and the choice matters. +/// Counting only `=>` and `==>` would make the denominator equal the numerator +/// — this rule has no arity or shape guard to reject a matched head — so a +/// clean corpus would report "0 findings over 0 candidates", which is the +/// false-clean a denominator exists to rule out. Counting *all four* asks the +/// question the rule actually adjudicates: of the threading macro calls in +/// this code, how many use a spelling core deprecated? +pub const THREADING_HEADS: [&str; 4] = ["->", "-->", "=>", "==>"]; + +/// How many threading macro calls this rule adjudicated: the denominator for a +/// zero-finding sweep. +/// +/// A clean run over a corpus that contains no threading macros at all is not +/// evidence of anything, and this is what distinguishes the two. +#[must_use] +pub fn candidate_count(dialect: Dialect, tree: &SyntaxTree) -> usize { + if !DIALECTS.contains(&dialect) { + return 0; + } + let root = tree.root_view(); + let mut count = 0; + let mut stack: Vec<&ExpressionView> = root.children.iter().collect(); + while let Some(view) = stack.pop() { + if head_symbol(view).is_some_and(|head| THREADING_HEADS.contains(&head)) { + count += 1; + } + stack.extend(view.children.iter()); + } + count +} + +#[cfg(test)] +mod tests { + use super::*; + + fn heads(source: &str, dialect: Dialect) -> Vec<&'static str> { + let tree = SyntaxTree::parse_with_dialect(source, dialect).expect("parse"); + collect(dialect, &tree) + .into_iter() + .map(|item| item.deprecation.head) + .collect() + } + + #[test] + fn flags_both_deprecated_spellings() { + assert_eq!(heads("(=> 8 inc inc)", Dialect::Carp), vec!["=>"]); + assert_eq!(heads("(==> results f)", Dialect::Carp), vec!["==>"]); + } + + #[test] + fn names_the_replacement_core_names() { + assert_eq!(deprecation_for("=>").expect("entry").replacement, "->"); + assert_eq!(deprecation_for("==>").expect("entry").replacement, "-->"); + } + + #[test] + fn leaves_the_supported_spellings_alone() { + assert!(heads("(-> 1 (- 10) (* 5))", Dialect::Carp).is_empty()); + assert!(heads("(--> 1 (- 10) (/ 45))", Dialect::Carp).is_empty()); + } + + /// `head_key` is verbatim for Carp, so these must not collide. + #[test] + fn a_head_that_merely_contains_the_same_bytes_is_not_one() { + assert!(heads("(=>> x f)", Dialect::Carp).is_empty()); + assert!(heads("(===> x f)", Dialect::Carp).is_empty()); + assert!(heads("(= a b)", Dialect::Carp).is_empty()); + assert!(heads("(x=> a b)", Dialect::Carp).is_empty()); + } + + /// The sigil is part of the atom text for Carp, so a `=>` appearing as an + /// *argument* rather than a head is not a call to it. + #[test] + fn the_operator_in_argument_position_is_not_a_call() { + assert!(heads("(map => xs)", Dialect::Carp).is_empty()); + } + + /// A `[…]` in Carp is an array literal or a parameter vector, never a + /// call, so its first element is not a head. + /// + /// The engine's head index happens to be paren-only too, so the dispatcher + /// would not hand the rule one of these — but `collect` and + /// `candidate_count` walk the tree themselves and do see them, and without + /// the delimiter test both would read an array of symbols as a call. + /// Mutation-testing found this: relaxing `head_symbol` to accept any list + /// killed no test until this one existed. + #[test] + fn a_bracket_or_brace_list_is_not_a_call() { + assert!(heads("(def ops [=> ==> ->])", Dialect::Carp).is_empty()); + assert!(heads("(def m {=> 1})", Dialect::Carp).is_empty()); + // And the denominator must not count them either. + let tree = SyntaxTree::parse_with_dialect("(def ops [=> ==> -> -->])", Dialect::Carp) + .expect("parse"); + assert_eq!(candidate_count(Dialect::Carp, &tree), 0); + } + + #[test] + fn a_nested_use_is_reached() { + assert_eq!( + heads("(defn f [state]\n (=> state inc))", Dialect::Carp), + vec!["=>"] + ); + } + + /// The reader gives `@(…)` an extra sibling. This pins that the rule still + /// fires through it, which an arity guard would have broken. + #[test] + fn an_inflated_arity_call_is_still_reached() { + assert_eq!(heads("(=> x @(f y))", Dialect::Carp), vec!["=>"]); + assert_eq!(heads("(==> x &(f y))", Dialect::Carp), vec!["==>"]); + } + + #[test] + fn other_dialects_are_out_of_scope() { + assert!(heads("(=> 8 inc)", Dialect::Clojure).is_empty()); + assert!(heads("(=> 8 inc)", Dialect::CommonLisp).is_empty()); + assert!(heads("(=> 8 inc)", Dialect::Fennel).is_empty()); + } + + #[test] + fn the_candidate_count_counts_every_threading_macro() { + let tree = + SyntaxTree::parse_with_dialect("(=> a b) (==> c d) (-> e f) (--> g h)", Dialect::Carp) + .expect("parse"); + // All four are adjudicated; two are deprecated. + assert_eq!(candidate_count(Dialect::Carp, &tree), 4); + assert_eq!(collect(Dialect::Carp, &tree).len(), 2); + // And nothing at all for a dialect out of scope. + let other = SyntaxTree::parse_with_dialect("(=> a b)", Dialect::Clojure).expect("parse"); + assert_eq!(candidate_count(Dialect::Clojure, &other), 0); + } + + /// The denominator has to be able to exceed the numerator, or it is not a + /// denominator. Correct code has candidates and no findings. + #[test] + fn correct_code_has_candidates_and_no_findings() { + let tree = SyntaxTree::parse_with_dialect("(-> a b) (--> c d) (-> e f)", Dialect::Carp) + .expect("parse"); + assert_eq!(candidate_count(Dialect::Carp, &tree), 3); + assert!(collect(Dialect::Carp, &tree).is_empty()); + } + + #[test] + fn the_head_span_covers_only_the_operator() { + let source = "(=> 8 inc)"; + let tree = SyntaxTree::parse_with_dialect(source, Dialect::Carp).expect("parse"); + let item = collect(Dialect::Carp, &tree).remove(0); + assert_eq!(item.head_span.slice(source), "=>"); + assert_eq!(item.span.slice(source), "(=> 8 inc)"); + } +} diff --git a/packages/feature/lint-carp-idiom/src/deprecated_thread_macro/mod.rs b/packages/feature/lint-carp-idiom/src/deprecated_thread_macro/mod.rs new file mode 100644 index 00000000..fafdfcd5 --- /dev/null +++ b/packages/feature/lint-carp-idiom/src/deprecated_thread_macro/mod.rs @@ -0,0 +1,5 @@ +//! `carp-deprecated-thread-macro`: a threading macro Carp's own standard +//! library marks deprecated. + +pub mod domain; +pub mod rule; diff --git a/packages/feature/lint-carp-idiom/src/deprecated_thread_macro/rule.rs b/packages/feature/lint-carp-idiom/src/deprecated_thread_macro/rule.rs new file mode 100644 index 00000000..c6a1e9e2 --- /dev/null +++ b/packages/feature/lint-carp-idiom/src/deprecated_thread_macro/rule.rs @@ -0,0 +1,118 @@ +//! `carp-deprecated-thread-macro`: a threading macro Carp's own standard +//! library marks deprecated. + +use paredit_core_lint_engine::LintResult; +use paredit_core_lint_engine::engine::{RuleContext, RuleSink}; +use paredit_core_lint_engine::model::{ + Fixability, HeadFilter, NormalizedHead, RuleCategory, RuleExplanation, RuleFix, RuleMeta, + Severity, +}; +use paredit_core_lint_engine::policy::RuleDialectScope; +use paredit_core_lint_engine::rule::LintRule; +use paredit_core_syntax::sexpr::ExpressionView; + +use crate::deprecated_thread_macro::domain::{self, examine}; +use crate::support::node_context; + +pub const META: RuleMeta = RuleMeta::new( + "carp-deprecated-thread-macro", + RuleCategory::Portability, + Severity::Warning, + "a threading macro Carp's standard library marks deprecated", + Fixability::Fixable, +) +.with_explanation( + RuleExplanation::new( + "Carp's `core/ControlMacros.carp` declares `(deprecated => \"deprecated in favor of \ + `->`.\")` and the same for `==>` against `-->`. That declaration only writes metadata: \ + the compiler reads the `deprecated` key in `primitiveInfo` (the REPL's `(info …)` \ + command) and in the HTML doc renderer, and nowhere on any compilation path. So the \ + deprecated spelling builds cleanly with no diagnostic at all, which is why it outlives \ + the deprecation — Carp's own `core/Binary.carp` still uses `==>`. The replacement is a \ + rename rather than a rewrite: `=>` and `->` are both defined as \ + `(defmacro _ [:rest forms] (thread-first-internal forms))`, with identical bodies.", + ) + .with_example( + "(=> state (update-pos) (draw))", + "(-> state (update-pos) (draw))", + ) + .with_caveat( + "The fix is withheld — the finding is still reported — when the same file defines its \ + own `->` or `-->`, because the rename is only meaning-preserving while the replacement \ + resolves to core's binding.", + ) + .with_caveat( + "A test that deliberately exercises the deprecated macro is reported like any other use. \ + Four of the nine findings in an audit of Carp's own tree are of that kind, all in \ + `test/produces-output/basics.carp`'s `threading` function, which prints and evaluates \ + `(=> …)` and `(==> …)` on purpose. They are correctly identified as deprecated uses but \ + are not defects to fix; suppress them at the file level.", + ), +); + +/// One head per entry of [`domain::DEPRECATIONS`], written out because +/// `NormalizedHead::new` is `const` and the table is not a `const` iterator. +/// +/// `head_key` is verbatim for Carp — there is no case folding — so these must +/// match the source spelling byte for byte. +const HEADS: [NormalizedHead; 2] = [NormalizedHead::new("=>"), NormalizedHead::new("==>")]; + +#[derive(Debug)] +pub struct Rule; + +pub const RULE: Rule = Rule; + +impl LintRule for Rule { + fn head_filter(&self) -> HeadFilter { + HeadFilter::Heads(&HEADS) + } + + fn dialect_scope(&self) -> RuleDialectScope { + // Reads the domain's own table, so the head set and the dialect gate + // cannot drift apart in a later edit. + RuleDialectScope::new(&domain::DIALECTS) + } + + fn check( + &self, + context: &RuleContext<'_>, + view: &ExpressionView, + sink: &mut RuleSink<'_, '_>, + ) -> LintResult { + let Some(item) = examine(context.dialect(), view) else { + return Ok(()); + }; + // Asked only once a finding exists, and asked *once* for both facts. + // The dispatcher hands a rule quoted nodes like any other, so a + // `(=> …)` inside a macro template reaches here — but this + // materializes the whole document, so asking before `examine` would + // charge every visited node for a walk that almost always answers + // "no", and asking twice cost four orders of magnitude when measured. + let context_at = node_context(context.tree(), view.span, item.deprecation.replacement); + if context_at.is_data { + return Ok(()); + } + let message = format!( + "{} is deprecated in favor of {} ({}); it still compiles because Carp's \ + deprecation metadata is only read by `(info …)` and the doc renderer", + item.deprecation.head, item.deprecation.replacement, item.deprecation.citation + ); + // A file that defines its own `->` would have the rename change + // meaning rather than preserve it, so the finding stands but the fix + // does not. + if context_at.defines_asked_name { + sink.report(item.span, message); + return Ok(()); + } + let fix = RuleFix::single( + item.head_span, + item.deprecation.replacement.to_owned(), + format!( + "Replace {} with {}", + item.deprecation.head, item.deprecation.replacement + ), + ); + sink.report_fixed(item.span, message, fix); + Ok(()) + } +} diff --git a/packages/feature/lint-carp-idiom/src/engine_pass_tests.rs b/packages/feature/lint-carp-idiom/src/engine_pass_tests.rs new file mode 100644 index 00000000..25259ed0 --- /dev/null +++ b/packages/feature/lint-carp-idiom/src/engine_pass_tests.rs @@ -0,0 +1,276 @@ +//! Every rule driven through the *real* engine rather than by calling +//! `examine` or `check` directly. +//! +//! Calling `check` bypasses the head index and the dialect filter, which is +//! where the two mistakes this package is most exposed to would hide: +//! +//! - A `HeadFilter::Heads` entry that does not match the source spelling. For +//! Carp `head_key` returns the head *verbatim*, so `NormalizedHead::new` and +//! the source have to agree byte for byte — there is no case folding to +//! rescue a mismatch, and a rule with a wrong head simply never runs. `=>` +//! and `==>` are pure punctuation, so this is the whole safety net. +//! - A forgotten `dialect_scope`. The trait's default is `COMMON_LISP_ONLY`, +//! so a rule that omits the override silently never fires on Carp while +//! every unit test on `examine`, which takes the dialect as an argument, +//! still passes. + +use std::path::Path; + +use paredit_core_lint_engine::engine::{build_head_index, collect_lint_outcomes}; +use paredit_core_lint_engine::policy::RuleSelection; +use paredit_core_lint_engine::rule::RuleCatalog; +use paredit_core_syntax::dialect::Dialect; +use paredit_core_syntax::sexpr::SyntaxTree; + +use crate::ENTRIES; + +fn path_for(dialect: Dialect) -> &'static Path { + match dialect { + Dialect::Carp => Path::new("t.carp"), + Dialect::Clojure => Path::new("t.clj"), + _ => Path::new("t.lisp"), + } +} + +fn outcomes(source: &str, dialect: Dialect) -> Vec<(String, String, bool)> { + let catalog = RuleCatalog::new(&ENTRIES); + let index = build_head_index(catalog); + let tree = SyntaxTree::parse_with_dialect(source, dialect).expect("parse"); + collect_lint_outcomes( + catalog, + &index, + path_for(dialect), + dialect, + &tree, + source, + RuleSelection::All, + ) + .expect("lint pass") + .into_iter() + .map(|outcome| { + let (finding, fix) = outcome.into_parts(); + ( + finding.rule.to_owned(), + finding.message.clone(), + fix.is_some(), + ) + }) + .collect() +} + +/// `source` with every offered fix applied, so a test can assert on the text +/// the user would actually end up with rather than merely that a fix exists. +/// +/// Applies right to left so an earlier edit cannot shift a later span. +fn fixed_source(source: &str, dialect: Dialect) -> String { + let catalog = RuleCatalog::new(&ENTRIES); + let index = build_head_index(catalog); + let tree = SyntaxTree::parse_with_dialect(source, dialect).expect("parse"); + let mut edits: Vec<(usize, usize, String)> = collect_lint_outcomes( + catalog, + &index, + path_for(dialect), + dialect, + &tree, + source, + RuleSelection::All, + ) + .expect("lint pass") + .into_iter() + .filter_map(|outcome| outcome.into_parts().1) + .flat_map(|fix| { + fix.replacements() + .map(|replacement| { + ( + replacement.span().start().get(), + replacement.span().end().get(), + replacement.text().to_owned(), + ) + }) + .collect::>() + }) + .collect(); + edits.sort_by_key(|(start, _, _)| std::cmp::Reverse(*start)); + + let mut out = source.to_owned(); + for (start, end, text) in edits { + out.replace_range(start..end, &text); + } + out +} + +/// The whole point of a `Fixability::Fixable` rule: the rewrite has to produce +/// the right bytes. Asserting only that a fix *exists* would pass just as well +/// for a fix that replaced the wrong span. +#[test] +fn the_fix_rewrites_only_the_operator() { + assert_eq!( + fixed_source("(=> state (update-pos) (draw))", Dialect::Carp), + "(-> state (update-pos) (draw))" + ); + assert_eq!( + fixed_source("(==> results (Array.copy-filter &f))", Dialect::Carp), + "(--> results (Array.copy-filter &f))" + ); +} + +/// Several fixes in one document must not corrupt each other's spans, and the +/// `@(…)` shape the reader mis-lexes must survive the rewrite untouched. +#[test] +fn several_fixes_in_one_document_compose() { + assert_eq!( + fixed_source("(=> a @(f b))\n(==> c &(g d))\n(-> e h)", Dialect::Carp), + "(-> a @(f b))\n(--> c &(g d))\n(-> e h)" + ); +} + +/// A fix that is offered must leave the file parseable, and re-linting the +/// result must find nothing left to do. +#[test] +fn the_fixed_source_is_clean_and_stable() { + let once = fixed_source("(=> a inc)\n(==> b dec)", Dialect::Carp); + SyntaxTree::parse_with_dialect(&once, Dialect::Carp).expect("the fixed source must parse"); + assert!( + fired(&once, Dialect::Carp).is_empty(), + "the rewrite must remove the finding, not move it" + ); + assert_eq!( + fixed_source(&once, Dialect::Carp), + once, + "and be idempotent" + ); +} + +/// The rule names that fire on `source`, sorted so the assertions do not +/// depend on registration order. +pub(crate) fn fired(source: &str, dialect: Dialect) -> Vec { + let mut names: Vec = outcomes(source, dialect) + .into_iter() + .map(|(rule, _, _)| rule) + .collect(); + names.sort(); + names +} + +// -- the rule reaches the engine ----------------------------------------- + +#[test] +fn every_rule_fires_through_the_real_dispatch() { + assert_eq!( + fired("(defn f [x] (=> x inc inc))", Dialect::Carp), + vec!["carp-deprecated-thread-macro"] + ); + assert_eq!( + fired("(defn f [x] (==> x inc inc))", Dialect::Carp), + vec!["carp-deprecated-thread-macro"] + ); +} + +/// Both heads have to be in `HEADS` independently. Deleting either one from +/// the array leaves the other's test green, which is exactly how a sibling +/// batch shipped a rule with a missing head. +#[test] +fn each_head_is_indexed_separately() { + assert_eq!(fired("(=> a b)", Dialect::Carp).len(), 1); + assert_eq!(fired("(==> a b)", Dialect::Carp).len(), 1); + // Both in one document: two findings, not one. + assert_eq!(fired("(=> a b)\n(==> c d)", Dialect::Carp).len(), 2); +} + +/// The dialect gate, through the engine. `=>` is a live operator in Clojure +/// test code, so firing there would be a real false positive. +#[test] +fn the_dialect_scope_holds_through_dispatch() { + assert!(fired("(=> a b)", Dialect::Clojure).is_empty()); + assert!(fired("(=> a b)", Dialect::CommonLisp).is_empty()); + assert!(fired("(=> a b)", Dialect::Fennel).is_empty()); + assert!(fired("(=> a b)", Dialect::Janet).is_empty()); +} + +#[test] +fn the_supported_spelling_never_fires() { + assert!(fired("(-> a b)\n(--> c d)", Dialect::Carp).is_empty()); +} + +// -- the fix ------------------------------------------------------------- + +#[test] +fn a_plain_use_carries_a_fix() { + let found = outcomes("(=> a inc)", Dialect::Carp); + assert_eq!(found.len(), 1); + assert!(found[0].2, "expected a fix to be offered"); +} + +/// The shadowing guard, through the engine: a file that defines its own `->` +/// still gets the finding, but not the rewrite. +#[test] +fn a_shadowed_replacement_reports_without_a_fix() { + let source = "(defmacro -> [:rest forms] (my-own-threading forms))\n(=> a inc)"; + let found = outcomes(source, Dialect::Carp); + assert_eq!(found.len(), 1, "the finding must still be reported"); + assert!( + !found[0].2, + "the fix must be withheld when `->` is shadowed" + ); +} + +/// Carp puts most definitions inside a `defmodule`, so the shadowing scan has +/// to descend into container bodies. A scan that only looked at top level +/// would miss this and offer a rewrite to a `->` that is not core's. +#[test] +fn a_replacement_shadowed_inside_a_module_still_withholds_the_fix() { + let source = "(defmodule M\n (defmacro -> [:rest forms] (mine forms))\n)\n(=> a inc)"; + let found = outcomes(source, Dialect::Carp); + assert_eq!(found.len(), 1, "the finding must still be reported"); + assert!( + !found[0].2, + "a `->` defined inside a defmodule shadows core's just as well" + ); +} + +/// Shadowing the *other* replacement must not suppress this one's fix. +#[test] +fn shadowing_is_matched_against_the_right_replacement() { + let source = "(defmacro --> [:rest forms] (mine forms))\n(=> a inc)"; + let found = outcomes(source, Dialect::Carp); + assert_eq!(found.len(), 1); + assert!( + found[0].2, + "`-->` being shadowed says nothing about rewriting `=>` to `->`" + ); +} + +// -- quoting ------------------------------------------------------------- + +/// A deprecated head inside quoted data is not a call to it. +#[test] +fn a_quoted_use_is_not_reported() { + // The `'` reader prefix. + assert!(fired("(def forms '(=> a b))", Dialect::Carp).is_empty()); + // And the long-hand spellings, which hand-written Carp macros use and + // which `docs/Quasiquotation.md` presents as the primary form. + assert!(fired("(quote (=> a b))", Dialect::Carp).is_empty()); + assert!(fired("(quasiquote (=> a b))", Dialect::Carp).is_empty()); +} + +/// But a macro that genuinely emits the deprecated spelling into a template is +/// still a use, and the conservative quote model means this one is suppressed. +/// Pinned so the limitation is visible rather than assumed away. +#[test] +fn a_quasiquoted_use_is_suppressed_by_the_conservative_quote_model() { + assert!( + fired("(defmacro m [x] `(=> %x inc))", Dialect::Carp).is_empty(), + "Carp's `%` unquote is not a reader prefix here, so the whole template reads as data" + ); +} + +// -- the reader's arity inflation ---------------------------------------- + +/// `@(…)` and `&(…)` give the enclosing call an extra child. The rule keys on +/// the head alone precisely so this cannot hide a finding. +#[test] +fn a_call_whose_arity_the_reader_inflates_still_fires() { + assert_eq!(fired("(=> x @(f y))", Dialect::Carp).len(), 1); + assert_eq!(fired("(==> x &(g y))", Dialect::Carp).len(), 1); + assert_eq!(fired("(=> @(a b) &(c d) @e)", Dialect::Carp).len(), 1); +} diff --git a/packages/feature/lint-carp-idiom/src/lib.rs b/packages/feature/lint-carp-idiom/src/lib.rs new file mode 100644 index 00000000..f9557bbe --- /dev/null +++ b/packages/feature/lint-carp-idiom/src/lib.rs @@ -0,0 +1,21 @@ +#![doc = include_str!("../README.md")] + +pub mod deprecated_thread_macro; +pub mod support; + +#[cfg(test)] +mod corpus_tests; +#[cfg(test)] +mod engine_pass_tests; + +/// The rules this package publishes, in the order a registry should list them. +/// +/// `cfg(test)` because the root crate owns the catalogue; this exists so the +/// package's own engine-driven tests and the eventual wiring pass name the +/// same rule. +#[cfg(test)] +pub(crate) static ENTRIES: [paredit_core_lint_engine::rule::RuleEntry; 1] = + [paredit_core_lint_engine::rule::RuleEntry::new( + &deprecated_thread_macro::rule::META, + &deprecated_thread_macro::rule::RULE, + )]; diff --git a/packages/feature/lint-carp-idiom/src/support.rs b/packages/feature/lint-carp-idiom/src/support.rs new file mode 100644 index 00000000..49afbfbd --- /dev/null +++ b/packages/feature/lint-carp-idiom/src/support.rs @@ -0,0 +1,242 @@ +//! What this crate's rules share: how to read an atom, how to read a call's +//! head, and how much of the surrounding reader syntax says "this is data". +//! +//! Nothing here runs per visited node. Every helper is called from inside a +//! rule that has already matched its head, and the expensive one +//! ([`is_unevaluated_at`]) is called only once a finding is otherwise ready to +//! report — it descends from [`SyntaxTree::root_view`], which materializes the +//! whole document, so calling it before the cheap head check costs orders of +//! magnitude more per invocation than the check it guards. +//! +//! # What this workspace's reader does with Carp +//! +//! Carp's own guide (`docs/LanguageGuide.md`, "Reader Macros") defines two: +//! +//! ```text +//! &x ;; same as (ref x) +//! @x ;; same as (copy x) +//! ``` +//! +//! This workspace's reader implements **neither**. `reader_policy.rs` routes +//! Carp through `classify_legacy`, which knows `#;`, `#_`, `#+`, `#-` and the +//! shared quote prefixes, and nothing about `@` or `&`. The consequences are +//! measured and recorded in this package's README; the two that constrain the +//! rules here are: +//! +//! - `@x` and `&x` lex as *one atom* whose text includes the sigil (`"@x"`), +//! so a head or symbol comparison must not assume the sigil was stripped. +//! - `@(f x)` and `&(f x)` lex as a **bare `@` atom followed by a sibling +//! list**, which inflates the enclosing call's arity by one. Over the +//! upstream corpus that is 1493 sites in 116 of 248 files, so **no rule in +//! this crate may index or count a call's arguments** — the count is not +//! trustworthy for Carp. Every rule here keys on the head symbol alone. +//! +//! Carp's unquote is `%` and its unquote-splicing is `%@` +//! (`docs/Quasiquotation.md`), neither of which the reader recognizes as a +//! prefix either. [`QuoteState::quasi`] therefore never counts back down for +//! Carp, so everything textually inside a `` ` `` template reads as data. That +//! suppresses findings rather than inventing them, which is the side to be +//! wrong on. + +use paredit_core_syntax::sexpr::{ + ByteSpan, Delimiter, ExpressionKind, ExpressionView, ReaderPrefix, SyntaxTree, +}; + +/// An atom's text, exactly as the source spells it — *including* any reader +/// prefix the reader did recognize. +/// +/// That is what makes this safe to compare against a bare symbol name without +/// also testing `reader_prefixes`: `'=>` has `text == "'=>"`, which is not +/// equal to `"=>"`, so quoted data can never be read as an operator. +/// The kind test is, today, redundant with the reader: `tree.rs:1022` +/// populates `text` with `(node.kind == NodeKind::Atom)`, so a list's `text` is +/// always `None` and no input can tell the two spellings apart. +/// Mutation-testing confirmed it — removing the kind test killed no test, and +/// unlike the paren-only test in [`head_symbol`] no test *could* be written +/// for it. +/// +/// It stays for the reason the head index states about itself: a helper that +/// leaned on another component's current shape for its notion of what an atom +/// is would be correct only by accident. +pub(crate) fn atom_text(view: &ExpressionView) -> Option<&str> { + (view.kind == ExpressionKind::Atom) + .then_some(view.text.as_deref()) + .flatten() +} + +/// The exact head symbol of a `(...)` list. +/// +/// Paren-delimited only. In Carp a `[...]` is an array literal or a parameter +/// vector and a `{...}` is a map literal — none of them is a call — so reading +/// a "head" off one would invent an operator the reader never produced. +pub(crate) fn head_symbol(view: &ExpressionView) -> Option<&str> { + (view.kind == ExpressionKind::List && view.delimiter == Some(Delimiter::Paren)) + .then(|| view.children.first()) + .flatten() + .and_then(atom_text) +} + +/// How much of the surrounding reader syntax says "this is data". +/// +/// Two independent counters, because `'` and `` ` `` are not the same thing. A +/// `%` inside `'(…)` is a percent character in a literal list, so `hard` never +/// clears; a `%` inside `` `(…) `` is meant to escape back to code, so `quasi` +/// would count up and down. A single `i32` depth counter cannot express that +/// and has shipped elsewhere in this workspace as a false-positive source. +/// +/// As the module header records, the reader gives Carp no `Unquote` prefix at +/// all, so in practice `quasi` only ever counts up here. The +/// [`ReaderPrefix::Unquote`] arm is kept because it is what makes this a +/// correct quote model rather than one that happens to work on the inputs the +/// reader can currently produce. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct QuoteState { + hard: bool, + quasi: u32, +} + +impl QuoteState { + const EVALUATED: Self = Self { + hard: false, + quasi: 0, + }; + + const fn is_data(self) -> bool { + self.hard || self.quasi > 0 + } + + /// The state inside a node, given the state outside it and the node's own + /// reader prefixes. + /// + /// `#`, `#'` and the rest are deliberately neutral: none of them turns + /// code into data. + fn after_prefixes(mut self, view: &ExpressionView) -> Self { + for prefix in &view.reader_prefixes { + match prefix { + ReaderPrefix::Quote => self.hard = true, + ReaderPrefix::Quasiquote => self.quasi += 1, + ReaderPrefix::Unquote | ReaderPrefix::UnquoteSplicing => { + self.quasi = self.quasi.saturating_sub(1); + } + _ => {} + } + } + self + } + + const fn quoted(mut self) -> Self { + self.hard = true; + self + } +} + +/// The long-hand `(quote …)`, which the reader also produces for `'…` but +/// which hand-written Carp macros spell out — and `(quasiquote …)`, which +/// `docs/Quasiquotation.md` presents as the primary spelling with `` ` `` as +/// the literal shorthand. +fn quoting_head(view: &ExpressionView) -> Option<&str> { + head_symbol(view).filter(|head| matches!(*head, "quote" | "quasiquote")) +} + +const fn span_contains(outer: ByteSpan, inner: ByteSpan) -> bool { + outer.start().get() <= inner.start().get() && inner.end().get() <= outer.end().get() +} + +/// The heads that bind a name in Carp. +/// +/// `defmacro`, `defndynamic` and `defdynamic` bind at macro-expansion time; +/// `defn` and `def` bind at runtime. Any of them shadowing a core threading +/// macro is enough to make a rename stop being meaning-preserving. +const DEFINING_HEADS: [&str; 5] = ["defmacro", "defndynamic", "defdynamic", "defn", "def"]; + +/// Everything one materialization of the document can answer at once. +#[derive(Debug, Clone, Copy)] +pub(crate) struct NodeContext { + /// Whether the node is unevaluated data rather than code. + pub(crate) is_data: bool, + /// Whether this file defines the name that was asked about. + pub(crate) defines_asked_name: bool, +} + +/// Both facts about `target`, from a single materialization of the document. +/// +/// This exists for cost, not tidiness. [`SyntaxTree::root_view`] materializes +/// the whole document into owned views, so its cost is the file's size, not +/// the node's depth; a rule that asked the two questions separately paid that +/// twice. Measured on a 67140-byte dense Carp fixture, splitting them cost +/// 8589447 ns/call — four orders of magnitude above the shipped comparator in +/// the same pass — and merging them is the single largest saving available +/// without changing the core API. +/// +/// This is why the caller applies its cheap head check first and reaches this +/// only with a finding otherwise ready to report. +/// +/// # The two answers +/// +/// **`is_data`** is read *at* the target and nowhere shallower. An ancestor +/// being quasiquoted does not settle it; being inside a hard `'` does, and +/// that is already modelled by `hard` never clearing. A span that names no +/// node is judged by the innermost node containing it, which is the honest +/// answer for a span a caller synthesized rather than took from the tree. +/// +/// **`defines_asked_name`** scans *definition sites only*: top-level forms and +/// the bodies of `defmodule`/`with`/`do` forms, which is where Carp puts +/// `defn` and friends. It deliberately does not descend into function bodies, +/// because Carp has no internal defines — a `defn` cannot appear inside +/// another `defn`'s body — so a full walk would pay for the whole document to +/// find nothing. It is otherwise blind to module nesting, answering "does this +/// file define this name anywhere?", which over-approximates shadowing. +/// Over-approximating *withholds a fix*, the safe direction; under-approximating +/// would rewrite code whose meaning it had not established. +#[must_use] +pub(crate) fn node_context(tree: &SyntaxTree, target: ByteSpan, name: &str) -> NodeContext { + let root = tree.root_view(); + + // -- the descent, for the quote verdict -------------------------------- + let mut view: &ExpressionView = &root; + let mut state = QuoteState::EVALUATED; + let is_data = loop { + let quoting = quoting_head(view).is_some(); + let Some(child) = view + .children + .iter() + .find(|child| span_contains(child.span, target)) + else { + break state.is_data(); + }; + state = state.after_prefixes(child); + if quoting { + state = state.quoted(); + } + view = child; + if view.span == target { + break state.is_data(); + } + }; + + // -- the definition-site scan, for the shadowing verdict --------------- + const CONTAINER_HEADS: [&str; 3] = ["defmodule", "with", "do"]; + let mut defines_asked_name = false; + let mut stack: Vec<&ExpressionView> = root.children.iter().collect(); + while let Some(view) = stack.pop() { + let Some(head) = head_symbol(view) else { + continue; + }; + if DEFINING_HEADS.contains(&head) { + let defined = view.children.get(1).and_then(atom_text); + if defined == Some(name) { + defines_asked_name = true; + break; + } + continue; + } + if CONTAINER_HEADS.contains(&head) { + stack.extend(view.children.iter()); + } + } + + NodeContext { + is_data, + defines_asked_name, + } +} diff --git a/src/lint/registry/catalog.rs b/src/lint/registry/catalog.rs index eb4b531d..69c84a2c 100644 --- a/src/lint/registry/catalog.rs +++ b/src/lint/registry/catalog.rs @@ -217,7 +217,10 @@ pub const PEDANTIC_RULES: [&str; tagged_count(RuleTag::Pedantic)] = { // registry-only — no `cli/` directory, so no standalone command and // `INTROSPECTION_COMMANDS` stays at 340 where the batch above left it. Three of // the four are Hy and the fourth is LFE; none of them runs on Common Lisp. -const _: () = assert!(RULE_COUNT == 320); +// 320 + 1 (`lint-carp-idiom`) = 321. One rule, not more: the Carp compiler +// already rejects the ownership defects that looked most promising, so the only +// one worth a lint is the one that builds silently. +const _: () = assert!(RULE_COUNT == 321); // Unchanged at 99: every one of this branch's 37 rules is // `Fixability::ReportOnly`. Each one reports a judgment the tool cannot make // on the author's behalf — whether an annotation or the parameter list under it @@ -278,7 +281,10 @@ const _: () = assert!(RULE_COUNT == 320); // meant to handle, which the tool cannot know. `lfe-catch-swallows-exit` is the // clearest of the four: rewriting `(catch Expr)` to `try … catch` requires // inventing the failure continuation the `catch` form never had. -const _: () = assert!(fixable_count() == 103); +// 103 + 1: `carp-deprecated-thread-macro` is the rare mechanical fix -- `=>` +// and `->` are byte-identical macro bodies in Carp's own stdlib, so the repair +// is a rename. It is still withheld when the file defines its own `->`. +const _: () = assert!(fixable_count() == 104); // 164 (through PR #82) + 31 of this branch's 37 rules. The other 6 are // `Severity::Error`: `when-unless-implicit-nil-misused` and the five // `lint-safety` rules that report an exploitable defect rather than a risk — @@ -349,7 +355,9 @@ const _: () = assert!(fixable_count() == 103); // trouble. Note `lfe-catch-swallows-exit` is one of the 3: it is a warning that // also carries `RuleTag::Pedantic`, so it counts here and is subtracted again // in the preset-filtered count `tests/cli/lint_report.rs` pins. -const _: () = assert!(warning_count() == 235); +// 235 + 1: the Carp rule is a `Warning`. Deprecated-but-working code is not an +// error, and Carp's own stdlib still uses the spelling. +const _: () = assert!(warning_count() == 236); const _: () = assert!(EXPERIMENTAL_RULES.is_empty()); // 6 (through PR #82) + 8 of this branch's rules: `lint-call-shape`'s four // threshold rules, whose limits are conventions a codebase either adopted or diff --git a/src/lint/registry/mod.rs b/src/lint/registry/mod.rs index 1e25ebeb..80908f09 100644 --- a/src/lint/registry/mod.rs +++ b/src/lint/registry/mod.rs @@ -145,7 +145,24 @@ use super::rule::RuleEntry; // inflating arity at exit 0. Same class as the Janet backtick defect fixed in // PR #91, and the same disposition: a reader repair belongs in `core/syntax`, // not in a rule package. -pub const RULE_COUNT: usize = 320; +// +// 320 + 1 (`lint-carp-idiom`) = 321, completing dialect coverage: every one of +// the ten dialects now has a rule written for it specifically. Only one rule +// because the Carp compiler already rejects the ownership and format-arity +// defects that looked most promising -- `docs/Memory.md` walks through each and +// every one ends "the memory management system detects this and reports an +// error", and `core/Format.carp:8-22` raises `macro-error` at expansion. +// `carp-deprecated-thread-macro` survives precisely because it is the one that +// *builds silently*: `deprecated` expands to `meta-set!`, and the key is read +// only by the REPL's `(info ...)` and the HTML doc renderer, never by a +// compilation path. Carp's own `core/Binary.carp:68,77` still uses `==>`. +// +// That batch's larger result is in its README: four Carp reader defects, of +// which `@`/`&` not being reader prefixes is the worst -- `@(f x)` splits into +// a bare sigil atom plus a sibling, inflating the enclosing call's arity in 116 +// of 248 files, and `@"..."` silently splits string literals. Same class as the +// Hy, LFE and Janet gaps above; a repair belongs in `core/syntax`. +pub const RULE_COUNT: usize = 321; /// Every rule, in report order: findings are grouped by this order, and the /// public `RULES`/`RULE_DOCS` arrays preserve it. @@ -1444,4 +1461,8 @@ pub const REGISTRY: [RuleEntry; RULE_COUNT] = [ &paredit_feature_lint_hy_lfe_idiom::catch_swallows_exit::rule::META, &paredit_feature_lint_hy_lfe_idiom::catch_swallows_exit::rule::RULE, ), + RuleEntry::new( + &paredit_feature_lint_carp_idiom::deprecated_thread_macro::rule::META, + &paredit_feature_lint_carp_idiom::deprecated_thread_macro::rule::RULE, + ), ]; diff --git a/src/presentation/cli/lint_report/workflow.rs b/src/presentation/cli/lint_report/workflow.rs index f0be2331..44ab33a2 100644 --- a/src/presentation/cli/lint_report/workflow.rs +++ b/src/presentation/cli/lint_report/workflow.rs @@ -2448,6 +2448,12 @@ mod tests { "(memq 101 smk)\n", // scheme-memq-assq-literal-key "(let sln ((slni 0)) (* slni 2))\n", // scheme-named-let-never-recurs ); + // The Carp half, added for the same reason the Scheme half was: its one + // fixable rule is `Dialect::Carp` only, so no fixture above reaches it. + // Note `=>` must not be shadowed by a local `->` definition in this + // file — the rule withholds its fix when it is, which would make this + // test fail for a reason that has nothing to do with the fix engine. + let carp_source = "(=> cdtm (f) (g))\n"; // carp-deprecated-thread-macro let active: Vec<&str> = RULES.to_vec(); let mut produced: BTreeSet<&str> = BTreeSet::new(); @@ -2455,6 +2461,7 @@ mod tests { (source, Dialect::CommonLisp, "fixture.lisp"), (elisp_source, Dialect::EmacsLisp, "fixture.el"), (scheme_source, Dialect::Scheme, "fixture.scm"), + (carp_source, Dialect::Carp, "fixture.carp"), ] { let tree = paredit_core_syntax::sexpr::SyntaxTree::parse_with_dialect(text, dialect) .expect("parse fixture"); diff --git a/tests/cli/lint_report.rs b/tests/cli/lint_report.rs index ba3ce28a..4c1d02d3 100644 --- a/tests/cli/lint_report.rs +++ b/tests/cli/lint_report.rs @@ -304,8 +304,9 @@ fn cli_lint_list_rules_prints_the_catalog_without_files() { // the suite's 320 less *16* — the divisor moved for the first time in // four batches, because `lfe-catch-swallows-exit` ships tagged // `pedantic`. So this rises by 3 where the suite rises by 4, and the - // two numbers are not the same arithmetic. - .stdout(predicate::str::contains("\"rule_count\": 304")) + // two numbers are not the same arithmetic. + this branch's 1 = 305, + // and the divisor holds at 16, so it is a plain +1 again. + .stdout(predicate::str::contains("\"rule_count\": 305")) .stdout(predicate::str::contains("\"self-assignment\"")) .stdout(predicate::str::contains( "a setq/setf/psetq/psetf that assigns a place to itself", @@ -997,7 +998,7 @@ fn cli_lint_list_rules_marks_severity() { // `lfe-catch-swallows-exit` is also tagged `pedantic`, so the default // preset holds it back and only 2 of the 3 are visible here: 235 less the // now-16 `pedantic` rules, all still warnings, = 219. - assert_eq!(warnings, 219); + assert_eq!(warnings, 220); } #[test] @@ -1030,8 +1031,14 @@ fn cli_lint_list_rules_marks_fixability() { // `lfe-catch-swallows-exit` is `ReportOnly` like the other three. The // sentence "none of the pedantic rules is fixable" is now load-bearing over // 16 rules rather than 15. + // + // This branch finally moves it: `carp-deprecated-thread-macro` is + // `Fixability::Fixable` and untagged, so it lands in both the suite total + // and the preset-filtered one. It is the rare mechanical repair -- `=>` + // and `->` are byte-identical macro bodies in Carp's own stdlib, so the + // fix is a rename. assert_eq!( - fixable_count, 103, + fixable_count, 104, "the fixable rules the default preset admits" ); diff --git a/tests/fixtures/lint_golden/expected/broad.json.golden b/tests/fixtures/lint_golden/expected/broad.json.golden index b87ca7de..a8023324 100644 --- a/tests/fixtures/lint_golden/expected/broad.json.golden +++ b/tests/fixtures/lint_golden/expected/broad.json.golden @@ -3660,6 +3660,12 @@ "count": 0, "description": "`(except [] …)` catches every BaseException, Ctrl-C and SystemExit included", "rule": "hy-bare-except" + }, + { + "category": "portability", + "count": 0, + "description": "a threading macro Carp's standard library marks deprecated", + "rule": "carp-deprecated-thread-macro" } ], "policy": { diff --git a/tests/fixtures/lint_golden/expected/broad.sarif.golden b/tests/fixtures/lint_golden/expected/broad.sarif.golden index fee617ce..5d72d6e1 100644 --- a/tests/fixtures/lint_golden/expected/broad.sarif.golden +++ b/tests/fixtures/lint_golden/expected/broad.sarif.golden @@ -10122,6 +10122,22 @@ "shortDescription": { "text": "`(catch Expr)` returns failures as ordinary terms, indistinguishable from success" } + }, + { + "fullDescription": { + "text": "Carp's `core/ControlMacros.carp` declares `(deprecated => \"deprecated in favor of `->`.\")` and the same for `==>` against `-->`. That declaration only writes metadata: the compiler reads the `deprecated` key in `primitiveInfo` (the REPL's `(info …)` command) and in the HTML doc renderer, and nowhere on any compilation path. So the deprecated spelling builds cleanly with no diagnostic at all, which is why it outlives the deprecation — Carp's own `core/Binary.carp` still uses `==>`. The replacement is a rename rather than a rewrite: `=>` and `->` are both defined as `(defmacro _ [:rest forms] (thread-first-internal forms))`, with identical bodies." + }, + "id": "carp-deprecated-thread-macro", + "properties": { + "category": "portability", + "fixable": true, + "rationale": "Carp's `core/ControlMacros.carp` declares `(deprecated => \"deprecated in favor of `->`.\")` and the same for `==>` against `-->`. That declaration only writes metadata: the compiler reads the `deprecated` key in `primitiveInfo` (the REPL's `(info …)` command) and in the HTML doc renderer, and nowhere on any compilation path. So the deprecated spelling builds cleanly with no diagnostic at all, which is why it outlives the deprecation — Carp's own `core/Binary.carp` still uses `==>`. The replacement is a rename rather than a rewrite: `=>` and `->` are both defined as `(defmacro _ [:rest forms] (thread-first-internal forms))`, with identical bodies.", + "severity": "warning", + "tags": [] + }, + "shortDescription": { + "text": "a threading macro Carp's standard library marks deprecated" + } } ] } diff --git a/tests/fixtures/lint_golden/expected/broad.text.golden b/tests/fixtures/lint_golden/expected/broad.text.golden index b03850cf..c93aa66c 100644 --- a/tests/fixtures/lint_golden/expected/broad.text.golden +++ b/tests/fixtures/lint_golden/expected/broad.text.golden @@ -303,6 +303,7 @@ rule defconstant-non-eql-value 0 rule hy-mutable-default-argument 0 rule hy-identity-comparison-with-literal 0 rule hy-bare-except 0 +rule carp-deprecated-thread-macro 0 finding self-assignment error suspicious fixable=false broad.lisp 6875 setq assigns place x to itself self-assignment/5f1a5b802aa99b47/0 finding duplicate-setf-places error duplicate fixable=false broad.lisp 6961 setf assigns variable total more than once; the earlier assignment is dead duplicate-setf-places/8104810ed815b9fd/0 finding setf-arity error arity fixable=false broad.lisp 7021 setf has 3 arguments; place/value pairs require an even count setf-arity/b0a5b422aad6d4d2/0 diff --git a/tests/fixtures/lint_golden/expected/emacs-lisp.json.golden b/tests/fixtures/lint_golden/expected/emacs-lisp.json.golden index aded1570..aa8a1e9d 100644 --- a/tests/fixtures/lint_golden/expected/emacs-lisp.json.golden +++ b/tests/fixtures/lint_golden/expected/emacs-lisp.json.golden @@ -1970,6 +1970,12 @@ "count": 0, "description": "`(except [] …)` catches every BaseException, Ctrl-C and SystemExit included", "rule": "hy-bare-except" + }, + { + "category": "portability", + "count": 0, + "description": "a threading macro Carp's standard library marks deprecated", + "rule": "carp-deprecated-thread-macro" } ], "policy": { diff --git a/tests/fixtures/lint_golden/expected/emacs-lisp.sarif.golden b/tests/fixtures/lint_golden/expected/emacs-lisp.sarif.golden index 06fddb90..08e953a9 100644 --- a/tests/fixtures/lint_golden/expected/emacs-lisp.sarif.golden +++ b/tests/fixtures/lint_golden/expected/emacs-lisp.sarif.golden @@ -4549,6 +4549,22 @@ "shortDescription": { "text": "`(catch Expr)` returns failures as ordinary terms, indistinguishable from success" } + }, + { + "fullDescription": { + "text": "Carp's `core/ControlMacros.carp` declares `(deprecated => \"deprecated in favor of `->`.\")` and the same for `==>` against `-->`. That declaration only writes metadata: the compiler reads the `deprecated` key in `primitiveInfo` (the REPL's `(info …)` command) and in the HTML doc renderer, and nowhere on any compilation path. So the deprecated spelling builds cleanly with no diagnostic at all, which is why it outlives the deprecation — Carp's own `core/Binary.carp` still uses `==>`. The replacement is a rename rather than a rewrite: `=>` and `->` are both defined as `(defmacro _ [:rest forms] (thread-first-internal forms))`, with identical bodies." + }, + "id": "carp-deprecated-thread-macro", + "properties": { + "category": "portability", + "fixable": true, + "rationale": "Carp's `core/ControlMacros.carp` declares `(deprecated => \"deprecated in favor of `->`.\")` and the same for `==>` against `-->`. That declaration only writes metadata: the compiler reads the `deprecated` key in `primitiveInfo` (the REPL's `(info …)` command) and in the HTML doc renderer, and nowhere on any compilation path. So the deprecated spelling builds cleanly with no diagnostic at all, which is why it outlives the deprecation — Carp's own `core/Binary.carp` still uses `==>`. The replacement is a rename rather than a rewrite: `=>` and `->` are both defined as `(defmacro _ [:rest forms] (thread-first-internal forms))`, with identical bodies.", + "severity": "warning", + "tags": [] + }, + "shortDescription": { + "text": "a threading macro Carp's standard library marks deprecated" + } } ] } diff --git a/tests/fixtures/lint_golden/expected/emacs-lisp.text.golden b/tests/fixtures/lint_golden/expected/emacs-lisp.text.golden index 3984048d..0085c4cf 100644 --- a/tests/fixtures/lint_golden/expected/emacs-lisp.text.golden +++ b/tests/fixtures/lint_golden/expected/emacs-lisp.text.golden @@ -303,6 +303,7 @@ rule defconstant-non-eql-value 0 rule hy-mutable-default-argument 0 rule hy-identity-comparison-with-literal 0 rule hy-bare-except 0 +rule carp-deprecated-thread-macro 0 finding elisp-macro-missing-declare warning malformed fixable=false emacs-lisp.el 344 missing editor declaration: macro `fixture-macro` is defined without one; add a leading (declare (indent ...)) and/or (declare (debug ...)) form so Emacs indents and Edebug instruments calls to this macro correctly elisp-macro-missing-declare/3f509c1d426873fb/0 finding elisp-missing-lexical-binding warning suspicious fixable=false emacs-lisp.el 0 no `lexical-binding` setting on the first line, so this file is evaluated with dynamic binding; add `-*- lexical-binding: t -*-` elisp-missing-lexical-binding/48686428ba8a57d0/0 finding elisp-unreachable-lexical-binding error suspicious fixable=false emacs-lisp.el 911 `lexical-binding` is read from the first line only, so this setting has no effect and the file is evaluated with dynamic binding elisp-unreachable-lexical-binding/7bec796cbb2809fb/0 diff --git a/tests/fixtures/lint_golden/expected/nested.json.golden b/tests/fixtures/lint_golden/expected/nested.json.golden index 5df84f1a..ae4a4a0a 100644 --- a/tests/fixtures/lint_golden/expected/nested.json.golden +++ b/tests/fixtures/lint_golden/expected/nested.json.golden @@ -2074,6 +2074,12 @@ "count": 0, "description": "`(except [] …)` catches every BaseException, Ctrl-C and SystemExit included", "rule": "hy-bare-except" + }, + { + "category": "portability", + "count": 0, + "description": "a threading macro Carp's standard library marks deprecated", + "rule": "carp-deprecated-thread-macro" } ], "policy": { diff --git a/tests/fixtures/lint_golden/expected/nested.sarif.golden b/tests/fixtures/lint_golden/expected/nested.sarif.golden index 0cad33c5..76dfc76e 100644 --- a/tests/fixtures/lint_golden/expected/nested.sarif.golden +++ b/tests/fixtures/lint_golden/expected/nested.sarif.golden @@ -5207,6 +5207,22 @@ "shortDescription": { "text": "`(catch Expr)` returns failures as ordinary terms, indistinguishable from success" } + }, + { + "fullDescription": { + "text": "Carp's `core/ControlMacros.carp` declares `(deprecated => \"deprecated in favor of `->`.\")` and the same for `==>` against `-->`. That declaration only writes metadata: the compiler reads the `deprecated` key in `primitiveInfo` (the REPL's `(info …)` command) and in the HTML doc renderer, and nowhere on any compilation path. So the deprecated spelling builds cleanly with no diagnostic at all, which is why it outlives the deprecation — Carp's own `core/Binary.carp` still uses `==>`. The replacement is a rename rather than a rewrite: `=>` and `->` are both defined as `(defmacro _ [:rest forms] (thread-first-internal forms))`, with identical bodies." + }, + "id": "carp-deprecated-thread-macro", + "properties": { + "category": "portability", + "fixable": true, + "rationale": "Carp's `core/ControlMacros.carp` declares `(deprecated => \"deprecated in favor of `->`.\")` and the same for `==>` against `-->`. That declaration only writes metadata: the compiler reads the `deprecated` key in `primitiveInfo` (the REPL's `(info …)` command) and in the HTML doc renderer, and nowhere on any compilation path. So the deprecated spelling builds cleanly with no diagnostic at all, which is why it outlives the deprecation — Carp's own `core/Binary.carp` still uses `==>`. The replacement is a rename rather than a rewrite: `=>` and `->` are both defined as `(defmacro _ [:rest forms] (thread-first-internal forms))`, with identical bodies.", + "severity": "warning", + "tags": [] + }, + "shortDescription": { + "text": "a threading macro Carp's standard library marks deprecated" + } } ] } diff --git a/tests/fixtures/lint_golden/expected/nested.text.golden b/tests/fixtures/lint_golden/expected/nested.text.golden index 3cee625c..6d130b4e 100644 --- a/tests/fixtures/lint_golden/expected/nested.text.golden +++ b/tests/fixtures/lint_golden/expected/nested.text.golden @@ -303,6 +303,7 @@ rule defconstant-non-eql-value 0 rule hy-mutable-default-argument 0 rule hy-identity-comparison-with-literal 0 rule hy-bare-except 0 +rule carp-deprecated-thread-macro 0 finding redundant-quote warning suspicious fixable=true nested.lisp 840 quoting number 5 is redundant redundant-quote/07bb8307b4864109/0 finding redundant-progn warning suspicious fixable=true nested.lisp 268 redundant progn: progn wraps a single form; it is equivalent to that form redundant-progn/9715182cb9f63314/0 finding redundant-progn warning suspicious fixable=true nested.lisp 833 redundant progn: progn wraps a single form; it is equivalent to that form redundant-progn/8d949e5fcf0552a8/0 diff --git a/tests/fixtures/lint_golden/expected/suppressed.json.golden b/tests/fixtures/lint_golden/expected/suppressed.json.golden index d8a54464..6968c2b9 100644 --- a/tests/fixtures/lint_golden/expected/suppressed.json.golden +++ b/tests/fixtures/lint_golden/expected/suppressed.json.golden @@ -1853,6 +1853,12 @@ "count": 0, "description": "`(except [] …)` catches every BaseException, Ctrl-C and SystemExit included", "rule": "hy-bare-except" + }, + { + "category": "portability", + "count": 0, + "description": "a threading macro Carp's standard library marks deprecated", + "rule": "carp-deprecated-thread-macro" } ], "policy": { diff --git a/tests/fixtures/lint_golden/expected/suppressed.sarif.golden b/tests/fixtures/lint_golden/expected/suppressed.sarif.golden index fa5118a6..bcd34c94 100644 --- a/tests/fixtures/lint_golden/expected/suppressed.sarif.golden +++ b/tests/fixtures/lint_golden/expected/suppressed.sarif.golden @@ -4315,6 +4315,22 @@ "shortDescription": { "text": "`(catch Expr)` returns failures as ordinary terms, indistinguishable from success" } + }, + { + "fullDescription": { + "text": "Carp's `core/ControlMacros.carp` declares `(deprecated => \"deprecated in favor of `->`.\")` and the same for `==>` against `-->`. That declaration only writes metadata: the compiler reads the `deprecated` key in `primitiveInfo` (the REPL's `(info …)` command) and in the HTML doc renderer, and nowhere on any compilation path. So the deprecated spelling builds cleanly with no diagnostic at all, which is why it outlives the deprecation — Carp's own `core/Binary.carp` still uses `==>`. The replacement is a rename rather than a rewrite: `=>` and `->` are both defined as `(defmacro _ [:rest forms] (thread-first-internal forms))`, with identical bodies." + }, + "id": "carp-deprecated-thread-macro", + "properties": { + "category": "portability", + "fixable": true, + "rationale": "Carp's `core/ControlMacros.carp` declares `(deprecated => \"deprecated in favor of `->`.\")` and the same for `==>` against `-->`. That declaration only writes metadata: the compiler reads the `deprecated` key in `primitiveInfo` (the REPL's `(info …)` command) and in the HTML doc renderer, and nowhere on any compilation path. So the deprecated spelling builds cleanly with no diagnostic at all, which is why it outlives the deprecation — Carp's own `core/Binary.carp` still uses `==>`. The replacement is a rename rather than a rewrite: `=>` and `->` are both defined as `(defmacro _ [:rest forms] (thread-first-internal forms))`, with identical bodies.", + "severity": "warning", + "tags": [] + }, + "shortDescription": { + "text": "a threading macro Carp's standard library marks deprecated" + } } ] } diff --git a/tests/fixtures/lint_golden/expected/suppressed.text.golden b/tests/fixtures/lint_golden/expected/suppressed.text.golden index 09dc1e03..985367c9 100644 --- a/tests/fixtures/lint_golden/expected/suppressed.text.golden +++ b/tests/fixtures/lint_golden/expected/suppressed.text.golden @@ -303,5 +303,6 @@ rule defconstant-non-eql-value 0 rule hy-mutable-default-argument 0 rule hy-identity-comparison-with-literal 0 rule hy-bare-except 0 +rule carp-deprecated-thread-macro 0 finding redundant-quote warning suspicious fixable=true suppressed.lisp 338 quoting number 6 is redundant redundant-quote/07bb8007b4863bf0/0 finding declarative-style-score warning suspicious fixable=false suppressed.lisp 0 this file's declarative-style score is 0% (5 top-level forms), below the 80% threshold declarative-style-score/2f534fdd67304360/0