From 2ec98bea9b2d86a55b889d18d5ca9683cd8ccc0b Mon Sep 17 00:00:00 2001 From: lukas Date: Thu, 1 Jun 2023 23:46:17 +0200 Subject: [PATCH 1/5] Unused function call extension --- pylint/extensions/unassigned_function_call.py | 91 ++++++++++++ .../test_unassigned_function_call.py | 134 ++++++++++++++++++ 2 files changed, 225 insertions(+) create mode 100644 pylint/extensions/unassigned_function_call.py create mode 100644 tests/extensions/test_unassigned_function_call.py diff --git a/pylint/extensions/unassigned_function_call.py b/pylint/extensions/unassigned_function_call.py new file mode 100644 index 0000000000..2c409ee9d2 --- /dev/null +++ b/pylint/extensions/unassigned_function_call.py @@ -0,0 +1,91 @@ +# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html +# For details: https://github.com/pylint-dev/pylint/blob/main/LICENSE +# Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt + +"""Looks for unassigned function/method calls that have non-nullable return type.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import astroid +from astroid import nodes + +from pylint.checkers import BaseChecker +from pylint.checkers.typecheck import TypeChecker +from pylint.checkers.utils import only_required_for_messages, safe_infer + +if TYPE_CHECKING: + from pylint.lint import PyLinter + + +class UnusedReturnValueChecker(BaseChecker): + name = "unused_return_value" + msgs = { + "W5486": ( + "Function returned value which is never used", + "unused-return-value", + "Function returns non-nullable value which is never used. " + "Use explicit `_ = func_call()` if you are not interested in returned value", + ) + } + + @only_required_for_messages("unused-return-value") + def visit_call(self, node: nodes.Call) -> None: + result_is_used = not isinstance(node.parent, nodes.Expr) + + if result_is_used: + return + + function_node = safe_infer(node.func) + funcs = (nodes.FunctionDef, astroid.UnboundMethod, astroid.BoundMethod) + + # FIXME: more elegant solution probably exists + # methods called on instances returned by functions in some libraries + # are having function_node None and needs to be handled here + # for example: + # attrs.evolve returned instances + # instances returned by any pyrsistent method (pmap.set, pvector.append, ...) + if function_node is None: + try: + for n in node.func.infer(): + if not isinstance(n, astroid.BoundMethod): + continue + function_node = n + break + except Exception: # pylint:disable=broad-exception-caught + pass + + if not isinstance(function_node, funcs): + return + + # Unwrap to get the actual function node object + if isinstance(function_node, astroid.BoundMethod) and isinstance( + function_node._proxied, astroid.UnboundMethod + ): + function_node = function_node._proxied._proxied + + # Make sure that it's a valid function that we can analyze. + # Ordered from less expensive to more expensive checks. + if ( + not function_node.is_function + or function_node.decorators + or TypeChecker._is_ignored_function(function_node) + ): + return + + return_nodes = list( + function_node.nodes_of_class(nodes.Return, skip_klass=nodes.FunctionDef) + ) + for ret_node in return_nodes: + if not ( + isinstance(ret_node.value, nodes.Const) + and ret_node.value.value is None + or ret_node.value is None + ): + self.add_message("unused-return-value", node=node) + return + + +def register(linter: PyLinter) -> None: + linter.register_checker(UnusedReturnValueChecker(linter)) diff --git a/tests/extensions/test_unassigned_function_call.py b/tests/extensions/test_unassigned_function_call.py new file mode 100644 index 0000000000..a41ebc1709 --- /dev/null +++ b/tests/extensions/test_unassigned_function_call.py @@ -0,0 +1,134 @@ +# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html +# For details: https://github.com/pylint-dev/pylint/blob/main/LICENSE +# Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt + +import astroid + +from pylint.extensions import unassigned_function_call +from pylint.testutils import CheckerTestCase, MessageTest + + +class TestUnassignedFunctionCall(CheckerTestCase): + CHECKER_CLASS = unassigned_function_call.UnusedReturnValueChecker + + def test_simple_func(self) -> None: + node = astroid.extract_node( + """ + def a(): + return 1 + a() + """, + ) + msg = MessageTest( + msg_id="unused-return-value", + node=node, + line=4, + end_line=4, + col_offset=0, + end_col_offset=3, + ) + with self.assertAddsMessages(msg): + self.checker.visit_call(node) + + def test_simple_method(self) -> None: + node = astroid.extract_node( + """ + class A: + def return_self(self): + return self + a = A() + a.return_self() + """, + ) + msg = MessageTest( + msg_id="unused-return-value", + node=node, + line=6, + end_line=6, + col_offset=0, + end_col_offset=15, + ) + with self.assertAddsMessages(msg): + self.checker.visit_call(node) + + def test_simple_method_returns_none(self) -> None: + node = astroid.extract_node( + """ + class A: + def return_none(self): + return None + a = A() + a.return_self() + """, + ) + + with self.assertNoMessages(): + self.checker.visit_call(node) + + def test_simple_function_returns_none(self) -> None: + node = astroid.extract_node( + """ + def a(): + print("doing some stuff") + a() + """, + ) + + with self.assertNoMessages(): + self.checker.visit_call(node) + + def test_unassigned_dataclass_replace(self) -> None: + node = astroid.extract_node( + """ + from dataclasses import dataclass, replace + + @dataclass + class A: + a: int + + inst = A(1) + replace(inst, a=3) + """, + ) + + msg = MessageTest( + msg_id="unused-return-value", + node=node, + line=9, + end_line=9, + col_offset=0, + end_col_offset=18, + ) + + with self.assertAddsMessages(msg): + self.checker.visit_call(node) + + def test_unassigned_method_call_on_replaced_dataclass_inst(self) -> None: + node = astroid.extract_node( + """ + from dataclasses import dataclass, replace + + @dataclass + class A: + a: int + + def return_something(self): + return self.a + + inst = A(1) + inst = replace(inst, a=3) + inst.return_something() + """, + ) + + msg = MessageTest( + msg_id="unused-return-value", + node=node, + line=13, + end_line=13, + col_offset=0, + end_col_offset=23, + ) + + with self.assertAddsMessages(msg): + self.checker.visit_call(node) From ec3410752794608bd01acbbddc0b85488d74fe6d Mon Sep 17 00:00:00 2001 From: lukas Date: Tue, 6 Jun 2023 00:05:43 +0200 Subject: [PATCH 2/5] rename from `unused-return-value` -> `function-return-not-assigned` --- ...ction_call.py => function_return_not_assigned.py} | 12 ++++++------ ...ction_call.py => function_return_not_assigned.py} | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) rename pylint/extensions/{unassigned_function_call.py => function_return_not_assigned.py} (89%) rename tests/extensions/{test_unassigned_function_call.py => function_return_not_assigned.py} (90%) diff --git a/pylint/extensions/unassigned_function_call.py b/pylint/extensions/function_return_not_assigned.py similarity index 89% rename from pylint/extensions/unassigned_function_call.py rename to pylint/extensions/function_return_not_assigned.py index 2c409ee9d2..3fe3dd8d86 100644 --- a/pylint/extensions/unassigned_function_call.py +++ b/pylint/extensions/function_return_not_assigned.py @@ -19,18 +19,18 @@ from pylint.lint import PyLinter -class UnusedReturnValueChecker(BaseChecker): - name = "unused_return_value" +class FunctionReturnNotAssignedChecker(BaseChecker): + name = "function_return_not_assigned" msgs = { "W5486": ( "Function returned value which is never used", - "unused-return-value", + "function-return-not-assigned", "Function returns non-nullable value which is never used. " "Use explicit `_ = func_call()` if you are not interested in returned value", ) } - @only_required_for_messages("unused-return-value") + @only_required_for_messages("function-return-not-assigned") def visit_call(self, node: nodes.Call) -> None: result_is_used = not isinstance(node.parent, nodes.Expr) @@ -83,9 +83,9 @@ def visit_call(self, node: nodes.Call) -> None: and ret_node.value.value is None or ret_node.value is None ): - self.add_message("unused-return-value", node=node) + self.add_message("function-return-not-assigned", node=node) return def register(linter: PyLinter) -> None: - linter.register_checker(UnusedReturnValueChecker(linter)) + linter.register_checker(FunctionReturnNotAssignedChecker(linter)) diff --git a/tests/extensions/test_unassigned_function_call.py b/tests/extensions/function_return_not_assigned.py similarity index 90% rename from tests/extensions/test_unassigned_function_call.py rename to tests/extensions/function_return_not_assigned.py index a41ebc1709..6ee921593c 100644 --- a/tests/extensions/test_unassigned_function_call.py +++ b/tests/extensions/function_return_not_assigned.py @@ -4,12 +4,12 @@ import astroid -from pylint.extensions import unassigned_function_call +from pylint.extensions import function_return_not_assigned from pylint.testutils import CheckerTestCase, MessageTest class TestUnassignedFunctionCall(CheckerTestCase): - CHECKER_CLASS = unassigned_function_call.UnusedReturnValueChecker + CHECKER_CLASS = function_return_not_assigned.FunctionReturnNotAssignedChecker def test_simple_func(self) -> None: node = astroid.extract_node( @@ -20,7 +20,7 @@ def a(): """, ) msg = MessageTest( - msg_id="unused-return-value", + msg_id="function-return-not-assigned", node=node, line=4, end_line=4, @@ -41,7 +41,7 @@ def return_self(self): """, ) msg = MessageTest( - msg_id="unused-return-value", + msg_id="function-return-not-assigned", node=node, line=6, end_line=6, @@ -92,7 +92,7 @@ class A: ) msg = MessageTest( - msg_id="unused-return-value", + msg_id="function-return-not-assigned", node=node, line=9, end_line=9, @@ -122,7 +122,7 @@ def return_something(self): ) msg = MessageTest( - msg_id="unused-return-value", + msg_id="function-return-not-assigned", node=node, line=13, end_line=13, From 4e0a984e173b3c5ae9bbc0cfc78cbb8050061afe Mon Sep 17 00:00:00 2001 From: lukas Date: Tue, 6 Jun 2023 00:09:26 +0200 Subject: [PATCH 3/5] adding changelog --- doc/whatsnew/fragments/7935.new_check | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 doc/whatsnew/fragments/7935.new_check diff --git a/doc/whatsnew/fragments/7935.new_check b/doc/whatsnew/fragments/7935.new_check new file mode 100644 index 0000000000..9d80757362 --- /dev/null +++ b/doc/whatsnew/fragments/7935.new_check @@ -0,0 +1,3 @@ +Add ``FunctionReturnNotAssignedChecker`` extension and new ``function-return-not-assigned`` message if return value not used. + +Refs #7935 From b502545a146472b6e8a3e40ec83fdb1d328316aa Mon Sep 17 00:00:00 2001 From: lukas Date: Tue, 6 Jun 2023 00:14:29 +0200 Subject: [PATCH 4/5] adding bad and good example --- doc/data/messages/f/function-return-not-assigned/bad.py | 5 +++++ doc/data/messages/f/function-return-not-assigned/good.py | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 doc/data/messages/f/function-return-not-assigned/bad.py create mode 100644 doc/data/messages/f/function-return-not-assigned/good.py diff --git a/doc/data/messages/f/function-return-not-assigned/bad.py b/doc/data/messages/f/function-return-not-assigned/bad.py new file mode 100644 index 0000000000..779ce785d5 --- /dev/null +++ b/doc/data/messages/f/function-return-not-assigned/bad.py @@ -0,0 +1,5 @@ +def return_int(): + return 1 + + +return_int() # [function-return-not-assigned] diff --git a/doc/data/messages/f/function-return-not-assigned/good.py b/doc/data/messages/f/function-return-not-assigned/good.py new file mode 100644 index 0000000000..f409a51e16 --- /dev/null +++ b/doc/data/messages/f/function-return-not-assigned/good.py @@ -0,0 +1,5 @@ +def return_int(): + return 1 + + +_ = return_int() From 900b7a40fbef6f94218ec8733873aebaf3f7041a Mon Sep 17 00:00:00 2001 From: lukas Date: Tue, 6 Jun 2023 00:17:29 +0200 Subject: [PATCH 5/5] switch to functional tests --- .../function_return_not_assigned.py | 134 ------------------ .../function_return_not_assigned.py | 56 ++++++++ .../function_return_not_assigned.rc | 2 + .../function_return_not_assigned.txt | 4 + .../f/function_return_not_assigned.txt | 0 5 files changed, 62 insertions(+), 134 deletions(-) delete mode 100644 tests/extensions/function_return_not_assigned.py create mode 100644 tests/functional/ext/function_return_not_assigned/function_return_not_assigned.py create mode 100644 tests/functional/ext/function_return_not_assigned/function_return_not_assigned.rc create mode 100644 tests/functional/ext/function_return_not_assigned/function_return_not_assigned.txt create mode 100644 tests/functional/f/function_return_not_assigned.txt diff --git a/tests/extensions/function_return_not_assigned.py b/tests/extensions/function_return_not_assigned.py deleted file mode 100644 index 6ee921593c..0000000000 --- a/tests/extensions/function_return_not_assigned.py +++ /dev/null @@ -1,134 +0,0 @@ -# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html -# For details: https://github.com/pylint-dev/pylint/blob/main/LICENSE -# Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt - -import astroid - -from pylint.extensions import function_return_not_assigned -from pylint.testutils import CheckerTestCase, MessageTest - - -class TestUnassignedFunctionCall(CheckerTestCase): - CHECKER_CLASS = function_return_not_assigned.FunctionReturnNotAssignedChecker - - def test_simple_func(self) -> None: - node = astroid.extract_node( - """ - def a(): - return 1 - a() - """, - ) - msg = MessageTest( - msg_id="function-return-not-assigned", - node=node, - line=4, - end_line=4, - col_offset=0, - end_col_offset=3, - ) - with self.assertAddsMessages(msg): - self.checker.visit_call(node) - - def test_simple_method(self) -> None: - node = astroid.extract_node( - """ - class A: - def return_self(self): - return self - a = A() - a.return_self() - """, - ) - msg = MessageTest( - msg_id="function-return-not-assigned", - node=node, - line=6, - end_line=6, - col_offset=0, - end_col_offset=15, - ) - with self.assertAddsMessages(msg): - self.checker.visit_call(node) - - def test_simple_method_returns_none(self) -> None: - node = astroid.extract_node( - """ - class A: - def return_none(self): - return None - a = A() - a.return_self() - """, - ) - - with self.assertNoMessages(): - self.checker.visit_call(node) - - def test_simple_function_returns_none(self) -> None: - node = astroid.extract_node( - """ - def a(): - print("doing some stuff") - a() - """, - ) - - with self.assertNoMessages(): - self.checker.visit_call(node) - - def test_unassigned_dataclass_replace(self) -> None: - node = astroid.extract_node( - """ - from dataclasses import dataclass, replace - - @dataclass - class A: - a: int - - inst = A(1) - replace(inst, a=3) - """, - ) - - msg = MessageTest( - msg_id="function-return-not-assigned", - node=node, - line=9, - end_line=9, - col_offset=0, - end_col_offset=18, - ) - - with self.assertAddsMessages(msg): - self.checker.visit_call(node) - - def test_unassigned_method_call_on_replaced_dataclass_inst(self) -> None: - node = astroid.extract_node( - """ - from dataclasses import dataclass, replace - - @dataclass - class A: - a: int - - def return_something(self): - return self.a - - inst = A(1) - inst = replace(inst, a=3) - inst.return_something() - """, - ) - - msg = MessageTest( - msg_id="function-return-not-assigned", - node=node, - line=13, - end_line=13, - col_offset=0, - end_col_offset=23, - ) - - with self.assertAddsMessages(msg): - self.checker.visit_call(node) diff --git a/tests/functional/ext/function_return_not_assigned/function_return_not_assigned.py b/tests/functional/ext/function_return_not_assigned/function_return_not_assigned.py new file mode 100644 index 0000000000..ac9e69ba80 --- /dev/null +++ b/tests/functional/ext/function_return_not_assigned/function_return_not_assigned.py @@ -0,0 +1,56 @@ +# pylint: disable=missing-function-docstring, missing-module-docstring, missing-class-docstring, expression-not-assigned, invalid-name +from dataclasses import dataclass, replace + + +def func_that_returns_something(): + return 1 + + +func_that_returns_something() # [function-return-not-assigned] + +_ = func_that_returns_something() + +if func_that_returns_something(): + pass + + +def func_that_returns_none(): + return None + + +def func_with_no_explicit_return(): + print("I am doing something") + + +func_that_returns_none() +func_with_no_explicit_return() + +some_var = "" +# next line should probably raise? +func_that_returns_something() if some_var else func_that_returns_none() +_ = func_that_returns_something() if some_var else func_that_returns_none() +func_with_no_explicit_return() if some_var else func_that_returns_none() + + +@dataclass +class TestClass: + value: int + + def return_self(self): + return self + + def return_none(self): + pass + + +inst = TestClass(1) +inst.return_self() # [function-return-not-assigned] +inst.return_none() + +replace(inst, value=3) # [function-return-not-assigned] + +inst = replace(inst, value=3) + +inst.return_self() # [function-return-not-assigned] +inst.return_none() +inst = inst.return_self() diff --git a/tests/functional/ext/function_return_not_assigned/function_return_not_assigned.rc b/tests/functional/ext/function_return_not_assigned/function_return_not_assigned.rc new file mode 100644 index 0000000000..3b66585607 --- /dev/null +++ b/tests/functional/ext/function_return_not_assigned/function_return_not_assigned.rc @@ -0,0 +1,2 @@ +[MAIN] +load-plugins=pylint.extensions.function_return_not_assigned, diff --git a/tests/functional/ext/function_return_not_assigned/function_return_not_assigned.txt b/tests/functional/ext/function_return_not_assigned/function_return_not_assigned.txt new file mode 100644 index 0000000000..5da590fa7a --- /dev/null +++ b/tests/functional/ext/function_return_not_assigned/function_return_not_assigned.txt @@ -0,0 +1,4 @@ +function-return-not-assigned:9:0:9:29::Function returned value which is never used:UNDEFINED +function-return-not-assigned:47:0:47:18::Function returned value which is never used:UNDEFINED +function-return-not-assigned:50:0:50:22::Function returned value which is never used:UNDEFINED +function-return-not-assigned:54:0:54:18::Function returned value which is never used:UNDEFINED diff --git a/tests/functional/f/function_return_not_assigned.txt b/tests/functional/f/function_return_not_assigned.txt new file mode 100644 index 0000000000..e69de29bb2