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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
30 changes: 30 additions & 0 deletions packages/feature/lint-carp-idiom/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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
113 changes: 113 additions & 0 deletions packages/feature/lint-carp-idiom/README.md
Original file line number Diff line number Diff line change
@@ -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.
166 changes: 166 additions & 0 deletions packages/feature/lint-carp-idiom/src/corpus_tests.rs
Original file line number Diff line number Diff line change
@@ -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::<String>::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"
);
}
Loading