shellcheck icon indicating copy to clipboard operation
shellcheck copied to clipboard

Add maintainability/complexity creep checks

Open mcornils opened this issue 8 years ago • 4 comments

Feature suggestions:

  • number of total lines in a shell script over a reasonable limit (recommend splitting it up in included files)
  • number of non-function lines (main part of script) over a reasonable limit (recommend splitting in functions)
  • number of lines in a function (recommend splitting it up)
  • cyclomatic complexity of a function (recommend simplifying it/splitting it up)
  • number of statements and/or characters in one line over a reasonable limit (add line breaks)

mcornils avatar Nov 30 '15 14:11 mcornils

I'm a bit dubious about imposing arbitrary limits, even just as warnings. A badly written 30-line script can be far more difficult to understand than a well-written 1000-line script. Splitting up into files and functions has some side effects that need to be taken into account, particularly the separation of parsing and running, and in the case of files, just locating them is a fraught problem in its own right. And how should cyclomatic complexity be measured? (May I suggest that variable assignments should carry negative weight in such a calculation?)

kurahaupo avatar Nov 30 '15 20:11 kurahaupo

this can be a command line option with e.g.

--checkstyle default|lazy|agressive

with different settings. Ofcourse the coding style differs from project to project, but we can at least give some values. For instance it is a good idea for functions() not beeing too wide (e.g. 80 or 120 chars) and not beeing longer than 1 screen (e.g. 45 lines). Dont get the numbers too serious, these are just "our" values...

bittorf avatar Dec 01 '15 08:12 bittorf

Hi,

the suggestions are mostly based on what Atlassian's SonarQube does for Java code (and other languages). That by itself does not mean they're valuable for shellcheck; but I think most apply to shell code too, if the limits (and severities) are not too strict and/or configurable. Besides adding that SonarQube calculates the CC, I cannot help much as my knowledge of Haskell is... well, almost non-existent. SonarQube allows to specify false positives, if you have the rare case of a one-function 1000-line script that is indeed written well. Also, tests can be generally skipped if they do not apply for your project. Anyway, these warnings do not automatically prevent writing bad scripts, but they aid in finding out where one should consider refactoring (some "dumb" ones, like the raw total line-count, are of comparatively low severity in SonarQube).

mcornils avatar Dec 01 '15 16:12 mcornils

For reference implementation: https://github.com/shellspec/shellmetrics CC @qequ

dgutson avatar Sep 12 '22 16:09 dgutson