Repository navigation
Allow # noqa: SIM103 on last return #16250
Description
Activity
- changed the title
[-]allow noqa SIM103 on last return[/-][+]Allow `# noqa SIM103` on last return[/+]on Feb 19, 2025 Irrelevant, but the comment should have a colon:
# noqa: SIM103. See alsoPGH004.Reacted by JKE-be- changed the title
[-]Allow `# noqa SIM103` on last return[/-][+]Allow `# noqa: SIM103` on last return[/+]on Feb 19, 2025 This is an interesting case, thank you!
As far as I can tell this would require some fairly substantial changes to how error suppression currently works for multi-line diagnostic ranges. Note that, in your example, that final return statement is part of the diagnostic range, it's just that in general the in-line suppression comment is supposed to be on the first line of a diagnostic.
So I think supporting this may require some design discussion to understand whether it should be done and what the ramifications might be.
Ha, @dylwil3 I was just about to write the same.
Red Knot implements your desired behavior (see #15046 (comment)) but I'd prefer to keep the behavior of
noqaconsistent with flake8. I'd have to check what behavior flake8 uses. Either way, this is a rather significant change.There are some trade offs involved with matching both the start and end of the range (see red knot PR) and the main mitigation is to strictly use error codes.
Not sure how easy it would be to detect, but I would actually argue that this should not be a violation of SIM103. The spirit of SIM103 is to replace simple if checks with a boolean expression, but fall through boolean checks is a very common idiom that should be allowed
# GOOD - easy to read and extend def check(s: str) -> bool: if s.startswith("/"): return True if s.endswith("/"): return False # TODO: more guards will be added in the future return True # BAD - difficult to read or extend def check(s: str) -> bool: return s.startswith("/") or not s.endswith("/")
The original ask would be a decent mitigation for this, but IMO a better resolution would be to exclude these situations, if possible.
Reacted by decibyte, Michael Van Delft and Jérémy Jauzion
Description
searched: SIM103 Ignore
snippet:
Command:
ruff check /tmp/ruff.pyOutput:
version: ruff 0.9.6
config:
Expected (Nice to have feature)
Allow ignoring
SIM103without requiring the# noqacomment on the lastifstatement itself. The goal is to enable other developers to add anotherifafter the last one without modifying the previous condition.This is why I don’t want to refactor the last condition as suggested by Ruff. It’s a special case, but I don’t want to ignore
SIM103for the entire file or all files, as I still find it useful in 99% of cases.Currently, I need to add
# noqaon the lastìfto getAll checks passed!def func(a, b): if a == 0: return False a = a + b / a if a != 1: return False a = a * b - if a > 1: # noqa SIM103 + if a > 1: return False - return True + return True # noqa SIM103