[PATCH 2/2] syntax-check: Introduce sc_prohibit_local_with_subshell rule

Michal Privoznik via Devel posted 2 patches 2 weeks, 2 days ago
There is a newer version of this series
[PATCH 2/2] syntax-check: Introduce sc_prohibit_local_with_subshell rule
Posted by Michal Privoznik via Devel 2 weeks, 2 days ago
From: Michal Privoznik <mprivozn@redhat.com>

As mentioned in the previous commit, the following function
doesn't echo '1' but '0':

  func() {
      local var=$(false)
      echo $?
  }

It's explained here [1]. Since this kind of error is not easy to
catch a new syntax-check rule is introduced. To avoid having
multiline grep and match only those patterns where '$?' is
examined, let's keep the rule simple and forbid all local
declarations with subshell.

1: https://www.shellcheck.net/wiki/SC2155
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 build-aux/syntax-check.mk | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index f605c9b0e3..ff44dfa2fe 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -394,6 +394,12 @@ sc_prohibit_g_autofree_const:
 	halt='‘g_autofree’ discards ‘const’ qualifier from pointer target type' \
 	  $(_sc_search_regexp)
 
+sc_prohibit_local_with_subshell:
+	@prohibit='local [a-zA-Z]+="?\$$\(.+' \
+	in_vc_files='\.sh(\.in)?$$' \
+	halt='local variable with subshell does not do what you think it does' \
+	  $(_sc_search_regexp)
+
 
 # Many of the function names below came from this filter:
 # git grep -B2 '\<_('|grep -E '\.c- *[[:alpha:]_][[:alnum:]_]* ?\(.*[,;]$' \
-- 
2.52.0

Re: [PATCH 2/2] syntax-check: Introduce sc_prohibit_local_with_subshell rule
Posted by Martin Kletzander via Devel 2 weeks, 2 days ago
On Fri, Jan 16, 2026 at 10:22:14AM +0100, Michal Privoznik via Devel wrote:
>From: Michal Privoznik <mprivozn@redhat.com>
>
>As mentioned in the previous commit, the following function
>doesn't echo '1' but '0':
>
>  func() {
>      local var=$(false)
>      echo $?
>  }
>
>It's explained here [1]. Since this kind of error is not easy to
>catch a new syntax-check rule is introduced. To avoid having
>multiline grep and match only those patterns where '$?' is
>examined, let's keep the rule simple and forbid all local
>declarations with subshell.
>
>1: https://www.shellcheck.net/wiki/SC2155

Man page of bash(1) and bash_builtins(1) both explain it well, too.

>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> build-aux/syntax-check.mk | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
>index f605c9b0e3..ff44dfa2fe 100644
>--- a/build-aux/syntax-check.mk
>+++ b/build-aux/syntax-check.mk
>@@ -394,6 +394,12 @@ sc_prohibit_g_autofree_const:
> 	halt='‘g_autofree’ discards ‘const’ qualifier from pointer target type' \
> 	  $(_sc_search_regexp)
>
>+sc_prohibit_local_with_subshell:
>+	@prohibit='local [a-zA-Z]+="?\$$\(.+' \

this won't catch variables with underscores etc. in the name, neither
will it catch (if anyone ever touches the script again and does such a
thing) an array initialization, e.g.:

     local domains=( $(run_virsh list --uuid) )

not to mention that there shouldn't be anything than a space before the
`local`, so I'd suggest we instead do something like:

     @prohibit='^ *local .*='

or, if we do want to catch someone using local x=... with TAB for
indentation before the sc_TAB_in_indentation warns them, then:

     @prohibit='^\s*local .*='

and if you want to be super-duper-extra careful, then

     @prohibit='^\s*(local|export|declare|readonly) .*='

>+	in_vc_files='\.sh(\.in)?$$' \
>+	halt='local variable with subshell does not do what you think it does' \

Well, it does what I think.  Maybe word it differently, like "don't use
that" or "this way you cannot know the exit code" or something.

>+	  $(_sc_search_regexp)
>+
>
> # Many of the function names below came from this filter:
> # git grep -B2 '\<_('|grep -E '\.c- *[[:alpha:]_][[:alnum:]_]* ?\(.*[,;]$' \
>-- 
>2.52.0
>