diff --git a/README.md b/README.md index cedfb4b..d111353 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,23 @@ export default [ ]; ``` +Or configure it manually: + +```js +import shell from "@eslint/shell"; + +export default [ + { + files: ["**/*.sh"], + plugins: { shell }, + language: "shell/bash", + rules: { + "shell/no-backticks": "error", + }, + }, +]; +``` + ### Languages The plugin provides one language per shell dialect: @@ -46,10 +63,19 @@ export default [ files: ["**/*.sh"], plugins: { shell }, language: "shell/posix", + rules: { "shell/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 shell scripts: diff --git a/docs/rules/no-backticks.md b/docs/rules/no-backticks.md new file mode 100644 index 0000000..d80402f --- /dev/null +++ b/docs/rules/no-backticks.md @@ -0,0 +1,57 @@ +# no-backticks + +Disallow legacy backtick command substitution in favor of `$(...)`. + +## Background + +Shells support 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 shell/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 shell/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 eb5bf79..8af1c9a 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -42,7 +42,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/shell/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 5ec2fb1..dd058c4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,8 +8,11 @@ // Plugin //------------------------------------------------------------------------------ import { ShellLanguage } from "./languages/shell-language.js"; +import noBackticks from "./rules/no-backticks.js"; -const rules = {}; +const rules = { + "no-backticks": noBackticks, +}; const plugin = { meta: { @@ -29,7 +32,9 @@ const plugin = { files: ["**/*.sh", "**/*.bash"], language: "shell/bash", plugins: {}, - rules: {}, + rules: { + "shell/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..8c6cf98 --- /dev/null +++ b/src/rules/no-backticks.spec.ts @@ -0,0 +1,61 @@ +/** + * @fileoverview Tests for the no-backticks rule. + */ + +import { RuleTester } from "eslint"; +import { ShellLanguage } from "../languages/shell-language.js"; +import rule from "./no-backticks.js"; + +const ruleTester = new RuleTester({ + plugins: { + shell: { + languages: { bash: new ShellLanguage() }, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- plugin shape is validated by ESLint at runtime. + } as any, + language: "shell/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..c14466b --- /dev/null +++ b/src/rules/no-backticks.ts @@ -0,0 +1,58 @@ +/** + * @fileoverview Rule to disallow legacy backtick command substitution. + * Mirrors ShellCheck SC2006. + */ + +import type { ShellRuleDefinition } from "../types.js"; + +const rule: ShellRuleDefinition<{ MessageIds: "useDollarParen" }> = { + meta: { + type: "suggestion", + docs: { + description: + "Disallow legacy backtick command substitution in favor of `$(...)`", + recommended: true, + url: "https://github.com/eslint/shell/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..0b533bf --- /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 shell from "../src/index.js"; + +function fix(code: string, rules: Record): Linter.FixReport { + const linter = new Linter(); + + return linter.verifyAndFix( + code, + [ + { + files: ["**/*.sh"], + plugins: { shell }, + language: "shell/bash", + rules: rules as never, + }, + ] as never, + "script.sh", + ); +} + +describe("autofix", () => { + it("should fix backticks to $()", () => { + const result = fix("echo `pwd` `date`\n", { + "shell/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..5d3d9a9 --- /dev/null +++ b/tests/rule-docs.test.ts @@ -0,0 +1,123 @@ +/** + * @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 shell from "../src/index.js"; + +interface Example { + kind: "incorrect" | "correct"; + code: string; +} + +const CONFIG_COMMENT = /^# eslint shell\//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: { shell }, language: "shell/bash" }, + ] as never, + "example.sh", + ); +} + +describe("rule documentation", () => { + for (const [ruleId, rule] of Object.entries(shell.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 === `shell/${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([]); + } + }); + }); + } +});