diff --git a/README.md b/README.md index 0bd2687..7fd586e 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,7 @@ export default [ language: "shell/bash", rules: { "shell/no-backticks": "error", + "shell/no-unquoted-expansions": "error", }, }, ]; @@ -84,10 +85,11 @@ to override the dialect of the language you chose. 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 | -| [`no-expansions-in-single-quotes`](./docs/rules/no-expansions-in-single-quotes.md) | SC2016 | Expressions don't expand in single quotes | | warn | +| Rule | ShellCheck | Description | Fixable | Recommended | +| ---------------------------------------------------------------------------------- | ---------- | ------------------------------------------------------- | ------- | ----------- | +| [`no-backticks`](./docs/rules/no-backticks.md) | SC2006 | Use `$(...)` instead of legacy backticks | ✅ | error | +| [`no-expansions-in-single-quotes`](./docs/rules/no-expansions-in-single-quotes.md) | SC2016 | Expressions don't expand in single quotes | | warn | +| [`no-unquoted-expansions`](./docs/rules/no-unquoted-expansions.md) | SC2086/46 | Quote expansions subject to word splitting and globbing | ✅ | error | ## Configuration comments diff --git a/docs/rules/no-unquoted-expansions.md b/docs/rules/no-unquoted-expansions.md new file mode 100644 index 0000000..03e8430 --- /dev/null +++ b/docs/rules/no-unquoted-expansions.md @@ -0,0 +1,79 @@ +# no-unquoted-expansions + +Require quoting parameter expansions and command substitutions that are subject to word splitting. + +## Background + +When an expansion such as `$file` or `$(cmd)` is not quoted, the shell splits its value on whitespace and then expands any glob characters in the pieces. A file named `my report.txt` becomes two arguments, and a value containing `*` can turn into a list of unrelated files. Wrapping the expansion in double quotes (`"$file"`) passes the value through as a single argument. + +## Rule Details + +This rule warns about unquoted parameter expansions (`$var`, `${var}`, `$@`, `$1`, and so on) and command substitutions (`$(...)` and backticks) in positions where word splitting happens: + +- a command's name and arguments, including arguments to `[ ... ]`; +- the word list of a `for ... in` loop; +- redirection targets, such as `> $file`, except when the `variant` language option is `"posix"`. + +The rule does not warn about: + +- expansions inside double quotes; +- parameters that can't contain whitespace: `$?`, `$$`, `$!`, `$#`, `$-`, and length expansions such as `${#array[@]}`; +- contexts where the shell doesn't split words: variable assignments (`x=$y`), `[[ ... ]]`, `case` subjects, arithmetic such as `$((...))`, and heredoc or herestring redirects (`<<`, `<<-`, `<<<`); +- redirection targets when the `variant` language option is `"posix"`, because POSIX `sh` doesn't split them. + +This rule is autofixable when the expansion is the entire word: `$var` becomes `"$var"`. Words that mix an expansion with other text, such as `prefix$var`, are reported but not fixed. + +Examples of **incorrect** code for this rule: + +```bash +# eslint shell/no-unquoted-expansions: "error" + +rm $file + +cp $source $destination + +for f in $files; do echo "$f"; done + +echo $(ls) + +sort data.txt > $output + +$command --verbose +``` + +Examples of **correct** code for this rule: + +```bash +# eslint shell/no-unquoted-expansions: "error" + +rm "$file" + +cp "$source" "$destination" + +for f in "$@"; do echo "$f"; done + +echo "$(ls)" + +name=$other + +[[ $a == "$b" ]] + +echo $? $# ${#items[@]} + +echo $((count + 1)) + +cat <<< $input +``` + +## Options + +This rule has no options. + +## When Not to Use It + +Occasionally word splitting is intended, such as passing a space-separated list of flags stored in a variable. Prefer an array (`"${flags[@]}"`) in that case; otherwise, use a disable comment for the specific line rather than disabling this rule. + +## Prior Art + +- [SC2086](https://www.shellcheck.net/wiki/SC2086) +- [SC2046](https://www.shellcheck.net/wiki/SC2046) diff --git a/src/index.spec.ts b/src/index.spec.ts index 9bfbf02..17ec809 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -37,6 +37,7 @@ describe("plugin", () => { expect(ruleIds.sort()).toEqual([ "no-backticks", "no-expansions-in-single-quotes", + "no-unquoted-expansions", ]); }); diff --git a/src/index.ts b/src/index.ts index ce7a916..9922626 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,10 +6,12 @@ import { ShellLanguage } from "./languages/shell-language.js"; import noBackticks from "./rules/no-backticks.js"; import noExpansionsInSingleQuotes from "./rules/no-expansions-in-single-quotes.js"; +import noUnquotedExpansions from "./rules/no-unquoted-expansions.js"; const rules = { "no-backticks": noBackticks, "no-expansions-in-single-quotes": noExpansionsInSingleQuotes, + "no-unquoted-expansions": noUnquotedExpansions, }; const plugin = { @@ -33,6 +35,7 @@ const plugin = { rules: { "shell/no-backticks": "error", "shell/no-expansions-in-single-quotes": "warn", + "shell/no-unquoted-expansions": "error", }, }, }, diff --git a/src/rules/no-unquoted-expansions.spec.ts b/src/rules/no-unquoted-expansions.spec.ts new file mode 100644 index 0000000..7190c1a --- /dev/null +++ b/src/rules/no-unquoted-expansions.spec.ts @@ -0,0 +1,155 @@ +/** + * @fileoverview Tests for the no-unquoted-expansions rule. + */ + +import { RuleTester } from "eslint"; +import { ShellLanguage } from "../languages/shell-language.js"; +import rule from "./no-unquoted-expansions.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("no-unquoted-expansions", rule as never, { + valid: [ + // Quoted expansions + 'echo "$var"', + 'echo "${var}"', + 'echo "$(pwd)"', + 'cp "$src" "$dest"', + 'for f in "$@"; do echo "$f"; done', + // Safe special parameters + "echo $?", + "echo $$", + "echo $#", + "echo $!", + "echo $-", + "echo ${#arr}", + // Assignments don't word-split + "x=$y", + "x=$(pwd)", + // [[ ]] doesn't word-split + "[[ $x == foo ]]", + // Case discriminants don't word-split + "case $x in a) ;; esac", + // Arithmetic contexts don't word-split + "echo $((x + 1))", + // Heredoc delimiters and herestrings + "cat <<< $var", + // POSIX sh doesn't word-split redirection targets + { + code: "cat > $out", + languageOptions: { variant: "posix" }, + }, + { + code: "cat < $(pwd)/in", + languageOptions: { variant: "posix" }, + }, + ], + invalid: [ + { + code: "echo $var", + output: 'echo "$var"', + errors: [ + { + messageId: "unquotedParameterExpansion", + data: { expansion: "$var" }, + line: 1, + column: 6, + endColumn: 10, + }, + ], + }, + { + code: "echo ${var}", + output: 'echo "${var}"', + errors: [{ messageId: "unquotedParameterExpansion" }], + }, + { + // The message quotes the expansion as written + code: "echo ${arr[@]}", + output: 'echo "${arr[@]}"', + errors: [ + { + messageId: "unquotedParameterExpansion", + data: { expansion: "${arr[@]}" }, + }, + ], + }, + { + code: "echo ${var:-default}", + output: 'echo "${var:-default}"', + errors: [ + { + messageId: "unquotedParameterExpansion", + data: { expansion: "${var:-default}" }, + }, + ], + }, + { + code: "echo $@", + output: 'echo "$@"', + errors: [{ messageId: "unquotedParameterExpansion" }], + }, + { + code: "echo $1", + output: 'echo "$1"', + errors: [{ messageId: "unquotedParameterExpansion" }], + }, + { + code: "rm $(ls)", + output: 'rm "$(ls)"', + errors: [{ messageId: "unquotedCommandSubstitution" }], + }, + { + // Expansion in the command-name position + code: "$cmd --help", + output: '"$cmd" --help', + errors: [{ messageId: "unquotedParameterExpansion" }], + }, + { + // Mixed word: report but do not autofix + code: "echo prefix$var", + output: null, + errors: [{ messageId: "unquotedParameterExpansion" }], + }, + { + code: "for f in $files; do echo ok; done", + output: 'for f in "$files"; do echo ok; done', + errors: [{ messageId: "unquotedParameterExpansion" }], + }, + { + code: "cat > $out", + output: 'cat > "$out"', + errors: [{ messageId: "unquotedParameterExpansion" }], + }, + { + code: "cat > $out", + output: 'cat > "$out"', + languageOptions: { variant: "mksh" }, + errors: [{ messageId: "unquotedParameterExpansion" }], + }, + { + // Arguments still word-split in POSIX sh + code: "cat $in > $out", + output: 'cat "$in" > $out', + languageOptions: { variant: "posix" }, + errors: [{ messageId: "unquotedParameterExpansion" }], + }, + { + code: "cp $a $b", + output: 'cp "$a" "$b"', + errors: [ + { messageId: "unquotedParameterExpansion" }, + { messageId: "unquotedParameterExpansion" }, + ], + }, + ], +}); diff --git a/src/rules/no-unquoted-expansions.ts b/src/rules/no-unquoted-expansions.ts new file mode 100644 index 0000000..7067c5c --- /dev/null +++ b/src/rules/no-unquoted-expansions.ts @@ -0,0 +1,118 @@ +/** + * @fileoverview Rule to require quoting expansions that undergo word + * splitting and globbing. Mirrors ShellCheck SC2086/SC2046. + */ + +import type { ShellRuleDefinition, WordNode } from "../types.js"; + +/** + * Parameters that always expand to values that cannot split, so quoting is + * unnecessary. + */ +const SAFE_PARAMETERS = new Set(["?", "$", "!", "#", "-"]); + +/** Heredoc-style redirect operators whose targets never word-split. */ +const NON_SPLITTING_REDIRECTS = new Set(["<<", "<<-", "<<<"]); + +const rule: ShellRuleDefinition<{ + MessageIds: "unquotedParameterExpansion" | "unquotedCommandSubstitution"; +}> = { + meta: { + type: "problem", + languages: ["shell/bash", "shell/posix", "shell/mksh"], + docs: { + description: + "Require quoting parameter expansions and command substitutions that are subject to word splitting", + recommended: true, + dialects: ["Bash", "POSIX sh", "mksh"], + url: "https://github.com/eslint/shell/blob/main/docs/rules/no-unquoted-expansions.md", + }, + fixable: "code", + schema: [], + messages: { + unquotedParameterExpansion: + 'Double quote "{{expansion}}" to prevent word splitting and globbing. (ShellCheck SC2086)', + unquotedCommandSubstitution: + "Quote this command substitution to prevent word splitting. (ShellCheck SC2046)", + }, + }, + + create(context) { + const { sourceCode } = context; + + // POSIX sh doesn't field-split redirection targets the way Bash does. + const splitsRedirects = context.languageOptions.variant !== "posix"; + + function checkWord(word: WordNode): void { + for (const part of word.parts) { + if ( + part.type !== "ParameterExpansion" && + part.type !== "CommandSubstitution" + ) { + continue; + } + + if (part.type === "ParameterExpansion") { + if (part.lengthOf || SAFE_PARAMETERS.has(part.name)) { + continue; + } + } + + // Only offer a fix when the expansion is the entire word. + const isWholeWord = + word.parts.length === 1 && + word.start === part.start && + word.end === part.end; + + context.report({ + node: part, + messageId: + part.type === "ParameterExpansion" + ? "unquotedParameterExpansion" + : "unquotedCommandSubstitution", + data: + part.type === "ParameterExpansion" + ? { expansion: sourceCode.getText(part) } + : {}, + fix: isWholeWord + ? fixer => + fixer.replaceText( + word, + `"${sourceCode.getText(word)}"`, + ) + : undefined, + }); + } + } + + return { + Command(node) { + if (node.name) { + checkWord(node.name); + } + + for (const argument of node.arguments) { + checkWord(argument); + } + }, + + ForStatement(node) { + for (const word of node.words) { + checkWord(word); + } + }, + + Redirect(node) { + if ( + splitsRedirects && + node.target && + !NON_SPLITTING_REDIRECTS.has(node.operator) + ) { + checkWord(node.target); + } + }, + }; + }, +}; + +export default rule; diff --git a/tests/autofix.test.ts b/tests/autofix.test.ts index 0b533bf..d8e8c73 100644 --- a/tests/autofix.test.ts +++ b/tests/autofix.test.ts @@ -33,4 +33,12 @@ describe("autofix", () => { expect(result.output).toBe("echo $(pwd) $(date)\n"); expect(result.messages).toEqual([]); }); + + it("should quote unquoted expansions", () => { + const result = fix("cp $src $dest\n", { + "shell/no-unquoted-expansions": "error", + }); + + expect(result.output).toBe('cp "$src" "$dest"\n'); + }); });