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

Fix false positives in typing.Protocol #313

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install --upgrade build coveralls setuptools tox==4.0.2 wheel
python -m pip install --upgrade build coveralls setuptools tox==4.0.2 wheel ast-scope==0.3.1

- name: Build Vulture wheel
run: python -m build
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
* Add `UnicodeEncodeError` exception handling to `core.py` (milanbalazs, #299).
* Add whitelist for `Enum` attributes `_name_` and `_value_` (Eugene Toder, #305).
* Add scoping info to handle false positives with `typing.Protocol` (pm3512, #309).

# 2.7 (2023-01-08)

Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def find_version(*parts):
"Programming Language :: Python :: Implementation :: PyPy",
"Topic :: Software Development :: Quality Assurance",
],
install_requires=["toml"],
install_requires=["toml", "ast_scope"],
entry_points={"console_scripts": ["vulture = vulture.core:main"]},
python_requires=">=3.6",
packages=setuptools.find_packages(exclude=["tests"]),
Expand Down
25 changes: 25 additions & 0 deletions tests/test_scavenging.py
Original file line number Diff line number Diff line change
Expand Up @@ -899,3 +899,28 @@ class Color(Enum):

check(v.unused_classes, [])
check(v.unused_vars, ["BLUE"])


def test_protocol(v):
v.scan(
"""\
from typing import Protocol, Mapping

class LoggerProtocol(Protocol):
def log(
self,
level: int,
msg: str,
*args,
extra: Mapping[str, object] | None = None
) -> None:
...

def foo(self) -> None:
...
"""
)

check(v.unused_vars, [])
check(v.unused_funcs, [])
check(v.unused_classes, ["LoggerProtocol"])
36 changes: 36 additions & 0 deletions vulture/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from vulture import utils
from vulture.config import make_config

import ast_scope


DEFAULT_CONFIDENCE = 60

Expand Down Expand Up @@ -253,6 +255,11 @@ def handle_syntax_error(e):
)
self.found_dead_code_or_error = True
else:
# ast_scope doesn't always parse, so need to handle errors.
try:
self.scope_info = ast_scope.annotate(node)
except Exception:
self.scope_info = None
# When parsing type comments, visiting can throw SyntaxError.
try:
self.visit(node)
Expand Down Expand Up @@ -505,6 +512,15 @@ def _define_variable(self, name, node, confidence=DEFAULT_CONFIDENCE):
def visit_arg(self, node):
"""Function argument"""
self._define_variable(node.arg, node, confidence=100)
if self.scope_info and node in self.scope_info:
scope = self.scope_info[node]
if isinstance(scope, ast_scope.scope.FunctionScope) and isinstance(
scope.parent, ast_scope.scope.ClassScope
):
method_class = scope.parent.class_node
is_protocol = self._is_subclass(method_class, "Protocol")
if is_protocol:
self.used_names.add(node.arg)

def visit_AsyncFunctionDef(self, node):
return self.visit_FunctionDef(node)
Expand Down Expand Up @@ -581,6 +597,20 @@ def _is_locals_call(node):
and not node.keywords
)

@staticmethod
def _is_subclass(node, class_name):
"""Return True if the node is a subclass of the given class."""
assert isinstance(node, ast.ClassDef)
for superclass in node.bases:
if (
isinstance(superclass, ast.Name)
and superclass.id == class_name
or isinstance(superclass, ast.Attribute)
and superclass.attr == class_name
):
return True
return False

def visit_ClassDef(self, node):
for decorator in node.decorator_list:
if _match(
Expand Down Expand Up @@ -624,6 +654,12 @@ def visit_FunctionDef(self, node):
self._define(
self.defined_methods, node.name, node, ignore=_ignore_method
)
if self.scope_info and node in self.scope_info:
scope = self.scope_info[node]
if isinstance(scope, ast_scope.scope.ClassScope):
method_class = scope.class_node
if self._is_subclass(method_class, "Protocol"):
self.used_names.add(node.name)
else:
self._define(
self.defined_funcs, node.name, node, ignore=_ignore_function
Expand Down