Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Correct parameter count for python function with trailing comma and parameterized type hints #385

Merged
merged 2 commits into from
Apr 25, 2024

Conversation

jmal0
Copy link

@jmal0 jmal0 commented Apr 12, 2024

Addresses #354

In several situations, the argument count reported for python is incorrect. In particular:

  • When a trailing comma is included in a function argument list
  • When a parameterized type hint is provided that includes commas, e.g. dict[str, tuple[int, float]]

Below is a code snippet that reproduces these issues:

def func_with_trailing_comma(
    a,
    b,
    c,
):
    pass


def func_with_type_parameterized_args(a: dict[str, tuple[int, float]]):
    pass

Running lizard -a 2 demo.py on this reports the following:

================================================
  NLOC    CCN   token  PARAM  length  location  
------------------------------------------------
       6      1     11      4       6 func_with_trailing_comma@1-6@demo.py
       2      1     18      3       2 func_with_type_parameterized_args@9-10@demo.py
1 file analyzed.
==============================================================
NLOC    Avg.NLOC  AvgCCN  Avg.token  function_cnt    file
--------------------------------------------------------------
      8       4.0     1.0       14.5         2     demo.py

=========================================================================================================
!!!! Warnings (cyclomatic_complexity > 15 or length > 1000 or nloc > 1000000 or parameter_count > 2) !!!!
================================================
  NLOC    CCN   token  PARAM  length  location  
------------------------------------------------
       6      1     11      4       6 func_with_trailing_comma@1-6@demo.py
       2      1     18      3       2 func_with_type_parameterized_args@9-10@demo.py
==========================================================================================
Total nloc   Avg.NLOC  AvgCCN  Avg.token   Fun Cnt  Warning cnt   Fun Rt   nloc Rt
------------------------------------------------------------------------------------------
         8       4.0     1.0       14.5        2            2      1.00    1.00

The correct result is reported with the fixes included in the second commit of this branch:

================================================
  NLOC    CCN   token  PARAM  length  location  
------------------------------------------------
       6      1     11      3       6 func_with_trailing_comma@1-6@demo.py
       2      1     18      1       2 func_with_type_parameterized_args@9-10@demo.py
1 file analyzed.
==============================================================
NLOC    Avg.NLOC  AvgCCN  Avg.token  function_cnt    file
--------------------------------------------------------------
      8       4.0     1.0       14.5         2     demo.py

=========================================================================================================
!!!! Warnings (cyclomatic_complexity > 15 or nloc > 1000000 or length > 1000 or parameter_count > 2) !!!!
================================================
  NLOC    CCN   token  PARAM  length  location  
------------------------------------------------
       6      1     11      3       6 func_with_trailing_comma@1-6@demo.py
==========================================================================================
Total nloc   Avg.NLOC  AvgCCN  Avg.token   Fun Cnt  Warning cnt   Fun Rt   nloc Rt
------------------------------------------------------------------------------------------
         8       4.0     1.0       14.5        2            1      0.50    0.75

I have included tests in the first commit of this branch that reproduce the issue and check for the correct result

@jmal0 jmal0 changed the title fix: Correct parameter count for python function with trailing comma and parameterized type hints Draft: fix: Correct parameter count for python function with trailing comma and parameterized type hints Apr 12, 2024
@jmal0 jmal0 force-pushed the 354-fix-python-parameter-count branch from 56106ad to 1b2b373 Compare April 12, 2024 23:42
@jmal0 jmal0 changed the title Draft: fix: Correct parameter count for python function with trailing comma and parameterized type hints fix: Correct parameter count for python function with trailing comma and parameterized type hints Apr 12, 2024
@jmal0
Copy link
Author

jmal0 commented Apr 12, 2024

The workflow initially failed due to the use of python 3.6+ syntax in my added tests, in the second push I work around this by embedding the code to be parsed in strings. The python tests are now passing when run with python 2.7, however there were failures in testErlang.py and testKotlin.py which are failing on master as well.

@jmal0 jmal0 changed the title fix: Correct parameter count for python function with trailing comma and parameterized type hints Draft: fix: Correct parameter count for python function with trailing comma and parameterized type hints Apr 12, 2024
@jmal0 jmal0 force-pushed the 354-fix-python-parameter-count branch from 1b2b373 to d2d7787 Compare April 12, 2024 23:59
@jmal0 jmal0 changed the title Draft: fix: Correct parameter count for python function with trailing comma and parameterized type hints fix: Correct parameter count for python function with trailing comma and parameterized type hints Apr 13, 2024
@jmal0 jmal0 force-pushed the 354-fix-python-parameter-count branch from d2d7787 to 0c744af Compare April 13, 2024 00:37
@mmmtastymmm
Copy link

My company needs this functionality as well. Would be interesting to see if there is any feedback.

@terryyin
Copy link
Owner

Thanks for the addition and fix. The test coverage looks great.

@terryyin terryyin merged commit e06407d into terryyin:master Apr 25, 2024
0 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants