RUF100 should probably delete an entire comment noqa is part of

[yury-fedotov]

I think it applies to all error codes, but let me give an example where I encountered that.

Consider:

data = do_some_transformations()
return data

Which is a violation of RET504 - unnecessary assignment.

I needed to perform the assignment and return data, not return do_some_transformations(), so added a noqa: RET504 with explanation of why I want to bypass the rule:

data = do_some_transformations()
return data  # noqa: RET504 - to be able to debug the returned object.

Then I disabled RET504 in ruff config completely, and this noqa became unused. Here is where RUF100 comes into play, as it complains on unused error codes, and provides a fix.

The fix was:

-    return data  # noqa: RET504 - to be able to debug the returned object
+    return data  # - to be able to debug the returned object

While I believe it should have been:

-    return data  # noqa: RET504 - to be able to debug the returned object
+    return data

As it's probably quite safe to assume that entire content of the comment noqa is part of is explaining why a rule should by bypassed.

• List of keywords you searched for before creating this issue: RUF100
• Most similar open issue: RUF100 for noqa: E501 not at end of line #7851
• A minimal code snippet that reproduces the bug: see above.
• The command you invoked: ruff check --fix
• The current Ruff version: 0.5.1.

[charliermarsh]

I'm unsure -- I think the current behavior is intentional. But either seems reasonable.

[MichaReiser]

I think deleting the entire comment is fine, for as long as it doesn't delete anything coming after another #:

-    return data  # noqa: RET504 # fmt:skip
+    return data  # fmt: skip

[charliermarsh]

I agree!

[charliermarsh]

I think this is all here if anyone is interested in a good issue:

ruff/crates/ruff_linter/src/fix/edits.rs

Line 65 in 940df67

pub(crate) fn delete_comment(range: TextRange, locator: &Locator) -> Edit {

[jeremeybingham]

Is there a standard or a strong feeling here on preserving trailing whitespace following 'Preserved' comments? Meaning:

can: x = 1 # noqa # type: ignore
and: x = 1 # noqa # type: ignore

both resolve to: x = 1 # type: ignore ?

or is there a conceivable edge case where that matters?

At any rate, I believe I've mostly got this but will spend some time tomorrow getting the testing framework and everything set up, should work regardless of the above, just changes the logic a bit to save those characters if we want 'em.

[dhruvmanila]

It should be fine to remove trailing whitespace but just a note that any other comments shouldn't be deleted (#12251 (comment)).

[jeremeybingham]

this was... much better than I expected the first pass to go, will have a PR on this once I figure out what's going on with these, but I'll take 4/2057 as a start!

[dhruvmanila]

@jeremeybingham Can you open the PR? I can help you debug these test failures.

[jeremeybingham]

will try asap, yes - have "fixed" my way into various fun new failure modes, and am now trying to consolidate the working bits of all that into something that compiles.