Skip to content

fix(validate-license): make the rule run, and match the conventional MIT line - #192

Open
mnaoumov wants to merge 3 commits into
obsidianmd:masterfrom
mnaoumov:validate-license
Open

mnaoumov wants to merge 3 commits into
obsidianmd:masterfrom
mnaoumov:validate-license

Conversation

@mnaoumov

@mnaoumov mnaoumov commented Sep 2, 2026 •

Copy link
Copy Markdown
Collaborator

Independent of #191 and #193: this branch is based directly on master and carries only the validate-license change, so it can be merged on its own and in any order.

What was wrong

validate-license could not report anything in configs.recommended, and would not have matched much if it had.

  • It never ran. It lived in recommendedPluginRulesConfigBase, which is only spread into the **/*.{js,cjs,mjs,jsx} and **/*.{ts,cts,mts,tsx} blocks, so no LICENSE could ever match. On the same fixture, ESLint previously answered "File ignored because no matching configuration was supplied" for LICENSE, and now reports.
  • The copyright regex missed the standard MIT line. It required Copyright (C) <year>[-<year>] by <holder>. The conventional line, Copyright (c) 2025 Dynalist Inc. (this repo's own LICENSE), has a lowercase marker and no " by ", so it matched nowhere and the rule passed silently on every repo using the standard MIT text.

What changes

  • LICENSE, LICENSE.md and LICENSE.txt get their own file-scoped block in the recommended config, read with a new plugin-contributed language, obsidianmd/plain-text (lib/plainTextLanguage.ts, built on @eslint/plugin-kit's TextSourceCodeBase, with one Line node per line and CRLF handled).
  • The rule is taken out of the JS/TS base block.
  • It's a language rather than a parser on purpose. languageOptions.parser is merged by key, so a config object that sets a parser without restricting files replaces it for every file. With a parser, the README's own example shape (...recommended followed by a global { languageOptions: { parser: tseslint.parser, ... } }) made LICENSE reach the TypeScript parser and fail fatally ("was not found by the project service because the extension for the file (``) is non-standard"). A language can only be replaced by another `language`, so that can't happen. I checked this across seven config shapes.
  • @eslint/plugin-kit is now a declared dependency. ESLint core already depended on it, so it was in the tree before.
  • lib/plainTextParser.ts is removed. It existed only for this rule, could only be reached by deep import, and was never wired up.
  • The copyright regex now accepts lowercase (c), the copyright sign and an optional " by ".
  • index.d.ts declares the plugin's languages export.

Tests

  • tests/validateLicense.test.ts covers the new copyright forms and the plain-text language.
  • tests/recommendedConfig.test.ts gets two regression guards:
    • a "file-scoped rules" block that resolves the config for LICENSE (this would have caught the scoping defect);
    • a block that lints LICENSE through a config carrying a global parser override and asserts no fatal error (this would have caught the parser-clobbering one).

Docs

LICENSE is matched by name, so ESLint has to be pointed at the project root. An eslint src lint script never visits it, and the check silently does nothing.

  • The README and docs/configuration.md now say so, under a new "Files linted beyond your source" section.
  • That section also covers a licence file under another name and explains why the plugin contributes a language rather than a parser.
  • The scanner-equivalent config switched this rule off inside a block globbed to **/*.{ts,...,jsx}, which LICENSE can't match. That "off" moves to a files: ["LICENSE", "LICENSE.md", "LICENSE.txt"] block.

Relation to #191

#191 fixes the same scoping defect for validate-manifest. The two PRs share no code, and neither needs the other, but they edit the same few spots, so whichever merges second will need a small textual conflict resolution in:

  • lib/index.ts (each removes its own rule from the base block and adds its own file-scoped block),
  • eslint.config.mjs,
  • README.md,
  • docs/configuration.md (both add the "Files linted beyond your source" section: this PR lists package.json and LICENSE, fix(validate-manifest): make the rule run, and stop it over-reporting #191 lists package.json and manifest.json; the merged text lists all three),
  • tests/recommendedConfig.test.ts (both add a "file-scoped rules" block).

In each file the resolution keeps both sides. I've checked that it reproduces exactly the combined tree the stacked version of this PR had.

@mnaoumov mnaoumov changed the title validate-license fix(validate-license): make the rule run, and match the conventional MIT line Sep 2, 2026
@mnaoumov
mnaoumov requested a review from saberzero1 September 4, 2026 18:41
@mnaoumov
mnaoumov requested a balanced review from Copilot September 29, 2026 23:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The documented COPYING configuration is silently ignored by the rule’s hard-coded filename guard.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Makes validate-license run on license files and recognize conventional MIT copyright lines.

Changes:

  • Adds and registers a plain-text ESLint language.
  • Scopes license validation to supported license filenames.
  • Expands tests and configuration documentation.
File Description
lib/​plainTextLanguage.ts Implements plain-text parsing and traversal.
lib/​plainTextParser.ts Removes the obsolete parser.
lib/​rules/​validateLicense.ts Updates matching and language integration.
lib/​index.ts Registers the language and license config.
index.d.ts Declares contributed languages.
package.json Adds plugin-kit dependency.
package-lock.json Locks the dependency.
tests/​validateLicense.test.ts Tests formats and line endings.
tests/​recommendedConfig.test.ts Tests scoping and parser overrides.
eslint.config.mjs Disables the rule for this repository.
README.md Documents root-level linting.
docs/​configuration.md Documents non-source files and custom setup.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/rules/validateLicense.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

One documented configuration crashes because it leaves no-irregular-whitespace enabled for the custom plain-text source code.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Disable no-irregular-whitespace for PlainTextSourceCode LICENSE linting

docs/​configuration.md:337

This snippet is used with the globally applied eslint.configs.recommended shown above, so it also enables no-irregular-whitespace for LICENSE. That core rule calls sourceCode.getAllComments() during setup, but PlainTextSourceCode does not provide that JavaScript-specific API, causing linting to fail before validate-license runs. Disable it here as the built-in LICENSE block and the custom-filename example do.

…MIT line

The rule could not report anything in `configs.recommended`, and would not have
matched much if it had.

It lived in `recommendedPluginRulesConfigBase`, which is only spread into the
`**/*.{js,cjs,mjs,jsx}` and `**/*.{ts,cts,mts,tsx}` blocks, so no LICENSE could
ever match. Measured before and after on the same fixture: ESLint previously
answered "File ignored because no matching configuration was supplied" for
LICENSE, and now reports.

LICENSE has no extension and is not code, so it also needs something that can
read it. That is now a LANGUAGE -- `obsidianmd/plain-text`, contributed by the
plugin -- rather than the `PlainTextParser` this repo carried but never wired up.

The distinction is load-bearing, not cosmetic. `languageOptions.parser` is merged
by key, so a config object that sets a parser WITHOUT restricting its `files`
replaces it for every file. With a parser, this config:

    export default defineConfig([
      ...obsidianmd.configs.recommended,
      { languageOptions: { parser: tseslint.parser, parserOptions: { ... } } },
    ]);

made LICENSE reach the TypeScript parser and fail:

    Parsing error: LICENSE was not found by the project service because the
    extension for the file (``) is non-standard.

That is a fatal error on a file the user never asked to lint, from a config shape
the README's own example invites. A `language` is only replaced by another
`language`, so it cannot happen -- the same reason `package.json` is immune, since it
uses `language: "json/json"`. Verified
across seven config shapes, including a global `parser`, a global
`parser` + `project`, and `extraFileExtensions: [".json"]`.

`lib/plainTextLanguage.ts` implements the language on `@eslint/plugin-kit`'s
`TextSourceCodeBase`, exposing each line as a `Line` node. It handles CRLF, and
its `validateLanguageOptions` deliberately accepts anything, so a stray
`parserOptions` from an unrestricted config block is ignored rather than fatal.
`@eslint/plugin-kit` becomes a declared dependency; it was already in the tree as
a dependency of ESLint core. `lib/plainTextParser.ts` is removed: it existed only
for this rule, was reachable only by deep import, and the rule could never run,
so nothing can be depending on it in a working setup.

The copyright regex required `Copyright (C) <year>[-<year>] by <holder>`. The
conventional MIT line -- `Copyright (c) 2025 Dynalist Inc.`, which is this repo's
own LICENSE -- has a lowercase marker and no " by ", so it matched nowhere and
the rule silently passed on every repo using the standard MIT text. Lowercase
`(c)`, the copyright sign and an optional " by " are now accepted, and
`LICENSE.md` / `LICENSE.txt` are recognised alongside `LICENSE`.

Two regression guards in `tests/recommendedConfig.test.ts`: the "file-scoped
rules" block resolves the config for LICENSE, and a new block lints LICENSE
through a config carrying a global parser override, asserting it does not go
fatal. The first would have caught the scoping defect; the second
would have caught the parser-clobbering one.

Docs. LICENSE is matched by name, so ESLint has to be pointed at the project
root: an `eslint src` lint script never visits it and the check silently does
nothing. README and configuration.md say so, under a new "Files linted beyond
your source" section, and configuration.md also covers a licence file under
another name and explains why the plugin contributes a language rather than a
parser.

The scanner-equivalent config switched this rule off
inside a block globbed to `**/*.{ts,...,jsx}`, which LICENSE cannot match. That
"off" is moved to a `files: ["LICENSE", "LICENSE.md", "LICENSE.txt"]` block.
The rule returned an empty visitor for any file not named LICENSE,
LICENSE.md or LICENSE.txt, so the COPYING block in the configuration
docs silently did nothing. The rule only visits plain-text lines, so
each config block's `files` already decides what it reads.

The documented block also needs `no-irregular-whitespace` off, as the
built-in LICENSE block has: without it ESLint's core rule crashes on
plain text. A test now lints COPYING through that exact block.
…SE snippet

The snippet sits under a config that applies eslint.configs.recommended
to every file, so no-irregular-whitespace also ran on LICENSE. That rule
calls sourceCode.getAllComments(), which the plain-text source code does
not have, so ESLint threw before validate-license ran. Turn it off here,
as the built-in LICENSE block and the custom-filename example already do.
@mnaoumov

mnaoumov commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

Confirmed and fixed in 6d51eac, and the branch is rebased on master. With eslint.configs.recommended applied to every file, that LICENSE block throws Error while loading rule 'no-irregular-whitespace': sourceCode.getAllComments is not a function before validate-license runs. The snippet now turns no-irregular-whitespace off, the same way the built-in LICENSE block and the COPYING example do. I checked the updated snippet with a real ESLint run and it reports the usual licence findings.

@saberzero1 saberzero1 self-assigned this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants