From f0f51e9051377a3dd2dee513e63fa9e3ca40d576 Mon Sep 17 00:00:00 2001 From: Michael Naumov Date: Wed, 2 Sep 2026 10:47:50 -0600 Subject: [PATCH] fix(validate-license): make the rule run, and match the conventional 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) [-] by `. 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. --- README.md | 5 + docs/configuration.md | 60 ++++++++++- eslint.config.mjs | 6 ++ index.d.ts | 7 ++ lib/index.ts | 26 ++++- lib/plainTextLanguage.ts | 181 ++++++++++++++++++++++++++++++++ lib/plainTextParser.ts | 51 --------- lib/rules/validateLicense.ts | 113 ++++++++++++-------- package-lock.json | 1 + package.json | 1 + tests/recommendedConfig.test.ts | 83 ++++++++++++++- tests/validateLicense.test.ts | 73 +++++++++++-- 12 files changed, 500 insertions(+), 107 deletions(-) create mode 100644 lib/plainTextLanguage.ts delete mode 100644 lib/plainTextParser.ts diff --git a/README.md b/README.md index feca049..502a610 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,11 @@ The `parserOptions` block is required because the recommended config includes ty > **Note:** You do not need to separately add `eslint.configs.recommended` or `tseslint.configs.recommended` — both are already included in the recommended config. +The recommended config also lints files that are not source, matching `package.json` and +`LICENSE` by name. **Run ESLint from the project root**: a lint script scoped to your sources — +`eslint src` — never visits them, and their checks silently do nothing. See +[Files linted beyond your source](docs/configuration.md#files-linted-beyond-your-source). + For advanced usage — layering stricter typescript-eslint configs, ignoring files, disabling rules for non-plugin code, and troubleshooting common errors — see the [configuration guide](docs/configuration.md). ## Configurations diff --git a/docs/configuration.md b/docs/configuration.md index f6322d3..13e5b3c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -55,9 +55,44 @@ The recommended config is an array of flat config objects that sets up: - **Third-party plugins**: `@microsoft/eslint-plugin-sdl`, `eslint-plugin-import`, `eslint-plugin-no-unsanitized`, `eslint-plugin-depend`, `@eslint-community/eslint-plugin-eslint-comments` - **Obsidian globals** (`activeDocument`, `activeWindow`, `createEl`, etc.) - **`package.json` linting** via `eslint-plugin-depend` (ban common micro-utilities) +- **`LICENSE` linting** via `validate-license`, read with the plugin's `obsidianmd/plain-text` language Because of this, you do **not** need to separately add `eslint.configs.recommended` or `tseslint.configs.recommended` — they are already included. +### Files linted beyond your source + +The recommended config lints more than `.ts` and `.js`. It matches `package.json` and `LICENSE` by +name, each read with its own language, so **ESLint has to be pointed at the project root for those +checks to run at all**. A lint script scoped to your sources — `eslint src` — will never visit +them, and the rules covering them silently do nothing: + +```jsonc +// package.json +"scripts": { + "lint": "eslint .", // package.json and LICENSE are checked + "lint:src": "eslint src" // they are not +} +``` + +`LICENSE` is matched exactly, alongside `LICENSE.md` and `LICENSE.txt`. A licence file under any +other name is not linted; point `validate-license` at it with your own `files` entry, using the +language the plugin contributes: + +```js +{ + files: ["COPYING"], + language: "obsidianmd/plain-text", + rules: { "obsidianmd/validate-license": "warn" }, +} +``` + +`plain-text` is a **language**, not a parser, and that distinction matters for more than tidiness. +`languageOptions.parser` is merged by key, so a config object that sets a parser **without +restricting its `files`** replaces it for every file — including `LICENSE`, which would then reach +(say) the TypeScript parser and fail with *"was not found by the project service because the +extension for the file (``) is non-standard"*. A `language` is only replaced by another `language`, +so that cannot happen. The same reasoning is why `package.json` uses `language: "json/json"`. + ## Using alongside stricter typescript-eslint configs If you want stricter rules than what the recommended config provides (e.g., `strictTypeChecked`, `stylisticTypeChecked`), layer them after the recommended spread: @@ -288,7 +323,17 @@ A few things to keep in mind with this approach: - **Obsidian globals** must be declared manually. The recommended config does this for you; here you need to add them yourself. The list above covers the most common ones. `DomElementInfo`, `SvgElementInfo`, `isBoolean`, `nextFrame`, and `ready` are also available. - **Third-party plugins** bundled by the recommended config (`@microsoft/eslint-plugin-sdl`, `eslint-plugin-import`, `eslint-plugin-no-unsanitized`, `eslint-plugin-depend`, `eslint-plugin-eslint-comments`) are not included. Add them separately if you want them. -- **`package.json` and `manifest.json` linting** (`validate-manifest`, `validate-license`, `depend/ban-dependencies`) is not set up. The `validate-manifest` and `validate-license` rules will work on `.json` files only if you have a JSON parser configured. +- **`package.json` and `manifest.json` linting** (`validate-manifest`, `depend/ban-dependencies`) is not set up. The `validate-manifest` rule will work on `.json` files only if you have a JSON parser configured. +- **`LICENSE` linting** (`validate-license`) is not set up either, and is not covered by `ruleConfigs` — those presets only carry rules that apply to source files. `validate-license` needs its own config block, because `LICENSE` matches no source glob and needs its own language: + + ```js + { + files: ["LICENSE", "LICENSE.md", "LICENSE.txt"], + language: "obsidianmd/plain-text", + plugins: { obsidianmd }, + rules: { "obsidianmd/validate-license": "warn" }, + } + ``` ## Community plugin scanner configuration @@ -384,9 +429,8 @@ export default defineConfig([ "@typescript-eslint/no-base-to-string": "off", "import/no-unresolved": "off", - // Scanner handles these separately + // Scanner handles this separately "obsidianmd/validate-manifest": "off", - "obsidianmd/validate-license": "off", // Old plugins should not change their command ids "obsidianmd/commands/no-command-in-command-id": "off", @@ -394,6 +438,16 @@ export default defineConfig([ }, }, + { + // Scanner handles this separately. It must be switched off under the glob + // the rule actually runs on -- LICENSE matches no source glob, so an "off" + // in the block above would not reach it. + files: ["LICENSE", "LICENSE.md", "LICENSE.txt"], + rules: { + "obsidianmd/validate-license": "off", + }, + }, + globalIgnores([ "node_modules", "dist", diff --git a/eslint.config.mjs b/eslint.config.mjs index e73bea5..87ef997 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -30,4 +30,10 @@ export default [ ...eslintPluginPlugin.configs["tests-recommended"], files: ["tests/**/*.ts"], }, + // This repo is the plugin, not an Obsidian plugin. Its LICENSE is Dynalist's + // own, so the rule is correct to fire here and the finding is unactionable. + { + files: ["LICENSE"], + rules: { "obsidianmd/validate-license": "off" }, + }, ]; diff --git a/index.d.ts b/index.d.ts index e7b1078..177bbc1 100644 --- a/index.d.ts +++ b/index.d.ts @@ -1,6 +1,13 @@ import { type Linter, type Rule } from "eslint"; declare module 'eslint-plugin-obsidianmd' { + /** + * Languages contributed by this plugin. `plain-text` exposes each line of a text file as a + * `Line` node and backs `validate-license`; reference it as `language: "obsidianmd/plain-text"`. + */ + export const languages: { + [key: string]: unknown; + }; export const meta: { name: string; version: string; diff --git a/lib/index.ts b/lib/index.ts index 1ff90f4..9ccd604 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -29,6 +29,7 @@ import ruleCustomMessage from "./rules/ruleCustomMessage.js"; import noNodejsModules from "./rules/noNodejsModules.js"; import noUnsupportedApi from "./rules/noUnsupportedApi.js"; import { getManifest } from "./manifest.js"; +import { PlainTextLanguage } from "./plainTextLanguage.js"; import { ui } from "./rules/ui/index.js"; // --- Import plugins and configs for the recommended config --- @@ -68,6 +69,13 @@ const plugin = { name: packageJson.name, version: packageJson.version, }, + // A language, not a parser: `languageOptions.parser` is merged by key, so + // any later config object without a `files` restriction replaces it, and + // LICENSE then reaches whatever parser that block names. `language` is only + // replaced by another `language`. + languages: { + "plain-text": PlainTextLanguage + }, rules: { "commands/no-command-in-command-id": commands.noCommandInCommandId, "commands/no-command-in-command-name": commands.noCommandInCommandName, @@ -171,7 +179,9 @@ const recommendedPluginRulesConfigBase: RulesConfig = { "obsidianmd/regex-lookbehind": "error", "obsidianmd/sample-names": "error", "obsidianmd/validate-manifest": "warn", - "obsidianmd/validate-license": ["warn"], + // validate-license is NOT here: it lints LICENSE, which cannot match the + // JS/TS globs this block is spread into. It gets its own file-scoped block + // below. "obsidianmd/ui/sentence-case": ["warn", { enforceCamelCaseLower: true }], } @@ -290,6 +300,20 @@ const flatRecommendedConfig: Config[] = defineConfig([ ] } }, + // LICENSE has no extension and is not code, so it needs both an explicit + // glob and a language that can read it. + { + files: ['LICENSE', 'LICENSE.md', 'LICENSE.txt'], + language: 'obsidianmd/plain-text', + extends: [tseslint.configs.disableTypeChecked as Config], + plugins: { + obsidianmd: plugin + }, + rules: { + "no-irregular-whitespace": "off", + "obsidianmd/validate-license": "warn" + } + }, { files: ['**/*.{ts,cts,mts,tsx,js,cjs,mjs,jsx}'], rules: { diff --git a/lib/plainTextLanguage.ts b/lib/plainTextLanguage.ts new file mode 100644 index 0000000..ffc9c08 --- /dev/null +++ b/lib/plainTextLanguage.ts @@ -0,0 +1,181 @@ +import type { + File, + Language, + LanguageContext, + OkParseResult, + ParseResult, + RuleVisitor, + SourceLocation, + SourceRange, + TraversalStep, +} from "@eslint/core"; +import { TextSourceCodeBase, VisitNodeStep } from "@eslint/plugin-kit"; + +/** A single line of a plain text file. */ +export interface PlainTextLineNode { + type: "Line"; + /** The text of the line, without its terminator. */ + value: string; + loc: SourceLocation; + range: SourceRange; +} + +/** The root node of a plain text file. */ +export interface PlainTextDocumentNode { + type: "Document"; + lines: PlainTextLineNode[]; + loc: SourceLocation; + range: SourceRange; +} + +export type PlainTextNode = PlainTextDocumentNode | PlainTextLineNode; + +/** Plain text has nothing to configure. */ +export type PlainTextLanguageOptions = Record; + +export interface PlainTextRuleVisitor extends RuleVisitor { + Document?(node: PlainTextDocumentNode): void; + Line?(node: PlainTextLineNode, parent?: PlainTextDocumentNode): void; + "Document:exit"?(node: PlainTextDocumentNode): void; + "Line:exit"?(node: PlainTextLineNode, parent?: PlainTextDocumentNode): void; +} + +const LINE_ENDING_PATTERN = /\r\n|[\r\n]/u; + +export class PlainTextSourceCode extends TextSourceCodeBase<{ + LangOptions: PlainTextLanguageOptions; + RootNode: PlainTextDocumentNode; + SyntaxElementWithLoc: PlainTextNode; + ConfigNode: never; +}> { + override ast: PlainTextDocumentNode; + + #steps: TraversalStep[] | undefined; + + constructor({ text, ast }: { text: string; ast: PlainTextDocumentNode }) { + super({ text, ast, lineEndingPattern: LINE_ENDING_PATTERN }); + this.ast = ast; + } + + // The AST is only two levels deep, so a node's parent is the document + // unless the node is the document itself. + override getParent(node: PlainTextNode): PlainTextNode | undefined { + return node.type === "Line" ? this.ast : undefined; + } + + override traverse(): Iterable { + // The AST does not mutate, so the steps can be cached. + if (this.#steps) { + return this.#steps.values(); + } + + const steps: TraversalStep[] = (this.#steps = []); + + steps.push( + new VisitNodeStep({ + target: this.ast, + phase: 1, + args: [this.ast], + }), + ); + + for (const line of this.ast.lines) { + steps.push( + new VisitNodeStep({ target: line, phase: 1, args: [line, this.ast] }), + new VisitNodeStep({ target: line, phase: 2, args: [line, this.ast] }), + ); + } + + steps.push( + new VisitNodeStep({ + target: this.ast, + phase: 2, + args: [this.ast], + }), + ); + + return steps; + } +} + +/** + * An ESLint language that exposes each line of a text file as a `Line` node. + * + * This is a language rather than a parser on purpose. A parser is set through + * `languageOptions.parser`, which ESLint merges by key, so any later config + * object without a `files` restriction replaces it -- and a LICENSE file then + * reaches, say, the TypeScript parser, which fails on an extensionless file. + * A `language` is only replaced by another `language`, so the LICENSE block + * cannot be clobbered by an unrelated `languageOptions` entry. + */ +export const PlainTextLanguage: Language<{ + LangOptions: PlainTextLanguageOptions; + Code: PlainTextSourceCode; + RootNode: PlainTextDocumentNode; + Node: PlainTextNode; +}> = { + fileType: "text", + lineStart: 1, + // 0 so that reported columns stay 1-based, as they are for source files. + columnStart: 0, + nodeTypeKey: "type", + visitorKeys: { + Document: ["lines"], + Line: [], + }, + + // Plain text takes no options. Anything else present on languageOptions -- + // a stray `parserOptions` from a config block that does not restrict its + // `files` -- is simply not ours to validate, and must not be an error. + validateLanguageOptions(): void { + // no options to validate + }, + + parse(file: File): ParseResult { + const text = String(file.body); + const lines: PlainTextLineNode[] = []; + + let index = 0; + let lineNumber = 1; + for (const line of text.split(LINE_ENDING_PATTERN)) { + lines.push({ + type: "Line", + value: line, + range: [index, index + line.length], + loc: { + start: { line: lineNumber, column: 0 }, + end: { line: lineNumber, column: line.length }, + }, + }); + // The terminator may be 1 or 2 characters, so recover the offset + // from the source text rather than assuming "\n". + const terminator = text.slice(index + line.length).match(/^\r\n|^[\r\n]/u); + index += line.length + (terminator ? terminator[0].length : 0); + lineNumber++; + } + + return { + ok: true, + ast: { + type: "Document", + lines, + range: [0, text.length], + loc: { + start: { line: 1, column: 0 }, + end: lines[lines.length - 1]?.loc.end ?? { line: 1, column: 0 }, + }, + }, + }; + }, + + createSourceCode( + file: File, + parseResult: OkParseResult, + _context: LanguageContext, + ): PlainTextSourceCode { + return new PlainTextSourceCode({ + text: String(file.body), + ast: parseResult.ast, + }); + }, +}; diff --git a/lib/plainTextParser.ts b/lib/plainTextParser.ts deleted file mode 100644 index c33703a..0000000 --- a/lib/plainTextParser.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { AST_NODE_TYPES, AST_TOKEN_TYPES } from "@typescript-eslint/types"; -import type { Parser } from "@typescript-eslint/utils/ts-eslint"; - -/** - * A plain text parser for ESLint. - * It treats each line as a separate token of type `Line`. - * This allows us to lint plain text files like LICENSE files. - */ -export const PlainTextParser: Parser.ParserModule = { - meta: { - name: "plain-text-parser", - version: "1.0.0", - }, - parseForESLint(text: string): Parser.ParseResult { - const lines = text.split("\n"); - - const tokens: Parser.ParseResult["ast"]["tokens"] = []; - - let index = 0; - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - tokens.push({ - // Line is originally supposed to represent a multi-line comment, - // but we use it to represent a line of text here. The identifier fits best. - type: AST_TOKEN_TYPES.Line, - value: line, - range: [index, index + line.length], - loc: { - start: { line: i + 1, column: 0 }, - end: { line: i + 1, column: line.length }, - }, - }); - index += line.length + 1; // +1 for the newline character - } - - return { - ast: { - type: AST_NODE_TYPES.Program, - sourceType: "script", - range: [0, text.length], - loc: { - start: tokens[0]?.loc.start ?? { line: 1, column: 0 }, - end: tokens[tokens.length - 1].loc.end ?? { line: 1, column: 0 }, - }, - body: [], - comments: [], - tokens: tokens, - }, - }; - } -} diff --git a/lib/rules/validateLicense.ts b/lib/rules/validateLicense.ts index b22e61e..fad8fc2 100644 --- a/lib/rules/validateLicense.ts +++ b/lib/rules/validateLicense.ts @@ -1,10 +1,46 @@ -import { AST_TOKEN_TYPES, TSESTree } from "@typescript-eslint/utils"; +import type { CustomRuleDefinitionType, CustomRuleTypeDefinitions } from "@eslint/core"; import path from "path"; -import { docsUrl, ruleCreator } from "../ruleCreator.js"; +import type { + PlainTextLanguageOptions, + PlainTextNode, + PlainTextRuleVisitor, + PlainTextSourceCode, +} from "../plainTextLanguage.js"; +import { docsUrl } from "../ruleCreator.js"; -export default ruleCreator({ +type MessageIds = "unchangedCopyright" | "unchangedYear"; + +interface ValidateLicenseOptions { + currentYear?: number; + disableUnchangedYear?: boolean; +} + +type PlainTextRuleDefinition = object> = + CustomRuleDefinitionType< + { + LangOptions: PlainTextLanguageOptions; + Code: PlainTextSourceCode; + Visitor: PlainTextRuleVisitor; + Node: PlainTextNode; + }, + Options + >; + +// We want to parse: Copyright (C) 2020-2025 by Dynalist Inc. +// We should check that the year is current and the holder is not "Dynalist Inc." +// +// The marker and the " by " are both optional in practice: the conventional MIT +// line reads "Copyright (c) 2025 Dynalist Inc.", with a lowercase marker and no +// " by ". Requiring the uppercase "(C) ... by" form meant this rule silently +// matched nothing on every repo using the standard MIT text. +const COPYRIGHT_REGEX = /^[ \t]*Copyright (?:\([Cc]\)|©) (\d{4})(?:\s*-\s*(\d{4}))? (?:by )?(.+)$/; + +const rule: PlainTextRuleDefinition<{ + MessageIds: MessageIds; + RuleOptions: [ValidateLicenseOptions?]; +}> = { meta: { - type: "problem" as const, + type: "problem", docs: { description: "Validate the structure of copyright notices in LICENSE files for Obsidian plugins.", url: docsUrl("validate-license"), @@ -30,53 +66,48 @@ export default ruleCreator({ unchangedYear: "Please change the copyright year from {{actual}} to the current year ({{expected}}).", }, }, - defaultOptions: [{ - currentYear: new Date().getFullYear(), - disableUnchangedYear: false, - }], - create(context, [options]) { + create(context) { const filename = context.physicalFilename; - if (!path.basename(filename).endsWith("LICENSE")) { + // Matches LICENSE as well as the equally common LICENSE.md / LICENSE.txt. + if (!/LICENSE(\.(?:md|txt))?$/.test(path.basename(filename))) { return {}; } - return { - Program(programNode: TSESTree.Program) { - // We want to parse: Copyright (C) 2020-2025 by Dynalist Inc. - // We should check that the year is current and the holder is not "Dynalist Inc." - - const copyrightRegex = /^(?: |\t)*Copyright \(C\) (\d{4})(?:-(\d{4}))? by (.+)$/; + const options = context.options[0] ?? {}; + const currentYear = options.currentYear ?? new Date().getFullYear(); + const disableUnchangedYear = options.disableUnchangedYear ?? false; - // we rely on our plain text parser to give us tokens as Line tokens - for (const token of programNode.tokens ?? []) { - if (token.type !== AST_TOKEN_TYPES.Line) continue; + return { + Line(node) { + const match = node.value.match(COPYRIGHT_REGEX); + if (!match) { + return; + } - const match = token.value.match(copyrightRegex); - if (match) { - const startYear = parseInt(match[1], 10); - const endYear = match[2] ? parseInt(match[2], 10) : startYear; - const holder = match[3].trim(); + const startYear = parseInt(match[1], 10); + const endYear = match[2] ? parseInt(match[2], 10) : startYear; + const holder = match[3].trim(); - if (!options.disableUnchangedYear && endYear < options.currentYear) { - context.report({ - messageId: "unchangedYear", - loc: token.loc, - data: { - expected: options.currentYear.toString(), - actual: endYear.toString(), - } - }); + if (!disableUnchangedYear && endYear < currentYear) { + context.report({ + messageId: "unchangedYear", + loc: node.loc, + data: { + expected: currentYear.toString(), + actual: endYear.toString(), } + }); + } - if (holder === "Dynalist Inc.") { - context.report({ - messageId: "unchangedCopyright", - loc: token.loc, - }); - } - } + if (holder === "Dynalist Inc.") { + context.report({ + messageId: "unchangedCopyright", + loc: node.loc, + }); } } }; }, -}); +}; + +export default rule; diff --git a/package-lock.json b/package-lock.json index d178716..83a02fd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,6 +13,7 @@ "@eslint/config-helpers": "^0.4.2", "@eslint/js": "^9.30.1", "@eslint/json": "0.14.0", + "@eslint/plugin-kit": "^0.4.1", "@microsoft/eslint-plugin-sdl": "^1.1.0", "@types/eslint": "9.6.1", "@types/node": "20.12.12", diff --git a/package.json b/package.json index 2d59249..ac41e3a 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "@eslint/config-helpers": "^0.4.2", "@eslint/js": "^9.30.1", "@eslint/json": "0.14.0", + "@eslint/plugin-kit": "^0.4.1", "@microsoft/eslint-plugin-sdl": "^1.1.0", "@types/eslint": "9.6.1", "@types/node": "20.12.12", diff --git a/tests/recommendedConfig.test.ts b/tests/recommendedConfig.test.ts index 2173f69..ff9bb87 100644 --- a/tests/recommendedConfig.test.ts +++ b/tests/recommendedConfig.test.ts @@ -1,5 +1,6 @@ import assert from "node:assert"; -import { ESLint } from "eslint"; +import { ESLint, type Linter } from "eslint"; +import tseslint from "typescript-eslint"; import plugin from "../lib/index.js"; async function rulesFor(configName: keyof typeof plugin.configs, filename: string): Promise> { @@ -82,6 +83,9 @@ describe("recommended config", () => { const fileSpecificRules = new Set([ "obsidianmd/ui/sentence-case-json", "obsidianmd/ui/sentence-case-locale-module", + // Scoped to LICENSE, which can never match a JS/TS glob. + // Asserted separately in "file-scoped rules" below. + "obsidianmd/validate-license", ]); const registeredRules = Object.keys(plugin.rules); for (const rule of registeredRules) { @@ -195,6 +199,82 @@ describe("recommendedWithLocalesEn config", () => { }); }); +// Regression guard for the defect these tests did not catch: validate-license +// used to live in the block spread into the JS and TS globs, so no LICENSE could +// ever match and the rule never ran. Resolving the config for the file the rule +// actually targets is the only assertion that notices. +describe("file-scoped rules", () => { + for (const configName of ["recommended", "recommendedWithLocalesEn"] as const) { + describe(configName, () => { + let licenseRules: Record; + let tsRules: Record; + + before(async () => { + licenseRules = await rulesFor(configName, "LICENSE"); + tsRules = await rulesFor(configName, "src/main.ts"); + }); + + it("validate-license should be 'warn' for LICENSE", () => { + assert.strictEqual( + getSeverity(licenseRules["obsidianmd/validate-license"]), + "warn" + ); + }); + + it("validate-license should not apply to source files", () => { + const rule = "obsidianmd/validate-license"; + const severity = getSeverity(tsRules[rule]); + assert.ok( + severity === "off" || !(rule in tsRules), + `${rule} targets a non-source file but is enabled for .ts, got: ${severity}` + ); + }); + }); + } +}); + +// LICENSE is read with a `language`, not a `parser`, and that is load-bearing: +// `languageOptions.parser` is merged by key, so a config object that sets a +// parser without restricting its `files` would replace it for every file. +// LICENSE would then reach the TypeScript parser and fail with "was not found +// by the project service because the extension for the file (``) is +// non-standard" -- a fatal error on a file the user never asked to lint. +describe("LICENSE survives a global parser override", () => { + const withGlobalParser: Linter.Config[] = [ + ...(plugin.configs.recommended as Linter.Config[]), + { + languageOptions: { + parser: tseslint.parser, + parserOptions: { + projectService: { allowDefaultProject: ["eslint.config.*"] }, + }, + }, + }, + ]; + + async function lint(filePath: string, code: string) { + const eslint = new ESLint({ + overrideConfigFile: true, + overrideConfig: withGlobalParser, + }); + const [result] = await eslint.lintText(code, { filePath }); + return result; + } + + it("LICENSE is still read by the plain text language", async () => { + const result = await lint("LICENSE", "Copyright (c) 2020 Dynalist Inc.\n"); + assert.deepStrictEqual( + result.messages.filter(m => m.fatal).map(m => m.message), + [], + "a global parser must not reach LICENSE" + ); + assert.ok( + result.messages.some(m => m.ruleId === "obsidianmd/validate-license"), + "validate-license should still report" + ); + }); +}); + describe("scanner-aligned severities", () => { let tsRules: Record; let jsRules: Record; @@ -274,7 +354,6 @@ describe("scanner-aligned severities", () => { "obsidianmd/prefer-abstract-input-suggest", "obsidianmd/prefer-window-timers", "obsidianmd/validate-manifest", - "obsidianmd/validate-license", "obsidianmd/ui/sentence-case", ]; for (const rule of warnRules) { diff --git a/tests/validateLicense.test.ts b/tests/validateLicense.test.ts index 8bbf1fd..6c27093 100644 --- a/tests/validateLicense.test.ts +++ b/tests/validateLicense.test.ts @@ -1,19 +1,22 @@ -import { RuleTester } from "@typescript-eslint/rule-tester"; +import { RuleTester, type Rule } from "eslint"; import licenseRule from "../lib/rules/validateLicense.js"; -import { PlainTextParser } from "lib/plainTextParser.js"; +import { PlainTextLanguage } from "../lib/plainTextLanguage.js"; +// LICENSE is not code, so the rule is exercised through the plain text language +// rather than through a JS/TS parser -- which is also how the recommended +// config runs it. const ruleTester = new RuleTester({ - languageOptions: { - parser: PlainTextParser, - parserOptions: { - extraFileExtensions: [""], - } + plugins: { + obsidianmd: { + languages: { "plain-text": PlainTextLanguage }, + }, }, - + language: "obsidianmd/plain-text", }); + const currentYear = new Date().getFullYear(); -ruleTester.run("validate-license", licenseRule, { +ruleTester.run("validate-license", licenseRule as unknown as Rule.RuleModule, { valid: [ { name: "copyright with year range ending at current year is valid", @@ -48,6 +51,16 @@ ruleTester.run("validate-license", licenseRule, { code: `Copyright (C) 2001 by John Doe`, options: [{ currentYear: currentYear, disableUnchangedYear: true }], }, + { + name: "conventional MIT line with lowercase marker and no 'by' is valid when current", + filename: "LICENSE", + code: `Copyright (c) ${currentYear} John Doe`, + }, + { + name: "copyright sign marker is valid when current", + filename: "LICENSE", + code: `Copyright © ${currentYear} John Doe`, + }, { name: "copyright embedded in other text is valid", filename: "LICENSE", @@ -58,6 +71,11 @@ ruleTester.run("validate-license", licenseRule, { filename: "LICENSE", code: `foo\nbar\nbaz`, }, + { + name: "a file that is not a licence is ignored", + filename: "NOTICE", + code: `Copyright (C) 2020 by Dynalist Inc.`, + }, ], invalid: [ { @@ -68,6 +86,34 @@ ruleTester.run("validate-license", licenseRule, { { messageId: "unchangedCopyright" } ], }, + { + // This is the line the standard MIT template produces, and the one + // this repo's own LICENSE carries. The rule used to match nothing + // here, so it silently passed on every such repo. + name: "conventional MIT line with lowercase marker and no 'by' is checked", + filename: "LICENSE", + code: `Copyright (c) ${currentYear} Dynalist Inc.`, + errors: [ + { messageId: "unchangedCopyright" } + ], + }, + { + name: "copyright sign marker is checked", + filename: "LICENSE", + code: `Copyright © 2022 Dynalist Inc.`, + errors: [ + { messageId: "unchangedYear", data: { expected: currentYear.toString(), actual: "2022" } }, + { messageId: "unchangedCopyright" } + ], + }, + { + name: "LICENSE.md is checked", + filename: "LICENSE.md", + code: `Copyright (c) ${currentYear} Dynalist Inc.`, + errors: [ + { messageId: "unchangedCopyright" } + ], + }, { name: "outdated year in range is forbidden", filename: "LICENSE", @@ -110,6 +156,15 @@ ruleTester.run("validate-license", licenseRule, { { messageId: "unchangedCopyright" } ], }, + { + name: "CRLF line endings are handled", + filename: "LICENSE", + code: `MIT License\r\n\r\nCopyright (c) 2022 Dynalist Inc.\r\n`, + errors: [ + { messageId: "unchangedYear", data: { expected: currentYear.toString(), actual: "2022" }, line: 3 }, + { messageId: "unchangedCopyright", line: 3 } + ], + }, { name: "year before custom currentYear is forbidden", filename: "LICENSE",