Line numbers reported by linters do not correspond to the committed files

[vkucera]

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

[nvuillam]

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 ?

[vkucera]

Yes, the Python linters seem to run in the following order:

1. black
2. isort
3. ruff
4. pylint
5. mypy
6. pyright
7. flake8
8. bandit

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. IMHO, this is an undesired behaviour which makes finding the problems in the code more confusing.

[vkucera]

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.

[echoix]

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.

[vkucera]

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?

[nvuillam]

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

[vkucera]

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?

[echoix]

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

[vkucera]

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.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

[vkucera]

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?

[nvuillam]

@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

[vkucera]

@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.

[nvuillam]

@vkucera the algorith is the following

https://github.com/oxsecurity/megalinter/blob/7cb18ee58c135a04d2bc07f05727000440b7d281/megalinter/MegaLinter.py#L299

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 ?

[vkucera]

@vkucera the algorith is the following

https://github.com/oxsecurity/megalinter/blob/7cb18ee58c135a04d2bc07f05727000440b7d281/megalinter/MegaLinter.py#L299

Groups of linters that can upate sources are run before groups of linters that can not update

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).

2024-05-10T11:13:56.2279237Z ##[group]❌ Linted [YAML] files with [yamllint]: Found 1 error(s) - (0.27s) (expand for details)
2024-05-10T11:13:56.2364557Z ##[endgroup]
2024-05-10T11:13:56.6316125Z ##[group]✅ Linted [YAML] files with [prettier] successfully - (0.6s) (expand for details)
2024-05-10T11:13:56.6345647Z ##[endgroup]
2024-05-10T11:13:59.5020765Z ##[group]✅ Linted [YAML] files with [v8r] successfully - (3.16s) (expand for details)
2024-05-10T11:13:59.5054149Z ##[endgroup]

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 ?

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?

[nvuillam]

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 ?

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

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

[vkucera]

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?

[nvuillam]

@vkucera I agree this is a bug yes :)

Now the game will be to find the time to solve it... ^^

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

[vkucera]

@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?

[nvuillam]

There is all the discussion here :)

[vkucera]

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.

[nvuillam]

It's here to make sure we keep only issues that really matter to people 😇

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.