From 6a276d48bc2ca294da33a70b5f067eb953181102 Mon Sep 17 00:00:00 2001 From: afadesigns Date: Fri, 26 Jun 2026 01:17:37 +0200 Subject: [PATCH] fix: lint inside logical-chain compound bodies A compound statement on the right of a `&&` / `||` chain (`cmd && { ... }`, `cmd && if ...; fi`, `cmd && for ...; done`, `cmd && case ... esac`) was wrapped in a stub identifier by keywordStmtToExpression, so the AST walker never descended into the body. Dangerous code inside the block, such as an unquoted `rm -rf $x`, went unlinted. These statement types already implement Expression, so return the real node, as the WHILE head already does; the walker now walks the body. Walking the bodies surfaced four latent kata false positives, fixed here: - ZC1071 fired on a single-element rebuild (`arr=(${arr[@]:#p})`, a filter, not an append). Require the whole-array reference plus at least one appended element. - ZC1075 fired on a `$var` glued to a literal command-name prefix (`_clear$fsuf`), which cannot elide the word. - ZC1075 fired on a `(q)`-quoted expansion, which renders an empty value as a quoted empty string and never elides. - ZC1149 fired on an error message already routed to stderr (`print -u2`, `>&2`). Net corpus effect: 28 real findings inside compound bodies now visible; 21 false positives removed. Signed-off-by: afadesigns --- .github/violation-baseline.txt | 85 ++++++++---------- pkg/katas/katatests/fp_round10_test.go | 118 +++++++++++++++++++++++++ pkg/katas/katatests/zc1000s_test.go | 17 ++-- pkg/katas/zc1000s.go | 35 +++++++- pkg/katas/zc1100s.go | 65 ++++++++++---- pkg/parser/parser_corpus_fixes_test.go | 37 ++++++++ pkg/parser/parser_stmt.go | 15 +++- 7 files changed, 294 insertions(+), 78 deletions(-) create mode 100644 pkg/katas/katatests/fp_round10_test.go diff --git a/.github/violation-baseline.txt b/.github/violation-baseline.txt index 7b52cb09..efa86522 100644 --- a/.github/violation-baseline.txt +++ b/.github/violation-baseline.txt @@ -7,7 +7,7 @@ agkozak-prompt/agkozak-zsh-prompt.plugin.zsh ZC1083 1 agkozak-prompt/agkozak-zsh-prompt.plugin.zsh ZC1091 1 agkozak-prompt/agkozak-zsh-prompt.plugin.zsh ZC1337 1 agkozak-prompt/agkozak-zsh-prompt.plugin.zsh ZC1604 2 -agkozak-prompt/lib/async.zsh ZC1001 5 +agkozak-prompt/lib/async.zsh ZC1001 6 agkozak-prompt/lib/async.zsh ZC1043 2 agkozak-prompt/lib/async.zsh ZC1046 1 agkozak-prompt/lib/async.zsh ZC1075 24 @@ -27,7 +27,6 @@ antidote/antidote.zsh ZC1043 1 antidote/antidote.zsh ZC1046 3 antidote/antidote.zsh ZC1059 1 antidote/antidote.zsh ZC1065 1 -antidote/antidote.zsh ZC1071 1 antidote/antidote.zsh ZC1073 2 antidote/antidote.zsh ZC1075 22 antidote/antidote.zsh ZC1091 14 @@ -98,7 +97,6 @@ autosuggestions/src/bind.zsh ZC1046 3 autosuggestions/src/bind.zsh ZC1075 11 autosuggestions/src/bind.zsh ZC1078 1 autosuggestions/src/bind.zsh ZC1191 1 -autosuggestions/src/fetch.zsh ZC1075 1 autosuggestions/src/highlight.zsh ZC1045 1 autosuggestions/src/strategies/completion.zsh ZC1001 1 autosuggestions/src/strategies/completion.zsh ZC1037 1 @@ -125,7 +123,7 @@ autosuggestions/zsh-autosuggestions.zsh ZC1045 2 autosuggestions/zsh-autosuggestions.zsh ZC1046 4 autosuggestions/zsh-autosuggestions.zsh ZC1068 1 autosuggestions/zsh-autosuggestions.zsh ZC1073 6 -autosuggestions/zsh-autosuggestions.zsh ZC1075 19 +autosuggestions/zsh-autosuggestions.zsh ZC1075 18 autosuggestions/zsh-autosuggestions.zsh ZC1078 6 autosuggestions/zsh-autosuggestions.zsh ZC1086 1 autosuggestions/zsh-autosuggestions.zsh ZC1091 1 @@ -152,7 +150,7 @@ bullet-train/bullet-train.zsh-theme ZC1046 2 bullet-train/bullet-train.zsh-theme ZC1055 1 bullet-train/bullet-train.zsh-theme ZC1064 2 bullet-train/bullet-train.zsh-theme ZC1068 2 -bullet-train/bullet-train.zsh-theme ZC1075 59 +bullet-train/bullet-train.zsh-theme ZC1075 58 bullet-train/bullet-train.zsh-theme ZC1083 1 bullet-train/bullet-train.zsh-theme ZC1086 1 bullet-train/bullet-train.zsh-theme ZC1091 5 @@ -187,7 +185,7 @@ fast-syntax-highlighting/test/to-parse.zsh ZC1001 17 fast-syntax-highlighting/test/to-parse.zsh ZC1043 2 fast-syntax-highlighting/test/to-parse.zsh ZC1046 2 fast-syntax-highlighting/test/to-parse.zsh ZC1073 3 -fast-syntax-highlighting/test/to-parse.zsh ZC1075 30 +fast-syntax-highlighting/test/to-parse.zsh ZC1075 29 fast-syntax-highlighting/test/to-parse.zsh ZC1830 1 forgit/forgit.plugin.zsh ZC1091 1 forgit/forgit.plugin.zsh ZC1300 3 @@ -208,15 +206,16 @@ F-Sy-H/share/parse/to-parse.zsh ZC1001 17 F-Sy-H/share/parse/to-parse.zsh ZC1043 2 F-Sy-H/share/parse/to-parse.zsh ZC1046 2 F-Sy-H/share/parse/to-parse.zsh ZC1073 3 -F-Sy-H/share/parse/to-parse.zsh ZC1075 30 +F-Sy-H/share/parse/to-parse.zsh ZC1075 29 F-Sy-H/share/parse/to-parse.zsh ZC1830 1 +fzf/shell/completion.zsh ZC1001 1 fzf/shell/completion.zsh ZC1003 3 -fzf/shell/completion.zsh ZC1010 10 +fzf/shell/completion.zsh ZC1010 12 fzf/shell/completion.zsh ZC1043 4 fzf/shell/completion.zsh ZC1046 4 fzf/shell/completion.zsh ZC1064 1 fzf/shell/completion.zsh ZC1074 1 -fzf/shell/completion.zsh ZC1075 8 +fzf/shell/completion.zsh ZC1075 7 fzf/shell/completion.zsh ZC1097 1 fzf/shell/completion.zsh ZC1098 3 fzf/shell/completion.zsh ZC1100 1 @@ -224,7 +223,7 @@ fzf/shell/completion.zsh ZC1256 1 fzf/shell/completion.zsh ZC1275 1 fzf/shell/completion.zsh ZC1602 1 fzf/shell/completion.zsh ZC1830 1 -fzf/shell/key-bindings.zsh ZC1010 2 +fzf/shell/key-bindings.zsh ZC1010 4 fzf/shell/key-bindings.zsh ZC1037 2 fzf/shell/key-bindings.zsh ZC1043 5 fzf/shell/key-bindings.zsh ZC1045 1 @@ -247,7 +246,7 @@ fzf-tab/fzf-tab.zsh ZC1073 1 fzf-tab/fzf-tab.zsh ZC1075 7 fzf-tab/fzf-tab.zsh ZC1086 2 fzf-tab/fzf-tab.zsh ZC1091 1 -fzf-tab/lib/zsh-ls-colors/ls-colors.zsh ZC1001 16 +fzf-tab/lib/zsh-ls-colors/ls-colors.zsh ZC1001 19 fzf-tab/lib/zsh-ls-colors/ls-colors.zsh ZC1045 1 fzf-tab/lib/zsh-ls-colors/ls-colors.zsh ZC1065 1 fzf-tab/lib/zsh-ls-colors/ls-colors.zsh ZC1075 8 @@ -317,7 +316,7 @@ git-open/git-open.plugin.zsh ZC1275 1 gitstatus/gitstatus.plugin.zsh ZC1001 7 gitstatus/gitstatus.plugin.zsh ZC1043 3 gitstatus/gitstatus.plugin.zsh ZC1046 1 -gitstatus/gitstatus.plugin.zsh ZC1075 60 +gitstatus/gitstatus.plugin.zsh ZC1075 48 gitstatus/gitstatus.plugin.zsh ZC1086 11 gitstatus/gitstatus.plugin.zsh ZC1167 1 gitstatus/gitstatus.plugin.zsh ZC1176 3 @@ -332,7 +331,7 @@ history-search-multi-word/test/to-parse.zsh ZC1001 17 history-search-multi-word/test/to-parse.zsh ZC1043 2 history-search-multi-word/test/to-parse.zsh ZC1046 2 history-search-multi-word/test/to-parse.zsh ZC1073 3 -history-search-multi-word/test/to-parse.zsh ZC1075 30 +history-search-multi-word/test/to-parse.zsh ZC1075 29 history-search-multi-word/test/to-parse.zsh ZC1830 1 mcfly/dev.zsh ZC1002 1 mcfly/dev.zsh ZC1015 1 @@ -489,7 +488,7 @@ purer/async.zsh ZC1001 5 purer/async.zsh ZC1004 1 purer/async.zsh ZC1043 4 purer/async.zsh ZC1046 1 -purer/async.zsh ZC1075 23 +purer/async.zsh ZC1075 22 purer/async.zsh ZC1078 1 purer/async.zsh ZC1097 3 purer/async.zsh ZC1134 1 @@ -499,13 +498,13 @@ purer/pure.zsh ZC1006 1 purer/pure.zsh ZC1017 3 purer/pure.zsh ZC1020 1 purer/pure.zsh ZC1036 1 -purer/pure.zsh ZC1043 10 +purer/pure.zsh ZC1043 11 purer/pure.zsh ZC1055 1 purer/pure.zsh ZC1073 1 -purer/pure.zsh ZC1075 4 +purer/pure.zsh ZC1075 5 purer/pure.zsh ZC1091 2 purer/pure.zsh ZC1293 1 -spaceship/async.zsh ZC1001 5 +spaceship/async.zsh ZC1001 6 spaceship/async.zsh ZC1043 2 spaceship/async.zsh ZC1046 1 spaceship/async.zsh ZC1075 24 @@ -521,7 +520,6 @@ spaceship/lib/cli.zsh ZC1043 1 spaceship/lib/cli.zsh ZC1045 2 spaceship/lib/cli.zsh ZC1046 2 spaceship/lib/cli.zsh ZC1064 1 -spaceship/lib/cli.zsh ZC1075 1 spaceship/lib/cli.zsh ZC1080 1 spaceship/lib/cli.zsh ZC1097 5 spaceship/lib/cli.zsh ZC1098 2 @@ -930,7 +928,7 @@ spaceship/tests/zig.test.zsh ZC1188 1 spaceship/tests/zig.test.zsh ZC1602 1 starship/src/init/starship.zsh ZC1043 3 starship/src/init/starship.zsh ZC1967 1 -typewritten/async.zsh ZC1001 5 +typewritten/async.zsh ZC1001 6 typewritten/async.zsh ZC1043 2 typewritten/async.zsh ZC1046 1 typewritten/async.zsh ZC1075 24 @@ -976,7 +974,6 @@ zaw/sources/applications.zsh ZC1003 2 zaw/sources/applications.zsh ZC1010 1 zaw/sources/applications.zsh ZC1012 1 zaw/sources/applications.zsh ZC1040 1 -zaw/sources/applications.zsh ZC1071 1 zaw/sources/applications.zsh ZC1075 2 zaw/sources/applications.zsh ZC1080 1 zaw/sources/applications.zsh ZC1086 2 @@ -1039,7 +1036,6 @@ zaw/sources/searcher.zsh ZC1076 1 zaw/sources/searcher.zsh ZC1086 2 zaw/sources/ssh-hosts.zsh ZC1001 6 zaw/sources/ssh-hosts.zsh ZC1010 6 -zaw/sources/ssh-hosts.zsh ZC1071 1 zaw/sources/ssh-hosts.zsh ZC1086 2 zaw/sources/tmux.zsh ZC1010 1 zaw/sources/tmux.zsh ZC1012 1 @@ -1069,7 +1065,7 @@ zcomet/zcomet.zsh ZC1043 7 zcomet/zcomet.zsh ZC1065 2 zcomet/zcomet.zsh ZC1068 1 zcomet/zcomet.zsh ZC1073 2 -zcomet/zcomet.zsh ZC1075 14 +zcomet/zcomet.zsh ZC1075 13 zcomet/zcomet.zsh ZC1078 1 zcomet/zcomet.zsh ZC1097 1 zcomet/zcomet.zsh ZC1160 1 @@ -1144,7 +1140,7 @@ zephyr/tests/__init__.zsh ZC1031 1 zephyr/tests/__init__.zsh ZC1037 6 zephyr/tests/__init__.zsh ZC1051 1 zephyr/tests/__init__.zsh ZC1073 3 -zephyr/tests/__init__.zsh ZC1075 10 +zephyr/tests/__init__.zsh ZC1075 9 zephyr/tests/__init__.zsh ZC1086 8 zephyr/tests/__init__.zsh ZC1136 1 zephyr/tests/__init__.zsh ZC1165 1 @@ -1173,7 +1169,7 @@ zimfw/zimfw.zsh ZC1043 23 zimfw/zimfw.zsh ZC1045 3 zimfw/zimfw.zsh ZC1046 1 zimfw/zimfw.zsh ZC1065 1 -zimfw/zimfw.zsh ZC1075 116 +zimfw/zimfw.zsh ZC1075 124 zimfw/zimfw.zsh ZC1080 1 zimfw/zimfw.zsh ZC1091 10 zimfw/zimfw.zsh ZC1637 19 @@ -1195,8 +1191,7 @@ zinit/zinit-additional.zsh ZC1001 2 zinit/zinit-additional.zsh ZC1043 1 zinit/zinit-additional.zsh ZC1045 1 zinit/zinit-additional.zsh ZC1046 3 -zinit/zinit-additional.zsh ZC1071 1 -zinit/zinit-additional.zsh ZC1075 3 +zinit/zinit-additional.zsh ZC1075 5 zinit/zinit-additional.zsh ZC1098 1 zinit/zinit-additional.zsh ZC1877 1 zinit/zinit-additional.zsh ZC1916 1 @@ -1207,14 +1202,14 @@ zinit/zinit-autoload.zsh ZC1015 2 zinit/zinit-autoload.zsh ZC1017 1 zinit/zinit-autoload.zsh ZC1040 2 zinit/zinit-autoload.zsh ZC1045 6 -zinit/zinit-autoload.zsh ZC1046 6 +zinit/zinit-autoload.zsh ZC1046 8 zinit/zinit-autoload.zsh ZC1049 1 zinit/zinit-autoload.zsh ZC1053 1 zinit/zinit-autoload.zsh ZC1064 1 zinit/zinit-autoload.zsh ZC1073 3 -zinit/zinit-autoload.zsh ZC1075 40 +zinit/zinit-autoload.zsh ZC1075 41 zinit/zinit-autoload.zsh ZC1083 2 -zinit/zinit-autoload.zsh ZC1091 32 +zinit/zinit-autoload.zsh ZC1091 33 zinit/zinit-autoload.zsh ZC1098 3 zinit/zinit-autoload.zsh ZC1111 1 zinit/zinit-autoload.zsh ZC1124 1 @@ -1239,7 +1234,6 @@ zinit/zinit-install.zsh ZC1045 10 zinit/zinit-install.zsh ZC1046 16 zinit/zinit-install.zsh ZC1053 1 zinit/zinit-install.zsh ZC1064 5 -zinit/zinit-install.zsh ZC1071 2 zinit/zinit-install.zsh ZC1073 1 zinit/zinit-install.zsh ZC1075 63 zinit/zinit-install.zsh ZC1083 2 @@ -1248,7 +1242,7 @@ zinit/zinit-install.zsh ZC1098 9 zinit/zinit-install.zsh ZC1099 1 zinit/zinit-install.zsh ZC1139 1 zinit/zinit-install.zsh ZC1166 1 -zinit/zinit-install.zsh ZC1179 2 +zinit/zinit-install.zsh ZC1179 3 zinit/zinit-install.zsh ZC1833 3 zinit/zinit-install.zsh ZC1867 1 zinit/zinit-install.zsh ZC1875 7 @@ -1260,20 +1254,19 @@ zinit/zinit-side.zsh ZC1075 5 zinit/zinit-side.zsh ZC1083 1 zinit/zinit-side.zsh ZC1091 2 zinit/zinit-side.zsh ZC1877 2 -zinit/zinit.zsh ZC1001 60 +zinit/zinit.zsh ZC1001 63 zinit/zinit.zsh ZC1002 1 zinit/zinit.zsh ZC1015 1 zinit/zinit.zsh ZC1017 1 -zinit/zinit.zsh ZC1043 6 +zinit/zinit.zsh ZC1043 7 zinit/zinit.zsh ZC1045 4 -zinit/zinit.zsh ZC1046 13 +zinit/zinit.zsh ZC1046 17 zinit/zinit.zsh ZC1064 1 -zinit/zinit.zsh ZC1071 3 -zinit/zinit.zsh ZC1075 69 +zinit/zinit.zsh ZC1075 83 zinit/zinit.zsh ZC1076 2 -zinit/zinit.zsh ZC1091 25 +zinit/zinit.zsh ZC1091 27 zinit/zinit.zsh ZC1097 1 -zinit/zinit.zsh ZC1098 5 +zinit/zinit.zsh ZC1098 8 zinit/zinit.zsh ZC1288 1 zinit/zinit.zsh ZC1414 3 zinit/zinit.zsh ZC1604 1 @@ -1320,11 +1313,11 @@ zsh-abbr/tests/index.ztr.zsh ZC1097 1 zsh-abbr/tests/index.ztr.zsh ZC1128 1 zsh-abbr/tests/_rename.ztr.zsh ZC1075 28 zsh-abbr/tests/_tmpdir.ztr.zsh ZC1075 2 -zsh-abbr/zsh-abbr.zsh ZC1001 25 +zsh-abbr/zsh-abbr.zsh ZC1001 28 zsh-abbr/zsh-abbr.zsh ZC1043 40 zsh-abbr/zsh-abbr.zsh ZC1046 2 zsh-abbr/zsh-abbr.zsh ZC1073 1 -zsh-abbr/zsh-abbr.zsh ZC1075 61 +zsh-abbr/zsh-abbr.zsh ZC1075 67 zsh-abbr/zsh-abbr.zsh ZC1078 1 zsh-abbr/zsh-abbr.zsh ZC1086 3 zsh-abbr/zsh-abbr.zsh ZC1098 1 @@ -1336,11 +1329,11 @@ zsh-async/async_test.zsh ZC1001 65 zsh-async/async_test.zsh ZC1004 4 zsh-async/async_test.zsh ZC1043 12 zsh-async/async_test.zsh ZC1048 1 -zsh-async/async_test.zsh ZC1075 8 +zsh-async/async_test.zsh ZC1075 5 zsh-async/async_test.zsh ZC1134 7 zsh-async/async_test.zsh ZC1149 1 zsh-async/async_test.zsh ZC1869 1 -zsh-async/async.zsh ZC1001 5 +zsh-async/async.zsh ZC1001 6 zsh-async/async.zsh ZC1043 2 zsh-async/async.zsh ZC1046 1 zsh-async/async.zsh ZC1075 24 @@ -1414,7 +1407,7 @@ zsh-completion-generator/zsh-completion-generator.plugin.zsh ZC1006 1 zsh-completion-generator/zsh-completion-generator.plugin.zsh ZC1010 1 zsh-completion-generator/zsh-completion-generator.plugin.zsh ZC1020 1 zsh-completion-generator/zsh-completion-generator.plugin.zsh ZC1036 1 -zsh-completion-generator/zsh-completion-generator.plugin.zsh ZC1037 1 +zsh-completion-generator/zsh-completion-generator.plugin.zsh ZC1037 2 zsh-completion-generator/zsh-completion-generator.plugin.zsh ZC1075 4 zsh-completion-generator/zsh-completion-generator.plugin.zsh ZC1293 1 zsh-defer/zsh-defer.plugin.zsh ZC1001 2 @@ -1467,7 +1460,7 @@ zsh-histdb/sqlite-history.zsh ZC1271 1 zsh-histdb/sqlite-history.zsh ZC1288 2 zsh-histdb/sqlite-history.zsh ZC1637 1 zsh-history-substring-search/zsh-history-substring-search.zsh ZC1001 3 -zsh-history-substring-search/zsh-history-substring-search.zsh ZC1037 1 +zsh-history-substring-search/zsh-history-substring-search.zsh ZC1037 2 zsh-history-substring-search/zsh-history-substring-search.zsh ZC1043 21 zsh-history-substring-search/zsh-history-substring-search.zsh ZC1046 3 zsh-history-substring-search/zsh-history-substring-search.zsh ZC1075 7 @@ -1475,6 +1468,7 @@ zsh-history-substring-search/zsh-history-substring-search.zsh ZC1076 3 zsh-history-substring-search/zsh-history-substring-search.zsh ZC1091 20 zsh-history-substring-search/zsh-history-substring-search.zsh ZC1098 3 zsh-hist/zsh-hist.plugin.zsh ZC1031 1 +zsh-interactive-cd/zsh-interactive-cd.plugin.zsh ZC1001 1 zsh-interactive-cd/zsh-interactive-cd.plugin.zsh ZC1003 2 zsh-interactive-cd/zsh-interactive-cd.plugin.zsh ZC1010 14 zsh-interactive-cd/zsh-interactive-cd.plugin.zsh ZC1037 15 @@ -1545,7 +1539,6 @@ zsh-vi-mode/zsh-vi-mode.zsh ZC1046 8 zsh-vi-mode/zsh-vi-mode.zsh ZC1055 3 zsh-vi-mode/zsh-vi-mode.zsh ZC1059 1 zsh-vi-mode/zsh-vi-mode.zsh ZC1065 2 -zsh-vi-mode/zsh-vi-mode.zsh ZC1071 2 zsh-vi-mode/zsh-vi-mode.zsh ZC1073 12 zsh-vi-mode/zsh-vi-mode.zsh ZC1075 104 zsh-vi-mode/zsh-vi-mode.zsh ZC1080 1 @@ -1568,6 +1561,6 @@ zsh-z/zsh-z.plugin.zsh ZC1073 6 zsh-z/zsh-z.plugin.zsh ZC1075 30 zsh-z/zsh-z.plugin.zsh ZC1076 1 zsh-z/zsh-z.plugin.zsh ZC1078 3 -zsh-z/zsh-z.plugin.zsh ZC1149 2 +zsh-z/zsh-z.plugin.zsh ZC1128 1 zsh-z/zsh-z.plugin.zsh ZC1337 1 zsh-z/zsh-z.plugin.zsh ZC1833 1 diff --git a/pkg/katas/katatests/fp_round10_test.go b/pkg/katas/katatests/fp_round10_test.go new file mode 100644 index 00000000..7c23cefb --- /dev/null +++ b/pkg/katas/katatests/fp_round10_test.go @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: MIT +// Copyright the ZShellCheck contributors. +package katas + +import ( + "testing" + + "github.com/afadesigns/zshellcheck/pkg/testutil" +) + +// TestZC1075GluedCommandNameNotFlagged pins the glued-command-name false +// positive. A `$var` glued with no separating space to a literal prefix +// (the command-name form `_clear$fsuf`, common in gitstatus) is part of a +// concatenation: the literal prefix keeps the word non-empty, so an empty +// value cannot elide it. The parser splits `name$var` in command position +// into the name plus this glued argument, which previously looked bare. +func TestZC1075GluedCommandNameNotFlagged(t *testing.T) { + for _, src := range []string{ + "_gitstatus_clear$fsuf", + "gitstatus_stop$fsuf", + "prefix$suffix arg", + } { + if n := len(testutil.Check(src, "ZC1075")); n != 0 { + t.Errorf("ZC1075 should not flag a glued-suffix expansion: %q (got %d)", src, n) + } + } + // The glued suffix is skipped, but a following space-separated bare + // expansion in the same command is a real elision hazard and fires. + if n := len(testutil.Check("gitstatus_stop$fsuf $name", "ZC1075")); n != 1 { + t.Errorf("ZC1075 should flag the space-separated `$name` (not the glued `$fsuf`): got %d", n) + } + // A space-separated bare expansion still elides and must fire. + for _, src := range []string{"rm $file", "cd $dir"} { + if n := len(testutil.Check(src, "ZC1075")); n == 0 { + t.Errorf("ZC1075 should still flag a bare space-separated expansion: %q", src) + } + } +} + +// TestZC1075QuoteFlagNotFlagged pins the `(q)`-flag false positive. A +// quoting flag (`q`, `qq`, `q-`, `q+`, or a combination such as `Vq-`) +// renders an empty value as a quoted empty string — a non-empty word — so +// the expansion never elides. +func TestZC1075QuoteFlagNotFlagged(t *testing.T) { + for _, src := range []string{ + "f ${(q)result}", + "f ${(qq)x}", + "f ${(Vq-)result[3]}", + "print -lr ${(qqqq)input[@]}", + } { + if n := len(testutil.Check(src, "ZC1075")); n != 0 { + t.Errorf("ZC1075 should not flag a (q)-quoted expansion: %q (got %d)", src, n) + } + } +} + +// TestZC1071SingleSelfReferenceNotFlagged pins that a single-element +// self-reference is an identity reassignment, not an append, so `+=` does +// not apply. The parser drops the `:#`/`//` modifier, so a filter rebuild +// (`arr=(${arr[@]:#pat})`) is indistinguishable from `arr=(${arr[@]})` at +// the AST level; the element-count guard separates a real append (whole +// array plus at least one new element) from both. +func TestZC1071SingleSelfReferenceNotFlagged(t *testing.T) { + for _, src := range []string{ + "arr=($arr)", + "del_list=(${del_list[@]:#pattern})", + "arr=(${arr[@]//a/b})", + } { + if n := len(testutil.Check(src, "ZC1071")); n != 0 { + t.Errorf("ZC1071 should not flag a single-element rebuild: %q (got %d)", src, n) + } + } + // A whole-array reference followed by appended elements is a real + // append and must still fire. + for _, src := range []string{"arr=($arr a b)", "arr=(${arr[@]} x)"} { + if n := len(testutil.Check(src, "ZC1071")); n == 0 { + t.Errorf("ZC1071 should still flag a true append: %q", src) + } + } +} + +// TestZC1149StderrNotFlagged pins the stderr false positive. A print/echo +// already routed to stderr (`print -u2`, `print -u 2`, or a `>&2` / `2>` +// redirection) has its error message on the right stream, so the kata has +// nothing to recommend. +func TestZC1149StderrNotFlagged(t *testing.T) { + for _, src := range []string{ + `print -u2 "Error: boom"`, + `print -u 2 "Error: boom"`, + `echo "Error: boom" >&2`, + } { + if n := len(testutil.Check(src, "ZC1149")); n != 0 { + t.Errorf("ZC1149 should not flag an error message already on stderr: %q (got %d)", src, n) + } + } + // An error message on stdout still warrants the redirect advice. + for _, src := range []string{`echo "Error: boom"`, `print "Error: boom"`} { + if n := len(testutil.Check(src, "ZC1149")); n == 0 { + t.Errorf("ZC1149 should still flag an error message on stdout: %q", src) + } + } +} + +// TestLogicalChainCompoundBodyLinted is the integration proof that the +// parser walk fix makes katas see code inside a `&&` / `||` compound body. +// A dangerous `rm -rf $unquoted` inside `cmd && { … }` was previously +// invisible; it now draws its findings. +func TestLogicalChainCompoundBodyLinted(t *testing.T) { + for _, src := range []string{ + "(( x > 0 )) && { rm -rf $target }", + "[[ -n $x ]] && { cp $a $b }", + "ok && if true; then rm -rf $target; fi", + } { + if n := len(testutil.Check(src, "ZC1075")); n == 0 { + t.Errorf("a body inside a logical-chain compound should be linted: %q drew no ZC1075", src) + } + } +} diff --git a/pkg/katas/katatests/zc1000s_test.go b/pkg/katas/katatests/zc1000s_test.go index c20a142c..1062bce8 100644 --- a/pkg/katas/katatests/zc1000s_test.go +++ b/pkg/katas/katatests/zc1000s_test.go @@ -3081,16 +3081,13 @@ func TestZC1071(t *testing.T) { expected []katas.Violation }{ { - name: "invalid append self reference single", - input: `arr=($arr)`, - expected: []katas.Violation{ - { - KataID: "ZC1071", - Message: "Appending to an array using `arr=($arr ...)` is verbose and slower. Use `arr+=(...)` instead.", - Line: 1, - Column: 1, - }, - }, + // A bare self-reference with no appended elements is an identity + // reassignment, not an append: `arr+=()` adds nothing, so the + // `+=` advice does not apply. A true append needs at least one + // element after the whole-array reference. + name: "single self reference is not an append", + input: `arr=($arr)`, + expected: []katas.Violation{}, }, { name: "valid append whole array first then new elements", diff --git a/pkg/katas/zc1000s.go b/pkg/katas/zc1000s.go index e1effa8d..1d962bf6 100644 --- a/pkg/katas/zc1000s.go +++ b/pkg/katas/zc1000s.go @@ -5041,8 +5041,15 @@ func checkZC1071(node ast.Node) []Violation { if !ok { return nil } + // A true append needs the whole array reference PLUS at least one + // appended element (`arr=($arr a)` ≡ `arr+=(a)`). A single-element + // rebuild is never an append: it is an identity (`arr=($arr)`) or a + // transform/filter (`arr=(${arr[@]:#pat})`, `arr=(${arr[@]//a/b})`). + // The parser collapses `${arr[@]:#pat}` and a plain `${arr[@]}` to the + // same indexed node — the `:#`/`//` modifier is not preserved — so the + // element-count guard is what separates a filter from an append. arrayLit, ok := infix.Right.(*ast.ArrayLiteral) - if !ok || len(arrayLit.Elements) == 0 { + if !ok || len(arrayLit.Elements) < 2 { return nil } // Only a true append rewrites to `+=`. That means the unmodified @@ -5445,6 +5452,19 @@ func zc1075HasWidthFlag(flags string) bool { return strings.ContainsAny(flags[:i], "lr") } +// zc1075HasQuoteFlag reports whether a `${(flags)name}` flag group +// carries a `q` quoting flag (`q`, `qq`, `q-`, `q+`, or a combination +// such as `Vq-`). A quoted expansion renders an empty value as a quoted +// empty string — a non-empty word — so it never elides. Only the leading +// flag characters (before any `:delimiter:` section) are inspected. +func zc1075HasQuoteFlag(flags string) bool { + head := flags + if i := strings.IndexByte(head, ':'); i >= 0 { + head = head[:i] + } + return strings.IndexByte(head, 'q') >= 0 +} + func zc1075HasDefaultModifier(left ast.Expression) bool { ident, ok := left.(*ast.Identifier) if !ok { @@ -5502,6 +5522,16 @@ func zc1075FlagCommand(cmd *ast.SimpleCommand, arrays map[string]bool, violation // If it's a bare IdentifierNode (variable expansion), it's unquoted. if ident, ok := arg.(*ast.Identifier); ok { + // A `$name` glued to the preceding word with no separating space + // (`prefix$name`, the command-name form `_clear$fsuf`) is part of + // a concatenation: the literal prefix keeps the word non-empty, so + // an empty value cannot elide it. The parser splits `name$var` in + // command position into the name plus this glued argument, so the + // arg looks bare; the no-preceding-space flag is what marks it as a + // concatenation tail. + if !ident.Token.HasPrecedingSpace { + continue + } // Only a bare `$name` expansion can elide to nothing; a suffixed // form like `$dir/foo` keeps a literal tail and never elides. if zc1075IsBareExpansion(ident.Value) && !arrays[zc1075BareName(ident.Value)] { @@ -5539,7 +5569,8 @@ func zc1075FlagArrayAccess(aa *ast.ArrayAccess, arrays map[string]bool, violatio if zc1075HasDefaultModifier(aa.Left) { return } - if zc1075HasSplitFlag(aa.Flags) || zc1075HasWidthFlag(aa.Flags) { + if zc1075HasSplitFlag(aa.Flags) || zc1075HasWidthFlag(aa.Flags) || + zc1075HasQuoteFlag(aa.Flags) { return } // A bare whole-array expansion `${arr}` joins under quoting; an element diff --git a/pkg/katas/zc1100s.go b/pkg/katas/zc1100s.go index 5c2a672f..00b7fe71 100644 --- a/pkg/katas/zc1100s.go +++ b/pkg/katas/zc1100s.go @@ -2597,6 +2597,23 @@ func init() { }) } +// zc1149GoesToStderr reports whether a print/echo command already directs +// its output to stderr — via the `print -u2` / `print -u 2` file-descriptor +// flag or a `>&2` / `2>` redirection word. Such a command needs no fix. +func zc1149GoesToStderr(cmd *ast.SimpleCommand) bool { + for i, arg := range cmd.Arguments { + s := arg.String() + if s == "-u2" || strings.Contains(s, ">&2") || strings.HasPrefix(s, "2>") { + return true + } + // The split `-u 2` form: a `-u` flag followed by the fd `2`. + if s == "-u" && i+1 < len(cmd.Arguments) && cmd.Arguments[i+1].String() == "2" { + return true + } + } + return false +} + func checkZC1149(node ast.Node) []Violation { cmd, ok := node.(*ast.SimpleCommand) if !ok { @@ -2612,30 +2629,44 @@ func checkZC1149(node ast.Node) []Violation { return nil } + // Already routed to stderr: `print -u2 …`, `print -u 2 …`, or any + // `>&2` / `2>` redirection. The error message is on the right stream, + // so the kata has nothing to recommend. + if zc1149GoesToStderr(cmd) { + return nil + } + for _, arg := range cmd.Arguments { - val := arg.String() - // Check for error-like messages - if len(val) > 5 { - clean := val - if len(clean) > 2 && (clean[0] == '\'' || clean[0] == '"') { - clean = clean[1 : len(clean)-1] - } - if len(clean) >= 5 && (clean[:5] == "Error" || clean[:5] == "error" || clean[:5] == "ERROR") { - return []Violation{{ - KataID: "ZC1149", - Message: "Error messages should go to stderr. Use `print -u2` or append `>&2` " + - "to separate error output from normal stdout.", - Line: cmd.Token.Line, - Column: cmd.Token.Column, - Level: SeverityInfo, - }} - } + if zc1149IsErrorPrefixed(arg.String()) { + return []Violation{{ + KataID: "ZC1149", + Message: "Error messages should go to stderr. Use `print -u2` or append `>&2` " + + "to separate error output from normal stdout.", + Line: cmd.Token.Line, + Column: cmd.Token.Column, + Level: SeverityInfo, + }} } } return nil } +// zc1149IsErrorPrefixed reports whether a print/echo argument is a string +// that begins with `Error`/`error`/`ERROR` (with surrounding quotes +// stripped) — the diagnostic-message shape the kata routes to stderr. +func zc1149IsErrorPrefixed(val string) bool { + if len(val) <= 5 { + return false + } + clean := val + if len(clean) > 2 && (clean[0] == '\'' || clean[0] == '"') { + clean = clean[1 : len(clean)-1] + } + return len(clean) >= 5 && + (clean[:5] == "Error" || clean[:5] == "error" || clean[:5] == "ERROR") +} + func init() { RegisterKata(ast.SimpleCommandNode, Kata{ ID: "ZC1151", diff --git a/pkg/parser/parser_corpus_fixes_test.go b/pkg/parser/parser_corpus_fixes_test.go index 9b0def7b..0c0145e0 100644 --- a/pkg/parser/parser_corpus_fixes_test.go +++ b/pkg/parser/parser_corpus_fixes_test.go @@ -5,6 +5,7 @@ package parser import ( "testing" + "github.com/afadesigns/zshellcheck/pkg/ast" "github.com/afadesigns/zshellcheck/pkg/lexer" ) @@ -490,3 +491,39 @@ func TestParseAssignmentGluedQuoteTail(t *testing.T) { } } } + +// A compound statement on the right of a `&&` / `||` logical chain +// (`cmd && { … }`, `cmd && if …; fi`, `cmd && for …; done`, +// `cmd && case … esac`) must keep its body in the tree so katas walk it. +// keywordStmtToExpression previously wrapped the compound in a stub +// Identifier, hiding the body — dangerous code inside the block went +// unlinted. Each real node implements Expression and is now returned +// directly; the walker descends into the body. Proxy for "body walked": +// the inner `rm` SimpleCommand is reachable (two SimpleCommands total). +func TestParseLogicalChainCompoundBodyWalked(t *testing.T) { + cases := []string{ + "x && { rm -rf $y }\n", + "x || { rm -rf $y }\n", + "x && if a; then rm -rf $y; fi\n", + "x && for i in a; do rm -rf $y; done\n", + "x && case $z in a) rm -rf $y ;; esac\n", + "x && select n in a; do rm -rf $y; done\n", + } + for _, src := range cases { + p := New(lexer.New(src)) + prog := p.ParseProgram() + if errs := p.Errors(); len(errs) != 0 { + t.Fatalf("unexpected parser errors for %q: %v", src, errs) + } + n := 0 + ast.Walk(prog, func(node ast.Node) bool { + if _, ok := node.(*ast.SimpleCommand); ok { + n++ + } + return true + }) + if n < 2 { + t.Errorf("compound RHS body not walked for %q: want >=2 SimpleCommands, got %d", src, n) + } + } +} diff --git a/pkg/parser/parser_stmt.go b/pkg/parser/parser_stmt.go index 23ff1085..b50bfc68 100644 --- a/pkg/parser/parser_stmt.go +++ b/pkg/parser/parser_stmt.go @@ -460,9 +460,18 @@ func keywordStmtToExpression(stmt ast.Statement) ast.Expression { if es, ok := stmt.(*ast.ExpressionStatement); ok { return es.Expression } - // Wrap in a stub Identifier so callers see a non-nil expression. - // The Token preserves the head keyword for kata-side walks of - // containing CallExpression / DollarParenExpression bodies. + // Compound statements (BlockStatement / IfStatement / ForLoopStatement + // / CaseStatement / SelectStatement) all implement Expression. Return + // the real node so the AST walker descends into the body. A stub + // Identifier hid the body from every kata, so dangerous code inside a + // logical-chain compound (`cmd && { rm -rf $x }`, `cmd && if …; then + // rm -rf $x; fi`) went unlinted. The WHILE head already returns its + // statement directly; this brings the other compound heads in line. + if expr, ok := stmt.(ast.Expression); ok { + return expr + } + // Fallback stub for a non-Expression statement: callers still see a + // non-nil expression carrying the head keyword. return &ast.Identifier{Token: stmt.TokenLiteralNode(), Value: stmt.TokenLiteral()} }