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

Does MegaLinter support flake8-cognitive-complexity? #2017

Closed
kcfedun-fincad opened this issue Oct 28, 2022 · 10 comments · Fixed by #2032
Closed

Does MegaLinter support flake8-cognitive-complexity? #2017

kcfedun-fincad opened this issue Oct 28, 2022 · 10 comments · Fixed by #2032
Labels
question Further information is requested

Comments

@kcfedun-fincad
Copy link

Hello there,

Does MegaLinter support flake8-cognitive-complexity? If not, can it be extended to do so? perhaps a new PYTHON_FLAKE8_ variable could be used to enable/disable these checks?

https://github.com/Melevir/flake8-cognitive-complexity

Many thanks.

@kcfedun-fincad kcfedun-fincad added the question Further information is requested label Oct 28, 2022
@kcfedun-fincad
Copy link
Author

I've applied a bit of a workaround for now using the following code, but a more sustainable solution would be ideal.

.mega-linter.yml
---
...

PYTHON_FLAKE8_PRE_COMMANDS:
  - command: |-
      echo $PWD
      cd /venvs/flake8 \
      && source bin/activate \
      && PYTHONDONTWRITEBYTECODE=1 pip3 install --no-cache-dir flake8-cognitive-complexity \
      && deactivate

@nvuillam
Copy link
Member

If we add it by default with flake8 installation, will it enforce new rules by default for all repos ?

@Kurt-von-Laven
Copy link
Collaborator

Can you elaborate on what is unsustainable about this approach? If you hadn't provided it, using PYTHON_FLAKE8_PRE_COMMANDS to install the desired flake8 plugins is what I would have recommended (and what we have been doing). For my education, did you encounter issues with a simple pip install flake8-cognitive-complexity? For the sake of folks who may copy and paste this snippet in the future, some may prefer a standalone shell script that is called from PYTHON_FLAKE8_PRE_COMMANDS. I am a big fan of MegaLinter's value of minimizing the configuration burden on its users, but it feels quite aggressive for MegaLinter to decide which of the many available flake8 plugins to run on our users' source code. This particular plugin looks quite appealing to me personally, but at face value it doesn't appear to be very mature or regularly maintained. It also may be viewed as redundant with and/or contradictory to the more popular mccabe plugin, for example.

@Kurt-von-Laven
Copy link
Collaborator

@nvuillam, there are some exceptions, but yes, most flake8 plugins will run by default if both they and flake8 are installed.

@kcfedun-fincad
Copy link
Author

Can you elaborate on what is unsustainable about this approach? If you hadn't provided it, using PYTHON_FLAKE8_PRE_COMMANDS to install the desired flake8 plugins is what I would have recommended (and what we have been doing). For my education, did you encounter issues with a simple pip install flake8-cognitive-complexity? For the sake of folks who may copy and paste this snippet in the future, some may prefer a standalone shell script that is called from PYTHON_FLAKE8_PRE_COMMANDS. I am a big fan of MegaLinter's value of minimizing the configuration burden on its users, but it feels quite aggressive for MegaLinter to decide which of the many available flake8 plugins to run on our users' source code. This particular plugin looks quite appealing to me personally, but at face value it doesn't appear to be very mature or regularly maintained. It also may be viewed as redundant with and/or contradictory to the more popular mccabe plugin, for example.

My sustainability concerns arise in cases where MegaLinter could decide to change the /venvs/flake8 directory or how it manages pip installations. That would have a trickle-down effect on my custom installs.

I did try running pip3 install --no-cache-dir flake8-cognitive-complexity in the PYTHON_FLAKE8_PRE_COMMANDS (without cd-ing into the /venvs/flake8 dir and activating the virtualenv. This resulted in none of the cognitive-complexity check being run. This implies that the pip package must be installed in the flake8 venv.

I wouldn't want other users to be forced to use the checks provided by the pip extensions, be it mccabe, cognitive-complexity, or others. Ideally there would be a switch/variable that could be used to enable them if the user so chooses.

@nvuillam
Copy link
Member

It depends if we want plugins rules to be considered as standard in megalinter:)
A good test could be to create a pull request, and as megalinter job does own ml code linting, wewould see the gap in ml code with such plugin :)

@nvuillam
Copy link
Member

(basically, if it triggers 657 errors within MegaLinter own code, I'll prefer to let it as an option and just add in the doc about how to add such plugin instead of adding/activating it by default ^^
The best would be to have a ML config var to enable additional rules (which would be disabled by default ^^)

@Kurt-von-Laven
Copy link
Collaborator

Kurt-von-Laven commented Oct 30, 2022

I did try running pip3 install --no-cache-dir flake8-cognitive-complexity in the PYTHON_FLAKE8_PRE_COMMANDS (without cd-ing into the /venvs/flake8 dir and activating the virtualenv. This resulted in none of the cognitive-complexity check being run. This implies that the pip package must be installed in the flake8 venv.

I am unable to reproduce this. Installing flake8 plugins into the virtualenv might be better practice, but at least I do observe that flake8-bugbear runs when installed via a simple pip install. If you want to use the virtualenv, it may at least be possible to remove the cd by specifying the absolute rather than relative filepath? You can ask flake8 which plugins it picked up via PYTHON_FLAKE8_ARGUMENTS: --version.

@nvuillam
Copy link
Member

nvuillam commented Nov 1, 2022

@kcfedun-fincad @Kurt-von-Laven I simplified the way to add pip plugins in venvs :)

PRE_COMMANDS:
  - command: pip install flake8-cognitive-complexity
    venv: flake8 # Will be run within flake8 python virtualenv. There is one virtualenv per python-based linter, with the same name

https://github.com/oxsecurity/megalinter/actions/runs/3370888552/jobs/5592370481

image

Already available in beta, and soon in next minor release :)

@Kurt-von-Laven
Copy link
Collaborator

That is quite a nice solution 👍.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants