From cb7e1f52498de099c801d1bdf01643af4b06b991 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 7 Nov 2025 17:06:23 +0100 Subject: [PATCH 1/2] Language Server: Add `Unnecessary` diagnostic for `ERBCaseNode` children --- .../language-server/src/diagnostics.ts | 65 ++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/javascript/packages/language-server/src/diagnostics.ts b/javascript/packages/language-server/src/diagnostics.ts index 360547e0b..d7ff79866 100644 --- a/javascript/packages/language-server/src/diagnostics.ts +++ b/javascript/packages/language-server/src/diagnostics.ts @@ -1,5 +1,15 @@ import { TextDocument } from "vscode-languageserver-textdocument" -import { Connection, Diagnostic } from "vscode-languageserver/node" +import { Connection, Diagnostic, DiagnosticSeverity, DiagnosticTag } from "vscode-languageserver/node" +import { Visitor } from "@herb-tools/core" + +import type { + Node, + ERBCaseNode, + ERBCaseMatchNode, + HTMLTextNode, +} from "@herb-tools/core" + +import { isHTMLTextNode } from "@herb-tools/core" import { ParserService } from "./parser_service" import { LinterService } from "./linter_service" @@ -36,10 +46,12 @@ export class Diagnostics { } else { const parseResult = this.parserService.parseDocument(textDocument) const lintResult = await this.linterService.lintDocument(textDocument) + const unreachableCodeDiagnostics = this.getUnreachableCodeDiagnostics(parseResult.document) allDiagnostics = [ ...parseResult.diagnostics, ...lintResult.diagnostics, + ...unreachableCodeDiagnostics, ] } @@ -47,6 +59,12 @@ export class Diagnostics { this.sendDiagnosticsFor(textDocument) } + private getUnreachableCodeDiagnostics(document: Node): Diagnostic[] { + const collector = new UnreachableCodeCollector() + collector.visit(document) + return collector.diagnostics + } + async refreshDocument(document: TextDocument) { await this.validate(document) } @@ -67,3 +85,48 @@ export class Diagnostics { this.diagnostics.delete(textDocument) } } + +export class UnreachableCodeCollector extends Visitor { + diagnostics: Diagnostic[] = [] + + visitERBCaseNode(node: ERBCaseNode): void { + this.checkUnreachableChildren(node.children) + this.visitChildNodes(node) + } + + visitERBCaseMatchNode(node: ERBCaseMatchNode): void { + this.checkUnreachableChildren(node.children) + this.visitChildNodes(node) + } + + private checkUnreachableChildren(children: Node[]): void { + for (const child of children) { + if (isHTMLTextNode(child) && child.content.trim() === "") { + continue + } + + const diagnostic: Diagnostic = { + range: { + start: { + line: this.toZeroBased(child.location.start.line), + character: child.location.start.column + }, + end: { + line: this.toZeroBased(child.location.end.line), + character: child.location.end.column + } + }, + message: "Unreachable code: content between case and when/in is never executed", + severity: DiagnosticSeverity.Hint, + tags: [DiagnosticTag.Unnecessary], + source: "Herb Language Server" + } + + this.diagnostics.push(diagnostic) + } + } + + private toZeroBased(line: number): number { + return line - 1 + } +} From c3efaf6c7e774e3efffa74a2bd7a35a911341e78 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 7 Nov 2025 23:15:24 +0100 Subject: [PATCH 2/2] Add tests --- .../test/unreachable_code.test.ts | 130 ++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 javascript/packages/language-server/test/unreachable_code.test.ts diff --git a/javascript/packages/language-server/test/unreachable_code.test.ts b/javascript/packages/language-server/test/unreachable_code.test.ts new file mode 100644 index 000000000..19dfca278 --- /dev/null +++ b/javascript/packages/language-server/test/unreachable_code.test.ts @@ -0,0 +1,130 @@ +import dedent from "dedent" + +import { describe, it, expect, beforeAll } from "vitest" +import { DiagnosticSeverity, DiagnosticTag } from "vscode-languageserver/node" + +import { UnreachableCodeCollector } from "../src/diagnostics" +import { Herb } from "@herb-tools/node-wasm" + +describe("Unreachable Code Diagnostics", () => { + beforeAll(async () => { + await Herb.load() + }) + + describe("ERB case statements", () => { + it("detects unreachable code between case and when", () => { + const content = dedent` + <% case abc %> + something here that is not renderable + <% when String %> + actual string + <% end %> + ` + + const parseResult = Herb.parse(content) + const collector = new UnreachableCodeCollector() + collector.visit(parseResult.value) + + expect(collector.diagnostics.length).toBeGreaterThan(0) + + const diagnostic = collector.diagnostics[0] + expect(diagnostic.message).toContain("Unreachable code") + expect(diagnostic.severity).toBe(DiagnosticSeverity.Hint) + expect(diagnostic.tags).toContain(DiagnosticTag.Unnecessary) + expect(diagnostic.source).toBe("Herb Language Server") + }) + + it("detects unreachable code in case/in statements", () => { + const content = dedent` + <% case abc %> + <% in String %> + actual string + <% end %> + ` + + const parseResult = Herb.parse(content) + const collector = new UnreachableCodeCollector() + collector.visit(parseResult.value) + + expect(collector.diagnostics.length).toBe(0) + }) + + it("detects unreachable HTML content between case and when", () => { + const content = dedent` + <% case status %> +
This will never render
+

Neither will this

+ <% when "active" %> +

Active

+ <% when "inactive" %> +

Inactive

+ <% end %> + ` + + const parseResult = Herb.parse(content) + const collector = new UnreachableCodeCollector() + collector.visit(parseResult.value) + + expect(collector.diagnostics.length).toBeGreaterThan(0) + }) + + it("does not report diagnostics for case without unreachable children", () => { + const content = dedent` + <% case status %> + <% when "active" %> +

Active

+ <% when "inactive" %> +

Inactive

+ <% else %> +

Unknown

+ <% end %> + ` + + const parseResult = Herb.parse(content) + const collector = new UnreachableCodeCollector() + collector.visit(parseResult.value) + + expect(collector.diagnostics.length).toBe(0) + }) + + it("detects unreachable code with mixed content", () => { + const content = dedent` + <% case type %> + Some text + <%= variable %> + HTML + <% when :foo %> +

Foo

+ <% end %> + ` + + const parseResult = Herb.parse(content) + const collector = new UnreachableCodeCollector() + collector.visit(parseResult.value) + + expect(collector.diagnostics.length).toBeGreaterThan(0) + }) + }) + + describe("nested case statements", () => { + it("detects unreachable code in nested case statements", () => { + const content = dedent` + <% case outer %> + unreachable outer + <% when "a" %> + <% case inner %> + unreachable inner + <% when "b" %> + reachable + <% end %> + <% end %> + ` + + const parseResult = Herb.parse(content) + const collector = new UnreachableCodeCollector() + collector.visit(parseResult.value) + + expect(collector.diagnostics.length).toBeGreaterThanOrEqual(2) + }) + }) +})