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

CFQ001 counts too many lines #9

Open
alexis974 opened this issue Oct 14, 2021 · 3 comments
Open

CFQ001 counts too many lines #9

alexis974 opened this issue Oct 14, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@alexis974
Copy link

Bug description:
CFQ001 takes into account empty lines and comment lines when counting number of lines inside a function.

To Reproduce:
Steps to reproduce the behavior:

  1. Create a function that contains some empty lines and/or comments lines
  2. Run flake8 with the --max-function-length=N where N is the number of line of your function (without counting the empty line and comment line)

Expected behavior:
CFQ001 should only count lines of actual code. Taking into account empty lines and comment lines may lead to developers removing some comments or empty lines to fix CFQ001 linting error. Thus, making the code less readable.

Desktop:

  • OS:Manjaro Linux x86_64
  • Python : 3.9.7
  • Flake8: 4.0.1
  • Flake8-functions : 0.0.6
@alexis974 alexis974 added the bug Something isn't working label Oct 14, 2021
@jeffnyman
Copy link

Agreed and seconding this as a concern. I'm finding exactly what was described in the initial report: developers started removing comments -- and necessary and helpful comments -- from functions just to satisfy this lint check.

Length from a linting perspective should only be taking into account executable lines of code. That said, I could see a desire by some to also check for all lines, including non-executable ones. In that case, maybe there should be a configuration option? Something like count-only-executable-lines or include-non-executable-lines or whatever makes sense.

@alexis974
Copy link
Author

Great idea to add a configuration option for this. I think the default behavior should be to omit non-executable lines.

I would be happy to implement this feature enhancement. I just need the approval of a maintainer.

Also, PEP8 says the following thing :

The closing brace/bracket/parenthesis on multiline constructs may either line up under the first non-whitespace character of the last line of list [...] or it may be lined up under the first character of the line that starts the multiline construct:

How should we consider lines with just a closing brace/bracket/parenthesis ? For example, should we count 3 or 4 lines for the following code :

my_dict = {
    “one”: 1,
    “two”: 2,
    }

@justkrys
Copy link

Agreed +1 on fixing this bug. Comments should not be counted and I am being bitten by this right now. :(
Thanks for the great tool though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants