Record the command's id in CFSetExitCode for built-in handlers

[Eljees]

Fixes #3490.

Symptom

SC2320 fires for echo but not for printf:

#!/bin/sh
mycommand
printf 'Command exited with %d\n' $?
if [ $? -ne 0 ]      # no SC2320 here
then
  echo "Failed"
fi

A four-way probe puts the discriminator on the command name rather than on quoting:

script	before	after
printf '...%d' $?	silent	SC2320
printf '...%d' "$?"	silent	SC2320
echo "...$?"	SC2320	SC2320
echo $?	SC2320	SC2320

Cause

handleCommand cmd vars args literalCmd receives the whole T_SimpleCommand as cmd, and the ordinary path registers the exit code against it:

regular = handleOthers (getId cmd) vars args literalCmd

regularExpansionWithStatus, which the built-in table uses, shadows that name with the command's first word:

regularExpansionWithStatus vars args@(cmd NE.:| _) p = do
    initial <- regularExpansion vars (NE.toList args) p
    status  <- newNodeRange $ CFSetExitCode (getId cmd)   -- id of the word, not the command

So printf, unset, wait, mapfile, readarray, read and the four DEFINE_* commands all register their exit code under the id of a T_NormalWord. A consumer that resolves that id through idMap then gets a word where it expects a command, and checkOverwrittenExitCode is exactly such a consumer: getCommandBasename is Nothing for a T_NormalWord, so isPrinting never matches. echo is not in the table, goes through handleOthers, and therefore works.

Fix

Drop the shadowing pattern so cmd refers to the command again, matching handleOthers. One token; the body of the helper is unchanged.

Tests

Two prop_ cases next to the existing ones:

prop_checkOverwrittenExitCode9  = verify    checkOverwrittenExitCode "x; printf '%d' $?; [ $? -eq 0 ]"
prop_checkOverwrittenExitCode10 = verifyNot checkOverwrittenExitCode "read -r x; [ $? -eq 0 ]"

The second pins the intended narrowness: read also goes through the helper and also gets a corrected id, but it is neither a condition nor a printing command, so nothing new is reported for it.

Verified on GHC 9.8.4:

• with the tests but without the fix, cabal test fails (*** Failed! Falsified (after 1 test), Test suite test-shellcheck: FAIL);
• with the fix, cabal test passes in full;
• end-to-end per CLAUDE.md: the reporter's script now reports SC2320, echo is unchanged, and mycommand; unset foo; [ $? -eq 0 ] stays silent apart from the unrelated SC2181.

AI usage

I used Claude to help trace the CFG path and to draft the patch and the two tests, following the workflow in this repository's .claude/CLAUDE.md. I read every line of the change, ran the four-way probe to establish the cause rather than assume it, ran the new tests against unpatched master first to confirm they fail without the fix, and ran the full cabal test and the end-to-end checks myself.

[Eljees]

Ping — this one and #3517 have been open since 9 August with no review.

Both are green (16 checks each) and independent of each other, so they can be taken in either order.

[e-kwsm]

POSIX specifies that printf shall return nonzero when an error occurs.

[ $# -eq 1 ] || exit
printf '%d' "$1" > /dev/null 2>&1
if [ $? -eq 0 ]; then
  echo "$1 is an integer"
else
  echo "$1 is NOT an integer"
fi

The snippet above checks whether $1 is an integer or not, and does not take $?, but shellcheck 566b295 issues SC2181.

ksh 2020.0.0 returns zero, whereas the followings return one:

• dash 0.5.13.4
• bash 5.3.15
• busybox sh 1.36.1
• GNU coreutils 9.11
• bsdutils 13.2

[Eljees]

SC2181 does not come from this PR. It is emitted by checkReturnAgainstZero in src/ShellCheck/Analytics.hs, which works purely on the parse tree via parentMap/getPath and never reads the CFG. This PR touches one line in CFG.hs (regularExpansionWithStatus) and adds two prop_checkOverwrittenExitCode cases; the check it fixes is SC2320.

As a control I ran your snippet through the 0.11.0 release binary, which predates this branch: it reports the same SC2181 (style) on if [ $? -eq 0 ]; then. So the diagnostic you are seeing is pre-existing behaviour that this branch leaves untouched.

On the POSIX point you are right — printf '%d' is required to exit nonzero on a non-numeric operand, and ksh 2020.0.0 is the outlier there.

If SC2320 is the check you actually meant, tell me and I will re-check on your snippet. And if you think SC2181 should not fire on that idiom, that is a separate question from this PR — I am happy to look at it in its own issue.

[e-kwsm]

I meant SC2181 is true positive but SC2320 is false positive, as it checks the return code of printf.

[Eljees]

You are right that SC2320 fires there and that it is wrong for that idiom - POSIX requires printf '%d' to exit non-zero on a non-numeric operand, so $? is exactly what the author wants.

What this PR changes is only that printf now behaves the way echo already does. Built from 566b295 and from master, same files:

script	master	this PR
printf '%d' "$1" >/dev/null 2>&1 then if [ $? -eq 0 ]	SC2181	SC2320 + SC2181
echo "$1" >/dev/null 2>&1 then if [ $? -eq 0 ]	SC2320 + SC2181	SC2320 + SC2181
somecmd; printf 'done\n'; [ $? -eq 0 ]	SC2181	SC2320 + SC2181
printf '%d' "$1" >/dev/null 2>&1; status=$?	clean	SC2320
echo "$1" >/dev/null 2>&1; status=$?	SC2320	SC2320
if printf '%d' "$1" >/dev/null 2>&1; then	clean	clean

Every printf row has an echo twin that master already reports the same way. So the false positive lives in SC2320's premise - isPrinting assumes nobody wants echo's or printf's own status - and not in the CFG id this PR corrects. #3490 asked for the two to be consistent; row 3 is the case it was filed for.

If the conclusion is that neither should warn when the status is used directly, that is a change to checkOverwrittenExitCode rather than to the CFG, and I am happy to take it on as its own PR.

Worth noting separately: rows 4 and 5 show the warning survives the fix it recommends. status=$? immediately after the printing command still gets "Assign to variable to avoid it being overwritten", on master for echo and here for printf too. SC2319 has a usedUnconditionally guard for that shape; SC2320 has none.

Tell me which way you would like this to go and I will follow it.