SC2178: Tell the user about the special MAPFILE variable

jkirk commented 1 year ago

For bugs

Rule Id: SC2178
My shellcheck version (shellcheck --version or "online"): online
[x] The rule's wiki page does not already cover this (e.g. https://shellcheck.net/wiki/SC2178)
[x] I tried on https://www.shellcheck.net/ and verified that this is still a problem on the latest commit

For new checks and feature suggestions

[x] https://www.shellcheck.net/ (i.e. the latest commit) currently gives no useful warnings about this
[x] I searched through https://github.com/koalaman/shellcheck/issues and didn't find anything related

Here's a snippet or screenshot that shows the problem:

#!/bin/bash

MAPFILE="mapfile.txt"
IFS=";"

echo "$MAPFILE"

Here's what shellcheck currently says:

$ shellcheck myscript

[Line 3:](javascript:setPosition(3, 1))
MAPFILE="mapfile.txt"
^-- [SC2178](https://www.shellcheck.net/wiki/SC2178) (warning): Variable was used as an array but is now assigned a string.

[Line 6:](javascript:setPosition(6, 7))
echo "$MAPFILE"
      ^-- [SC2128](https://www.shellcheck.net/wiki/SC2128) (warning): Expanding an array without an index only gives the first element.

Here's what I wanted or expected to see:

Look at this:

#!/bin/bash

FILE="mapfile.txt"
IFS=";"

echo "$FILE"

$ shellcheck myscript
No issues detected!

I am not sure, if a MAPFILE variable itself is a problem when not using the bash builtin mapfile, but please let the user know, that there IS such a builtin and variable and/or that this variable name is reserved. The SC2178 wiki page (currently) does not mention MAPFILE.

Thank you very much for shellcheck! 😄

brother commented 1 year ago

The wiki is open to edits if you think it is important to extend the page. I am not sure really. And I am not actually sure that you (or I?) have parsed the manual text about the variable MAPFILE correct. As far as Ican tell the the default variable used for storage by the mapfile command if no target variable is specified is MAPFILE. A UPPERCASE variable named MAPFILE is not treated as an array just because of the name (your second example is a bit unclear because of the name change in this regard).

Hints on using the mapfile command are present in SC2046, SC2206 and SC2207.

jkirk commented 1 year ago

Thx @brother for your quick reply!

I somehow was not aware that this Wiki is open for everyone (funny how the visual impression can mislead one (or me(!): official wiki page vs Github Wiki apge).

But I am by no means (or at least I wouldn't call myself that way) an expert in bash programming (that's why I use shellcheck in the first place 😉 - and I also haven't heard of the mapfile builtin before), so I do not really feel confident in editing the Wiki all by myself (there are no PRs which could be reviewed). So I need some advice and hope I can ask for help here.

About the MAPFILE variable: Quoting bash-builtins(7):

   mapfile [-d delim] [-n count] [-O origin] [-s count] [-t] [-u fd] [-C callback] [-c quantum] [array]
   [...]
   Read lines from the standard input into the indexed array variable array, or from file descriptor fd if the -u option is supplied.
   The variable MAPFILE is the default array.

And yes, I read it the same way. If no array (aka target variable) is given, the array variable will be called MAPFILE.

That said, I would also say - as you already said -, an uppercase variable named MAPFILE should not be treated as an array, just because of the name. I was very confused about why shellcheck issued a SC2178-warning because I "accidentally" named my variable MAPFILE.

So if an uppercase variable name MAPFILE is nothing special, I'd call this a bug. If yes, I'd change the title.

koalaman / shellcheck