astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
28.59k stars 926 forks source link

Reqression: ruff 0.5.0 expects noqa directive at a different line #12086

Closed anz-ableton closed 3 days ago

anz-ableton commented 3 days ago

I've searched for the following keywords before opening this issue:

Ruff in version 0.4.10 will not complain about the following snippet:

def run(a_long_command_to_make_this_wrap_at_the_line_end, use_shell):
    subprocess.run(
        a_long_command_to_make_this_wrap_at_the_line_end,
        capture_output=True,
        check=True,
        shell=use_shell,  #  noqa: S603
    )

However, when using 0.5.0 it will complain with

$ ruff check --select S .
ruff_s603_regression.py:5:5: S603 `subprocess` call: check for execution of untrusted input
  |
4 | def run(a_long_command_to_make_this_wrap_at_the_line_end, use_shell):
5 |     subprocess.run(
  |     ^^^^^^^^^^^^^^ S603
6 |         a_long_command_to_make_this_wrap_at_the_line_end,
7 |         capture_output=True,
  |

Found 1 error.

Moving the noqa directive to the line where subprocess.run is called will make it succeed:

diff --git a/ruff_s603_regression.py b/ruff_s603_regression.py
index eda9e6c..306eb56 100644
--- a/ruff_s603_regression.py
+++ b/ruff_s603_regression.py
@@ -2,11 +2,11 @@ import subprocess

 def run(a_long_command_to_make_this_wrap_at_the_line_end, use_shell):
-    subprocess.run(
+    subprocess.run(  #  noqa: S603
         a_long_command_to_make_this_wrap_at_the_line_end,
         capture_output=True,
         check=True,
-        shell=use_shell,  #  noqa: S603
+        shell=use_shell,
     ) 

For completeness, below is the content of the full example file:

import subprocess

def run(a_long_command_to_make_this_wrap_at_the_line_end, use_shell):
    subprocess.run(  #  noqa: S603
        a_long_command_to_make_this_wrap_at_the_line_end,
        capture_output=True,
        check=True,
        shell=use_shell,
    )

run("echo 'Hello, World!'", True)
MichaReiser commented 3 days ago

Hi.

It is documented in the changelog under rule changes but I should have mentioned it in the breaking changes section, to make it more visible. Let me update the changelog real quick

[flake8-bandit] Modify diagnostic ranges for shell-related rules (https://github.com/astral-sh/ruff/pull/10667)

You can use --add-noqa and https://docs.astral.sh/ruff/rules/unused-noqa/ to automatically move the noqa comments (add the new once and remove the no longer necessary comments)

anz-ableton commented 3 days ago

@MichaReiser Thanks for the swift response and for updating the changelog! I did not know about the --add-noqa flag, that's pretty helpful! Will you take care of closing this issue once the changelog got updated?