Skip to content

Conversation

@Flu
Copy link
Contributor

@Flu Flu commented Jan 7, 2025

Adds an optional check for triggering a SC2331 for variables that are declared (and/or initialized) but never assigned to again. If a variable is found that matches, a note is given to maybe declare it as readonly.

Also implemented tests for it, as well as for the option itself in Checker.hs

In accordance to #2987, but it's not the full issue.

Let me know if you want to implement the full issue suggestion, as part of the same optional check, another optional check, etc.

Repository owner deleted a comment from aaaass5566 Mar 11, 2025
@koalaman
Copy link
Owner

This a neat check, but unfortunately ShellCheck's poor DFA makes it a lot trickier than it ought to be:

  • It doesn't handle loops, e.g. for file in *; do name=${f%.*}; echo "$name.txt"; done will trigger for both file and name
  • Similarly, functions like f() { last=$1; }; f bar; echo $last; f baz; echo $last;
  • [ -z "$file" ] triggers for $file due to the original DFA algorithm treating this as a signal that the user has checked the variable for existence (awful hack on my part)

ShellCheck does have a significantly more correct DFA algorithm that does track control flow properly (ShellCheck.CFG), but it is correspondingly more complex and would require a whole lot more compiler type graph operations in order to determine that the variable is only set once.

@Flu
Copy link
Contributor Author

Flu commented Oct 7, 2025

ShellCheck does have a significantly more correct DFA algorithm that does track control flow properly (ShellCheck.CFG), but it is correspondingly more complex and would require a whole lot more compiler type graph operations in order to determine that the variable is only set once.

So does this mean that using ShellCheck.CFG would make this kind of check possible, but it would be complicated to take into account all of the possible edge cases? Cause if so, I could still try. I don't know if it's worth the effort for such a small and optional check, but I could give it a shot if it's technically possible

@koalaman koalaman force-pushed the master branch 4 times, most recently from 50074dc to eac8eff Compare November 5, 2025 03:37
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.

2 participants