shellspec / shellmetrics

Cyclomatic Complexity Analyzer for bash, mksh, zsh and POSIX shells
MIT License
52 stars 5 forks source link

A syntax analysis problem #3

Open lutzmad opened 4 years ago

lutzmad commented 4 years ago

Hello, nice tool, works well with AIX 7.2.1.4 and bash 4.3.30. The shellmetrics 0.5.0 tool pass all your tests, after I changed all "run script" statements to "run" (I use shellspec 0.20.2). Unfortunately my favorit shell, the "ksh", is not supported today. On the other hand shellspec and shellmetrics does not support "csh" or "tcsh" also.

I do some tests based of my scripts, the calculated value seems to be correct. But sometimes your syntax analysis will not fit.

#!/bin/bash
function func1 { echo '1'; }
case "$1" in
  1) func1 ;&   << should handled like ;;
  2) func1 ;;
  3|5) echo '3' ;;  << only one path is counted, but this are two
  4) echo '4' ;;
  *) : ;;   
esac
exit 0

On the other hand, the number of pathes are not counted well for some "if" statements also.

#!/bin/bash
function func1 { echo '1'; }
if [ "$1" -lt 0 -a "$2" -gt 10 ]; then  << only one path is counted, but this are two
  func1
fi
exit 0

In a short form, nice tool. The most other tools I know are not able to handle concatenated "if" or "case" terms and the old McCabe samples does not count the number of all pathes also, in these circumstances. But you should add ";&" to your list of closeing statements, like the ";;", this is available in the bash also (but used only in "ksh" mostly).

A nice tool to give me some useful information to prepare new tests. With regards, Lutz

ko1nksm commented 4 years ago

Hi @lutzmad, thank you for reporting.

Unfortunately my favorit shell, the "ksh", is not supported today.

Unfortunately ksh cannot support. This is ksh limitation.

Parsing shell script code is very difficult. In fact, ShellMetrics does very few code parsing. Supported shells has built in code formatter. I don't think it was made as a code formatter, but when display defined function, it will be formatted.

$ bash -c 'foo() { case a in (*) echo ok; esac }; typeset -f foo'
foo ()
{
    case a in
        *)
            echo ok
        ;;
    esac
}

Using this technique greatly reduces the code patterns to be analyzed. However, ksh displays defined function as is it.

$ ksh -c 'foo() { case a in (*) echo ok; esac }; typeset -f foo'
foo() { case a in (*) echo ok; esac };

To support ksh, we need true shell script parser. There seem to be some parsers, but I haven't tried them.

By the way, I think the syntax of ksh is almost the same as bash, so I think you can parse it using bash. Do you know the ksh syntax that bash cannot parse? I would like to know that syntax.

On the other hand shellspec and shellmetrics does not support "csh" or "tcsh" also.

ShellSpec and ShellMetrics is targeted POSIX compliant shells. It cannot be supported because the syntax of csh and tcsh is very different. And I am not familiar with csh and tcsh.

1) func1 ;& << should handled like ;;

Oh. It is a bashism (kshism) I forgot. There is also ;;&. This is probably easy to support. But I little busy now. I welcome the pull request. 😀

3|5) echo '3' ;; << only one path is counted, but this are two

if [ "$1" -lt 0 -a "$2" -gt 10 ]; then << only one path is counted, but this are two

ShellMetrics does not support styles like foo && bar. To tell the truth, I am not familiar with how to calculate cyclomatic complexity. There is some variants and I think depends on it. I know there is CCN1, CCN2 and modified CCN aka CCN3.

https://blog.feabhas.com/2018/07/code-quality-cyclomatic-complexity/ https://www.fujitsu.com/jp/group/fst/en/services/pgr/function/mccabe.html

lizard has --modified option.

-m, --modified Calculate modified cyclomatic complexity number, which count a switch/case with multiple cases as one CCN.

We need some more research is needed before making any changes.

I think first example ( 3|5) ) can be supported. Removing the spaces around | as preprocessing, and formatting, then it can be distinguished from the string because space is added.

$ bash -c 'foo() { case a in a|b) :; esac; echo "a|b)"; }; typeset -f foo'
foo ()
{
    case a in
        a | b)
            :
        ;;
    esac;
    echo "a|b)"
}

Second example ([ "$1" -lt 0 -a "$2" -gt 10 ]) is difficult. However, I do not recommend using it because ShellCheck warns you.

if [ "$1" -lt 0 -a "$2" -gt 10 ]; then
                ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

For more information:
  https://www.shellcheck.net/wiki/SC2166 -- Prefer [ p ] && [ q ] as [ p -a q...
lutzmad commented 4 years ago

Hello Koichi Nakashima, thanks for your detailed answer.

To support ksh, we need true shell script parser. There seem to be some parsers, but I haven't tried them.

By the way, I think the syntax of ksh is almost the same as bash, so I think you can parse it using bash. Do you know the ksh syntax that bash cannot parse? I would like to know that syntax.

On the other hand shellspec and shellmetrics does not support "csh" or "tcsh" also.

ShellSpec and ShellMetrics is targeted POSIX compliant shells. It cannot be supported because the syntax of csh and tcsh is very different. And I am not familiar with csh and tcsh.

    func1 ;& << should handled like ;;

Oh. It is a bashism (kshism) I forgot. There is also ;;&. This is probably easy to support. But I little busy now. I welcome the pull request. :-)

You are right, I use the bash mode of shellspec and shellmetrics to check my ksh scripts. This works well in general. As long as the bash 4 is available. The only new syntax moved from ksh to bash is ;& (in case). And ;;& became available with zsh. Yes, csh and tcsh are not POSIX compliant and use a different syntax.

Thanks for your nice tools, with regards, Lutz

p.s. Sorry for the sample, shellcheck does not aggree to my way to write scripts. But the suggestions from chellcheck are sometimes not verry usefull, if you are on AIX systems. The shellcheck tool is a nice tool to check Linux shell scripts.