diff --git a/README.md b/README.md index 834b11e..bcb9ddc 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,7 @@ Each rule mirrors a well-known ShellCheck check. | [`no-unquoted-expansions`](./docs/rules/no-unquoted-expansions.md) | SC2086/46 | Quote expansions subject to word splitting and globbing | ✅ | error | | [`no-useless-cat`](./docs/rules/no-useless-cat.md) | SC2002 | Don't pipe from a single-file `cat` | | error | | [`no-useless-echo`](./docs/rules/no-useless-echo.md) | SC2116 | Disallow `$(echo ...)` | | error | +| [`require-cd-guard`](./docs/rules/require-cd-guard.md) | SC2164 | Handle `cd` failure with `\|\| exit` (has suggestions) | | error | | [`require-read-r`](./docs/rules/require-read-r.md) | SC2162 | Use `read -r` so backslashes aren't mangled | ✅ | error | ## Configuration comments diff --git a/docs/rules/require-cd-guard.md b/docs/rules/require-cd-guard.md new file mode 100644 index 0000000..31ae2f1 --- /dev/null +++ b/docs/rules/require-cd-guard.md @@ -0,0 +1,62 @@ +# require-cd-guard + +Require `cd` failures to be handled, e.g. `cd ... || exit`. + +## Background + +If `cd` fails, for example because the directory doesn't exist, the script keeps running in the directory it was already in. Every command after that runs in the wrong place, which can be destructive: `cd "$build_dir"; rm -rf *` deletes the wrong files when the `cd` fails. Checking the result of `cd` stops the script before that happens. + +## Rule Details + +This rule warns about `cd` commands whose exit status isn't checked. The status counts as checked when `cd` is: + +- the left side of `||` or `&&`, as in `cd dir || exit` or `cd dir && make`; +- the condition of an `if`, `while`, or `until`; +- negated with `!`. + +A `cd` on the right side of `&&` is still reported, because its own failure isn't handled. + +The rule doesn't take `set -e` into account, so scripts that rely on it still get warnings. It also reports `cd` inside subshells, such as `(cd dir; make)`. + +This rule provides a suggestion, not an autofix, to append `|| exit`. It's a suggestion because `return` is often the better choice inside functions. The suggestion isn't offered when the `cd` has redirects or runs in the background. + +Examples of **incorrect** code for this rule: + +```bash +# eslint shell/require-cd-guard: "error" + +cd /tmp + +cd "$build_dir" +make + +test -d build && cd build + +(cd src; make) +``` + +Examples of **correct** code for this rule: + +```bash +# eslint shell/require-cd-guard: "error" + +cd /tmp || exit + +cd "$build_dir" || return 1 + +cd build && make + +if cd build; then make; fi +``` + +## Options + +This rule has no options. + +## When Not to Use It + +If your scripts use `set -e` so that a failed `cd` already stops execution, and you rely on that behavior, you can disable this rule. + +## Prior Art + +- [SC2164](https://www.shellcheck.net/wiki/SC2164) diff --git a/src/index.spec.ts b/src/index.spec.ts index 93b87ab..2bdd813 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -41,6 +41,7 @@ describe("plugin", () => { "no-unquoted-expansions", "no-useless-cat", "no-useless-echo", + "require-cd-guard", "require-read-r", ]); }); diff --git a/src/index.ts b/src/index.ts index fd28790..9a425e8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -10,6 +10,7 @@ import noLsIteration from "./rules/no-ls-iteration.js"; import noUnquotedExpansions from "./rules/no-unquoted-expansions.js"; import noUselessCat from "./rules/no-useless-cat.js"; import noUselessEcho from "./rules/no-useless-echo.js"; +import requireCdGuard from "./rules/require-cd-guard.js"; import requireReadR from "./rules/require-read-r.js"; const rules = { @@ -19,6 +20,7 @@ const rules = { "no-unquoted-expansions": noUnquotedExpansions, "no-useless-cat": noUselessCat, "no-useless-echo": noUselessEcho, + "require-cd-guard": requireCdGuard, "require-read-r": requireReadR, }; @@ -47,6 +49,7 @@ const plugin = { "shell/no-unquoted-expansions": "error", "shell/no-useless-cat": "error", "shell/no-useless-echo": "error", + "shell/require-cd-guard": "error", "shell/require-read-r": "error", }, }, diff --git a/src/rules/require-cd-guard.spec.ts b/src/rules/require-cd-guard.spec.ts new file mode 100644 index 0000000..edae35a --- /dev/null +++ b/src/rules/require-cd-guard.spec.ts @@ -0,0 +1,79 @@ +/** + * @fileoverview Tests for the require-cd-guard rule. + */ + +import { RuleTester } from "eslint"; +import { ShellLanguage } from "../languages/shell-language.js"; +import rule from "./require-cd-guard.js"; + +const ruleTester = new RuleTester({ + plugins: { + shell: { + meta: { namespace: "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("require-cd-guard", rule as never, { + valid: [ + "cd /tmp || exit", + "cd /tmp || return", + "cd /tmp && make", + "precheck && cd /tmp && make", + "precheck || cd /tmp || make", + "if cd /tmp; then make; fi", + "while cd /tmp; do break; done", + "! cd /tmp", + // Not cd + "cdx /tmp", + ], + invalid: [ + { + code: "cd /tmp", + errors: [ + { + messageId: "uncheckedCd", + suggestions: [ + { + messageId: "addGuard", + output: "cd /tmp || exit", + }, + ], + }, + ], + }, + { + code: 'cd "$dir"\nmake', + errors: [ + { + messageId: "uncheckedCd", + line: 1, + suggestions: [ + { + messageId: "addGuard", + output: 'cd "$dir" || exit\nmake', + }, + ], + }, + ], + }, + { + // Right side of && is still unchecked + code: "test -d /tmp && cd /tmp", + errors: [ + { + messageId: "uncheckedCd", + suggestions: [ + { + messageId: "addGuard", + output: "test -d /tmp && cd /tmp || exit", + }, + ], + }, + ], + }, + ], +}); diff --git a/src/rules/require-cd-guard.ts b/src/rules/require-cd-guard.ts new file mode 100644 index 0000000..cd9572f --- /dev/null +++ b/src/rules/require-cd-guard.ts @@ -0,0 +1,92 @@ +/** + * @fileoverview Rule to require handling `cd` failure. + * Mirrors ShellCheck SC2164. + */ + +import { getCommandName } from "./utils.js"; +import type { ShellRuleDefinition, StatementNode } from "../types.js"; + +const rule: ShellRuleDefinition<{ + MessageIds: "uncheckedCd" | "addGuard"; +}> = { + meta: { + type: "problem", + languages: ["shell/bash", "shell/posix", "shell/mksh"], + docs: { + description: + "Require `cd` failures to be handled, e.g. `cd ... || exit`", + recommended: true, + dialects: ["Bash", "POSIX sh", "mksh"], + url: "https://github.com/eslint/shell/blob/main/docs/rules/require-cd-guard.md", + }, + hasSuggestions: true, + schema: [], + messages: { + uncheckedCd: + "Use 'cd ... || exit' or 'cd ... || return' in case cd fails. (ShellCheck SC2164)", + addGuard: "Add '|| exit' after the cd command.", + }, + }, + + create(context) { + const { sourceCode } = context; + + return { + Command(node) { + if (getCommandName(node) !== "cd") { + return; + } + + // `! cd ...` uses the exit status explicitly. + if (node.negated) { + return; + } + + let expression: StatementNode = node; + let parent = sourceCode.getParent(expression); + + while (parent?.type === "LogicalExpression") { + // A failed command can propagate through a left-associative + // chain to an expression whose status is checked. + if (parent.left === expression) { + return; + } + + expression = parent; + parent = sourceCode.getParent(expression); + } + + if (parent) { + // `if cd ...`, `while cd ...`, `until cd ...` + if ( + (parent.type === "IfStatement" || + parent.type === "WhileStatement" || + parent.type === "UntilStatement") && + parent.test.includes(node) + ) { + return; + } + } + + const canSuggest = + !node.background && node.redirects.length === 0; + + context.report({ + node, + messageId: "uncheckedCd", + suggest: canSuggest + ? [ + { + messageId: "addGuard", + fix: fixer => + fixer.insertTextAfter(node, " || exit"), + }, + ] + : [], + }); + }, + }; + }, +}; + +export default rule;