1826209 - ruff warnings are not displayed (intentionally but non-obviously)

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement)

Description

[Steve Fink [:sfink] [:s:]]

I got a message https://phabricator.services.mozilla.com/D172060#5743559 from reviewbot that I had 1 warning from ruff. I ran mach lint -l ruff -r . locally to see what the warning was. It said:

warning: linting the entire repo takes a long time, using --outgoing and --workdir instead. If you want to lint the entire repo, run `./mach lint .`
✖ 0 problems (0 errors, 3 warnings, 0 fixed)

(1) the -r . should have suppressed the "entire repo" warning, but that's minor. And (2) it's reporting 3 warnings rather than the 1 from reviewbot, but I guess I'm not too bothered by that either. The real problem is (3) that it still didn't tell me what the warnings are. I tried my usual workaround of adding --fix and then doing an hg diff, but it didn't make any changes. I tried adding -v to see if it would display the warnings, but it still didn't. It did tell me the path to ruff, so I ran it manually, and it said Found 3 errors. I don't know if those are the same as the 3 warnings, but if they aren't, I would write it off to configuration differences (I don't know if directly running ruff picks up the same config or not.)

Is mach eating the warning output?

Comment 1

[Andrew Halberstadt [:ahal]]

This is by design, though you could argue the behaviour is confusing and should be more obvious. You need to pass -W/--warnings to see them. The logic behind this is that since warnings don't cause backouts, there's a very good chance they weren't caused by you, so we hide them by default (and even if they were caused by you, it's fine to ignore them).

As for there being 3 warnings instead of 1, I think this is because the code review bot has some extra logic to determine if the warning appears on a line you touched or not and filters out the ones that you didn't touch (this isn't a perfect solution but works reasonably well at least). So the extra 2 warnings are likely in places you didn't touch with your patch.

As far as -r . suppressing the "entire repo" warning... yeah that seems like a bug! Bug 1740598 in fact.

Comment 2

[Steve Fink [:sfink] [:s:]]

(In reply to Andrew Halberstadt [:ahal] from comment #1)

This is by design, though you could argue the behaviour is confusing and should be more obvious. You need to pass -W/--warnings to see them. The logic behind this is that since warnings don't cause backouts, there's a very good chance they weren't caused by you, so we hide them by default (and even if they were caused by you, it's fine to ignore them).

Oh! Ok, that makes sense. I guess I would like it to be more obvious; I think I've been running into this for a while and ignoring it.

As for there being 3 warnings instead of 1, I think this is because the code review bot has some extra logic to determine if the warning appears on a line you touched or not and filters out the ones that you didn't touch (this isn't a perfect solution but works reasonably well at least). So the extra 2 warnings are likely in places you didn't touch with your patch.

That's smart. I think it was correct in my case, too.

As far as -r . suppressing the "entire repo" warning... yeah that seems like a bug! Bug 1740598 in fact.

Oh, it actually does something different too? Hm, that's uncomfortably close to code that I wrote in the first place. Fix submitted. Sorry for stealing a good-first-bug.

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1826209 - Display message when nonfatal warnings are present

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6da75607c674
Display message when nonfatal warnings are present r=ahal

Comment 5

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/6da75607c674