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

PowerShell linter does not count errors properly #3320

Closed
efrecon opened this issue Jan 24, 2024 · 10 comments
Closed

PowerShell linter does not count errors properly #3320

efrecon opened this issue Jan 24, 2024 · 10 comments
Labels
bug Something isn't working

Comments

@efrecon
Copy link
Contributor

efrecon commented Jan 24, 2024

Describe the bug
The powershell linter does not count errors properly. Instead of reporting the number of errors, it report the number of erroneous files.

To Reproduce
Steps to reproduce the behavior:

  1. Run the powershell linter on a file with several warnings/errors
  2. Megalinter reports "1" error.

Expected behavior
The MegaLinter should count the number of errors and probably report warnings as non-blocking errors.

note
I believe this is because the descriptor does not provide a cli_lint_errors_count and cli_lint_errors_regex or similar.

@efrecon efrecon added the bug Something isn't working label Jan 24, 2024
@echoix
Copy link
Collaborator

echoix commented Jan 24, 2024

Does the tool itself have a way of reporting it? It's a bit harder to be smarter than the tool ;)

@efrecon
Copy link
Contributor Author

efrecon commented Jan 24, 2024

Not directly, not that I know. But, as I understand the implementation of the MegaLinter, the two descriptor keys that I mentioned above are able to count the number of occurrences of matches for the cli_lint_errors_regex regular expression. Is it so?

In that case, I think that it should be possible to control the order of the columns output from Format-Table (see #3318) and arrange for the severity to be placed first. We could then count the occurrences of ^[A-Za-z]*Error (or ^Warning for non-blocking errors).

@nvuillam
Copy link
Member

Indeed cli_lint_errors_count and cli_lint_errors_regex are the way to count errors using output text :)

@efrecon please feel free to add them, even in your current PR about Powershell if you want :)

@efrecon
Copy link
Contributor Author

efrecon commented Jan 29, 2024

Merci @nvuillam. I've started to look at that but stumbled upon the following:

If I understand the code correctly, when the total_errors is 0 as of this line -- because there are only warnings or information (possible levels here) -- the python code will issue a warning. And, since there was some output, this will end up being reported as 1 error (there was 1 file with some problems).

  • Is my understanding correct?
  • Is this correct behaviour?
  • How can I support "non-breaking" errors, e.g. Warning in the severity table linked above.

@echoix
Copy link
Collaborator

echoix commented Jan 31, 2024

I'm not sure I understand your message correctly.

If I'm not mistaken, the configuration key DISABLE_ERRORS_LINTERS enables you to have a linter run, but if the linter reports errors, instead of failing the job, it will only report it as warnings.

https://megalinter.io/latest/configuration/
https://megalinter.io/latest/configuration/#activation-and-deactivation

Is it a bit what you are trying to achieve?

Or you are trying to "upgrade" the existing linter to be able to report both kinds (warnings AND errors) when it does (or doesn't) currently report it when used by itself manually.

@efrecon
Copy link
Contributor Author

efrecon commented Feb 5, 2024

I needed a helping hand. This is now implemented as part of #3318. The implementation places the severity at the beginning of the lines and count the known severity levels in the output. This means that even messages with a severity of Information will report errors, but it is possible to control the level of severity through the configuration file pointed at by POWERSHELL_POWERSHELL_CONFIG_FILE.

I have run the tests as here. They pass. Is there anything I need to do for this to be considered for approval and merge?

@nvuillam
Copy link
Member

nvuillam commented Feb 7, 2024

@efrecon if the CI jobs pass, I'll merge the PR :)

@efrecon
Copy link
Contributor Author

efrecon commented Feb 8, 2024

@nvuillam They.. didn't. One line too long, the rest being internal: a network error and a "no space on device error"

@nvuillam
Copy link
Member

nvuillam commented Feb 8, 2024

Let's cross fingers for the next run :)

I updated your branch from main, the no space left on device has been solved in another PR

@nvuillam
Copy link
Member

Appears to be solved by #3318 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants