emacs-vs / codemetrics

Plugin shows complexity information
GNU General Public License v3.0
66 stars 7 forks source link

Lua tests and fix for excessive binary_expression calculations #16

Closed themkat closed 7 months ago

themkat commented 7 months ago

General

Test cases for Lua are mostly inspired by JavaScript. My idea is that it would be easier to work with if we kept some consistency with test names and what we are testing.

binary_expression error

I guess the original idea for the binary_expression rule was to test the logical expressions and their nesting? If not, please let me know 🙂

Almost everything in Lua is a binary_expression (overstatement, but still...). Let us take some quick examples of how many places they actually occur and see the calculations in the MAIN codebase. This is to show why the code fix was done.

Simple.lua

Notice that each little multiplication, equality check and string concatenation is a binary_expression. If we see the debug prints for the Simple.lua file, it looks like this:

image

We can easily verify that this is from the binary_expression operations with the output from tree-sitter:

chunk:
  function_declaration:
    identifier:
    parameters:
      identifier:
    block:
      return_statement:
        expression_list:
          binary_expression:
            identifier:
            identifier:
  function_call:
    identifier:
    arguments:
      binary_expression:
        string:
        function_call:
          identifier:
          arguments:
            number:
  for_statement:
    for_numeric_clause:
      identifier:
      number:
      number:
    block:
      if_statement:
        binary_expression:
          number:
          identifier:

The calculation is correct here, but that is due to no real nesting of binary_expressions.

If we try write a quad function instead of square, we see the issue quite clearly:

image

As you can see, we now get a score of 1 for the first multiplication. From my understanding of cognitive complexity, only logical expressions should increase the score. I see nothing in any documentation (e.g, the Sonar paper) that arithmetic should increase it.

Recursion.lua

To really drive the point home, let us look at the recursion example.

image

Here we see something similar. (one of the +1s is correct due to recursive call!). Because the multiplication n * factorial(n - 1) is a binary_expression that has a binary_expression (i.e, n - 1) in a child of child nodes, it will get a score. That is not (at least to my knowledge) something that should happen. If it should happen, then it is wrong for ALL other languages...

We could probably continue down this road, and check each test. I don't think that is necessary. Feel free to do so if you want 🙂

binary_expression fix

if it was not clear, this PR fixes the above consistencies. This is by changing the binary_expression checks to only give any score for logical operators that has a logical operator operation as a child. If not, the calculations above will happen resulting in the situation we see in the recursion example.

jcs090218 commented 7 months ago

Make sense to me! Thank you! 🚀