Close guideline coverage gaps for JavaScript and TypeScript - #1
Merged
Merged
Conversation
Every numbered clause of the pinned guide had a catalog row, but several rows claimed enforcement that the resolved settings did not provide, the guideline policy reversed only one prose/sample conflict, and TypeScript files ran pinned JavaScript rules that misreport TypeScript syntax. Catalog: record a disposition per policy (compatibility and guideline), add a `partial` enforcement level, scope mappings to the base preset, the guideline policy, or an opt-in profile, and record the TypeScript implementation of each replaced rule. Dispositions for 5.2, 10.4, 10.10, 10.11, 16.1, 17.1, 18.1, 18.6, and 7.2 now match actual behavior. Guideline policy: enforce the prose for clauses 5.2, 6.2/18.13, 10.1, 10.2, 10.6, 10.10 (selectable `importGroups: example | prose`), 10.11, 16.1, 17.1, 17.2, 18.1 (JSX), 18.6, 18.7, 22.1, and 22.7. Add rules no-multiline-string-concat, multiline-condition-layout, constant-name, and a lineCommentRuns option for require-docblock. TypeScript: replace pinned rules that report valid TypeScript or ignore TypeScript syntax with TypeScript-aware implementations of the same rules and options (native no-dupe-class-members, no-array-constructor, default-param-last, no-use-before-define; Stylistic comma-dangle, comma-spacing, key-spacing, lines-between-class-members, lines-around-comment, object-curly-spacing, semi, space-before-blocks, space-infix-ops; merge-aware ag-ts/no-redeclare). no-accessors covers abstract, signature, and auto-accessors; unknown-catch covers every catch binding pattern. `--check-tsconfig` verifies clauses 26.1/26.2. Profile: move AdGuard project conventions that the guide does not require (stricter JSDoc, MV2/MV3 imports, import-newlines, boundaries, logger context, @typescript-eslint rules) from the base preset into the opt-in `adguard-projects` profile with accurate provenance. Tests: run every enforced clause's good and bad examples through the complete preset for both policies, require partial and overridden dispositions to be demonstrated, and require an idiomatic TypeScript corpus to pass the full TypeScript preset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCeDV2VhbUtLyAP9XRG1zL
The TypeScript port of eslint-plugin-import-newlines 1.4.0 mistranslated
the fixer's type check. Upstream lib/enforce.js reads
`currentNode.imported.parent.importKind`; the parent of the imported
name is the ImportSpecifier, so only an inline `type` modifier is
reprinted. The port read `currentNode.parent.importKind`, the
ImportDeclaration's kind, instead. Every specifier of `import type { ... }`
was printed as `type X` (TS2206: the 'type' modifier cannot be used on a
named import when 'import type' is used), and inline modifiers in
`import { type A, B }` were dropped.
This is not an Oxc AST difference. Oxc and @typescript-eslint/parser
both set importKind 'type' on the declaration for `import type { A }`
and 'value' on its specifiers, and 'type' only on specifiers that carry
an inline modifier. With the upstream rule and @typescript-eslint/parser
the fix output is valid. An adapter in newlines.ts would have had to
fake specifier parents to hide a port bug, so the reviewed patch in
scripts/vendor-patches/import-newlines/src/enforce.ts.json now reads the
imported name's parent, as upstream does. The vendored file is
reproduced from the pinned upstream source and that patch, and
vendor/manifest.json was updated with `pnpm run snapshots:record`.
A new CLI test runs `oxlint --fix` with only ag-newlines/enforce on
type-only, aliased, inline, default-plus-inline, and joined imports and on
`export type` forms. It checks the exact fixed text and requires a second
run to report no diagnostics, which include Oxc's TS1363 and TS2206
parser errors. The members stay unindented, as upstream emits them; the
indent rule owns their indentation.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCeDV2VhbUtLyAP9XRG1zL
An independent audit ran every good and bad example of the guide under both policies in JavaScript and TypeScript, and linted three AdGuard codebases with the guideline policy. It found clauses marked enforced whose examples passed, rules that rejected the guide's own good examples, and TypeScript syntax that pinned rules still ignored. Dispositions: mark 3.3, 6.3, 7.4, 7.6, 7.10, 7.12, 9.1, 9.2, 14.1, 17.3, 21.2, 21.3, and 22.2 partial where the configured rules check only part of the clause, 18.13 overridden under compatibility, and explain 6.2, 18.14, 19.2, and 21.4. Rule lists now match what enforces them. Guideline policy: add ag/prefer-array-from (4.3), ag/prefer-template-over-join (6.3), ag/no-arguments (7.4), and ag/docblock-spacing (17.3); report prototype changes through calls (9.1, 9.2); drop Airbnb's exemptions from no-param-reassign (7.10) and eqeqeq (14.1); enable no-implicit-coercion (21.2, 21.3), inline duplicate-import merging (10.4), and TypeScript no-require-imports (10.1); turn off func-names, which rejects the guide's examples. Keep built-in imports first in the `example` import grouping and put aliases after packages; allow braced cases back to back, TODO/FIXME comment runs, and tool directives next to code; report multiline conditions only when their operands span lines. Both policies: no-default-side-effects reports `delete`; require-docblock keeps `/*!` banners and directive blocks; enum-name accepts V1-style names; unknown-catch reports unions with `any`. TypeScript files use Stylistic brace-style, keyword-spacing, and space-before-function-paren and native class-methods-use-this that exempts overrides. The adguard-projects profile adds member-delimiter-style. --check-tsconfig resolves `extends` as TypeScript does (existing files as written, package tsconfig fields, never `main`) and reports disabled strict-family options. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCeDV2VhbUtLyAP9XRG1zL
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Moderate issues remain in prototype-call operand validation and TypeScript declaration-merge handling.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR closes JavaScript and TypeScript guideline coverage gaps with policy/profile layering, TypeScript-aware linting, compiler checks, custom rules, and an import-fixer correction.
Changes:
- Adds compatibility/guideline policies and the
adguard-projectsprofile. - Adds TypeScript rule replacements, declaration handling, and tsconfig validation.
- Expands tests, fixtures, documentation, and import-fix behavior.
| File | Description |
|---|---|
vendor/manifest.json |
Updates vendor hashes. |
vendor/import-newlines/src/enforce.ts |
Corrects TypeScript import handling. |
tests/typescript-syntax.test.ts |
Tests TypeScript syntax coverage. |
tests/typescript-rules.test.ts |
Tests TypeScript-aware rules. |
tests/typescript-preset.test.ts |
Tests the complete TypeScript preset. |
tests/integration.test.ts |
Tests policy and profile APIs. |
tests/gaps.test.ts |
Tests scoped rule coverage. |
tests/gap-cases.ts |
Adds policy and profile cases. |
tests/fixtures/typescript/types.ts |
Adds TypeScript syntax fixtures. |
tests/fixtures/typescript/square.ts |
Adds class fixtures. |
tests/fixtures/typescript/overloads.ts |
Adds overload fixtures. |
tests/fixtures/typescript/namespace-merge.ts |
Adds namespace merge fixtures. |
tests/fixtures/typescript/merging.ts |
Adds declaration merge fixtures. |
tests/fixtures/typescript/imports.ts |
Adds TypeScript import fixtures. |
tests/fixtures/typescript/declarations.d.ts |
Adds ambient declaration fixtures. |
tests/fixtures/typescript/component.tsx |
Adds TSX fixtures. |
tests/fixtures/typescript/classes.ts |
Adds abstract-class fixtures. |
tests/custom.test.ts |
Tests configurable custom rules. |
tests/custom-cases.ts |
Adds custom-rule edge cases. |
tests/consumer.test.ts |
Tests packaged presets and consumers. |
tests/config-cli.test.ts |
Tests new CLI options. |
tests/compiler.test.ts |
Tests tsconfig validation. |
tests/clauses.test.ts |
Tests clause examples under both policies. |
tests/catalog.test.ts |
Validates scoped catalog mappings. |
tests/behavior.test.ts |
Tests import fixes and typed linting. |
scripts/write-configs.ts |
Generates project-profile presets. |
scripts/vendor-patches/import-newlines/src/enforce.ts.json |
Records the vendor patch. |
scripts/source-policy.ts |
Registers permitted external loaders. |
scripts/generate.ts |
Generates policies, profiles, and mappings. |
README.md |
Documents policies and TypeScript support. |
packages/rule-catalog/src/index.ts |
Extends catalog types. |
packages/rule-catalog/README.md |
Documents catalog layers. |
packages/oxlint-plugin/src/typescript.ts |
Adds TypeScript-aware redeclaration handling. |
packages/oxlint-plugin/src/index.ts |
Adds custom guideline rules. |
packages/oxlint-plugin/src/helpers.ts |
Adds shared AST and mutation helpers. |
packages/oxlint-plugin/README.md |
Documents plugin rules. |
packages/oxlint-plugin/package.json |
Exposes the TypeScript plugin. |
packages/oxlint-config/src/index.ts |
Implements policy and profile configuration. |
packages/oxlint-config/src/compiler.ts |
Adds tsconfig checking. |
packages/oxlint-config/src/cli.ts |
Adds policy, profile, and tsconfig options. |
packages/oxlint-config/README.md |
Documents configuration options. |
packages/oxlint-config/package.json |
Exposes project-profile presets. |
docs/conformance.md |
Documents TypeScript rule substitutions. |
.oxlintrc.json |
Enables the project profile. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… accepts ag-ts/no-redeclare suppressed reports for every declaration merge that TypeScript permits, including enum + enum, type alias + value, and interface + value. The TypeScript version of the rule, @typescript-eslint/no-redeclare (used by AdGuard projects through airbnb-typescript), accepts fewer merges with its default ignoreDeclarationMerge: interfaces, namespaces, one class with interfaces and namespaces, one function implementation with namespaces, and one enum with namespaces. It reports the rest, although TypeScript itself allows enum and type/value merges. Match that set so the TypeScript replacement enforces the same rule as its reference implementation. Overload signatures and ambient function declarations still merge with their implementation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCeDV2VhbUtLyAP9XRG1zL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every numbered clause of the pinned AdguardTeam/CodeGuidelines JavaScript guide already had a catalog row, but the rows were only checked for existence. Running each clause's good and bad examples through the complete preset showed three kinds of problem:
new Array<string>(); missed enum/tuple commas, union spacing, and type-alias semicolons.The guide repository is not modified.
Policies and catalog
compatibilitypolicy (the sample.eslintrc.jswins) and for the opt-inguidelinepolicy (prose wins).partialmarks clauses whose rules enforce only part of the requirement.guidelinepolicy now resolves every documented prose/sample conflict, not justprefer-default-export. Clause 10.10's grouping supports both readings of the guide viaimportGroups: 'example' | 'prose'.ag/no-multiline-string-concat,ag/multiline-condition-layout,ag/constant-nameag/prefer-array-from,ag/prefer-template-over-join,ag/no-arguments,ag/docblock-spacingrequire-docblockline-comment runsObject.assign/definePropertyon a prototype,setPrototypeOf,inherits)profile: 'adguard-projects', with accurate provenance:.pcssimports,import-newlines,boundaries, logger context@typescript-eslintrules, plusmember-delimiter-styleTypeScript
no-dupe-class-members,no-array-constructor,default-param-last,no-use-before-define,class-methods-use-thiscomma-dangle(enums/tuples/generics),comma-spacing,key-spacing,semi,space-infix-ops,object-curly-spacing,space-before-blocks,lines-between-class-members,lines-around-comment,brace-style,keyword-spacing,space-before-function-parenag-ts/no-redeclareno-accessorsandunknown-catchcover TypeScript forms (abstract, signature, and auto-accessors; catch bindings of any shape;anyunions).ag-oxlint-config --check-tsconfigverifies clauses 26.1/26.2, resolvingextendsthe way TypeScript does.Fix
import-newlines/enforceautofix produced invalidimport type { type A }and dropped inlinetypemodifiers. The TypeScript port read the declaration'simportKindinstead of the specifier's; the reviewed vendor patch now matches upstream.Breaking changes
profile: 'adguard-projects'; type-awareconsistent-type-exportsnow also requires that profile. The repository's own lint config extends the new profile preset.Test plan
pnpm run check(source policy, build, typecheck, self-lint, catalog/snapshot checks, coverage): 45,381 tests, coverage 98.74% statements / 95.54% branches.tests/clauses.test.ts:partial/overriddenlabel is backed by an example that demonstrates the gap.tests/typescript-preset.test.ts: an idiomatic TypeScript corpus passes the full preset under both policies, and each replacement reports TypeScript-only violations.🤖 Generated with Claude Code
https://claude.ai/code/session_01MCeDV2VhbUtLyAP9XRG1zL
Generated by Claude Code