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

feat: add ruff linter #2458

Merged
merged 13 commits into from
Apr 1, 2023
Merged

feat: add ruff linter #2458

merged 13 commits into from
Apr 1, 2023

Conversation

lars-reimann
Copy link
Contributor

@lars-reimann lars-reimann commented Mar 13, 2023

Fixes #2153

Proposed Changes

  1. Add the ruff Python linter.

Readiness Checklist

Author/Contributor

  • Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
  • If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

@lars-reimann lars-reimann marked this pull request as ready for review March 13, 2023 23:32
@Kurt-von-Laven
Copy link
Collaborator

Ruff markets itself as a much faster replacement for flake8, isort, and possibly other linters we already include in the Python flavor. It feels like it defeats the purpose of adding Ruff to leave the linters it duplicates in, but on the other hand maybe we should wait to remove the other linters until MegaLinter v7. What do folks think?

@nvuillam
Copy link
Member

I think ruff and the others can be both in MegaLinter for now, and users will decide which one they use :)

@lars-reimann
Copy link
Contributor Author

lars-reimann commented Mar 14, 2023

Ruff can be configured either in pyproject.toml (preferred) or ruff.toml. Currently, I've chosen pyproject.toml. But this might be problematic for projects that have a pyproject.toml file in the workspace folder (e.g. because they use poetry for dependency management) but still want to get linter configs using LINTER_RULES_PATH, which can point to an external repo. With the current setup, the workspace pyproject.toml would be found, which doesn't contain the linter config, so it runs with default settings.

Would ruff.toml be a better choice?

Edit: It seems like .ruff.toml is also possible, which would be in line with the existing linters, so I'll go for this.

@lars-reimann lars-reimann marked this pull request as draft March 14, 2023 08:58
@lars-reimann lars-reimann marked this pull request as ready for review March 14, 2023 10:53
@echoix
Copy link
Collaborator

echoix commented Mar 14, 2023

I think that in the middle-term future, we should find a way to properly handle what's defined in pyproject.toml, since it's where Python projects should go, and it's supposed to be there where the tools used are defined. But that means parsing that file, not only detecting its presence.

That pyproject.toml handling doesn't need to be ready for the first inclusion of ruff.

@nvuillam @Kurt-von-Laven what are your opinions on this?

@nvuillam
Copy link
Member

That could be an option on desceptors to avoid using the default config file while there is something in the pyproject, indeed :)

@lars-reimann
Copy link
Contributor Author

lars-reimann commented Mar 14, 2023

Maybe config_file_name could accept an array of objects containing

  1. the file name to look for
  2. how to check that the file should be used as config (file exists, file contains some text matching a regex)
  3. any further options needed for 2 (like the regex to look for).

The options could then be checked in order and the first matching file could be used. For ruff it would be something like

- config_file_name:
  - file: "pyproject.toml"
    check: matches_regex
    regex: "\\[tool\\.ruff"
  - file: "ruff.toml"
    check: file_exists
  - file: ".ruff.toml"
    check: file_exists

Property names, config_file_name specifically, need some work.

@nvuillam
Copy link
Member

I think we discussed that in another PR, that's definitely something to do :)

Meanwhile, config file name can be forced in local repo config

@lars-reimann
Copy link
Contributor Author

Do I need to configure somewhere how to get the number of errors that was fixed by the linter so it shows up in this table?

image

@nvuillam
Copy link
Member

@lars-reimann the fixes number is computed from the list of files that are detected as updated after the linting :)

@lars-reimann
Copy link
Contributor Author

Alright, this is done from my side then.

@nvuillam
Copy link
Member

The PR looks great :)
I'll merge it after today's release so it can be used with beta version until next release :)

@Kurt-von-Laven
Copy link
Collaborator

Personally, I feel that we should make LINTER_DEFAULT the default config file for of all linters in one shot in a major release. That would keep MegaLinter as simple as possible and make it more user-friendly at the same time.

@lars-reimann
Copy link
Contributor Author

Looks like Trivy was also happy this time round.

Copy link
Member

@nvuillam nvuillam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great :)

Many many thanks for this contribution :)

@nvuillam nvuillam merged commit dc98b15 into oxsecurity:main Apr 1, 2023
@Kurt-von-Laven
Copy link
Collaborator

Note that Ruff only enables a tiny subset of available rules by default, so if one wishes to replace Bandit, Flake8, and isort for best performance, they would need to configure Ruff explicitly.

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.

Replace flake8/isort with Ruff
4 participants