diff --git a/README.md b/README.md index 3e82b1d..b54d515 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,23 @@ export default [ ]; ``` +Or configure it manually: + +```js +import bash from "@eslint/bash"; + +export default [ + { + files: ["**/*.sh"], + plugins: { bash }, + language: "bash/bash", + rules: { + "bash/no-backticks": "error", + }, + }, +]; +``` + ### Language options | Option | Values | Default | Description | @@ -43,10 +60,19 @@ export default [ plugins: { bash }, language: "bash/bash", languageOptions: { variant: "posix" }, + rules: { "bash/no-backticks": "error" }, }, ]; ``` +## Rules + +Each rule mirrors a well-known ShellCheck check. + +| Rule | ShellCheck | Description | Fixable | Recommended | +| ---------------------------------------------- | ---------- | ---------------------------------------- | ------- | ----------- | +| [`no-backticks`](./docs/rules/no-backticks.md) | SC2006 | Use `$(...)` instead of legacy backticks | ✅ | error | + ## Configuration comments Standard ESLint configuration comments work inside Bash files: diff --git a/docs/rules/no-backticks.md b/docs/rules/no-backticks.md new file mode 100644 index 0000000..3799aa7 --- /dev/null +++ b/docs/rules/no-backticks.md @@ -0,0 +1,57 @@ +# no-backticks + +Disallow legacy backtick command substitution in favor of `$(...)`. + +## Background + +Bash supports two syntaxes for command substitution: the legacy backtick form (`` `cmd` ``) and the POSIX `$(cmd)` form. The backtick form is harder to read and harder to get right: + +- Nesting requires escaping the inner backticks (`` `outer \`inner\`` ``). +- Backslashes inside backticks are processed differently than elsewhere, which makes quoting surprising. +- Backticks are easy to confuse with single quotes. + +`$(...)` nests without escaping and treats its contents like any other code. + +## Rule Details + +This rule warns on every backtick command substitution, including ones inside double-quoted strings. Nested backtick substitutions are reported individually. + +This rule is autofixable: it rewrites `` `cmd` `` as `$(cmd)`. Substitutions that contain a backslash or a nested backtick are reported but not fixed, because backslash escaping inside backticks differs from `$(...)` and a textual rewrite could change the meaning. + +Examples of **incorrect** code for this rule: + +```bash +# eslint bash/no-backticks: "error" + +today=`date +%F` + +echo "Now in `pwd`" + +for f in `find . -name '*.sh'`; do echo "$f"; done +``` + +Examples of **correct** code for this rule: + +```bash +# eslint bash/no-backticks: "error" + +today=$(date +%F) + +echo "Now in $(pwd)" + +parent=$(basename "$(dirname "$PWD")") + +echo 'Backticks in single quotes, like `this`, are just text' +``` + +## Options + +This rule has no options. + +## When Not to Use It + +If your scripts must run on very old Bourne shells that predate POSIX `$(...)` support, you can safely disable this rule. + +## Prior Art + +- [SC2006](https://www.shellcheck.net/wiki/SC2006) diff --git a/src/index.spec.ts b/src/index.spec.ts index e8af10c..382d5f2 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -24,7 +24,20 @@ describe("plugin", () => { it("should expose all rules", () => { const ruleIds = Object.keys(plugin.rules); - expect(ruleIds.sort()).toEqual([]); + expect(ruleIds.sort()).toEqual(["no-backticks"]); + }); + + it("should give every rule meta docs and messages", () => { + for (const [ruleId, rule] of Object.entries(plugin.rules)) { + expect(rule.meta?.docs?.description, ruleId).toBeTruthy(); + expect(rule.meta?.docs?.recommended, ruleId).toBe(true); + expect(rule.meta?.messages, ruleId).toBeTruthy(); + expect(rule.meta?.schema, ruleId).toBeDefined(); + expect(rule.meta?.docs?.url, ruleId).toBe( + `https://github.com/eslint/bash/blob/main/docs/rules/${ruleId}.md`, + ); + expect(typeof rule.create, ruleId).toBe("function"); + } }); describe("recommended config", () => { diff --git a/src/index.ts b/src/index.ts index daf01c4..284e201 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,8 +4,11 @@ */ import { BashLanguage } from "./languages/bash-language.js"; +import noBackticks from "./rules/no-backticks.js"; -const rules = {}; +const rules = { + "no-backticks": noBackticks, +}; const plugin = { meta: { @@ -23,7 +26,9 @@ const plugin = { files: ["**/*.sh", "**/*.bash"], language: "bash/bash", plugins: {}, - rules: {}, + rules: { + "bash/no-backticks": "error", + }, }, }, }; diff --git a/src/rules/no-backticks.spec.ts b/src/rules/no-backticks.spec.ts new file mode 100644 index 0000000..4330efe --- /dev/null +++ b/src/rules/no-backticks.spec.ts @@ -0,0 +1,62 @@ +/** + * @fileoverview Tests for the no-backticks rule. + */ + +import { RuleTester } from "eslint"; +import { BashLanguage } from "../languages/bash-language.js"; +import rule from "./no-backticks.js"; + +const ruleTester = new RuleTester({ + plugins: { + bash: { + meta: { namespace: "shell" }, + languages: { bash: new BashLanguage() }, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- plugin shape is validated by ESLint at runtime. + } as any, + language: "bash/bash", +}); + +ruleTester.run("no-backticks", rule as never, { + valid: [ + "echo $(pwd)", + 'echo "$(date)"', + "x=$(ls | wc -l)", + "echo plain text", + "echo 'literal `backticks` in single quotes are text? no...'", + ], + invalid: [ + { + code: "echo `pwd`", + output: "echo $(pwd)", + errors: [ + { + messageId: "useDollarParen", + line: 1, + column: 6, + endColumn: 11, + }, + ], + }, + { + code: "x=`date +%s`", + output: "x=$(date +%s)", + errors: [{ messageId: "useDollarParen" }], + }, + { + code: 'echo "today is `date`"', + output: 'echo "today is $(date)"', + errors: [{ messageId: "useDollarParen" }], + }, + { + // Backslashes inside backticks change meaning, so no autofix. + // Both the outer and the nested substitution are reported. + code: "echo `echo \\`x\\``", + output: null, + errors: [ + { messageId: "useDollarParen" }, + { messageId: "useDollarParen" }, + ], + }, + ], +}); diff --git a/src/rules/no-backticks.ts b/src/rules/no-backticks.ts new file mode 100644 index 0000000..a8b397a --- /dev/null +++ b/src/rules/no-backticks.ts @@ -0,0 +1,60 @@ +/** + * @fileoverview Rule to disallow legacy backtick command substitution. + * Mirrors ShellCheck SC2006. + */ + +import type { BashRuleDefinition } from "../types.js"; + +const rule: BashRuleDefinition<{ MessageIds: "useDollarParen" }> = { + meta: { + type: "suggestion", + languages: ["shell/bash"], + docs: { + description: + "Disallow legacy backtick command substitution in favor of `$(...)`", + recommended: true, + dialects: ["Bash", "POSIX sh", "mksh"], + url: "https://github.com/eslint/bash/blob/main/docs/rules/no-backticks.md", + }, + fixable: "code", + schema: [], + messages: { + useDollarParen: + "Use $(...) notation instead of legacy backticks. (ShellCheck SC2006)", + }, + }, + + create(context) { + const { sourceCode } = context; + + return { + CommandSubstitution(node) { + if (!node.backquotes) { + return; + } + + const [start, end] = sourceCode.getRange(node); + const inner = sourceCode.text.slice(start + 1, end - 1); + + context.report({ + node, + messageId: "useDollarParen", + + // Escapes behave differently inside backticks, so only + // fix substitutions without backslashes or nesting. + fix: /[\\`]/u.test(inner) + ? undefined + : fixer => [ + fixer.replaceTextRange( + [start, start + 1], + "$(", + ), + fixer.replaceTextRange([end - 1, end], ")"), + ], + }); + }, + }; + }, +}; + +export default rule; diff --git a/tests/autofix.test.ts b/tests/autofix.test.ts new file mode 100644 index 0000000..4827220 --- /dev/null +++ b/tests/autofix.test.ts @@ -0,0 +1,36 @@ +/** + * @fileoverview Integration tests for autofixing through the Linter API. + */ + +import { describe, expect, it } from "vitest"; +import { Linter } from "eslint"; +import bash from "../src/index.js"; + +function fix(code: string, rules: Record): Linter.FixReport { + const linter = new Linter(); + + return linter.verifyAndFix( + code, + [ + { + files: ["**/*.sh"], + plugins: { bash }, + language: "bash/bash", + rules: rules as never, + }, + ] as never, + "script.sh", + ); +} + +describe("autofix", () => { + it("should fix backticks to $()", () => { + const result = fix("echo `pwd` `date`\n", { + "bash/no-backticks": "error", + }); + + expect(result.fixed).toBe(true); + expect(result.output).toBe("echo $(pwd) $(date)\n"); + expect(result.messages).toEqual([]); + }); +}); diff --git a/tests/rule-docs.test.ts b/tests/rule-docs.test.ts new file mode 100644 index 0000000..467c095 --- /dev/null +++ b/tests/rule-docs.test.ts @@ -0,0 +1,121 @@ +/** + * @fileoverview Verifies that every rule has documentation in docs/rules + * and that the examples in it behave as documented. + */ + +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; +import { Linter } from "eslint"; +import bash from "../src/index.js"; + +interface Example { + kind: "incorrect" | "correct"; + code: string; +} + +const CONFIG_COMMENT = /^# eslint bash\//u; + +function readDoc(ruleId: string): string { + return readFileSync( + new URL(`../docs/rules/${ruleId}.md`, import.meta.url), + "utf8", + ).replace(/\r\n/gu, "\n"); +} + +/** + * Finds each "Examples of **incorrect|correct** code" heading and the + * bash code block that follows it. + */ +function getExamples(doc: string): Example[] { + const pattern = + /Examples of \*\*(incorrect|correct)\*\* code[^\n]*\n+```bash\n([\s\S]*?)```/gu; + + return [...doc.matchAll(pattern)].map(match => ({ + kind: match[1] as Example["kind"], + code: match[2] as string, + })); +} + +function lint(code: string): Linter.LintMessage[] { + return new Linter().verify( + code, + [ + { files: ["**/*.sh"], plugins: { bash }, language: "bash/bash" }, + ] as never, + "example.sh", + ); +} + +describe("rule documentation", () => { + for (const [ruleId, rule] of Object.entries(bash.rules)) { + describe(ruleId, () => { + const doc = readDoc(ruleId); + const examples = getExamples(doc); + + it("should start with the rule name and description", () => { + const [title, , description] = doc.split("\n"); + + expect(title).toBe(`# ${ruleId}`); + expect(description).toBe(`${rule.meta?.docs?.description}.`); + }); + + it("should have the standard sections", () => { + for (const heading of [ + "## Rule Details", + "## Options", + "## When Not to Use It", + "## Prior Art", + ]) { + expect(doc).toContain(`\n${heading}\n`); + } + }); + + it("should have incorrect and correct examples", () => { + const kinds = new Set(examples.map(example => example.kind)); + + expect(kinds).toEqual(new Set(["incorrect", "correct"])); + }); + + it("should report every incorrect example", () => { + for (const example of examples.filter( + e => e.kind === "incorrect", + )) { + const lines = example.code.split("\n"); + const config = lines.find(line => + CONFIG_COMMENT.test(line), + ); + + expect(config, "config comment").toBeDefined(); + + const snippets = lines + .filter(line => line !== config) + .join("\n") + .split(/\n\s*\n/u) + .filter(snippet => snippet.trim() !== ""); + + for (const snippet of snippets) { + const messages = lint(`${config}\n${snippet}\n`); + + expect( + messages.filter(m => m.ruleId === `bash/${ruleId}`), + snippet, + ).not.toHaveLength(0); + expect( + messages.filter(m => m.fatal), + snippet, + ).toHaveLength(0); + } + } + }); + + it("should not report correct examples", () => { + for (const example of examples.filter( + e => e.kind === "correct", + )) { + expect(example.code).toMatch(CONFIG_COMMENT); + expect(lint(example.code), example.code).toEqual([]); + } + }); + }); + } +});