psprint/zsh-sweep

FUNCTION_SETS_GLOBAL_VARS does not understand context

Opened this issue · 2 comments

The global vars check creates a lot of false positives because of how simple it is. It seems to ignore almost all context. Consider this example:

(
  a='a'
)

bar() (
  b='b'
)

c='c' true

local arr=(
  d='d'
)

e='e'; export e

python - <<'EOF'
f='f'
print(f)
EOF

All of those variable-like patterns should be fine ('d' and 'f' are not even variables), but zsh-sweep complains about all of them. 'f' is the most problematic, because it's not always practical to add a shell-style comment inside of a heredoc in order to suppress it. Another problem is that zsh-sweep complains about implicitly global variables being set outside of functions, even in --script mode, where there is no practical difference between local and global AFAIK.

Notice: running check: CHECK_FUNCTION_SETS_GLOBAL_VARS_PRE…
Notice: running check: CHECK_FUNCTION_SETS_GLOBAL_VARS_POST…
[ZSweep][zsfilt:184]: Error: Error near line #2 ↔  a='a':
[ZSweep][zsfilt:184]: Error: Error near line #6 ↔  b='b':
[ZSweep][zsfilt:184]: Error: Error near line #9 ↔ c='c' true:
[ZSweep][zsfilt:184]: Error: Error near line #12 ↔  d='d':
[ZSweep][zsfilt:184]: Error: Error near line #15 ↔ e='e'; export e:
[ZSweep][zsfilt:184]: Error: Error near line #18 ↔ f='f':
[ZSweep][zscan:125]: Warning: A global variable has been created without declaration, by assignment. Add a line like local VAR or typeset -g VAR so that the variable is defined before use.

Less concerning are the false negatives like this, which zsh-sweep does not see as variables:

{ a='a' }

foo() { b='b' }
(
  a='a'
)

I think that a should be reported here. Was your thought that var created in subshell isn't a global? I think that it's good to expect a typeset in (…), for readability.

I'm reading the error code as "a global was implicitly created inside of a function", and I see no function in that example. I suppose if you go by the description it might be referring to any undeclared variable, which seems like a purely stylistic note and not a diagnostic, since typeset can be used before or after a value is assigned to a variable. In that case, the error code should probably be changed to make its meaning clearer.