Rule javascript:S2703 (BLOCKER Code Smell) states:
“JavaScript variable scope can be particularly difficult to understand and get right. The situation gets even worse when you consider the accidental creation of global variables, which is what happens when you declare a variable inside a function or the for
clause of a for-loop without using the let
, const
or var
keywords.”
This is true, but sometimes a prior developer actually intends to set a global variable subsequently read elsewhere, making this a false positive. Completing the ‘fix’ of a local-scope declaration would actually break functionality.
Contrary to the “2min effort” nominally associated with this type of issue, actually fixing this might require hours of manually reviewing the thousands of occurrences of that token in the project to determine if any of them are reading from a globally-scoped variable with the same name, prior to either fixing the issue as suggested or closing it as a false positive. Efficiency & accuracy would be improved if Sonarqube could also scan the rest of the project to find all other uses of the same token in reference to a variable of global scope (e.g. not declared within a non-global scope of the read).
If there are any reads of the global other than in the same function/context where this issue is being flagged, Sonarqube should say where those are (they might also be bugs, especially if the same token is implicitly-global twice) and if there aren’t any, Sonarqube should say that too. Only when there’s confidence that the globally-scoped variable is never read is this a “2min effort.” Sonarqube could also likely determine whether a correct local declaration would be var
(e.g. if read before declaration), let
(e.g. if set after declaration), or const
, and be more specific in its suggestion.
Sonarqube version: Version 8.9.5 (build 50698)