Conversation
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.
…not only a manifest
A repo that is not a plugin -- a library, tooling, a standalone CLI alongside
the plugin -- currently has to ship a fake manifest.json to use this config
without noise.
The rule was configured as `manifest && manifest.isDesktopOnly ? "off" : "warn"`,
so the only way to tell it that Node APIs are available was a manifest with
`isDesktopOnly: true`. It now takes an `isDesktopOnly` option, defaulting to that
same manifest value, and the config sets a plain "warn". Behaviour for an
existing plugin repo is identical; a repo with no manifest can say
`["warn", { isDesktopOnly: true }]` instead of inventing one. This matters
because the rule is listed in eslint-comments/no-restricted-disable, so a disable
comment is not an available escape.
`defaultOptions` had to become `[{}]` rather than `[]`: applyDefault iterates the
defaults array, so an empty one silently discards the user's options -- the
option had no effect at all until that changed. Worth checking wherever else a
rule takes options with an empty default.
The other half of the same problem is in `getManifest()`, which is why it is in
this commit rather than its own: it caught the ENOENT from a missing
manifest.json, printed "Failed to load JSON file: <err>" to console.error, and
returned null, so a repo with no manifest got that dump on every lint run. Its
absence is not an error and is now checked for first. A manifest that exists but
does not parse is a real defect and still reports.
The node globals half of the manifest read is unchanged -- a repo that wants
them can add `globals.node` to its own config in one line, which does not seem
worth new API surface.
Closes obsidianmd#178.
This touches getManifest(), which obsidianmd#107 also edits to walk up the directory tree;
the two should combine cleanly but will conflict textually.
Docs. configuration.md gains a "Code that only runs on desktop" section under
"Disabling rules for specific files": it shows the option, notes that a
desktop-only plugin still needs no configuration because the default reads the
manifest, and says why the option exists at all -- the no-manifest case. Nothing
in the README changes, since the default behaviour is unchanged.
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
1 participant
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.
Based on #192