From 1c07acc47e66efd2cd026efe7a31b8ed266caeb1 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Thu, 9 Oct 2025 14:36:18 +0200 Subject: [PATCH 1/4] Linter: Add support for defining custom Linter rules --- javascript/packages/formatter/src/cli.ts | 50 ++++- .../language-server/src/linter_service.ts | 106 +++++++++- .../packages/language-server/src/server.ts | 23 ++- .../packages/language-server/src/service.ts | 2 +- javascript/packages/linter/README.md | 109 ++++++++++ javascript/packages/linter/package.json | 6 + javascript/packages/linter/rollup.config.mjs | 40 ++++ javascript/packages/linter/src/cli.ts | 5 +- .../linter/src/cli/argument-parser.ts | 8 +- .../packages/linter/src/cli/file-processor.ts | 37 ++++ .../packages/linter/src/custom-rule-loader.ts | 189 ++++++++++++++++++ javascript/packages/linter/src/linter.ts | 93 ++++++++- javascript/packages/linter/src/loader.ts | 4 + javascript/packages/linter/src/types.ts | 22 +- .../herb-disable-comment-unnecessary.test.ts | 2 + ...rb-disable-comment-valid-rule-name.test.ts | 8 +- .../herb-disable-precise-locations.test.ts | 8 +- javascript/packages/rewriter/README.md | 44 +++- .../rewriter/src/custom-rewriter-loader.ts | 21 +- .../packages/vscode/src/analysis-service.ts | 5 +- .../packages/vscode/src/parse-worker.js | 13 +- 21 files changed, 746 insertions(+), 49 deletions(-) create mode 100644 javascript/packages/linter/src/custom-rule-loader.ts create mode 100644 javascript/packages/linter/src/loader.ts diff --git a/javascript/packages/formatter/src/cli.ts b/javascript/packages/formatter/src/cli.ts index 37d8508c7..e8f163dbc 100644 --- a/javascript/packages/formatter/src/cli.ts +++ b/javascript/packages/formatter/src/cli.ts @@ -5,6 +5,7 @@ import { resolve, relative } from "path" import { Herb } from "@herb-tools/node-wasm" import { Config, addHerbExtensionRecommendation, getExtensionsJsonRelativePath } from "@herb-tools/config" +import { colorize } from "@herb-tools/highlighter" import { Formatter } from "./formatter.js" import { ASTRewriter, StringRewriter, CustomRewriterLoader, builtinRewriters, isASTRewriterClass, isStringRewriterClass } from "@herb-tools/rewriter/loader" @@ -208,7 +209,19 @@ export class CLI { allRewriterClasses.push(...builtinRewriters) const loader = new CustomRewriterLoader({ baseDir }) - const { rewriters: customRewriters, duplicateWarnings } = await loader.loadRewritersWithInfo() + const { rewriters: customRewriters, rewriterInfo, duplicateWarnings } = await loader.loadRewritersWithInfo() + + if (customRewriters.length > 0) { + console.error(colorize(`\nLoaded ${customRewriters.length} custom ${pluralize(customRewriters.length, 'rewriter')}:`, "green")) + + for (const { name, path } of rewriterInfo) { + const relativePath = config.projectPath ? path.replace(config.projectPath + '/', '') : path + + console.error(colorize(` • ${name}`, "cyan") + colorize(` (${relativePath})`, "dim")) + } + + console.error() + } allRewriterClasses.push(...customRewriters) warnings.push(...duplicateWarnings) @@ -274,18 +287,41 @@ export class CLI { } if (preRewriters.length > 0 || postRewriters.length > 0) { - const parts: string[] = [] + const customRewriterPaths = new Map(rewriterInfo.map(r => [r.name, r.path])) if (preRewriters.length > 0) { - parts.push(`${preRewriters.length} pre-format ${pluralize(preRewriters.length, 'rewriter')}: ${rewriterNames.pre.join(', ')}`) + console.error(colorize(`\nUsing ${preRewriters.length} pre-format ${pluralize(preRewriters.length, 'rewriter')}:`, "green")) + + for (const rewriter of preRewriters) { + const customPath = customRewriterPaths.get(rewriter.name) + + if (customPath) { + const relativePath = config.projectPath ? customPath.replace(config.projectPath + '/', '') : customPath + console.error(colorize(` • ${rewriter.name}`, "cyan") + colorize(` (${relativePath})`, "dim")) + } else { + console.error(colorize(` • ${rewriter.name}`, "cyan") + colorize(` (built-in)`, "dim")) + } + } + + console.error() } if (postRewriters.length > 0) { - parts.push(`${postRewriters.length} post-format ${pluralize(postRewriters.length, 'rewriter')}: ${rewriterNames.post.join(', ')}`) - } + console.error(colorize(`\nUsing ${postRewriters.length} post-format ${pluralize(postRewriters.length, 'rewriter')}:`, "green")) - console.error(`Using ${parts.join(', ')}`) - console.error() + for (const rewriter of postRewriters) { + const customPath = customRewriterPaths.get(rewriter.name) + + if (customPath) { + const relativePath = config.projectPath ? customPath.replace(config.projectPath + '/', '') : customPath + console.error(colorize(` • ${rewriter.name}`, "cyan") + colorize(` (${relativePath})`, "dim")) + } else { + console.error(colorize(` • ${rewriter.name}`, "cyan") + colorize(` (built-in)`, "dim")) + } + } + + console.error() + } } if (warnings.length > 0) { diff --git a/javascript/packages/language-server/src/linter_service.ts b/javascript/packages/language-server/src/linter_service.ts index 0b19b4efa..73b19b000 100644 --- a/javascript/packages/language-server/src/linter_service.ts +++ b/javascript/packages/language-server/src/linter_service.ts @@ -1,24 +1,37 @@ -import { Diagnostic, Range, Position, CodeDescription } from "vscode-languageserver/node" +import { Diagnostic, Range, Position, CodeDescription, Connection } from "vscode-languageserver/node" import { TextDocument } from "vscode-languageserver-textdocument" -import { Linter } from "@herb-tools/linter" +import { Linter, rules, type RuleClass } from "@herb-tools/linter" +import { CustomRuleLoader } from "@herb-tools/linter/loader" import { Herb } from "@herb-tools/node-wasm" import { Config } from "@herb-tools/config" import { Settings } from "./settings" +import { Project } from "./project" import { lintToDignosticSeverity } from "./utils" +const OPEN_CONFIG_ACTION = 'Open .herb.yml' + export interface LintServiceResult { diagnostics: Diagnostic[] } export class LinterService { + private readonly connection: Connection private readonly settings: Settings + private readonly project: Project private readonly source = "Herb Linter " private linter?: Linter - - constructor(settings: Settings) { + private allRules: RuleClass[] = rules + private customRulesLoaded = false + private failedCustomRules: Map = new Map() + private hasShownCustomRuleWarning = false + private customRulePaths: Map = new Map() + + constructor(connection: Connection, settings: Settings, project: Project) { + this.connection = connection this.settings = settings + this.project = project } /** @@ -27,6 +40,78 @@ export class LinterService { */ rebuildLinter(): void { this.linter = undefined + this.allRules = rules + this.customRulesLoaded = false + this.hasShownCustomRuleWarning = false + this.failedCustomRules.clear() + this.customRulePaths.clear() + } + + /** + * Load custom rules from the project and merge with built-in rules + */ + private async loadCustomRules(): Promise { + if (this.customRulesLoaded) { + return + } + + const baseDir = this.project.projectPath + const loader = new CustomRuleLoader({ baseDir, silent: true }) + + try { + const { rules: customRules, ruleInfo, duplicateWarnings } = await loader.loadRulesWithInfo() + + if (customRules.length > 0) { + this.connection.console.log(`[Linter] Loaded ${customRules.length} custom rules: ${ruleInfo.map(r => r.name).join(', ')}`) + + ruleInfo.forEach(({ name, path }) => { + this.customRulePaths.set(name, path) + }) + + this.allRules = [...rules, ...customRules] + + if (duplicateWarnings.length > 0) { + duplicateWarnings.forEach(warning => { + this.connection.console.warn(`[Linter] ${warning}`) + }) + } + } + + this.customRulesLoaded = true + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error) + + this.connection.console.error(`[Linter] Failed to load custom rules: ${errorMessage}`) + this.failedCustomRules.set('custom-rules', errorMessage) + this.customRulesLoaded = true + } + } + + /** + * Show warning message to user about failed custom rules + */ + private showCustomRuleWarnings(): void { + if (this.failedCustomRules.size === 0 || this.hasShownCustomRuleWarning) { + return + } + + this.hasShownCustomRuleWarning = true + + const failures = Array.from(this.failedCustomRules.entries()) + const message = failures.length === 1 + ? `Failed to load custom linter rules: ${failures[0][1]}` + : `Failed to load custom linter rules:\n${failures.map(([_, error], i) => `${i + 1}. ${error}`).join('\n')}` + + if (this.settings.hasShowDocumentCapability) { + this.connection.window.showWarningMessage(message, { title: OPEN_CONFIG_ACTION }).then(action => { + if (action?.title === OPEN_CONFIG_ACTION) { + const configPath = `${this.project.projectPath}/.herb.yml` + this.connection.window.showDocument({ uri: `file://${configPath}`, takeFocus: true }) + } + }) + } else { + this.connection.window.showWarningMessage(message) + } } async lintDocument(textDocument: TextDocument): Promise { @@ -40,6 +125,10 @@ export class LinterService { const projectConfig = this.settings.projectConfig if (!this.linter) { + await this.loadCustomRules() + + this.showCustomRuleWarnings() + const linterConfig = projectConfig?.config?.linter || { enabled: true, rules: {} } const config = Config.fromObject({ @@ -52,7 +141,9 @@ export class LinterService { } }, { projectPath: projectConfig?.projectPath || process.cwd() }) - this.linter = Linter.from(Herb, config) + const filteredRules = Linter.filterRulesByConfig(this.allRules, config.linter?.rules) + + this.linter = new Linter(Herb, filteredRules, config, this.allRules) } const content = textDocument.getText() @@ -64,8 +155,11 @@ export class LinterService { Position.create(offense.location.end.line - 1, offense.location.end.column), ) + const customRulePath = this.customRulePaths.get(offense.rule) const codeDescription: CodeDescription = { - href: `https://herb-tools.dev/linter/rules/${offense.rule}` + href: customRulePath + ? `file://${customRulePath}` + : `https://herb-tools.dev/linter/rules/${offense.rule}` } return { diff --git a/javascript/packages/language-server/src/server.ts b/javascript/packages/language-server/src/server.ts index b5e93fc8a..0404dcf12 100644 --- a/javascript/packages/language-server/src/server.ts +++ b/javascript/packages/language-server/src/server.ts @@ -90,6 +90,8 @@ export class Server { ...patterns, { globPattern: `**/.herb.yml` }, { globPattern: `**/**/.herb-lsp/config.json` }, + { globPattern: `**/.herb/rules/**/*.mjs` }, + { globPattern: `**/.herb/rewriters/**/*.mjs` }, ], }) }) @@ -117,9 +119,28 @@ export class Server { this.connection.onDidChangeWatchedFiles(async (params) => { for (const event of params.changes) { - if (event.uri.endsWith("/.herb.yml") || event.uri.endsWith("/.herb-lsp/config.json")) { + const isConfigChange = event.uri.endsWith("/.herb.yml") || event.uri.endsWith("/.herb-lsp/config.json") + const isCustomRuleChange = event.uri.includes("/.herb/rules/") + const isCustomRewriterChange = event.uri.includes("/.herb/rewriters/") + + if (isConfigChange) { await this.service.refreshConfig() + const documents = this.service.documentService.getAll() + await Promise.all(documents.map(document => + this.service.diagnostics.refreshDocument(document) + )) + } else if (isCustomRuleChange || isCustomRewriterChange) { + if (isCustomRuleChange) { + this.connection.console.log(`[Linter] Custom rule changed: ${event.uri}`) + this.service.linterService.rebuildLinter() + } + + if (isCustomRewriterChange) { + this.connection.console.log(`[Rewriter] Custom rewriter changed: ${event.uri}`) + await this.service.formattingService.refreshConfig(this.service.config) + } + const documents = this.service.documentService.getAll() await Promise.all(documents.map(document => this.service.diagnostics.refreshDocument(document) diff --git a/javascript/packages/language-server/src/service.ts b/javascript/packages/language-server/src/service.ts index 0ee567081..27679058a 100644 --- a/javascript/packages/language-server/src/service.ts +++ b/javascript/packages/language-server/src/service.ts @@ -37,7 +37,7 @@ export class Service { this.documentService = new DocumentService(this.connection) this.project = new Project(connection, this.settings.projectPath.replace("file://", "")) this.parserService = new ParserService() - this.linterService = new LinterService(this.settings) + this.linterService = new LinterService(this.connection, this.settings, this.project) this.formattingService = new FormattingService(this.connection, this.documentService.documents, this.project, this.settings) this.autofixService = new AutofixService(this.connection, this.config) this.configService = new ConfigService(this.project.projectPath) diff --git a/javascript/packages/linter/README.md b/javascript/packages/linter/README.md index 3b51ff3ae..08838fcd4 100644 --- a/javascript/packages/linter/README.md +++ b/javascript/packages/linter/README.md @@ -413,6 +413,115 @@ herb-lint --force app/views/excluded-file.html.erb When using `--force` on an excluded file, the linter will show a warning but proceed with linting. +### Custom Rules + +Create custom linter rules for project-specific requirements by placing ES module files (`.mjs`) in `.herb/rules/`: + +::: info File Extension +Custom rules must use the `.mjs` extension to avoid Node.js module type warnings. The `.mjs` extension explicitly marks files as ES modules. +::: + +::: code-group + + +```js [.herb/rules/no-inline-styles.mjs] +import { BaseRuleVisitor, ParserRule } from "@herb-tools/linter" + +class NoDivTagsVisitor extends BaseRuleVisitor { + visitHTMLOpenTagNode(node) { + if (!node.tag_name) return + if (node.tag_name.value !== "div") return + + this.addOffense( + `Avoid using \`
\` tags. Consider using semantic HTML elements like \`
\`, \`
\`, \`