-
-
Notifications
You must be signed in to change notification settings - Fork 235
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
Line numbers reported by linters do not correspond to the committed files #3399
Comments
When linters are in the same descriptor, the linters that can update (formatters,linters with auto-fix) are always run before the others But... with "generic" formatters that's probably not the case... Do you have concrete examples in mind ? |
Yes, the Python linters seem to run in the following order:
If black, isort and ruff format the files on the fly and change the line numbering, line numbers reported by pylint refer to the formatted files and don't match to lines in the committed files anymore. |
Besides, another benefit of running the formatters last would be the possibility to speedup MegaLinter's running time by skipping the formatters if some of the linters have failed. |
But on the other hand, some easily fixed issues, like line length, types of quotes etc, that can be fixed by a formatter without failing, will make the linters fail afterwards. Some of these tools are slower too. |
Which non-formatting linter can fail because of a formatting issue? |
That's the chicken or the egg problem... :) If we don't run linters that can fix first, the following linters will find issues that would have been fixed before by the formatters/fixers... In case of failure, formatted/fixed files are available in artifacts, so they can be copy-pasted locally to apply the fixes anyway, so have the correct number of lines That's far from ideal but I think it has less impacts to use such workaround than to change MegaLinter automated execution order |
I would agree with you if the non-formatting linters could fail because of a formatting issue (hence my last, yet unanswered, question). But I don't think that is the case. Do you have an example? |
Flake8 for sure, Pylint also, ruff implements checks for some of these rules too, so that's just the top of my head of thing I saw elsewhere last week when upgrading a codebase from black 22.3 to 24.3 |
OK, if you say that formatting by Black can fix something that would make Pylint fail, then I agree that it makes sense to run the formatters first. |
This issue has been automatically marked as stale because it has not had recent activity. If you think this issue should stay open, please remove the |
I am now experiencing a situation where yamllint runs before prettier and fails because of formatting issues which are later fixed by prettier. So, although you convinced me that formatters should run first, it does not seem to be the case which points to another problem. Could it be that yamllint and prettier run in parallel jobs and yamllint is just faster? |
@vkucera I think a local run of prettier and a commit in your repo should solve this mess for a while ^^ Of just make yamllint non blocking by adding YAML_TAMLLINT in DISABLE_ERRORS_LINTERS property |
@nvuillam thanks, that can work as a workaround but, since you said that formatters always run before other linters, I am wondering whether the case of yamllint running before prettier is a feature or a bug. |
@vkucera the algorith is the following megalinter/megalinter/MegaLinter.py Line 299 in 7cb18ee
Groups of linters that can upate sources are run before groups of linters that can not update There is nothing telling "wait for linters that can fix to run before running linters that can not fix", maybe this is the bug you suspect ? |
But this is apparently not true. If it was the case, it would be impossible to see yamllint (which cannot update sources) running before prettier (the only enabled linter which can update sources).
If the linters that cannot update sources are not starting after the formatters in the given format group finish, I would indeed consider that to be a major bug. If the point of formatting is to avoid later failures of other linter (as @echoix pointed out), then it would be incorrect to run the non-formatting linters on files which have not been formatted yet. Am I missing anything? |
We are back on that answer ^^ Indeed we could implement some wait mechanism, or maybe something like "group of groups" run to make sure not updating linters are run after updating linters In your example, the fixing group is called after the not-fixing one, but as it's fast its result appears before |
This issue has been automatically marked as stale because it has not had recent activity. If you think this issue should stay open, please remove the |
So, since you have confirmed that the timing of running the linters is not the desired one, do you agree that this issue should be considered a bug and should be fixed? |
@vkucera I agree this is a bug yes :) Now the game will be to find the time to solve it... ^^ |
MegaLinter does not create an automatic formatting PR if linters have found errors. The reported error messages are referring to lines in formatted files which can make it difficult to locate the corresponding lines in the committed files. See oxsecurity/megalinter#3399 Formatted files can be uploaded in artifacts, which gives the PR author the possibility to see which lines the linters complain about.
MegaLinter does not create an automatic formatting PR if linters have found errors. The reported error messages are referring to lines in formatted files which can make it difficult to locate the corresponding lines in the committed files. See oxsecurity/megalinter#3399 Formatted files can be uploaded in artifacts, which gives the PR author the possibility to see which lines the linters complain about.
MegaLinter does not create an automatic formatting PR if linters have found errors. The reported error messages are referring to lines in formatted files which can make it difficult to locate the corresponding lines in the committed files. See oxsecurity/megalinter#3399 Formatted files can be uploaded in artifacts, which gives the PR author the possibility to see which lines the linters complain about.
This issue has been automatically marked as stale because it has not had recent activity. If you think this issue should stay open, please remove the |
@nvuillam Should this issue be reopened or do you prefer to open a new issue that will be a bug report regarding the wrong order? |
There is all the discussion here :) |
There is indeed and it would be a pity if it becomes part of forgotten history because of the automatic closing of the issue. |
It's here to make sure we keep only issues that really matter to people 😇 |
This issue has been automatically marked as stale because it has not had recent activity. If you think this issue should stay open, please remove the |
Is your feature request related to a problem? Please describe.
Line numbers reported by linters do not correspond to the committed files, probably as a result of formatting by the formatters running before them.
Describe the solution you'd like
Line numbers referred in the reports of linters should be the numbers of lines where the errors appear in the committed files.
Describe alternatives you've considered
If formatters run after the linters, the line numbers of errors found by linters will be correct.
Additional context
The text was updated successfully, but these errors were encountered: