diff --git a/its/ruling/src/test/resources/expected/python-S6542.json b/its/ruling/src/test/resources/expected/python-S6542.json index 23474c4d63..aab4cabdae 100644 --- a/its/ruling/src/test/resources/expected/python-S6542.json +++ b/its/ruling/src/test/resources/expected/python-S6542.json @@ -1,74 +1,55 @@ { -'project:mypy-0.782/misc/actions_stubs.py':[ -14, +"project:mypy-0.782/misc/actions_stubs.py": [ +14 ], -'project:mypy-0.782/misc/analyze_cache.py':[ +"project:mypy-0.782/misc/analyze_cache.py": [ 80, 80, 109, -109, +109 ], -'project:mypy-0.782/misc/upload-pypi.py':[ -161, +"project:mypy-0.782/misc/upload-pypi.py": [ +161 ], -'project:mypy-0.782/mypy/build.py':[ +"project:mypy-0.782/mypy/build.py": [ 831, -1391, +1391 ], -'project:mypy-0.782/mypy/dmypy_server.py':[ -804, +"project:mypy-0.782/mypy/dmypy_server.py": [ +804 ], -'project:mypy-0.782/mypy/dmypy_util.py':[ -16, +"project:mypy-0.782/mypy/dmypy_util.py": [ +16 ], -'project:mypy-0.782/mypy/fastparse.py':[ +"project:mypy-0.782/mypy/fastparse.py": [ 310, -1086, -], -'project:mypy-0.782/mypy/fastparse2.py':[ -180, -], -'project:mypy-0.782/mypy/fixup.py':[ -173, -198, -202, -205, -208, -211, -253, -], -'project:mypy-0.782/mypy/main.py':[ -142, -266, -266, -], -'project:mypy-0.782/mypy/messages.py':[ +1086 +], +"project:mypy-0.782/mypy/fastparse2.py": [ +180 +], +"project:mypy-0.782/mypy/messages.py": [ 337, -338, +338 ], -'project:mypy-0.782/mypy/metastore.py':[ -178, +"project:mypy-0.782/mypy/metastore.py": [ +178 ], -'project:mypy-0.782/mypy/plugin.py':[ +"project:mypy-0.782/mypy/plugin.py": [ 335, -477, -710, +477 ], -'project:mypy-0.782/mypy/report.py':[ +"project:mypy-0.782/mypy/report.py": [ 556, -568, +568 ], -'project:mypy-0.782/mypy/split_namespace.py':[ -24, -30, +"project:mypy-0.782/mypy/stubdoc.py": [ +48 ], -'project:mypy-0.782/mypy/stubdoc.py':[ -48, +"project:mypy-0.782/mypy/stubgenc.py": [ +113 ], -'project:mypy-0.782/mypy/stubgenc.py':[ -113, -], -'project:mypy-0.782/mypy/stubtest.py':[ +"project:mypy-0.782/mypy/stubtest.py": [ 45, 45, 55, @@ -77,31 +58,30 @@ 392, 399, 749, -869, +869 ], -'project:mypy-0.782/mypy/subtypes.py':[ -1065, +"project:mypy-0.782/mypy/subtypes.py": [ +1065 ], -'project:mypy-0.782/mypy/test/data.py':[ -261, +"project:mypy-0.782/mypy/test/data.py": [ 486, -501, +501 ], -'project:mypy-0.782/mypy/test/teststubtest.py':[ +"project:mypy-0.782/mypy/test/teststubtest.py": [ 64, 64, -743, +743 ], -'project:mypy-0.782/mypyc/ir/ops.py':[ -204, +"project:mypy-0.782/mypyc/ir/ops.py": [ +204 ], -'project:mypy-0.782/mypyc/irbuild/builder.py':[ -895, +"project:mypy-0.782/mypyc/irbuild/builder.py": [ +895 ], -'project:mypy-0.782/mypyc/irbuild/util.py':[ -40, +"project:mypy-0.782/mypyc/irbuild/util.py": [ +40 ], -'project:mypy-0.782/mypyc/test-data/fixtures/ir.py':[ +"project:mypy-0.782/mypyc/test-data/fixtures/ir.py": [ 62, 62, 166, @@ -113,41 +93,40 @@ 170, 197, 215, -215, +215 ], -'project:mypy-0.782/mypyc/test/test_run.py':[ -304, +"project:mypy-0.782/mypyc/test/test_run.py": [ +304 ], -'project:mypy-0.782/mypyc/test/test_serialization.py':[ +"project:mypy-0.782/mypyc/test/test_serialization.py": [ 18, 33, -33, +33 ], -'project:mypy-0.782/test-data/samples/crawl.py':[ +"project:mypy-0.782/test-data/samples/crawl.py": [ 94, -97, +97 ], -'project:mypy-0.782/test-data/samples/crawl2.py':[ +"project:mypy-0.782/test-data/samples/crawl2.py": [ 95, -98, +98 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/genericpath.py':[ -72, +"project:mypy-0.782/test-data/stdlib-samples/3.2/genericpath.py": [ +72 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/pprint.py':[ +"project:mypy-0.782/test-data/stdlib-samples/3.2/pprint.py": [ 89, 92, -92, +92 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/random.py':[ -100, +"project:mypy-0.782/test-data/stdlib-samples/3.2/random.py": [ 164, 666, 666, 666, -702, +702 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/shutil.py':[ +"project:mypy-0.782/test-data/stdlib-samples/3.2/shutil.py": [ 273, 278, 395, @@ -155,27 +134,27 @@ 542, 571, 636, -655, +655 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/tempfile.py':[ +"project:mypy-0.782/test-data/stdlib-samples/3.2/tempfile.py": [ 381, 598, 604, 607, -626, +626 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_base64.py':[ -232, +"project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_base64.py": [ 232, +232 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_fnmatch.py':[ -14, +"project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_fnmatch.py": [ +14 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_getopt.py':[ -23, +"project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_getopt.py": [ 23, +23 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_shutil.py':[ +"project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_shutil.py": [ 45, 45, 49, @@ -186,22 +165,19 @@ 643, 643, 863, -878, +878 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_tempfile.py':[ +"project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_tempfile.py": [ 935, 935, 953, -962, +962 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_textwrap.py':[ -41, +"project:mypy-0.782/test-data/stdlib-samples/3.2/test/test_textwrap.py": [ +41 ], -'project:mypy-0.782/test-data/stdlib-samples/3.2/textwrap.py':[ +"project:mypy-0.782/test-data/stdlib-samples/3.2/textwrap.py": [ 309, -322, -], -'project:mypy-0.782/test-data/unit/plugins/config_data.py':[ -10, -], +322 +] } diff --git a/python-checks/src/main/java/org/sonar/python/checks/UseOfAnyAsTypeHintCheck.java b/python-checks/src/main/java/org/sonar/python/checks/UseOfAnyAsTypeHintCheck.java index 91d4c1cbc7..6f77e2a127 100644 --- a/python-checks/src/main/java/org/sonar/python/checks/UseOfAnyAsTypeHintCheck.java +++ b/python-checks/src/main/java/org/sonar/python/checks/UseOfAnyAsTypeHintCheck.java @@ -20,31 +20,46 @@ package org.sonar.python.checks; import java.util.Optional; +import java.util.Set; +import java.util.function.Predicate; import javax.annotation.Nullable; import org.sonar.check.Rule; import org.sonar.plugins.python.api.PythonSubscriptionCheck; import org.sonar.plugins.python.api.SubscriptionContext; +import org.sonar.plugins.python.api.tree.Decorator; +import org.sonar.plugins.python.api.tree.FunctionDef; import org.sonar.plugins.python.api.tree.Tree; import org.sonar.plugins.python.api.tree.TypeAnnotation; +import org.sonar.python.semantic.SymbolUtils; import org.sonar.python.tree.TreeUtils; @Rule(key = "S6542") public class UseOfAnyAsTypeHintCheck extends PythonSubscriptionCheck { private static final String MESSAGE = "Use a more specific type than `Any` for this type hint."; + private static final Set OVERRIDE_FQNS = Set.of("typing.override", "typing.overload"); @Override public void initialize(Context context) { - context.registerSyntaxNodeConsumer(Tree.Kind.RETURN_TYPE_ANNOTATION, UseOfAnyAsTypeHintCheck::checkForAnyInTypeHint); - context.registerSyntaxNodeConsumer(Tree.Kind.PARAMETER_TYPE_ANNOTATION, UseOfAnyAsTypeHintCheck::checkForAnyInTypeHint); + context.registerSyntaxNodeConsumer(Tree.Kind.RETURN_TYPE_ANNOTATION, UseOfAnyAsTypeHintCheck::checkForAnyInReturnTypeAndParameters); + context.registerSyntaxNodeConsumer(Tree.Kind.PARAMETER_TYPE_ANNOTATION, UseOfAnyAsTypeHintCheck::checkForAnyInReturnTypeAndParameters); context.registerSyntaxNodeConsumer(Tree.Kind.VARIABLE_TYPE_ANNOTATION, UseOfAnyAsTypeHintCheck::checkForAnyInTypeHint); } private static void checkForAnyInTypeHint(SubscriptionContext ctx) { + Optional.of((TypeAnnotation) ctx.syntaxNode()) + .filter(UseOfAnyAsTypeHintCheck::isTypeAny) + .ifPresent(typeAnnotation -> ctx.addIssue(typeAnnotation.expression(), MESSAGE)); + } + + private static void checkForAnyInReturnTypeAndParameters(SubscriptionContext ctx) { TypeAnnotation typeAnnotation = (TypeAnnotation) ctx.syntaxNode(); - if (isTypeAny(typeAnnotation)) { - ctx.addIssue(typeAnnotation.expression(), MESSAGE); - } + Optional.of(typeAnnotation) + .filter(UseOfAnyAsTypeHintCheck::isTypeAny) + .map(annotation -> (FunctionDef) TreeUtils.firstAncestorOfKind(annotation, Tree.Kind.FUNCDEF)) + .filter(Predicate.not(UseOfAnyAsTypeHintCheck::hasFunctionOverrideOrOverloadDecorator)) + .filter(Predicate.not(UseOfAnyAsTypeHintCheck::canFunctionBeAnOverride)) + .ifPresent(functionDef -> ctx.addIssue(typeAnnotation.expression(), MESSAGE)); } private static boolean isTypeAny(@Nullable TypeAnnotation typeAnnotation) { @@ -52,4 +67,18 @@ private static boolean isTypeAny(@Nullable TypeAnnotation typeAnnotation) { .map(annotation -> "typing.Any".equals(TreeUtils.fullyQualifiedNameFromExpression(annotation.expression()))) .orElse(false); } + + private static boolean hasFunctionOverrideOrOverloadDecorator(FunctionDef currentFunctionDef) { + return currentFunctionDef.decorators().stream() + .map(Decorator::expression) + .map(TreeUtils::fullyQualifiedNameFromExpression) + .anyMatch(OVERRIDE_FQNS::contains); + } + + private static boolean canFunctionBeAnOverride(FunctionDef currentMethodDef) { + return Optional.ofNullable(TreeUtils.getFunctionSymbolFromDef(currentMethodDef)) + .map(SymbolUtils::canBeAnOverridingMethod) + .orElse(false); + } + } diff --git a/python-checks/src/test/resources/checks/useOfAnyAsTypeHint.py b/python-checks/src/test/resources/checks/useOfAnyAsTypeHint.py index f874691f85..c424b9a39d 100644 --- a/python-checks/src/test/resources/checks/useOfAnyAsTypeHint.py +++ b/python-checks/src/test/resources/checks/useOfAnyAsTypeHint.py @@ -1,4 +1,6 @@ -from typing import Any +import typing +from typing import Any, override, overload + def foo(test: str, param: Any) -> str: # Noncompliant {{Use a more specific type than `Any` for this type hint.}} #^^^ @@ -41,3 +43,74 @@ def success(param: str | int) -> None: def success_without_hint(param): pass + + +class Parent1: + ... + + + +class Child1(Parent1): + + @override + def something(): + test: Any # Noncompliant + + @override(Parent1) + def add_item(self, param1: Any) -> None: # Compliant + ... + + @overload + def add_item(self, param2: Any) -> None: # Compliant + ... + + @override + def add_item(self, text: str, param1: Any) -> None: # Compliant + ... + + @typing.overload + def add_item(self, text: str, param2: Any) -> None: # Compliant + ... + + def over(): + def wrapper(): + ... + return wrapper + + @over + def add_item(self, text: str, param3: Any) -> None: # Noncompliant + ... + + +class Parent2: + def add_item(self, text: str, param1: Any) -> None: # Noncompliant + ... + + def some_function(self, text: str) -> None: + ... + + def text_function(self, text: Any) -> None: # Noncompliant + ... + + def other_function(self, text: str, other: Any) -> None: # Noncompliant + ... + + def return_type_check(self, text) -> Any: # Noncompliant + ... + +class Child2(Parent2, object): + def add_item(self, text: str, param1: Any) -> None: # Compliant it is an override + ... + + def some_function(self, text: str, extra_param: Any) -> None: # Compliant FN + ... + + def text_function(self, text: Any, other_param) -> None: # Compliant + ... + + # Here we consider this `other_function` as an override even if the parameters are in different order. + def other_function(self, other: Any, text: str) -> None: # Compliant + ... + + def return_type_check(self, text) -> Any: # Compliant + ...