Generalize and apply to subprocess-shell-false

src/codemodder/codemods/check_annotations.py

@@ -2,14 +2,14 @@
 from typing import Mapping
 
 import libcst as cst
-from libcst import CSTVisitor, ensure_type, matchers
+from libcst import CSTVisitor, matchers
 from libcst.metadata import ParentNodeProvider
 from libcst.metadata.base_provider import ProviderT  # noqa: F401
 from libcst._nodes.base import CSTNode  # noqa: F401
 
 from pylint.utils.pragma_parser import parse_pragma
 
-NOQA_PATTERN = re.compile(r"^#\s*noqa", re.IGNORECASE)
+NOQA_PATTERN = re.compile(r"^#\s*noqa(:\s+[A-Z]+[A-Z0-9]+)?", re.IGNORECASE)
 
 
 __all__ = ["is_disabled_by_annotations"]
@@ -33,53 +33,66 @@ def __init__(
     def leave_Comment(self, original_node: cst.Comment) -> None:
         self.comments.append(original_node)
 
-    def _is_disabled_by_linter(self, node: cst.CSTNode) -> bool:
+    def _process_simple_statement_line(self, stmt: cst.SimpleStatementLine) -> bool:
+        # has a trailing comment string anywhere in the node
+        stmt.body[0].visit(self)
+        # has a trailing comment string anywhere in the node
+        if stmt.trailing_whitespace.comment:
+            self.comments.append(stmt.trailing_whitespace.comment)
+
+        for comment in self.comments:
+            trailing_comment_string = comment.value
+            if trailing_comment_string and self._noqa_message_match(
+                trailing_comment_string
+            ):
+                return True
+            if trailing_comment_string and self._is_pylint_disable_unused_imports(
+                trailing_comment_string
+            ):
+                return True
+
+        # has a comment right above it
+        if matchers.matches(
+            stmt,
+            matchers.SimpleStatementLine(
+                leading_lines=[
+                    matchers.ZeroOrMore(),
+                    matchers.EmptyLine(comment=matchers.Comment()),
+                ]
+            ),
+        ):
+            comment_string = stmt.leading_lines[-1].comment.value
+            if self._noqa_message_match(comment_string):
+                return True
+            if comment_string and self._is_pylint_disable_next_unused_imports(
+                comment_string
+            ):
+                return True
+
+        return False
+
+    def is_disabled_by_linter(self, node: cst.CSTNode) -> bool:
         """
         Check if the import has a #noqa or # pylint: disable(-next)=unused_imports comment attached to it.
         """
         match self.get_metadata(ParentNodeProvider, node):
-            case cst.SimpleStatementLine() as parent:
-                stmt = ensure_type(parent, cst.SimpleStatementLine)
-
-                # has a trailing comment string anywhere in the node
-                stmt.body[0].visit(self)
-                # has a trailing comment string anywhere in the node
-                if stmt.trailing_whitespace.comment:
-                    self.comments.append(stmt.trailing_whitespace.comment)
-
-                for comment in self.comments:
-                    trailing_comment_string = comment.value
-                    if trailing_comment_string and NOQA_PATTERN.match(
-                        trailing_comment_string
-                    ):
-                        return True
-                    if (
-                        trailing_comment_string
-                        and self._is_pylint_disable_unused_imports(
-                            trailing_comment_string
-                        )
-                    ):
-                        return True
-
-                # has a comment right above it
-                if matchers.matches(
-                    stmt,
-                    matchers.SimpleStatementLine(
-                        leading_lines=[
-                            matchers.ZeroOrMore(),
-                            matchers.EmptyLine(comment=matchers.Comment()),
-                        ]
-                    ),
-                ):
-                    comment_string = stmt.leading_lines[-1].comment.value
-                    if NOQA_PATTERN.match(comment_string):
-                        return True
-                    if comment_string and self._is_pylint_disable_next_unused_imports(
-                        comment_string
-                    ):
-                        return True
+            case cst.SimpleStatementLine() as stmt:
+                return self._process_simple_statement_line(stmt)
+            case cst.Expr() as expr:
+                match self.get_metadata(ParentNodeProvider, expr):
+                    case cst.SimpleStatementLine() as stmt:
+                        return self._process_simple_statement_line(stmt)
         return False
 
+    def _noqa_message_match(self, comment: str) -> bool:
+        if not (match := NOQA_PATTERN.match(comment)):
+            return False
+
+        if match.group(1):
+            return match.group(1).strip(":").strip() in self.messages
+
+        return True
+
     def _is_pylint_disable_unused_imports(self, comment: str) -> bool:
         # If pragma parse fails, ignore
         try:
@@ -117,4 +130,4 @@ def is_disabled_by_annotations(
     """
     visitor = _GatherCommentNodes(metadata, messages)
     node.visit(visitor)
-    return visitor._is_disabled_by_linter(node)
+    return visitor.is_disabled_by_linter(node)

src/core_codemods/remove_unused_imports.py

@@ -1,5 +1,3 @@
-import re
-
 import libcst as cst
 from libcst.codemod.visitors import GatherUnusedImportsVisitor
 from libcst.metadata import (
@@ -44,7 +42,7 @@ def transform_module_impl(self, tree: cst.Module) -> cst.Module:
             pos = self.get_metadata(PositionProvider, import_alias)
             if self.filter_by_path_includes_or_excludes(pos):
                 if not is_disabled_by_annotations(
-                    importt, self.metadata, messages=["unused-import", "W0611"]  # type: ignore
+                    importt, self.metadata, messages=["unused-import", "W0611", "F401"]  # type: ignore
                 ):
                     self.file_context.codemod_changes.append(
                         Change(pos.start.line, self.change_description)

src/core_codemods/subprocess_shell_false.py

@@ -1,5 +1,8 @@
 import libcst as cst
 from libcst import matchers
+from libcst.metadata import ParentNodeProvider
+
+from codemodder.codemods.check_annotations import is_disabled_by_annotations
 from codemodder.codemods.utils_mixin import NameResolutionMixin
 from codemodder.codemods.libcst_transformer import NewArg
 from core_codemods.api import (
@@ -31,6 +34,8 @@ class SubprocessShellFalse(SimpleCodemod, NameResolutionMixin):
         for func in {"run", "call", "check_output", "check_call", "Popen"}
     ]
 
+    METADATA_DEPENDENCIES = SimpleCodemod.METADATA_DEPENDENCIES + (ParentNodeProvider,)
+
     def leave_Call(self, original_node: cst.Call, updated_node: cst.Call):
         if not self.filter_by_path_includes_or_excludes(
             self.node_position(original_node)
@@ -44,6 +49,10 @@ def leave_Call(self, original_node: cst.Call, updated_node: cst.Call):
                     matchers.Arg(
                         keyword=matchers.Name("shell"), value=matchers.Name("True")
                     ),
+                ) and not is_disabled_by_annotations(
+                    original_node,
+                    self.metadata,  # type: ignore
+                    messages=["S603"],
                 ):
                     self.report_change(original_node)
                     new_args = self.replace_args(

tests/codemods/test_subprocess_shell_false.py

@@ -77,3 +77,14 @@ def test_has_noqa(self, tmpdir, func):
         subprocess.{func}(args, shell=True) # noqa: S603
         """
         self.run_and_assert(tmpdir, input_code, expected)
+
+    def test_different_noqa_message(self, tmpdir):
+        input_code = """
+        import subprocess
+        subprocess.run(args, shell=True) # noqa: S604
+        """
+        expected = """
+        import subprocess
+        subprocess.run(args, shell=False) # noqa: S604
+        """
+        self.run_and_assert(tmpdir, input_code, expected)