Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
anz-ableton opened this issue Jun 28, 2024 · 2 comments · Fixed by #12090
Closed

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

anz-ableton opened this issue Jun 28, 2024 · 2 comments · Fixed by #12090
Labels
documentation Improvements or additions to documentation

Comments

@anz-ableton
Copy link

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

  • noqa,
  • S603, S602
  • subprocess

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
Copy link
Member

MichaReiser commented Jun 28, 2024

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 (#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)

@MichaReiser MichaReiser added the documentation Improvements or additions to documentation label Jun 28, 2024
@anz-ableton
Copy link
Author

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants