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

Too-many-branches scanning returns different values if directories to scan are explicitly specified. #6242

Closed
jackdewinter opened this issue Apr 9, 2022 · 3 comments · Fixed by #7747
Labels
Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@jackdewinter
Copy link

jackdewinter commented Apr 9, 2022

Bug description

In scanning my PyMarkdown project, a switch to pylint==2.13.5 caused some errors to show up with pylint. Specifically, when pipenv run pylint --rcfile=setup.cfg pymarkdown pymarkdown/plugins was executed, this was the response:

************* Module pymarkdown.plugins.rule_md_033
pymarkdown\plugins\rule_md_033.py:68:4: R0912: Too many branches (14/12) (too-many-branches)

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

However, upon using either pipenv run pylint --rcfile=setup.cfg --recursive=y pymarkdown or specifying the file directly, the errors were not reported.

Command used

Command to generate bad behavior:

pipenv run pylint --rcfile=setup.cfg pymarkdown pymarkdown/plugins

Commands that did not generate the bad behavior:

pipenv run pylint --rcfile=setup.cfg --recursive=y pymarkdown
pipenv run pylint --rcfile=setup.cfg --recursive=y pymarkdown/plugins/rule_md_021.py

Pylint output

Output for the bad case:

pymarkdown\plugins\rule_md_033.py:68:4: R0912: Too many branches (14/12) (too-many-branches)

No errors were output for the good cases.

Expected behavior

My expectations are that the same results should be generated, regardless of what form is used to get to that file.

Pylint version

pylint 2.13.5
astroid 2.11.2
Python 3.8.10 (tags/v3.8.10:3d8993a, May  3 2021, 11:48:03) [MSC v.1928 64 bit (AMD64)]

OS / Environment

Win10

@jackdewinter jackdewinter added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Apr 9, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Bug 🪲 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Apr 9, 2022
@Pierre-Sassoulas Pierre-Sassoulas added the Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning label Jul 14, 2022
@emcd
Copy link
Contributor

emcd commented Nov 6, 2022

I have also run into this bug. It is counting the number of branches for each way that the scanned file is being reached by Pylint. So, reaching the same module two different ways doubles the count of branches in each of its functions.

Here is a simple reproduction.

  1. Assume that I have the following file, bugs/pylint_branches.py:
''' Simple reproducer for 'too-many-branches' bug. '''

def too_many_branches( lol ):
    ''' Each branch is counted once per file reference. '''
    aaa = 1
    if aaa > lol: aaa = lol
    else: aaa = 42
  1. Running pylint --report n --score n --max-branches 1 bugs/pylint_branches.py bugs produces the following output:
************* Module pylint_branches
bugs/pylint_branches.py   3, 0 [too-many-branches] Too many branches (2/1)
bugs/pylint_branches.py   3, 0 [too-many-branches] Too many branches (4/1)

Environment:

$ pylint --version
pylint 2.15.5
astroid 2.12.12
Python 3.7.15 (default, Oct 15 2022, 11:29:20)
[GCC 9.4.0]

@Pierre-Sassoulas
Copy link
Member

Thank you for investigating @emcd, do you want to contribute a fix ?

@Pierre-Sassoulas Pierre-Sassoulas added Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning labels Nov 6, 2022
emcd added a commit to emcd/pylint that referenced this issue Nov 11, 2022
emcd added a commit to emcd/pylint that referenced this issue Nov 11, 2022
@emcd
Copy link
Contributor

emcd commented Nov 11, 2022

do you want to contribute a fix ?

@Pierre-Sassoulas : "Want to contribute" is a relative term, but, sure, I'll contribute. :) I've benefited from Pylint much over the years and Daniel and Tyler have fixed other issues I've filed. PR #7747 seems to fix this issue. I still need to consider whether I can do anything to validate it with the test suite, but it is otherwise ready.

@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 2.15.6, 2.15.7 Nov 15, 2022
Pierre-Sassoulas added a commit that referenced this issue Nov 18, 2022
* Use dict for 'expand_modules' result rather than list.
With 'path' as the key, we get deduplication for free
and do not need to reprocess the list for deduplication later.
* Fix deduplication to account for CLI args marker.
* Fix corner case with CLI arg flag handling during deduplication. 
* Add 'deduplication' to custom Pyenchant dict.

Closes #6242
Closes #4053

Co-authored-by: Eric McDonald <emcd@users.noreply.github.com>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 2.15.7, 2.15.6 Nov 18, 2022
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Nov 23, 2022
…#7747)

* Use dict for 'expand_modules' result rather than list.
With 'path' as the key, we get deduplication for free
and do not need to reprocess the list for deduplication later.
* Fix deduplication to account for CLI args marker.
* Fix corner case with CLI arg flag handling during deduplication.
* Add 'deduplication' to custom Pyenchant dict.

Closes pylint-dev#6242
Closes pylint-dev#4053

Co-authored-by: Eric McDonald <emcd@users.noreply.github.com>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Pierre-Sassoulas added a commit that referenced this issue Nov 23, 2022
* Use dict for 'expand_modules' result rather than list.
With 'path' as the key, we get deduplication for free
and do not need to reprocess the list for deduplication later.
* Fix deduplication to account for CLI args marker.
* Fix corner case with CLI arg flag handling during deduplication.
* Add 'deduplication' to custom Pyenchant dict.

Closes #6242
Closes #4053

Co-authored-by: Eric McDonald <emcd@users.noreply.github.com>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants