diff --git a/README.md b/README.md index bcb9ddc..f4faf13 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 | +| [`no-variables-in-printf-format`](./docs/rules/no-variables-in-printf-format.md) | SC2059 | Don't put variables in the `printf` format string | | 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 | diff --git a/docs/rules/no-variables-in-printf-format.md b/docs/rules/no-variables-in-printf-format.md new file mode 100644 index 0000000..39a6861 --- /dev/null +++ b/docs/rules/no-variables-in-printf-format.md @@ -0,0 +1,53 @@ +# no-variables-in-printf-format + +Disallow variables in the printf format string; use %s placeholders instead. + +## Background + +The first argument to `printf` is a format string: `%` starts a conversion and `\` starts an escape sequence. When a variable is part of the format, any `%` or `\` in its value is interpreted too. A value such as `100% done` produces garbled output or an error. Passing the value as a separate argument with a `%s` placeholder prints it exactly. + +## Rule Details + +This rule warns when the format argument of `printf` contains a parameter expansion or command substitution, whether or not it's quoted. The format is the first argument after an optional `-v name` or `-vname` and an optional `--`. + +Variables in the arguments after the format are fine. Arithmetic expansions such as `$((...))` in the format are not checked. + +Examples of **incorrect** code for this rule: + +```bash +# eslint shell/no-variables-in-printf-format: "error" + +printf "$message" + +printf "Hello, $name\n" + +printf -v line "$format" "$value" + +printf "Files: $(ls | wc -l)\n" +``` + +Examples of **correct** code for this rule: + +```bash +# eslint shell/no-variables-in-printf-format: "error" + +printf '%s\n' "$message" + +printf 'Hello, %s\n' "$name" + +printf -v line '%s' "$value" + +printf 'Files: %d\n' "$(ls | wc -l)" +``` + +## Options + +This rule has no options. + +## When Not to Use It + +Keeping a format in a variable is sometimes intentional, such as `fmt='%-10s %s\n'` reused across several `printf` calls. If the variable always holds a format you control, use a disable comment for those lines. + +## Prior Art + +- [SC2059](https://www.shellcheck.net/wiki/SC2059) diff --git a/src/index.spec.ts b/src/index.spec.ts index 2bdd813..50c4f1b 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", + "no-variables-in-printf-format", "require-cd-guard", "require-read-r", ]); diff --git a/src/index.ts b/src/index.ts index 9a425e8..c50dae2 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 noVariablesInPrintfFormat from "./rules/no-variables-in-printf-format.js"; import requireCdGuard from "./rules/require-cd-guard.js"; import requireReadR from "./rules/require-read-r.js"; @@ -20,6 +21,7 @@ const rules = { "no-unquoted-expansions": noUnquotedExpansions, "no-useless-cat": noUselessCat, "no-useless-echo": noUselessEcho, + "no-variables-in-printf-format": noVariablesInPrintfFormat, "require-cd-guard": requireCdGuard, "require-read-r": requireReadR, }; @@ -49,6 +51,7 @@ const plugin = { "shell/no-unquoted-expansions": "error", "shell/no-useless-cat": "error", "shell/no-useless-echo": "error", + "shell/no-variables-in-printf-format": "error", "shell/require-cd-guard": "error", "shell/require-read-r": "error", }, diff --git a/src/rules/no-variables-in-printf-format.spec.ts b/src/rules/no-variables-in-printf-format.spec.ts new file mode 100644 index 0000000..c520cac --- /dev/null +++ b/src/rules/no-variables-in-printf-format.spec.ts @@ -0,0 +1,73 @@ +/** + * @fileoverview Tests for the no-variables-in-printf-format rule. + */ + +import { RuleTester } from "eslint"; +import { ShellLanguage } from "../languages/shell-language.js"; +import rule from "./no-variables-in-printf-format.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-variables-in-printf-format", rule as never, { + valid: [ + 'printf "%s\\n" "$var"', + 'printf \'%s: %d\\n\' "$name" "$count"', + 'printf -v result "%s" "$var"', + 'printf -v result -- "%s" "$var"', + 'printf -vresult -- "%s" "$var"', + 'printf -- "%s" "$var"', + 'printf "static text\\n"', + // Not printf + 'echo "$var"', + ], + invalid: [ + { + code: 'printf "$var"', + errors: [ + { + messageId: "variableInFormat", + line: 1, + column: 8, + endColumn: 14, + }, + ], + }, + { + code: 'printf "$var\\n"', + errors: [{ messageId: "variableInFormat" }], + }, + { + code: "printf $format arg", + errors: [{ messageId: "variableInFormat" }], + }, + { + code: 'printf -v out "$fmt" x', + errors: [{ messageId: "variableInFormat" }], + }, + { + code: 'printf -v out -- "$fmt" x', + errors: [{ messageId: "variableInFormat" }], + }, + { + code: 'printf -vout -- "$fmt" x', + errors: [{ messageId: "variableInFormat" }], + }, + { + code: 'printf -- "$fmt" x', + errors: [{ messageId: "variableInFormat" }], + }, + { + code: 'printf "count: $(wc -l < file)\\n"', + errors: [{ messageId: "variableInFormat" }], + }, + ], +}); diff --git a/src/rules/no-variables-in-printf-format.ts b/src/rules/no-variables-in-printf-format.ts new file mode 100644 index 0000000..26f7984 --- /dev/null +++ b/src/rules/no-variables-in-printf-format.ts @@ -0,0 +1,69 @@ +/** + * @fileoverview Rule to disallow variables in the printf format string. + * Mirrors ShellCheck SC2059. + */ + +import { getCommandName, getExpansions, getStaticText } from "./utils.js"; +import type { ShellRuleDefinition } from "../types.js"; + +const rule: ShellRuleDefinition<{ MessageIds: "variableInFormat" }> = { + meta: { + type: "problem", + languages: ["shell/bash", "shell/posix", "shell/mksh"], + docs: { + description: + "Disallow variables in the printf format string; use %s placeholders instead", + recommended: true, + dialects: ["Bash", "POSIX sh", "mksh"], + url: "https://github.com/eslint/shell/blob/main/docs/rules/no-variables-in-printf-format.md", + }, + schema: [], + messages: { + variableInFormat: + "Don't use variables in the printf format string. Use printf '...%s...' \"$foo\". (ShellCheck SC2059)", + }, + }, + + create(context) { + return { + Command(node) { + if (getCommandName(node) !== "printf") { + return; + } + + let formatIndex = 0; + const first = node.arguments[0] + ? getStaticText(node.arguments[0]) + : null; + + if (first === "-v") { + // `printf -v var format ...` + formatIndex = 2; + } else if (first !== null && first.startsWith("-v")) { + // `printf -vvar format ...` + formatIndex = 1; + } + + const possibleFormat = node.arguments[formatIndex]; + if (possibleFormat && getStaticText(possibleFormat) === "--") { + formatIndex++; + } + + const format = node.arguments[formatIndex]; + + if (!format) { + return; + } + + if (getExpansions(format, { includeQuoted: true }).length > 0) { + context.report({ + node: format, + messageId: "variableInFormat", + }); + } + }, + }; + }, +}; + +export default rule;