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

What's the best practice for a linter's custom args in CLI call to process the listed files for linting ? #3000

Closed
fawazsalah opened this issue Oct 11, 2023 · 6 comments
Labels
question Further information is requested

Comments

@fawazsalah
Copy link

fawazsalah commented Oct 11, 2023

For example in Ruff linter with its "format" feature, I'd like to call:

PYTHON_RUFF_ARGUMENTS: "format 'Dir/ListedFilesForLint/' "

I'm not referring to the workspace/repo dirs, rather than to the files detected by MegaLinter.

@fawazsalah fawazsalah added the question Further information is requested label Oct 11, 2023
@fawazsalah fawazsalah changed the title What's the best practice for a linter's custom args in CLI call to process the listed files for lint ? What's the best practice for a linter's custom args in CLI call to process the listed files for linting ? Oct 11, 2023
@echoix
Copy link
Collaborator

echoix commented Oct 11, 2023

I'm not sure to fully understand your question, but I understand it relates to the list_of_files option for cli_lint_mode.

Usually a linter is configured by its descriptor. Here, ruff was defined in the https://github.com/oxsecurity/megalinter/blob/7e19651f1b8c0d67a0332fadf9d45b1c49d95c69/megalinter/descriptors/python.megalinter-descriptor.yml file, near the end.
The available options to use in a descriptor are noted in the schema here https://megalinter.io/latest/json-schemas/descriptor.html#linters_items_cli_lint_mode, take a special look at the fields starting with cli_lint and cli.

Usually, the argument is defined there, and Megalinter adds the list of files (already filtered, applying rules to include exclude files, so the search for all files is only done once for all the linters). I don't remember if it is possible to override every field from the descriptor as an runtime variable, you could check it out. When a linter is too complex to handle using these, then a Python class is used to interface the linter.

I think what you're trying to do is integrate the ruff tool as a formater instead of a linter, that isn't officially announced yet. Look at this PR for the updates of their docs, since the latest published docs don't apply yet for this mode.

astral-sh/ruff#7732

I'm not sure we had another linter that works in that way yet, so that change might be harder than usual to make use of. The problems I expect to arise include, "will the ruff check --fix include the fixes from ruff format ...?", "Can a file that was run through ruff format ... still have formatting errors that would be triggered from ruff check --fix?", "is it necessary to run ruff in two passes in order to make use of all the functionality that ruff offers?", "does ruff format ... emit some diagnostics too?", "does ruff check --fix ... and ruff format ... use the same config/rules, and if integrated in Megalinter, would it be a single config (like the same file, not two different files in other locations)?", "what does fix do differently than format, when talking about formatting fixes?", etc.

@Kurt-von-Laven
Copy link
Collaborator

Since ruff format takes a list of files and PYTHON_RUFF_CLI_LINT_MODE defaults to list_of_files, you would simply set PYTHON_RUFF_ARGUMENTS: format in your MegaLinter config.

@fawazsalah
Copy link
Author

I'm not sure to fully understand your question, but I understand it relates to the list_of_files option for cli_lint_mode.

Usually a linter is configured by its descriptor. Here, ruff was defined in the https://github.com/oxsecurity/megalinter/blob/7e19651f1b8c0d67a0332fadf9d45b1c49d95c69/megalinter/descriptors/python.megalinter-descriptor.yml file, near the end. The available options to use in a descriptor are noted in the schema here https://megalinter.io/latest/json-schemas/descriptor.html#linters_items_cli_lint_mode, take a special look at the fields starting with cli_lint and cli.

Usually, the argument is defined there, and Megalinter adds the list of files (already filtered, applying rules to include exclude files, so the search for all files is only done once for all the linters). I don't remember if it is possible to override every field from the descriptor as an runtime variable, you could check it out. When a linter is too complex to handle using these, then a Python class is used to interface the linter.

I think what you're trying to do is integrate the ruff tool as a formater instead of a linter, that isn't officially announced yet. Look at this PR for the updates of their docs, since the latest published docs don't apply yet for this mode.

astral-sh/ruff#7732

I'm not sure we had another linter that works in that way yet, so that change might be harder than usual to make use of. The problems I expect to arise include, "will the ruff check --fix include the fixes from ruff format ...?", "Can a file that was run through ruff format ... still have formatting errors that would be triggered from ruff check --fix?", "is it necessary to run ruff in two passes in order to make use of all the functionality that ruff offers?", "does ruff format ... emit some diagnostics too?", "does ruff check --fix ... and ruff format ... use the same config/rules, and if integrated in Megalinter, would it be a single config (like the same file, not two different files in other locations)?", "what does fix do differently than format, when talking about formatting fixes?", etc.

Hi @echoix thanks for the feedback,

My intention is to format files using the "ruff format" on the list_of_files, as "ruff format" (as of now) works either by specifying a file path or dir path. In the local run, I was able to do that with "ruff format '/tmp/lint' ", where that's the dir for list_of_files (?)

In GitHub Actions that's a different story as the workspace now is the repo itself, hence '/tmp/lint' isn't found (again based on my tests.) . So that was the obstacle.

I think what you're trying to do is integrate the ruff tool as a formater instead of a linter, that isn't officially announced yet. Look at this PR for the updates of their docs, since the latest published docs don't apply yet for this mode.

Correct.

I'm not sure we had another linter that works in that way yet, so that change might be harder than usual to make use of. The problems I expect to arise include, "will the ruff check --fix include the fixes from ruff format ...?", "Can a file that was run through ruff format ... still have formatting errors that would be triggered from ruff check --fix?", "is it necessary to run ruff in two passes in order to make use of all the functionality that ruff offers?", "does ruff format ... emit some diagnostics too?", "does ruff check --fix ... and ruff format ... use the same config/rules, and if integrated in Megalinter, would it be a single config (like the same file, not two different files in other locations)?", "what does fix do differently than format, when talking about formatting fixes?", etc.

Valid questions. As of now, it seems both formatting and fix args in one command line not working as expected. I think the formula that I reached so far is, formatting, then fixing, is decent (I benchmarked the errors post-formatting-fixing with flake8 errors so I'm satisfied with the result.)

@fawazsalah
Copy link
Author

fawazsalah commented Oct 12, 2023

Since ruff format takes a list of files and PYTHON_RUFF_CLI_LINT_MODE defaults to list_of_files, you would simply set PYTHON_RUFF_ARGUMENTS: format in your MegaLinter config.

That's what I expected at first, however ruff (as of now) expects either a file path or dir to format. So "format" alone will throw errors.

However, I found a workaround with PYTHON_RUFF_PRE_COMMANDS :

git fetch && IFS=$'\n' changed_files=$(git diff --name-only origin/main -- "*.py") && for file in $changed_files; do echo "Formatting '$file'" && ruff format "$file" --config "pyproject.toml"; done,

Closing this issue as I believe I found my answer, yet feel free to discuss it further if it can be improved.

@Kurt-von-Laven
Copy link
Collaborator

As of now, it seems both formatting and fix args in one command line not working as expected.

As MegaLinter doesn't specifically support running the same linter twice, my understanding is that it behaves as expected currently. Your workaround seems reasonable and creative. Other approaches would be to run ruff format outside of MegaLinter (e.g., via pre-commit, CI, and/or an IDE extension) or via a MegaLinter plugin. You could alternatively write a very short script that ferries the files passed to it through to ruff format and ruff sequentially, and invoke that script via PYTHON_RUFF_CLI_EXECUTABLE.

That's what I expected at first, however ruff (as of now) expects either a file path or dir to format. So "format" alone will throw errors.

If you can reproduce that at the latest version of ruff format, I would file a bug with Ruff since the docs do state that format accepts a list of files, and that is a better design for a CLI tool.

@nvuillam
Copy link
Member

@fawazsalah very creative workaround indeed, congrats :)

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

No branches or pull requests

4 participants