Skip to content

fix(linter): align S001 indirect expansion parity#596

Merged
ewhauser merged 1 commit intomainfrom
codex/s001-indirect-parity
Apr 25, 2026
Merged

fix(linter): align S001 indirect expansion parity#596
ewhauser merged 1 commit intomainfrom
codex/s001-indirect-parity

Conversation

@ewhauser
Copy link
Copy Markdown
Owner

@ewhauser ewhauser commented Apr 25, 2026

Summary

  • Align S001 indirect expansion handling with the ShellCheck SC2086 oracle: visible safe indirection names such as name=target; echo ${!name} are now treated as safe for S001, while dynamic names such as name=$1; echo ${!name} still report.
  • Remove the two resolved indirect-expansion records from crates/shuck-cli/tests/testdata/corpus-metadata/s001.yaml.
  • Add regression coverage for literal, loop-provided, and dynamic indirect expansion names.
  • Add docs/S001_BUGS.md to classify the current reviewed S001 divergence buckets and track the remaining work, including the rule that fixed records must also be removed from S001 corpus metadata.

Root Cause

S001 was checking whether the resolved indirect target value was safe. Black-box ShellCheck probes show SC2086 instead suppresses based on whether the visible indirection carrier is field-safe.

Remaining Arithmetic/Operator Notes

The remaining two records in the arithmetic/operator bucket are still open and remain in metadata. Follow-up probing showed they are separate shapes:

  • meshrdf_generate_table.sh:3374: likely needs a narrow field-safe rule for assignments whose whole RHS is an arithmetic expansion, not any value that merely contains arithmetic.
  • dehydrated:1077: likely depends on helper/call-order value knowledge for safe literal assignments before substring expansion, not a broad substring suppression.

Validation

  • cargo test -p shuck-linter indirect_ -- --nocapture
  • cargo test -p shuck-linter unquoted_expansion
  • cargo fmt --check
  • make large-corpus-report SHUCK_LARGE_CORPUS_RULES=S001

The targeted corpus report remains structurally clean with implementation_diffs=0 and mapping_issues=0; reviewed divergences dropped from 72 to 70.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 25, 2026

Macro CLI Benchmark Deltas

Compared 667724b against macro CLI baseline af9a089.

Positive deltas mean slower shuck check --no-cache runs.

  • Baseline source: downloaded from the latest successful main CI artifact
  • Baseline preparation: success
  • Comparison run: success
  • Workflow run: details

Aggregate

Benchmark Baseline Head Delta
all 183.47 ms +/- 7.92 ms 170.03 ms +/- 8.66 ms -7.32%

Flagged aggregate regressions over +5.0%: 0

Per-fixture macro deltas
Benchmark Baseline Head Delta
all 183.47 ms +/- 7.92 ms 170.03 ms +/- 8.66 ms -7.32%
fzf-install 10.77 ms +/- 66.95 us 10.79 ms +/- 164.62 us +0.16%
homebrew-install 20.82 ms +/- 220.02 us 21.15 ms +/- 424.25 us +1.59%
nvm 124.48 ms +/- 4.72 ms 121.88 ms +/- 2.82 ms -2.09%
pyenv-python-build 71.06 ms +/- 2.77 ms 67.18 ms +/- 1.17 ms -5.46%
ruby-build 41.34 ms +/- 893.37 us 40.85 ms +/- 464.64 us -1.20%
Largest changes

Regressions

Benchmark Baseline Head Delta
homebrew-install 20.82 ms +/- 220.02 us 21.15 ms +/- 424.25 us +1.59%
fzf-install 10.77 ms +/- 66.95 us 10.79 ms +/- 164.62 us +0.16%

Improvements

Benchmark Baseline Head Delta
all 183.47 ms +/- 7.92 ms 170.03 ms +/- 8.66 ms -7.32%
pyenv-python-build 71.06 ms +/- 2.77 ms 67.18 ms +/- 1.17 ms -5.46%
nvm 124.48 ms +/- 4.72 ms 121.88 ms +/- 2.82 ms -2.09%
ruby-build 41.34 ms +/- 893.37 us 40.85 ms +/- 464.64 us -1.20%
Non-zero benchmark exit codes
  • all baseline exit codes: 1,1,1,1,1,1,1,1,1,1; head exit codes: 1,1,1,1,1,1,1,1,1,1
  • fzf-install baseline exit codes: 1,1,1,1,1,1,1,1,1,1; head exit codes: 1,1,1,1,1,1,1,1,1,1
  • nvm baseline exit codes: 1,1,1,1,1,1,1,1,1,1; head exit codes: 1,1,1,1,1,1,1,1,1,1
  • pyenv-python-build baseline exit codes: 1,1,1,1,1,1,1,1,1,1; head exit codes: 1,1,1,1,1,1,1,1,1,1

@ewhauser ewhauser force-pushed the codex/s001-indirect-parity branch from d99006e to 667724b Compare April 25, 2026 23:13
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d99006e7b4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/shuck-linter/src/rules/common/safe_value.rs
@ewhauser ewhauser merged commit 6341fc9 into main Apr 25, 2026
19 checks passed
@ewhauser ewhauser deleted the codex/s001-indirect-parity branch April 25, 2026 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant