ruff
ruff copied to clipboard
astral-sh
[feature request] Support ignore block of code with noqa
First of all, thank you so much for this tool, it's fantastic 🚀 !
I would like to request the possibility to support ignoring a block of code. To my surprise, this is a feature some people have needed as well from flake8, see SO question, though it is not supported.
In my current use case, I have a bunch of templates, that are too long, and it becomes cumbersome to add noqa to all of them.
I was wondering if adding something like this would be possible (maybe with a different syntax):
# ruff: noqa: on: E501
TEMPLATE = """
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Pellentesque sit amet ligula magna. Nulla ${var1} vehicula leo, eget tincidunt leo pharetra in. Ut egestas felis et vehicula commodo. Aliquam ${var2} placerat tortor sed magna faucibus facilisis ut et mauris. Maecenas vitae sem augue. Cras egestas felis nulla, id porta nisl ${var3} vel
"""
# ruff: noqa: off: E501
Suggested changes
- [ ] Add section-level suppression/ignoring of rules (ruff: noqa: on and ruff: noqa: off)
- [ ] Add scope-level suppression/ignoring of rules (matches pylint block disables)
Thanks!
I'd really like to support something like this. But we may want to couple it with a more holistic redesign of suppression comments. \cc @MichaReiser
Hello, just to add to the already wonderful suggestion, I think also adding a simple and and off switch with just # ruff: noqa: on and # ruff: noqa: off to completely ignore a block of code would be fantastic!
What's your expectation if a file only contains an off comment without a corresponding on comment later in the file?
That the program ignores the lines under that comment.
A markdown linter named markdownlint has that option and if I am not mistaken, if there is no enable comment down the line, then the program just ignores all of it.
This program could also have an option to warn the user when an off flag/comment is unterminated (has no corresponding on switch) cuz disabling linting for the rest of the file is not recommended???? I dont knowwwwwwww
I'm pretty interested in this one. Obviously pretty new to the project, but I think that the design proposed above makes sense.
@MichaReiser saw you were mentioned above, are we good to move forward on this design? Do we want to support ignoring multiple rules at once?
@MichaReiser saw you were mentioned above, are we good to move forward on this design? Do we want to support ignoring multiple rules at once?
Technically: I recommend waiting till after #3931 lands because it significantly changes how we handle noqa comments internally.
Regarding the design. I haven't had much time to look into suppression comments, I got sidetracked by figuring out how a setting format could look like in a world where ruff does formatting and linting (and more?).
My personal objective for suppression comments is that we should try to keep it simple. I really like Rust's approach where there's a single way to enable or disable lints: Using allow and deny. The scope is defined by where the attributes are put: In a module -> applies for the whole module, at the top of a function -> applies for the function, at the top of an expression -> applies to the expression. Having a single format keeps things simple and easy to remember. I've had to google ESLint's suppression comment format so many time in the past, got it wrong because I forgot the on comment after disabling a rule for a section, or accidentally deleted the on comment in a refactor :hand_over_mouth: .
I'm not proposing that we need to implement the same semantics but I think its worthwhile to list the use cases with examples on how the new suppression comments should work. Do we support whole file suppression? Do we want to warn about missing on comments after an off. etc.
That makes sense. I'll start working on a design/use-cases - since #1054 asks for block/file-scoped noqa directives, closing this issue should also close that one (file-scoped noqas are already implemented)
Editing out the design approach I had here to link to the discussion instead: https://github.com/charliermarsh/ruff/discussions/4051
@woile Thanks for opening this issue!
Per discussion in #3868 , would you please add the following checklist to the beginning of this issue for PR tracking:
- [ ] Add section-level suppression/ignoring of rules (
ruff: noqa: onandruff: noqa: off) - [ ] Add scope-level suppression/ignoring of rules (matches
pylintblock disables)
There are several feature requests to add lint ignoring/suppression capabilities, each with slight variations, and some of the project maintainers have asked to keep track of them here. They may need to be implemented at different times or in several stages, so a checklist would be helpful for tracking to PRs.
Thank you!
FWIW, I believe suppression for any scope as in pylint block disables in addition to section-level suppression (i.e. turning on/off linting) would be wise.
Without knowing the internals, I imagine the former would require reparsing the AST, and thus a significant rewrite. However,
- it would match existing
pylintbehavior - I’m slowly seeing several different feature requests for suppression in different scopes (global level, function level, etc.).
Rather than rewrite the source for every kind of scope feature request, this would be done in one change and would likely cover 99% of use cases for most users.
I'd like to emphasize the importance of this feature for ruff's usability. There are some situations where you need more sophisticated rule control, for example this:
def get_all(cursor, table_name):
return cursor.execute(
'''
SELECT *
FROM {table_name};
'''.format(table_name=table_name)
)
ruff rightfully complains that this code contains a possible sql injection (S608). We know that the value of table_name is safe, so we want to disable S608 for that statement.
We can't disable the rule with # noqa because the affected line opens a multi-line string, so the only other option we have is to disable it for the entire file. Which is bad, because that would also disable it for any other sql statement in that file.
The only correct solution would be to wrap the entire sql string in an off-on-block or utilizing a disable-next pragma.
Why don't you pass it as a param like you should? Sorry, but this is a perfectly true positive, even if in THIS particular case it's safe - until someone decides to make cond come from let's say user-provided input. Boom, SQL injection!
Passing it as a param works in this simple case, but when you build a whole query programmatically …?
Why don't you pass it as a param like you should? Sorry, but this is a perfectly true positive, even if in THIS particular case it's safe - until someone decides to make
condcome from let's say user-provided input. Boom, SQL injection!
Sorry, my example was chosen very poorly, I've replaced it with a less frivolous one where you can't get away with parameter-passing.
@flbraun I believe your example might be worth raising a separate issue. I would have expected that one of those work, but none of them do.
def get_all(cursor, table_name):
return cursor.execute(
f'''
SELECT *
FROM {table_name};
''' # noqa: S608
)
def get_all(cursor, table_name):
return cursor.execute(
(
"""
SELECT *
FROM {table_name};
""" # noqa: S608
).format(table_name=table_name)
)
We are also waiting for this feature
Our use case is mainly in tests, we like to disable whitespaces rules the time of an assertions, to present a list/tuple/dict as a table, with aligned rows.
self.assertEquals(
# pylint: disable=bad-whitespace
report,
[
['Header', 400.00, 1600.00, 100.00],
['Other header', 00.00, 600.00, 10000.00],
['Some Header', 140.00, 0.00, 1000.00],
['', 1400.00, 600.00, 100.00],
['Finally', 10400.00, 600.00, 100.00],
]
)
Maybe instead of # ruff: noqa: on and ... off simply:
# ruff: off
...
# ruff: on
# ruff: off: T201
print()
if ...:
print("...", a, b, c)
...
# ruff: on
And since we have already # ruff: noqa ... to do it file level I would warn about non-paired off with a suggestion of either close it or replace with ruff: noqa: whatever.
I think there's a performance aspect to consider. In pylint the fine grained message control sometime prevented us from bypassing costly processing because checking if a message is disabled or not (on a directory, file, scope, line, node, or token) is costly in itself.
I stumbled upon another use case where this would be useful:
A line of code causes one error for mypy and one for bandit. Both support inline-comments for ignoring it, but they don't support ignoring code blocks. I can only ignore the error for one of them and have to ignore the whole file for another (not nice). If I used Ruff (with this feature) as a replacement for Bandit, I could ignore the block of code (1 line) and also add the inline ignore comment for mypy. So it would improve compatibility with mypy (they also have an ongoing thread about implementing this) and other linters without this feature.
I stumbled upon another use case where this would be useful:
A line of code causes one error for mypy and one for bandit. Both support inline-comments for ignoring it, but they don't support ignoring code blocks. I can only ignore the error for one of them and have to ignore the whole file for another (not nice). If I used Ruff (with this feature) as a replacement for Bandit, I could ignore the block of code (1 line) and also add the inline ignore comment for mypy. So it would improve compatibility with mypy (they also have an ongoing thread about implementing this) and other linters without this feature.
Why not just put two inline comments on the same line?
Just to chime in with my $0.02, I also think a good linter should have the ability to ignore a block
What's your expectation if a file only contains an
offcomment without a correspondingoncomment later in the file?
It should ignore the rest of the file (but turn itself back on at the end of the file)
Another use case for this.
I wanted to leave a comment explaining why I'm turning another rule off, which includes a snippet of a minimal example for a pandas warning. Since it's commented python code, it lights up with ERA001. In order to suppress this, I have to do # noqa: ERA001 for each line, which is unnecessary imo.
# import numpy as np # noqa: ERA001
# import pandas as pd # noqa: ERA001
# df = pd.DataFrame({"col1": [1, 2, 3, "", 5]}, dtype=object) # noqa: ERA001
# print(df) # noqa: ERA001
# print(df.dtypes) # noqa: ERA001
# df2 = df.replace("", 0) # noqa: ERA001
# print(df2["col1"]) # noqa: ERA001
# df2.iloc[0, 0] = "a" # noqa: ERA001
# print(df2["col1"]) # noqa: ERA001
Use case:
from a_module import ( # noqa: I001
# functions
do_something, # noqa: I001
do_something_else, # noqa: I001
# variables
some_module_config_var, # noqa: I001
some_other_module_config_var,# noqa: I001
fifty_more_of_these, # fifty more noqa: I001
)
because I wasn't doing that awfulness, and there's no way to disable this rule for a block of code, now my code has no import sorting.
@AntiSol You shouldn't require to specify noqa comment for each line, just one should suffice: https://play.ruff.rs/4c44cfef-9821-4cd6-98fd-6e3187228e68
@dhruvmanila I actually tried that before posting.
I don't think you're running the checker there? just the formatter? Isort rules are applied by the checker, not the formatter. I don't see any way to run the checker / isort rules there?
You'll note that removing the noqa comment from your example has no effect on the formatter output
Maybe I'm missing something?
Correct me if I'm wrong but I'm assuming that you're trying to ignore the names in that import statement from being sorted, right?
https://github.com/user-attachments/assets/74627bdd-f4a6-4843-9e81-d7709b525ee7
The video shows how to use code actions to apply a fix from the rules in the playground.
that is what I'm trying to do, and that is what I tried before posting. Maybe I got it wrong somehow. I'll try again when I have time.
Aha, thanks... but I assume you're right-clicking in that video? I don't see that context menu, just the regular one
Which means that there appears to be no way to run the checker in the playground? Might be worth a separate bug report I guess