[ruff] Fix --add-noqa breaking shebangs

[getehen]

Hello,

This PR fixes the bug #14442 about. It adds a check on noqa application:
If the first line is a shebang, errors with range 0..0 can be suppressed by a noqa on the second line.

Example:

For D100,

#!/usr/bin/env python #noqa: D100

broke the shebang.
Thus, you can now suppress errors like this with:

#!/usr/bin/env python
#noqa:D100 

This also works for errors like CPY001, ... (anything that creates errors on range 0..0).

Implementation details

When suppressing errors in check_noqa, adds a check: if the range of the error to suppress is 0..0, a shebang is present, try to get noqas on the second line. This is done by another function find_noqa_on_line_mut, that gather noqas of a specific line (with a binary search, similarly to the used function find_line_with_directive_mut).

Test Plan

The check_noqa is tested for this specific case with other cases in linter.rs, in the function test_shebang_noqa. Several cases are tested, including cases where the range is not 0..0, the file doesn't begin with a shebang, or the noqa is on the third line. Cases where suppression works are also tested.

The find_noqa_on_line_mut is tested in noqa_on_certain_line in the file it is declared.

[astral-sh-bot]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[amyreese]

The ecosystem report show a large number of panics. Please take a look and resolve that.

[amyreese]

This might just need a rebase onto latest main to clear up the panics.

[ntBre]

I think that's just a stale base branch from my earlier panic issue. I'll close and reopen to retrigger CI!

[amyreese]

Overall I think there is a better approach that relies on existing mechanisms in the noqa system. We already build a NoqaMapping data structure for mapping #noqa from one line to apply to other lines. I would suggest looking at where we build that mapping (ruff_linter/src/directives.rs in extract_noqa_line_for()) and add a new mapping when the file contains a shebang. I believe that solution will be much smaller and more direct than what you have proposed.

[amyreese]

Since a shebang can only occur on the first line, this should use something like comment_ranges.first().map(...), and verify that the comment range starts at 0, so that it's not iterating through every single comment in the file or giving a false positive if there's a "shebang" on any line other than the first one.

[getehen]

Alright, I wil look into this

[getehen]

I have a working solution using NoqaMapping, but with the way it works now, the noqa directive is expected to be on the next line. Thus, a noqa on the same line than the shebang won't work. It is not really a problem because with a shebang, the noqa on the same line would be a problem.

[amyreese]

@MichaReiser we triaged this as a bug, but since using NoqaMapping results in changing the expected location for a #noqa comment, is this something you would consider a "breaking" change that needs to be gated by preview mode?

To be clear, this would only affect diagnostics:

• in files with shebangs on line one
• from rules targeted at TextRange::ZERO, like D100 or EXE00*

But in those cases, if the user actually added a #noqa to the end of the shebang, this PR will ignore that noqa in favor of looking for one on line two instead.

[MichaReiser]

It's nice that we can solve this generically in noqa mapping. The only downside is that we'll need the same treatment for every ignore comment style (e.g. ruff: ignore[] once added). I don't think we need to change this now but an alternative would be to add an extra field to Diagnostic that stores an additional allowed suppression range. This is a feature that has come up in other contexts where it's sometimes desired to allow suppressions on lines other than the primary diagnostic range. I don't mind getting forward with the approach taken here.

I consider this a bugfix.