diff --git a/CHANGELOG.md b/CHANGELOG.md index d5291a8e..a91c37a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,8 @@ -# 1.5 (2020-05-24) +# 1.6 (unreleased) + +* Differentiate between functions and methods (Jendrik Seipp, #112, #209). + + # 1.5 (2020-05-24) * Support flake8 "noqa" error codes F401 (unused import) and F841 (unused local variable) (RJ722, #195). diff --git a/README.md b/README.md index aa479c60..f803c3e1 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,7 @@ results in the following output: dead_code.py:1: V104 unused import 'os' (90% confidence) dead_code.py:4: V103 unused function 'greet' (60% confidence) - dead_code.py:8: V106 unused variable 'message' (60% confidence) + dead_code.py:8: V107 unused variable 'message' (60% confidence) Vulture correctly reports "os" and "message" as unused, but it fails to detect that "greet" is actually used. The recommended method to deal @@ -181,7 +181,7 @@ Passing both the original program and the whitelist to Vulture makes Vulture ignore the `greet` method: dead_code.py:1: V104 unused import 'os' (90% confidence) - dead_code.py:8: V106 unused variable 'message' (60% confidence) + dead_code.py:8: V107 unused variable 'message' (60% confidence) diff --git a/TODO.md b/TODO.md index cd664778..a1be0261 100644 --- a/TODO.md +++ b/TODO.md @@ -15,10 +15,6 @@ * Ignore hidden files and directories (might be unexpected, use --exclude instead). -* Differentiate between functions and methods. For our purposes - methods are functions with a "self" parameter and they are stored as - attributes, not as variables. However, we can't differentiate - between function and method executions. * Use Assign instead of Name AST nodes for estimating the size of assignments (KISS). * Only count lines for unused code by storing a function `get_size` in diff --git a/tests/test_noqa.py b/tests/test_noqa.py index b511850b..7b364ad8 100644 --- a/tests/test_noqa.py +++ b/tests/test_noqa.py @@ -80,9 +80,9 @@ def test_noqa_specific_issue_codes(v): @underground # noqa: V102 class Cellar: - @property # noqa: V105 + @property # noqa: V106 def wine(self): - grapes = True # noqa: V106 + grapes = True # noqa: V107 @without_ice # noqa: V103 def serve(self, quantity=50): @@ -95,6 +95,7 @@ def serve(self, quantity=50): check(v.unused_classes, []) check(v.unused_funcs, []) check(v.unused_imports, []) + check(v.unused_methods, ["serve"]) check(v.unused_props, []) check(v.unreachable_code, []) check(v.unused_vars, []) @@ -104,7 +105,7 @@ def test_noqa_attributes(v): v.scan( """\ something.x = 'x' # noqa: V101 -something.z = 'z' # noqa: V106 (code for unused variable) +something.z = 'z' # noqa: V107 (code for unused variable) something.u = 'u' # noqa """ ) @@ -191,7 +192,7 @@ def test_noqa_multiple_decorators(v): """\ @bar # noqa: V102 class Foo: - @property # noqa: V105 + @property # noqa: V106 @make_it_cool @log def something(self): @@ -199,7 +200,7 @@ def something(self): @coolify @property - def something_else(self): # noqa: V105 + def something_else(self): # noqa: V106 pass @a @@ -235,10 +236,10 @@ def shave_sheep(sheep): def test_noqa_variables(v): v.scan( """\ -mitsi = "Mother" # noqa: V106 +mitsi = "Mother" # noqa: V107 harry = "Father" # noqa shero = "doggy" # noqa: V101, V104 (code for unused import, attr) -shinchan.friend = ['masao'] # noqa: V106 (code for unused variable) +shinchan.friend = ['masao'] # noqa: V107 (code for unused variable) """ ) check(v.unused_vars, ["shero"]) @@ -254,7 +255,7 @@ def world(axis): # noqa: V103, V201 for _ in range(3): continue - xyz = hello(something, else): # noqa: V201, V106 + xyz = hello(something, else): # noqa: V201, V107 """ ) check(v.get_unused_code(), []) diff --git a/tests/test_report.py b/tests/test_report.py index f4168ffc..de9d7542 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -20,6 +20,9 @@ def bar(self): @property def myprop(self): pass + +def myfunc(): + pass """ @@ -39,11 +42,12 @@ def test_item_report(check_report): expected = """\ {filename}:1: unused import 'foo' (90% confidence) {filename}:3: unused class 'Foo' (60% confidence) -{filename}:7: unused function 'bar' (60% confidence) +{filename}:7: unused method 'bar' (60% confidence) {filename}:8: unused attribute 'foobar' (60% confidence) {filename}:9: unused variable 'foobar' (60% confidence) {filename}:11: unreachable code after 'return' (100% confidence) {filename}:13: unused property 'myprop' (60% confidence) +{filename}:17: unused function 'myfunc' (60% confidence) """ check_report(mock_code, expected) @@ -52,10 +56,11 @@ def test_make_whitelist(check_report): expected = """\ foo # unused import ({filename}:1) Foo # unused class ({filename}:3) -bar # unused function ({filename}:7) +_.bar # unused method ({filename}:7) _.foobar # unused attribute ({filename}:8) foobar # unused variable ({filename}:9) # unreachable code after 'return' ({filename}:11) _.myprop # unused property ({filename}:13) +myfunc # unused function ({filename}:17) """ check_report(mock_code, expected, make_whitelist=True) diff --git a/tests/test_scavenging.py b/tests/test_scavenging.py index 88a5309b..b4934b13 100644 --- a/tests/test_scavenging.py +++ b/tests/test_scavenging.py @@ -95,9 +95,10 @@ async def bar(self): """ ) check(v.defined_classes, ["Foo"]) - check(v.defined_funcs, ["bar"]) + check(v.defined_funcs, []) + check(v.defined_methods, ["bar"]) check(v.unused_classes, ["Foo"]) - check(v.unused_funcs, ["bar"]) + check(v.unused_methods, ["bar"]) def test_function_and_method1(v): @@ -114,9 +115,11 @@ def func(): """ ) check(v.defined_classes, ["Bar"]) - check(v.defined_funcs, ["func", "func"]) + check(v.defined_funcs, ["func"]) + check(v.defined_methods, ["func"]) check(v.unused_classes, ["Bar"]) check(v.unused_funcs, []) + check(v.unused_methods, ["func"]) def test_attribute1(v): @@ -162,7 +165,8 @@ def foo(self): ) check(v.used_attrs, ["foo"]) check(v.defined_classes, ["Bar"]) - check(v.defined_funcs, ["foo"]) + check(v.defined_funcs, []) + check(v.defined_methods, ["foo"]) check(v.unused_classes, []) check(v.unused_funcs, []) @@ -273,12 +277,22 @@ def __init__(self): class Bar(object): def foo(self): pass + + @classmethod + def bar(cls): + pass + + @staticmethod + def foobar(): + pass """ ) check(v.defined_classes, ["Bar"]) - check(v.defined_funcs, ["foo"]) + check(v.defined_funcs, []) + check(v.defined_methods, ["foo", "bar", "foobar"]) check(v.unused_classes, ["Bar"]) check(v.unused_funcs, []) + check(v.unused_methods, ["bar", "foobar"]) def test_token_types(v): diff --git a/vulture/core.py b/vulture/core.py index 381fe31d..11f84a7b 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -52,9 +52,10 @@ "class": "V102", "function": "V103", "import": "V104", - "property": "V105", + "method": "V105", + "property": "V106", + "variable": "V107", "unreachable_code": "V201", - "variable": "V106", } @@ -95,8 +96,12 @@ def _ignore_import(filename, import_name): def _ignore_function(filename, function_name): - return _is_special_name(function_name) or ( - function_name.startswith("test_") and _is_test_file(filename) + return function_name.startswith("test_") and _is_test_file(filename) + + +def _ignore_method(filename, method_name): + return _is_special_name(method_name) or ( + method_name.startswith("test_") and _is_test_file(filename) ) @@ -171,7 +176,9 @@ def get_whitelist_string(self): self.message, filename, self.first_lineno ) else: - prefix = "_." if self.typ in ["attribute", "property"] else "" + prefix = "" + if self.typ in ["attribute", "method", "property"]: + prefix = "_." return "{}{} # unused {} ({}:{:d})".format( prefix, self.name, self.typ, filename, self.first_lineno ) @@ -207,6 +214,7 @@ def get_set(typ): self.defined_classes = get_list("class") self.defined_funcs = get_list("function") self.defined_imports = get_list("import") + self.defined_methods = get_list("method") self.defined_props = get_list("property") self.defined_vars = get_list("variable") self.unreachable_code = get_list("unreachable_code") @@ -312,6 +320,7 @@ def by_size(item): + self.unused_classes + self.unused_funcs + self.unused_imports + + self.unused_methods + self.unused_props + self.unused_vars + self.unreachable_code @@ -360,6 +369,10 @@ def unused_imports(self): self.defined_imports, self.used_names | self.used_attrs ) + @property + def unused_methods(self): + return _get_unused_items(self.defined_methods, self.used_attrs) + @property def unused_props(self): return _get_unused_items(self.defined_props, self.used_attrs) @@ -557,7 +570,26 @@ def visit_FunctionDef(self, node): utils.get_decorator_name(decorator) for decorator in node.decorator_list ] - typ = "property" if "@property" in decorator_names else "function" + + first_arg = None + if node.args.args: + try: + first_arg = node.args.args[0].arg + except AttributeError: + # Python 2.7 + first_arg = node.args.args[0].id + + if "@property" in decorator_names: + typ = "property" + elif ( + "@staticmethod" in decorator_names + or "@classmethod" in decorator_names + or first_arg == "self" + ): + typ = "method" + else: + typ = "function" + if any( _match(name, self.ignore_decorators) for name in decorator_names ): @@ -568,8 +600,11 @@ def visit_FunctionDef(self, node): ) elif typ == "property": self._define(self.defined_props, node.name, node) + elif typ == "method": + self._define( + self.defined_methods, node.name, node, ignore=_ignore_method + ) else: - # Function is not a property. self._define( self.defined_funcs, node.name, node, ignore=_ignore_function ) diff --git a/vulture/noqa.py b/vulture/noqa.py index f8d9cc12..70eded9c 100644 --- a/vulture/noqa.py +++ b/vulture/noqa.py @@ -42,7 +42,7 @@ # flake8 F401: module imported but unused. "F401": "V104", # flake8 F841: local variable is assigned to but never used. - "F841": "V106", + "F841": "V107", }