Files
Stirling-PDF/scripts/lint/comment-rules.mjs
T
ConnorYoh 4ab2505a6c Comment-quality standard, and the gate that enforces it (#7663)
## The problem

AI PRs write comments that restate the line below them, mark sections
with box drawing, and narrate the diff. Nothing in the repo said not to,
and nothing checked. `AGENTS.md` had one line about comments and it was
buried in the Python section.

Banners and `Step N:` narration have zero occurrences in the 15 months
before Aug 2025, so this is new.

## The fix

A written standard, plus a linter that enforces the mechanical part of
it on added lines only.

-
[devGuide/CODE_COMMENTS.md](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/devGuide/CODE_COMMENTS.md)
holds the reasoning and worked examples; a section in
[AGENTS.md](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/AGENTS.md)
holds the operative rules, kept short so they stay in an agent's
context. The two are split by kind rather than duplicated, because the
same prose in two places drifts.
- Rules in
[comment-rules.mjs](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs),
shared by both engines.
- Two engines. `.ts` / `.tsx` / `.mjs` go to an [oxlint JS
plugin](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-lint-oxlint-plugin.mjs)
so comments come from the parser rather than a line scan; `.java` /
`.py` go to a [line
scanner](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-lint.mjs).
Neither reads the other's files, so they cannot disagree about one file.
- Between them they read every comment form the repo writes: `//` and
`/* */`, Javadoc and JSDoc, JSX comments, `#`, and Python docstrings.
- Runs in `task pre-commit`, so the git hook and the `pre_commit.yml` CI
job both get it, and as a Claude Code [`Stop`
hook](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-lint-hook.mjs)
so an agent fixes the comment inside the turn that wrote it.

## The rules

The part worth arguing about. **Every rule blocks.** A rule that only
warns is a rule nobody acts on, so a finding you believe is wrong is a
bug in the rule: narrow it, or mark the line and say why.

| | Fires on |
| --- | --- |
|
[CMT001](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L71)
| Every word in the comment already appears in the code below it. Max 6
words, skipped for prose punctuation and for a bare Arrange/Act/Assert
marker |
|
[CMT002](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L92)
| 4+ rule or box-drawing characters, or a bare section label from [a
fixed
list](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L84)
(`Types`, `Helpers`, `State`, `Handlers`, ...) |
|
[CMT003](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L110)
| `Step N:` with a separator, or `Then,` / `Next,` / `Finally,`.
Suppressed in test files |
|
[CMT004](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L129)
| A comment about the code's own past: `this used to`, `renamed from`,
`was previously called`. Suppressed in test files |
|
[CMT005](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L154)
| 3+ consecutive comment lines where 2/3 [parse as
code](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L143)
|
|
[CMT006](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L31)
| A run of implementation comment over 12 lines, outside the first 5
lines of a file. Doc blocks are exempt, because the standard asks for
thorough contracts |
|
[CMT007](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L180)
| A parameter or return description that adds no word its name lacks.
Reads Javadoc/JSDoc `@param`, Sphinx `:param name:` and Google `name:
description` |
|
[CMT008](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L239)
| An allow directive naming a rule that does not exist, or one that
silenced nothing |
|
[CMT009](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L219)
| A `TODO` / `FIXME` / `HACK` naming no issue or link. An owner is not
accepted: a username goes stale, an issue outlives it |

Each rule carries the readings it deliberately excludes, next to the
rule. Those exclusions came from running the rules over this repo, not
from taste: `CMT004` does not match a bare "no longer needed" because
that is as often about runtime lifecycle as about history, and `CMT003`
needs a separator after the number so a wrapped line beginning "step 2
unmounts + remounts the panel" reads as the prose it is.

A comment sharing a line with code is judged by the rules that do not
depend on the code below it, so a trailing `// TODO fix this` or `/*
this used to run before the flush */` still reports, while `50L * 1024 *
1024 // 50 MB` does not. `CMT001` would have been wrong about six in
seven trailing comments here, so it stays out of them.

If a finding is wrong, `// comment-lint-allow: CMT002` on the line
above. Rule-specific, [no blanket
disable](https://github.com/Stirling-Tools/Stirling-PDF/blob/claude/ai-pr-comment-quality-dd970e/scripts/lint/comment-rules.mjs#L229).
A directive naming a rule that does not exist, or silencing nothing, is
itself a `CMT008` failure, so a typo cannot quietly disable a rule and a
stale one gets deleted rather than accumulating.

No native linter covers `CMT007`. `eslint-plugin-jsdoc`'s
`require-param-description`, Checkstyle's `NonEmptyAtclauseDescription`
and ruff's D-rules all check that a description exists, not whether it
says anything.

## Scoping

Added comment **text** only, not lines git calls new. Reindenting a file
or moving a block makes git mark untouched comments as added; findings
are matched against the comment text at the base, so only genuinely new
content reports.

The whole file is read and every comment in it evaluated. Only the
*reporting* is filtered, so a rule still sees the code a comment
introduces, the full run it belongs to, and the base version of the
file.

Existing tree is untouched. `task pre-commit:comment-lint:all` reports
it and always exits 0:

| | java | ts/js | py |
| --- | --- | --- | --- |
| findings | 1,218 | 741 | 204 |

2,163 across 542 files, mostly `CMT002` banners (1,482) and `CMT001`
restatements (456). Clearing it is separate work, by directory.

Not in this PR: an advisory LLM review layer for the things no pattern
can judge.

## Verification

Run against
[#7494](https://github.com/Stirling-Tools/Stirling-PDF/pull/7494) as CI
would, in a throwaway worktree: **two findings on a 78 file, +4,512 line
change, both genuine banners, in 952ms**. A whole-file scan of those
same files gives 11; the other 9 were withheld because that PR's author
did not write them, and they are the `@param teamId the team ID` shape
this standard exists to stop.

Both scanners blank string and character literals before looking for
comment markers, because a partial lex desynchronises everything after
it: one apostrophe in a Java comment, or one Python template whose
closing quotes start a line, is enough to read dozens of lines of code
as a single comment. Two fixtures carry canaries that stop being
reported if either engine ever desynchronises again.

The [fixture
corpus](https://github.com/Stirling-Tools/Stirling-PDF/tree/claude/ai-pr-comment-quality-dd970e/scripts/lint/fixtures)
pins all 9 rules against both engines, and `--selftest` fails if the two
disagree about the same file.

## Two things reviewers should know

**The oxlint JS plugin API is alpha.** oxlint itself is stable and
already this repo's frontend linter; the plugin API is the new
dependency. Its documented failure mode
([oxc#25203](https://github.com/oxc-project/oxc/issues/25203)) is being
skipped silently while oxlint still reports success. That affects the
standalone release binary rather than the npm package this invokes, but
the class of failure reads exactly like clean code, so the run asserts
`number_of_rules >= 1` from oxlint's own report and a broken engine
exits 2 rather than passing. If the API ever breaks, the fallback is
folding these rules into the line scanner, which already implements all
nine for Java and Python.

**`.claude/settings.json` is now committed**, carrying the hook and
nothing else: 19 lines, no `permissions`, nothing machine-specific. That
partly reverts `c35546a212` ("Ignore claude dir"), which existed because
this file had twice been committed by accident with a personal
`permissions` allowlist, once with absolute machine paths. Personal
config still belongs in `.claude/settings.local.json`, which the new
pattern keeps ignored, and hook entries merge across the two so nobody's
own hooks are lost.

If you already hand-wrote a `.claude/settings.json`, copy it somewhere
first: that path used to be git-ignored, and git overwrites an ignored
file without warning when a commit starts tracking it. Across 19 local
checkouts here, 13 have `settings.local.json` and none has a
hand-written `settings.json`.

To turn the hook off, `{ "env": { "COMMENT_LINT_HOOK": "0" } }` in local
settings. Claude Code can only disable all hooks at once, hence the
switch. The commit-time gate still applies.

## How to test

```bash
task pre-commit:comment-lint:ci
```

The fixture corpus, then the diff. The corpus checks the rules
themselves rather than the code under review, so it runs on CI and
before a rule change, not on every local commit.

```bash
task comment-lint:branch
```

`clean (34 files in scope)`. `task comment-lint` is the same thing
scoped to uncommitted work, which is what the git hook and CI run.

To watch it bite, add `// Is banner` above `export function isBanner` in
`scripts/lint/comment-rules.mjs` and run `task comment-lint`: one
`CMT001`, exit 1. The gate covers its own source, which is why these
scripts have no section dividers.

```bash
task pre-commit:comment-lint:all
```

The standing backlog, report-only.

Verified on the pinned oxlint 1.77.0, not only the 1.79 the plugin was
prototyped against.
2026-08-28 10:56:50 +00:00

501 lines
21 KiB
JavaScript

// The comment-quality rule set, shared by both engines so a rule means the same
// thing everywhere: the oxlint JS plugin (which owns .ts/.tsx, and has real
// comment tokens and an AST) and comment-lint.mjs (which owns .java and .py, and
// has only lines). Neither engine ever scans the other's files, so the two can
// differ in precision without producing contradictory findings on one file.
//
// The standard these rules enforce is devGuide/CODE_COMMENTS.md. Changing a rule
// here without changing that document leaves the repo with two answers.
//
// Between them the engines read every comment form the repo writes: // and /* */,
// Javadoc and JSDoc, JSX comments, # and Python docstrings.
export const SEVERITY = { ERROR: "error", WARN: "warn" };
// Every rule blocks. A rule that only warns is a rule nobody acts on, so a
// finding that turns out to be wrong is a bug in the rule: narrow it, or mark the
// line with comment-lint-allow and say why. Each rule below carries the readings
// it deliberately excludes, which is where to start when one misfires.
export const RULES = {
CMT001: { name: "restates-code", severity: SEVERITY.ERROR },
CMT002: { name: "banner", severity: SEVERITY.ERROR },
CMT003: { name: "step-narration", severity: SEVERITY.ERROR },
CMT004: { name: "diff-narration", severity: SEVERITY.ERROR },
CMT005: { name: "dead-code", severity: SEVERITY.ERROR },
CMT006: { name: "block-too-long", severity: SEVERITY.ERROR },
CMT007: { name: "doc-restates-signature", severity: SEVERITY.ERROR },
CMT008: { name: "bad-allow", severity: SEVERITY.ERROR },
CMT009: { name: "unowned-todo", severity: SEVERITY.ERROR },
};
export const MAX_BLOCK_LINES = 12;
// CMT001 compares a comment against the code it introduces. Both sides are
// reduced to the same shape first: lowercased, camel/snake/kebab split into
// words, stop words and short words dropped. What survives is the information
// each side actually carries, so "Handle drag start" and `handleDragStart` land
// on the same set and the comment is shown to add nothing.
const STOP_WORDS = new Set(
(
"a an the and or but if then else for to of in on at by with from into is are be was were this that these those it its as we our you your do" +
" does done use uses used using will would should can could may might not no yes new only also just so such via per each all any some more" +
" most other another same when while where which what who how why here there now next finally first second third let const var function" +
" return set get"
).split(" "),
);
const WORD_RE = /[a-z][a-z0-9]*/g;
export function contentWords(text) {
return (text.toLowerCase().match(WORD_RE) ?? []).filter((w) => w.length > 2 && !STOP_WORDS.has(w));
}
export function identWords(text) {
const split = text.replace(/([a-z0-9])([A-Z])/g, "$1 $2").replace(/[_\-.]/g, " ");
return contentWords(split);
}
// Sentence punctuation marks prose, which is usually saying something the code
// does not. A single trailing full stop does not count.
const PROSE_PUNCT = /[.;:?!]/;
const MAX_RESTATE_WORDS = 6;
// Arrange/Act/Assert and Given/When/Then label the shape of a test rather than
// describe the line beneath. Exempt only as a bare marker, so
// `// Assert the cap is clamped to the tier maximum` is prose and judged on its
// merits.
const TEST_STRUCTURE = /^(arrange|act|assert|given|when|then)\b/i;
const MAX_MARKER_WORDS = 4;
export function restatesCode(body, codeText) {
if (TEST_STRUCTURE.test(body.trim()) && body.trim().split(/\s+/).length <= MAX_MARKER_WORDS) return false;
if (PROSE_PUNCT.test(body.replace(/\.$/, ""))) return false;
const comment = contentWords(body);
if (comment.length === 0 || comment.length > MAX_RESTATE_WORDS) return false;
const code = identWords(codeText);
if (code.length === 0) return false;
// Prefix matching either way, so "config" covers "configuration" and vice versa.
return comment.every((w) => code.some((k) => k.startsWith(w) || w.startsWith(k)));
}
const RULE_CHARS = /^[=~_*#+\-]{4,}|[─-╿]{4,}|[=~_*+]{4,}$/;
const SECTION_LABEL = new RegExp(
"^(imports?|exports?|types?|interfaces?|constants?|config|helpers?|utils?|utilities|state|handlers?|callbacks?|effects?" +
"|render|rendering|styles?|props?|hooks?|setup|teardown|cleanup|main|public|private|internal|api|queries|mutations" +
"|selectors?|actions?|reducers?|components?|fields?|getters?|setters?|lifecycle|boilerplate)" +
"\\s*(section|area|block)?\\s*$",
"i",
);
export function isBanner(body) {
if (RULE_CHARS.test(body.trim())) return true;
// A label wrapped in decoration is still a label: strip the decoration first.
const bare = body
.replace(/[=~_*#+\-─-╿]/g, " ")
.replace(/\s+/g, " ")
.trim();
return bare.length > 0 && SECTION_LABEL.test(bare);
}
// A bare "1." is not narration: numbered lists are how a doc block enumerates
// conditions or alternatives, and matching them buries the rule in false
// positives. Only the explicit step form and sequencing adverbs qualify, and the
// number needs a separator after it, so a wrapped line beginning "step 2 unmounts
// + remounts the panel" reads as the prose it is.
const STEP = /^(step\s*\d+(\.\d+)?\s*[:.)\-]|(then|next|finally|afterwards|lastly)\s*[,:]\s+\S)/i;
export function isStepNarration(body) {
return STEP.test(body.trim());
}
// Only phrases that can be talking about the code's own past. Excluded because
// each has an innocent reading that fires constantly:
// "used to" alone - "Used to clamp the live line" means "is used to"
// "previously" alone - "re-show even if previously dismissed" is runtime state
// "was called" - collides with "verify getSession was called"
// "left over from" - "no cards left over from the unfiltered grid"
const DIFF_NARRATION = new RegExp(
"\\b((this|it|we|they|that) used to|used to (be|live|sit)" +
"|(this|it|that|which|the (class|method|field|code|file|module|palette|banner)) is no longer (needed|used)" +
"|renamed from|was (previously|formerly) (called|named|known)" +
"|instead of the old|has been (removed|replaced) )",
"i",
);
const REMOVAL_SUFFIX = /(^|\s)[-(—]\s*(removed|deleted|dropped|no longer needed)\s*\)?\s*$/i;
export function isDiffNarration(body) {
const t = body.trim();
return DIFF_NARRATION.test(t) || REMOVAL_SUFFIX.test(t);
}
const CODE_KEYWORD = new RegExp(
"^(import|package|public|private|protected|static|final|abstract|class|interface|enum|record|extends|implements" +
"|def|async|await|const|let|var|function|export|return|if|else|elif|for|while|do|try|catch|finally|switch|case" +
"|throw|new|super|this|@[A-Za-z])\\b",
);
const STATEMENT_TAIL = /[;{}]\s*$/;
const CALL_ONLY = /^[\w.$]+\s*\([^)]*\)\s*;?\s*$/;
const ASSIGNMENT = /\S\s*=\s*\S/;
export function looksLikeCode(line) {
const t = line.trim();
if (t.length === 0) return false;
if (CODE_KEYWORD.test(t)) return true;
if (CALL_ONLY.test(t)) return true;
return STATEMENT_TAIL.test(t) && ASSIGNMENT.test(t);
}
export const MIN_DEAD_CODE_RUN = 3;
const DEAD_CODE_SHARE = 2 / 3;
export function isDeadCodeRun(bodies) {
if (bodies.length < MIN_DEAD_CODE_RUN) return false;
const codeish = bodies.filter(looksLikeCode).length;
return codeish / bodies.length >= DEAD_CODE_SHARE;
}
// Every documented-parameter form this repo writes, so the rule is not quietly
// Javadoc-only:
// Javadoc / JSDoc @param blob The blob to download
// Sphinx :param blob: The blob to download
// Google docstring blob: The blob to download (under an Args: heading)
// NumPy style is deliberately absent: it splits the name and the description
// across two lines, and there is one instance of it in the tree.
const PARAM_TAG = /^@param\s+(?:\{[^}]*\}\s+)?([\w$.]+)\s*-?\s*(.+)$/;
const SPHINX_PARAM = /^:(?:param|arg|key)\s+(?:\S+\s+)?([\w.]+)\s*:\s*(.+)$/;
const GOOGLE_PARAM = /^([a-z_][\w]*)\s*(?:\([^)]*\))?\s*:\s*(.+)$/;
const RETURN_TAG = /^@returns?\s+(.+)$/;
const SPHINX_RETURN = /^:returns?\s*:\s*(.+)$/;
// Description adds nothing when every word in it already appears in the thing
// being described. `@param blob - The blob to download` is the canonical case.
//
// No native linter covers this. eslint-plugin-jsdoc's require-param-description,
// Checkstyle's NonEmptyAtclauseDescription and ruff's D-rules all check that a
// description exists, not whether it says anything.
export function docRestatesSignature(body, ownerName = "") {
const t = body.trim();
for (const pattern of [PARAM_TAG, SPHINX_PARAM, GOOGLE_PARAM]) {
const match = pattern.exec(t);
if (!match) continue;
const [, name, description] = match;
// Google form is just `name: description`, which also matches ordinary prose
// containing a colon. Require the description to be short and unpunctuated so
// "Note: the cap is clamped" is not read as a parameter called "note".
if (pattern === GOOGLE_PARAM && /[.;,]/.test(description)) return false;
return addsNothing(description, name, 5);
}
const returns = RETURN_TAG.exec(t) ?? SPHINX_RETURN.exec(t);
if (returns && ownerName) return addsNothing(returns[1], ownerName, 4);
return false;
}
function addsNothing(description, subject, limit) {
const words = contentWords(description);
if (words.length === 0 || words.length > limit) return false;
const known = identWords(subject);
return known.length > 0 && words.every((w) => known.some((k) => k.startsWith(w) || w.startsWith(k)));
}
// A TODO with no reference has nothing that will ever close it. An owner is not
// accepted in its place: a username goes stale when someone leaves and means
// nothing to an outside contributor, while an issue outlives both.
//
// Anchored at the start, so this catches a comment that *is* a TODO rather than
// prose that mentions the word.
const TODO_MARKER = /^(TODO|FIXME|HACK|XXX)\b/;
// What counts as something that will close it: an issue, a link, or a security
// advisory. Checked across the whole comment run, so the reference can sit on a
// continuation line.
const HAS_REFERENCE = /(#\d+|https?:\/\/|CVE-\d|GHSA-|[A-Z]{2,}-\d+)/;
export function isUnownedTodo(body, runText = body) {
return TODO_MARKER.test(body) && !HAS_REFERENCE.test(runText);
}
// A rule id is silenced by `comment-lint-allow: CMT002`, on the comment itself or
// on the line above it. There is deliberately no form that disables every rule.
//
// The whole comment must be the directive. Matching it anywhere in the text meant
// prose that merely mentions the syntax silenced a rule, which this file's own
// paragraph above did.
const DIRECTIVE = /^comment-lint-allow:\s*(.+?)\s*$/i;
export function isDirective(body) {
return DIRECTIVE.test(body.trim());
}
// A directive that names nothing real, or that suppresses nothing, is dead
// configuration: it reads as a silenced rule while silencing nothing, and it
// blinds the line for whoever inherits it. Reported for the same reason ESLint
// has --report-unused-disable-directives and ruff has RUF100.
class Allowance {
constructor(directives) {
this.entries = [];
for (const directive of directives) {
for (const token of directiveTokens(directive.body)) {
this.entries.push({ token, directive, known: token in RULES, used: false });
}
}
}
// Called only once a rule has decided it would report, so a directive counts
// as used when it actually silenced something. Asking before the rule decided
// marked every consulted directive as used, which hid the unused ones.
suppresses(rule) {
let allowed = false;
for (const entry of this.entries) {
if (entry.token !== rule) continue;
entry.used = true;
allowed = true;
}
return allowed;
}
reportUnused(report) {
for (const entry of this.entries) {
if (entry.used) continue;
const detail = entry.known ? `${entry.token} is allowed here but nothing reported it` : `${entry.token} is not a rule`;
report("CMT008", entry.directive.line, entry.directive.column, detail, entry.directive.body);
}
}
}
// Every token a directive names, valid or not, so an unknown one is reported
// rather than quietly ignored. Matching only real ids would let `CMT999` through
// as a silent no-op: it looks like a rule and silences nothing.
export function directiveTokens(body) {
const match = DIRECTIVE.exec(body.trim());
if (!match) return [];
return match[1]
.split(",")
.map((token) => token.trim().toUpperCase())
.filter(Boolean);
}
// Generated files carry whatever the generator emits, and editing them to
// satisfy a lint rule would be undone on the next regeneration.
const GENERATED_MARKER = /AUTO-?GENERATED|@generated|DO NOT EDIT|Code generated by/i;
const GENERATED_HEADER_LINES = 10;
export function isGenerated(source) {
return GENERATED_MARKER.test(source.split("\n", GENERATED_HEADER_LINES).join("\n"));
}
export const EXCLUDED_PATHS = [
/(^|\/)node_modules\//,
/(^|\/)dist(-\w+)?\//,
/(^|\/)build\//,
/(^|\/)target\//,
/(^|\/)vendor\//,
/pdfjs/i,
/thirdParty/i,
/\.min\./,
/src-tauri\/gen\//,
/public\/locales\//,
/\.d\.ts$/,
/(^|\/)storybook-static\//,
/(^|\/)playwright-report\//,
/(^|\/)org\/apache\//,
];
export function isExcludedPath(file) {
const normalised = file.replace(/\\/g, "/");
return EXCLUDED_PATHS.some((re) => re.test(normalised));
}
// Comment text reduced to what a reader would call "the same comment": trimmed,
// whitespace collapsed, comment markers and decoration stripped. Both sides of
// the pre-existing check normalise through here so indentation and marker style
// cannot make an unchanged comment look new.
export function normaliseComment(text) {
return String(text)
.replace(/^[\s{]*(\/\/+|\/\*+|#+|\*+)/gm, " ")
.replace(/\*+\/[\s}]*$/gm, " ")
.replace(/\s+/g, " ")
.trim()
.toLowerCase();
}
// Every comment in a source file, normalised. Deliberately permissive and
// language-agnostic: it only ever decides whether a finding is pre-existing, so
// over-matching suppresses a duplicate comment and under-matching just reports
// something the author can look at.
export function commentBodiesOf(source) {
const bodies = new Set();
for (const raw of source.split(/\r?\n/)) {
const marker = /(\/\/+|\/\*+|^\s*\*+|#+)/.exec(raw);
if (!marker) continue;
const body = normaliseComment(raw.slice(marker.index));
if (body.length > 0) bodies.add(body);
}
return bodies;
}
export function ruleLabel(id) {
return `${id} ${RULES[id].name}`;
}
// Both engines funnel into this. They differ only in how they build `runs`: the
// oxlint plugin reads real comment tokens, comment-lint.mjs scans lines. Keeping
// the rule application here is what stops the two drifting apart.
//
// A "run" is a group of comment lines with no code between them, which is the
// unit CMT005 and CMT006 judge. Shape:
// { startLine, kind: "line" | "block" | "doc", lines: [{ line, column, body }] }
// `line` is 1-based to match every editor and every diff.
export function analyse({ lines, runs, isTestFile = false }) {
const findings = [];
// `body` is the comment's own text, kept alongside the formatted detail so the
// caller can ask whether this exact comment already existed before the change.
// That is what stops a reindent or a code move reporting comments nobody wrote.
const report = (rule, line, column, detail, body) => {
if (isTestFile && SUPPRESSED_IN_TESTS.has(rule)) return;
findings.push({ rule, line, column, detail, body: normaliseComment(body ?? detail), severity: RULES[rule].severity });
};
for (const run of runs) {
// A directive is scaffolding, not content. Leaving it in the run made it two
// lines long, and CMT001 only judges a one-line run, so any directive
// silenced CMT001 whatever rule it named.
const directives = run.lines.filter((l) => isDirective(l.body));
const content = run.lines.filter((l) => !isDirective(l.body));
const allowed = new Allowance(directives);
if (content.length === 0) {
allowed.reportUnused(report);
continue;
}
const bodies = content.map((l) => l.body);
const runText = bodies.join("\n");
const first = content[0];
// Only CMT004 and CMT009 judge a trailing comment. The others depend on the
// comment introducing the code below it, and a trailing comment sits beside
// it: `0x25 // "%PDF"` overlaps in words while adding the decoding, which is
// the kind of lower-altitude fact the standard asks for.
if (run.trailing) {
for (const entry of content) {
const body = entry.body.trim();
if (body.length === 0) continue;
if (isDiffNarration(body) && !allowed.suppresses("CMT004")) {
report("CMT004", entry.line, entry.column, truncate(body), body);
continue;
}
if (isUnownedTodo(body, runText) && !allowed.suppresses("CMT009")) {
report("CMT009", entry.line, entry.column, truncate(body), body);
}
}
allowed.reportUnused(report);
continue;
}
if (isDeadCodeRun(bodies) && !allowed.suppresses("CMT005")) {
report("CMT005", run.startLine, first.column, `${bodies.length} commented-out lines`, runText);
allowed.reportUnused(report);
continue; // Every other rule would pile onto the same block of dead code.
}
// A doc block is exempt: the standard asks for thorough contracts, so capping
// their length would argue with itself. This judges runs of implementation
// comment, where an essay means the code needs restructuring.
const essay = run.kind !== "doc" && content.length > MAX_BLOCK_LINES && run.startLine > FILE_HEADER_LINES;
if (essay && !allowed.suppresses("CMT006")) {
report("CMT006", run.startLine, first.column, `${content.length} lines, limit ${MAX_BLOCK_LINES}`, runText);
}
const owner = run.kind === "line" ? "" : nextCodeLine(lines, run);
for (const entry of content) {
const body = entry.body.trim();
if (body.length === 0) continue;
if (isBanner(body) && !allowed.suppresses("CMT002")) {
report("CMT002", entry.line, entry.column, truncate(body), body);
continue;
}
if (isStepNarration(body) && !allowed.suppresses("CMT003")) {
report("CMT003", entry.line, entry.column, truncate(body), body);
continue;
}
if (isDiffNarration(body) && !allowed.suppresses("CMT004")) {
report("CMT004", entry.line, entry.column, truncate(body), body);
continue;
}
if (docRestatesSignature(body, owner) && !allowed.suppresses("CMT007")) {
report("CMT007", entry.line, entry.column, truncate(body), body);
continue;
}
if (isUnownedTodo(body, runText) && !allowed.suppresses("CMT009")) {
report("CMT009", entry.line, entry.column, truncate(body), body);
continue;
}
}
// CMT001 judges a whole single-line run against the code it introduces, so
// a two-line comment that happens to echo one identifier is left alone.
// A one-line `/* … */` counts, which is how JSX `{/* Cap editor */}` above
// `<CapEditor …>` is caught. A doc block does not: it is a contract, and
// CMT007 is the rule that judges those.
if (run.kind !== "doc" && content.length === 1) {
const entry = first;
const body = entry.body.trim();
const code = nextCodeLine(lines, run);
if (code && !isBanner(body) && restatesCode(body, code) && !allowed.suppresses("CMT001")) {
report("CMT001", entry.line, entry.column, `${truncate(body)} -> ${truncate(code)}`, body);
}
}
allowed.reportUnused(report);
}
return findings.sort((a, b) => a.line - b.line || a.column - b.column);
}
// A file header is allowed to be as long as it needs to be.
const FILE_HEADER_LINES = 5;
const DETAIL_WIDTH = 58;
// Both of these say something real in a test and nothing anywhere else. A
// regression test explains itself by describing the old behaviour, and the e2e
// specs number their comments to match a written manual test procedure.
const SUPPRESSED_IN_TESTS = new Set(["CMT003", "CMT004"]);
function precedingLine(lines, startLine) {
return lines[startLine - 2] ?? "";
}
function nextCodeLine(lines, run) {
const commentLines = new Set(run.lines.map((l) => l.line));
for (let i = run.startLine; i < lines.length; i++) {
const lineNumber = i + 1;
if (commentLines.has(lineNumber)) continue;
const text = lines[i]?.trim() ?? "";
if (text.length === 0) continue;
if (text.startsWith("//") || text.startsWith("#") || text.startsWith("*") || text.startsWith("/*")) continue;
if (text === "}" || text === "};" || text === ")" || text === ");") return "";
return text;
}
return "";
}
function truncate(text) {
const flat = text.replace(/\s+/g, " ").trim();
return flat.length > DETAIL_WIDTH ? `${flat.slice(0, DETAIL_WIDTH - 1)}…` : flat;
}
export const TEST_FILE = /\.(test|spec)\.[jt]sx?$|(^|\/)src\/test\/|Test\.java$|Tests\.java$|(^|\/)test_[^/]+\.py$|_test\.py$/;
export function isTestPath(file) {
return TEST_FILE.test(file.replace(/\\/g, "/"));
}