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/formatter/test/cli/__snapshots__/rewriters.test.ts.snap b/javascript/packages/formatter/test/cli/__snapshots__/rewriters.test.ts.snap new file mode 100644 index 000000000..ccf53a111 --- /dev/null +++ b/javascript/packages/formatter/test/cli/__snapshots__/rewriters.test.ts.snap @@ -0,0 +1,77 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`CLI > Rewriters > should actually apply Tailwind class sorting 1`] = ` +"✓ Using Herb config file at test-rewriters/.herb.yml +⚠️ Experimental Preview: The formatter is in early development. Please report any unexpected behavior or bugs to https://github.com/marcoroth/herb/issues/new?template=formatting-issue.md + + +Using 1 pre-format rewriter: + • tailwind-class-sorter (built-in) + +" +`; + +exports[`CLI > Rewriters > should format from stdin with rewriters configured 1`] = ` +"✓ Using Herb config file at test-rewriters/.herb.yml +⚠️ Experimental Preview: The formatter is in early development. Please report any unexpected behavior or bugs to https://github.com/marcoroth/herb/issues/new?template=formatting-issue.md + + +Using 1 pre-format rewriter: + • tailwind-class-sorter (built-in) + +" +`; + +exports[`CLI > Rewriters > should format from stdin with rewriters configured 2`] = ` +"
+" +`; + +exports[`CLI > Rewriters > should handle both pre and post rewriters 1`] = ` +"✓ Using Herb config file at test-rewriters/.herb.yml +⚠️ Experimental Preview: The formatter is in early development. Please report any unexpected behavior or bugs to https://github.com/marcoroth/herb/issues/new?template=formatting-issue.md + + +Using 1 pre-format rewriter: + • tailwind-class-sorter (built-in) + +" +`; + +exports[`CLI > Rewriters > should not show rewriter info when no rewriters are configured 1`] = ` +"✓ Using Herb config file at test-rewriters/.herb.yml +⚠️ Experimental Preview: The formatter is in early development. Please report any unexpected behavior or bugs to https://github.com/marcoroth/herb/issues/new?template=formatting-issue.md + +" +`; + +exports[`CLI > Rewriters > should show multiple rewriters on stderr 1`] = ` +"✓ Using Herb config file at test-rewriters/.herb.yml +⚠️ Experimental Preview: The formatter is in early development. Please report any unexpected behavior or bugs to https://github.com/marcoroth/herb/issues/new?template=formatting-issue.md + + +Using 1 pre-format rewriter: + • tailwind-class-sorter (built-in) + +" +`; + +exports[`CLI > Rewriters > should show rewriter info on stderr when pre-format rewriters are configured 1`] = ` +"✓ Using Herb config file at test-rewriters/.herb.yml +⚠️ Experimental Preview: The formatter is in early development. Please report any unexpected behavior or bugs to https://github.com/marcoroth/herb/issues/new?template=formatting-issue.md + + +Using 1 pre-format rewriter: + • tailwind-class-sorter (built-in) + +" +`; + +exports[`CLI > Rewriters > should show warning for unknown rewriters on stderr 1`] = ` +"✓ Using Herb config file at test-rewriters/.herb.yml +⚠️ Experimental Preview: The formatter is in early development. Please report any unexpected behavior or bugs to https://github.com/marcoroth/herb/issues/new?template=formatting-issue.md + +⚠️ Pre-format rewriter "non-existent-rewriter" not found. Skipping. + +" +`; diff --git a/javascript/packages/formatter/test/cli/rewriters.test.ts b/javascript/packages/formatter/test/cli/rewriters.test.ts index aa6992921..1ec0c9d97 100644 --- a/javascript/packages/formatter/test/cli/rewriters.test.ts +++ b/javascript/packages/formatter/test/cli/rewriters.test.ts @@ -5,6 +5,15 @@ import { join } from "path" import { execBinary, expectExitCode, } from "./cli-helpers" +/** + * Normalizes stderr output by replacing absolute paths with relative paths + * This makes snapshots consistent across different environments (local vs CI) + */ +function normalizeStderr(stderr: string): string { + const cwd = process.cwd() + return stderr.replace(new RegExp(cwd + '/', 'g'), '') +} + describe("CLI", () => { describe("Rewriters", () => { const testDir = "test-rewriters" @@ -35,7 +44,7 @@ describe("CLI", () => { const result = await execBinary([testFile]) expectExitCode(result, 0) - expect(result.stderr).toContain("Using 1 pre-format rewriter: tailwind-class-sorter") + expect(normalizeStderr(result.stderr)).toMatchSnapshot() }) it("should show multiple rewriters on stderr", async () => { @@ -55,7 +64,7 @@ describe("CLI", () => { const result = await execBinary([testFile]) expectExitCode(result, 0) - expect(result.stderr).toContain("Using 1 pre-format rewriter: tailwind-class-sorter") + expect(normalizeStderr(result.stderr)).toMatchSnapshot() }) it("should actually apply Tailwind class sorting", async () => { @@ -77,7 +86,7 @@ describe("CLI", () => { const result = await execBinary([testFile]) expectExitCode(result, 0) - expect(result.stdout).toContain("Formatted: test-rewriters/test.html.erb") + expect(normalizeStderr(result.stderr)).toMatchSnapshot() const formattedContent = await readFile(testFile, 'utf-8') expect(formattedContent).toBe(`
\n`) @@ -98,8 +107,8 @@ describe("CLI", () => { const result = await execBinary(["--config-file", configPath], input) expectExitCode(result, 0) - expect(result.stderr).toContain(`Using 1 pre-format rewriter: tailwind-class-sorter`) - expect(result.stdout).toContain(`class="bg-blue-500 px-4 text-white"`) + expect(normalizeStderr(result.stderr)).toMatchSnapshot() + expect(result.stdout).toMatchSnapshot() }) it("should show warning for unknown rewriters on stderr", async () => { @@ -119,9 +128,7 @@ describe("CLI", () => { const result = await execBinary([testFile]) expectExitCode(result, 0) - expect(result.stderr).toContain("⚠️") - expect(result.stderr).toContain("non-existent-rewriter") - expect(result.stderr).toContain("not found") + expect(normalizeStderr(result.stderr)).toMatchSnapshot() }) it("should not show rewriter info when no rewriters are configured", async () => { @@ -138,9 +145,7 @@ describe("CLI", () => { const result = await execBinary([testFile]) expectExitCode(result, 0) - expect(result.stderr).not.toContain("pre-format rewriter") - expect(result.stderr).not.toContain("post-format rewriter") - expect(result.stderr).toContain("⚠️ Experimental Preview") + expect(normalizeStderr(result.stderr)).toMatchSnapshot() }) it("should handle both pre and post rewriters", async () => { @@ -160,8 +165,7 @@ describe("CLI", () => { const result = await execBinary([testFile]) expectExitCode(result, 0) - expect(result.stderr).toContain("Using 1 pre-format rewriter: tailwind-class-sorter") - expect(result.stderr).not.toContain("post-format") + expect(normalizeStderr(result.stderr)).toMatchSnapshot() }) }) }) diff --git a/javascript/packages/formatter/test/rewriters/fixtures/uppercase-tags.js b/javascript/packages/formatter/test/rewriters/fixtures/uppercase-tags.js index 357c8557c..cc9e0b89f 100644 --- a/javascript/packages/formatter/test/rewriters/fixtures/uppercase-tags.js +++ b/javascript/packages/formatter/test/rewriters/fixtures/uppercase-tags.js @@ -30,7 +30,7 @@ class UppercaseTagsVisitor extends Visitor { } } -export class UppercaseTagsRewriter extends ASTRewriter { +export default class UppercaseTagsRewriter extends ASTRewriter { get name() { return "uppercase-tags" } diff --git a/javascript/packages/language-server/src/linter_service.ts b/javascript/packages/language-server/src/linter_service.ts index 0b19b4efa..775e3428f 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 { loadCustomRules as loadCustomRulesFromFs } 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,77 @@ 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 + + try { + const { rules: customRules, ruleInfo, warnings: duplicateWarnings } = await loadCustomRulesFromFs({ baseDir, silent: true }) + + 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 +124,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 +140,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 +154,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/language-server/test/linter_service.test.ts b/javascript/packages/language-server/test/linter_service.test.ts index 4a32db687..8cb4a152b 100644 --- a/javascript/packages/language-server/test/linter_service.test.ts +++ b/javascript/packages/language-server/test/linter_service.test.ts @@ -4,6 +4,7 @@ import { TextDocument } from "vscode-languageserver-textdocument" import { LinterService } from "../src/linter_service" import { Settings } from "../src/settings" +import { Project } from "../src/project" import { Herb } from "@herb-tools/node-wasm" import type { Connection, InitializeParams } from "vscode-languageserver/node" @@ -16,6 +17,11 @@ describe("LinterService", () => { const mockConnection = { workspace: { getConfiguration: vi.fn() + }, + console: { + log: vi.fn(), + warn: vi.fn(), + error: vi.fn() } } as unknown as Connection @@ -26,6 +32,10 @@ describe("LinterService", () => { workspaceFolders: null } + const mockProject = { + projectPath: process.cwd() + } as Project + const createTestDocument = (content: string) => { return TextDocument.create("file:///test.html.erb", "erb", 1, content) } @@ -35,7 +45,7 @@ describe("LinterService", () => { const settings = new Settings(mockParams, mockConnection) settings.getDocumentSettings = vi.fn().mockResolvedValue(null) - const linterService = new LinterService(settings) + const linterService = new LinterService(mockConnection, settings, mockProject) const textDocument = createTestDocument("
Test
\n") const result = await linterService.lintDocument(textDocument) @@ -51,7 +61,7 @@ describe("LinterService", () => { // linter is undefined }) - const linterService = new LinterService(settings) + const linterService = new LinterService(mockConnection, settings, mockProject) const textDocument = createTestDocument("
Test
\n") const result = await linterService.lintDocument(textDocument) @@ -66,7 +76,7 @@ describe("LinterService", () => { linter: { enabled: false } }) - const linterService = new LinterService(settings) + const linterService = new LinterService(mockConnection, settings, mockProject) const textDocument = createTestDocument("
Test
\n") const result = await linterService.lintDocument(textDocument) @@ -80,7 +90,7 @@ describe("LinterService", () => { linter: { enabled: true } }) - const linterService = new LinterService(settings) + const linterService = new LinterService(mockConnection, settings, mockProject) const textDocument = createTestDocument("
Hello
") const result = await linterService.lintDocument(textDocument) @@ -92,7 +102,7 @@ describe("LinterService", () => { const settings = new Settings(mockParams, mockConnection) settings.hasConfigurationCapability = false - const linterService = new LinterService(settings) + const linterService = new LinterService(mockConnection, settings, mockProject) const textDocument = createTestDocument("
Test
") const result = await linterService.lintDocument(textDocument) @@ -106,7 +116,7 @@ describe("LinterService", () => { linter: { enabled: true } }) - const linterService = new LinterService(settings) + const linterService = new LinterService(mockConnection, settings, mockProject) const textDocument = createTestDocument("

Content

") const result = await linterService.lintDocument(textDocument) @@ -142,7 +152,7 @@ describe("LinterService", () => { applySeverityOverrides: (offenses) => offenses } as any - const linterService = new LinterService(settings) + const linterService = new LinterService(mockConnection, settings, mockProject) const textDocument = createTestDocument("
Content
") const result = await linterService.lintDocument(textDocument) 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 \`
\`, \`
\`, \`