From 3a0d45c85b46d26cdc40febaa871871f158ab513 Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Sun, 16 Feb 2025 14:50:16 -0500 Subject: [PATCH] [`flake8-debugger`] Also flag `sys.breakpointhook` and `sys.__breakpointhook__` (`T100`) (#16191) ## Summary Fixes #16189. Only `sys.breakpointhook` is flagged by the upstream linter: https://github.com/pylint-dev/pylint/blob/007a745c8619c2cbf59f829a8f09fc6afa6eb0f1/pylint/checkers/stdlib.py#L38 but I think it makes sense to flag [`__breakpointhook__`](https://docs.python.org/3/library/sys.html#sys.__breakpointhook__) too, as suggested in the issue because it > contain[s] the original value of breakpointhook [...] in case [it happens] to get replaced with broken or alternative objects. ## Test Plan New T100 test cases --- .../test/fixtures/flake8_debugger/T100.py | 18 +++++++ .../rules/flake8_debugger/rules/debugger.rs | 5 +- ..._flake8_debugger__tests__T100_T100.py.snap | 54 +++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_debugger/T100.py b/crates/ruff_linter/resources/test/fixtures/flake8_debugger/T100.py index 8154f1a4cd..d876c575df 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_debugger/T100.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_debugger/T100.py @@ -23,3 +23,21 @@ debugpy.listen(1234) enable_attach() break_into_debugger() wait_for_attach() + + +# also flag `breakpointhook` from `sys` but obviously not `sys` itself. see +# https://github.com/astral-sh/ruff/issues/16189 +import sys # ok + +def scope(): + from sys import breakpointhook # error + + breakpointhook() # error + +def scope(): + from sys import __breakpointhook__ # error + + __breakpointhook__() # error + +sys.breakpointhook() # error +sys.__breakpointhook__() # error diff --git a/crates/ruff_linter/src/rules/flake8_debugger/rules/debugger.rs b/crates/ruff_linter/src/rules/flake8_debugger/rules/debugger.rs index 7477cfdc89..b216e69f5a 100644 --- a/crates/ruff_linter/src/rules/flake8_debugger/rules/debugger.rs +++ b/crates/ruff_linter/src/rules/flake8_debugger/rules/debugger.rs @@ -109,14 +109,15 @@ fn is_debugger_call(qualified_name: &QualifiedName) -> bool { | ["builtins" | "", "breakpoint"] | ["debugpy", "breakpoint" | "listen" | "wait_for_client"] | ["ptvsd", "break_into_debugger" | "wait_for_attach"] + | ["sys", "breakpointhook" | "__breakpointhook__"] ) } fn is_debugger_import(qualified_name: &QualifiedName) -> bool { // Constructed by taking every pattern in `is_debugger_call`, removing the last element in // each pattern, and de-duplicating the values. - // As a special-case, we omit `builtins` to allow `import builtins`, which is far more general - // than (e.g.) `import celery.contrib.rdb`. + // As special-cases, we omit `builtins` and `sys` to allow `import builtins` and `import sys` + // which are far more general than (e.g.) `import celery.contrib.rdb`. matches!( qualified_name.segments(), ["pdb" | "pudb" | "ipdb" | "debugpy" | "ptvsd"] diff --git a/crates/ruff_linter/src/rules/flake8_debugger/snapshots/ruff_linter__rules__flake8_debugger__tests__T100_T100.py.snap b/crates/ruff_linter/src/rules/flake8_debugger/snapshots/ruff_linter__rules__flake8_debugger__tests__T100_T100.py.snap index 3b8f68333c..75baaa0379 100644 --- a/crates/ruff_linter/src/rules/flake8_debugger/snapshots/ruff_linter__rules__flake8_debugger__tests__T100_T100.py.snap +++ b/crates/ruff_linter/src/rules/flake8_debugger/snapshots/ruff_linter__rules__flake8_debugger__tests__T100_T100.py.snap @@ -183,3 +183,57 @@ T100.py:25:1: T100 Trace found: `ptvsd.wait_for_attach` used 25 | wait_for_attach() | ^^^^^^^^^^^^^^^^^ T100 | + +T100.py:33:5: T100 Import for `sys.breakpointhook` found + | +32 | def scope(): +33 | from sys import breakpointhook # error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ T100 +34 | +35 | breakpointhook() # error + | + +T100.py:35:5: T100 Trace found: `sys.breakpointhook` used + | +33 | from sys import breakpointhook # error +34 | +35 | breakpointhook() # error + | ^^^^^^^^^^^^^^^^ T100 +36 | +37 | def scope(): + | + +T100.py:38:5: T100 Import for `sys.__breakpointhook__` found + | +37 | def scope(): +38 | from sys import __breakpointhook__ # error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ T100 +39 | +40 | __breakpointhook__() # error + | + +T100.py:40:5: T100 Trace found: `sys.__breakpointhook__` used + | +38 | from sys import __breakpointhook__ # error +39 | +40 | __breakpointhook__() # error + | ^^^^^^^^^^^^^^^^^^^^ T100 +41 | +42 | sys.breakpointhook() # error + | + +T100.py:42:1: T100 Trace found: `sys.breakpointhook` used + | +40 | __breakpointhook__() # error +41 | +42 | sys.breakpointhook() # error + | ^^^^^^^^^^^^^^^^^^^^ T100 +43 | sys.__breakpointhook__() # error + | + +T100.py:43:1: T100 Trace found: `sys.__breakpointhook__` used + | +42 | sys.breakpointhook() # error +43 | sys.__breakpointhook__() # error + | ^^^^^^^^^^^^^^^^^^^^^^^^ T100 + |