Skip to content

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

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

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

Conversation

@mnaoumov

@mnaoumov mnaoumov commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Based on #191

The rule could not report anything in `configs.recommended`.

It registered a `Program` visitor and read `Program.body[0].expression` as an
ESTree `ObjectExpression`. Under `@eslint/json`'s `json/json` language -- the
language this plugin already uses for its `package.json` block -- the visitor key
is `Document` and the AST is Momoa, so `Program` never fired.

It looked correct only because its tests ran `@typescript-eslint/parser` with
`extraFileExtensions: [".json"]`, where TypeScript parses a `.json` filename in
JSON mode and does yield `ExpressionStatement -> ObjectExpression` -- an AST no
real lint run produces for this file. Ported to the Momoa AST
(`Document -> Object -> Member`, key at `member.name.value`), with the tests
moved onto the JSON language.

It was also scoped to globs it could never match: it lived in
`recommendedPluginRulesConfigBase`, which is only spread into the
`**/*.{js,cjs,mjs,jsx}` and `**/*.{ts,cts,mts,tsx}` blocks. It now gets a
file-scoped block, modelled on the existing `package.json` one. Measured before
and after on the same fixture: ESLint previously answered "File ignored because
no matching configuration was supplied" for manifest.json, and now reports.

Two checks also report on manifests the community directory itself passed
untouched.

`descriptionFormat` required `^[A-Za-z0-9\s.,!?'"-]+$`, which rejects backticks,
em dashes, parentheses, colons and slashes. Seven live listings carry those and
the directory flagged none, so as written the check is noise rather than a gate.
It now rejects emoji and invisible characters -- `\p{Extended_Pictographic}`,
`\p{Cc}`, `\p{Cf}`, `\p{Cs}` -- and accepts ordinary punctuation. Length, leading
capital and trailing period are unchanged. Behaviour change worth calling out:
the existing "description with special characters is forbidden" case used
`@ # $ % ^ & *`, which now passes; that case moves to the valid set and an
invisible-character case takes its place.

`FORBIDDEN_WORDS` defaults are unchanged -- `obsidian` and `plugin` are still
banned in `id`, `name` and `description`. What is new is a way to opt out, for
findings that can never be acted on: a published plugin id can never change, so
a plugin whose id contains `obsidian` carries it forever.

The bypass has to be config-level. A manifest cannot carry an ignore marker of
its own -- the only marker ESLint understands is a comment, a comment in strict
JSON is a parse error, and it would also stop Obsidian loading the plugin, since
both the loader and the directory use `JSON.parse`. An ignore key survives
parsing but ships to every user and is itself reported as a disallowed key. So
the rule gains two options:

    allowedWords: { id?: string[], name?: string[], description?: string[] }
    ignore: ("noForbiddenWords" | "descriptionFormat")[]

Both are narrow by construction and tested as such: `allowedWords` un-bans one
word in one field and leaves every other field reporting, and ignoring one check
leaves the other running.

`tests/recommendedConfig.test.ts` gains a "file-scoped rules" block that resolves
the config for manifest.json, which is the only assertion that would have caught
the scoping defect.

Docs. `manifest.json` 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.

Two existing statements in configuration.md are now wrong and are corrected:

- The scanner-equivalent config switched `validate-manifest` off inside a block
  globbed to `**/*.{ts,...,jsx}`. manifest.json matches no source glob, so that
  "off" never reached the rule -- harmless while the rule could not run, and
  wrong now. Moved to a `files: ["manifest.json"]` block.
- The "using only obsidianmd rules" caveat said the rule "will work on .json
  files only if you have a JSON parser configured". It now shows the config block
  that actually enables it, and notes that `ruleConfigs` deliberately does not
  carry it, since those presets only hold rules that apply to source files.
…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 `manifest.json` and
`package.json` are immune, since they use `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 and
manifest.json through a config carrying a global parser override, asserting
neither goes 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, and configuration.md also covers a licence file under
another name and explains why the plugin contributes a language rather than a
parser.

As with validate-manifest, 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.
@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
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.

1 participant