PY-60104 Don't try to infer side effects of not type hinted decorators

Assume that such decorators as well as "well-known" decorators, which we special-case,
don't change signatures of decorated functions and classes.

This change effectively stops the long-standing policy of safe-listing a few recognized
"well-known" decorators and assuming everything else can change a definition in any
way. This approach doesn't apply well to the current state of the Python world, where most
of the common side effects of decorators, such as adding new parameters, can be expressed
in type hints.

In 2021.1 we added PyDecoratedFunctionTypeProvider that was able to infer a return type of
decorator over its body, as for any other function, and then correctly apply this information
to a decorated definition. It led to a number of problems.

First of all, depending on whether TypeEvalContext allowed us to access AST of a decorator's
body, we inferred different signatures for functions decorated with an imported decorator in
inspections and in user-initiated actions, such as Parameter Info.

Secondly, we started inferring useless `(*args, **kwargs)` signatures in case of decorators
defined following the common pattern of returning a wrapper function accepting arbitrary
parameters and itself decorated with @functools.wraps (PY-48338). In some sense, our code
analysis was "too smart" in its type inference in this case.

Lastly, we diluted the return types of functions decorated with unknown decorators, even
fully typed, by uniting these types with Any (so-called "weak" types). This logic
existed before PyDecoratedFunctionTypeProvider, but it became more problematic now
than we were able to propagate this artificial union through generic decorators.

This change in behavior might lead to some false positives for untyped Python code
with non-pure decorators. However, given that other type checkers are also likely to hit these
problems, there is now a stronger incentive to add type hints for such problematic APIs.
In the worst case, we can special-case some heavily requested decorators as we did before.

GitOrigin-RevId: db11fb3573bda5da155cb921a30adc31d5c841e2
This commit is contained in:
Mikhail Golubev
2024-01-09 20:49:13 +00:00
committed by intellij-monorepo-bot
parent 52f21cc60d
commit 3079150697
13 changed files with 74 additions and 76 deletions
@@ -514,7 +514,6 @@
<dialectsTokenSetContributor implementation="com.jetbrains.python.PythonTokenSetContributor"/>
<typeProvider implementation="com.jetbrains.python.psi.types.PyCollectionTypeByModificationsProvider" order="last"/>
<typeProvider implementation="com.jetbrains.python.codeInsight.stdlib.PyDataclassTypeProvider"/>
<typeProvider implementation="com.jetbrains.python.codeInsight.decorator.PyDecoratedFunctionTypeProvider"/>
@@ -558,6 +557,7 @@
<!--stdlib-->
<canonicalPathProvider implementation="com.jetbrains.python.codeInsight.stdlib.PyStdlibCanonicalPathProvider"/>
<inspectionExtension implementation="com.jetbrains.python.inspections.stdlib.PyStdlibInspectionExtension"/>
<typeProvider implementation="com.jetbrains.python.codeInsight.stdlib.PyDataclassTypeProvider"/>
<typeProvider implementation="com.jetbrains.python.codeInsight.stdlib.PyNamedTupleTypeProvider"/>
<!-- This provider should be able to override the results on anything else, including types coming from .pyi stubs -->
<typeProvider implementation="com.jetbrains.python.codeInsight.stdlib.PyStdlibTypeProvider" order="first"/>
@@ -91,7 +91,7 @@ object PyDataclassNames {
* It should be used only to map arguments to parameters and
* determine what settings dataclass has.
*/
val DECORATOR_AND_TYPE_AND_PARAMETERS = listOf(
private val DECORATOR_AND_TYPE_AND_PARAMETERS = listOf(
Triple(KnownDecorator.DATACLASSES_DATACLASS, PyDataclassParameters.PredefinedType.STD, PyDataclassNames.Dataclasses.DECORATOR_PARAMETERS),
Triple(KnownDecorator.ATTR_S, PyDataclassParameters.PredefinedType.ATTRS, PyDataclassNames.Attrs.DECORATOR_PARAMETERS),
Triple(KnownDecorator.ATTR_ATTRS, PyDataclassParameters.PredefinedType.ATTRS, PyDataclassNames.Attrs.DECORATOR_PARAMETERS),
@@ -5,9 +5,8 @@ import com.intellij.openapi.util.RecursionManager;
import com.intellij.openapi.util.Ref;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.PsiElement;
import com.intellij.psi.util.QualifiedName;
import com.intellij.util.containers.ContainerUtil;
import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil;
import com.jetbrains.python.codeInsight.typing.PyTypingTypeProvider;
import com.jetbrains.python.psi.*;
import com.jetbrains.python.psi.resolve.PyResolveUtil;
import com.jetbrains.python.psi.types.PyType;
@@ -16,9 +15,6 @@ import com.jetbrains.python.psi.types.TypeEvalContext;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import static com.jetbrains.python.psi.types.PyTypeUtil.notNullToRef;
@@ -39,17 +35,10 @@ public final class PyDecoratedFunctionTypeProvider extends PyTypeProviderBase {
return null;
}
List<PyKnownDecoratorUtil.KnownDecorator> filteredDecorators = new ArrayList<>();
filteredDecorators.add(PyKnownDecoratorUtil.KnownDecorator.TYPING_OVERLOAD);
filteredDecorators.add(PyKnownDecoratorUtil.KnownDecorator.STATICMETHOD);
filteredDecorators.add(PyKnownDecoratorUtil.KnownDecorator.CLASSMETHOD);
var decorators = Arrays.stream(decoratorList.getDecorators()).toList();
var haveSpecialCase = ContainerUtil.exists(decorators, d ->
ContainerUtil.exists(PyKnownDecoratorUtil.asKnownDecorators(d, context),
it -> ContainerUtil.exists(filteredDecorators, filtered -> filtered.equals(it))));
if (haveSpecialCase) return null;
List<PyDecorator> decorators = ContainerUtil.filter(decoratorList.getDecorators(), d -> !isTransparentDecorator(d, context));
if (decorators.isEmpty()) {
return null;
}
/* Our goal is to infer the type of reference of decorated object.
* For that we going to infer a type of expression <code>decorator(reference)<code>.
@@ -63,52 +52,39 @@ public final class PyDecoratedFunctionTypeProvider extends PyTypeProviderBase {
);
}
private static boolean isTransparentDecorator(@NotNull PyDecorator decorator, @NotNull TypeEvalContext context) {
return !PyKnownDecoratorUtil.asKnownDecorators(decorator, context).isEmpty() || isUntypedDecorator(decorator, context);
}
private static boolean isUntypedDecorator(@NotNull PyDecorator decorator, @NotNull TypeEvalContext context) {
QualifiedName qualifiedName = decorator.getQualifiedName();
if (qualifiedName == null) return false;
// Decorator is the only expression persisted in PSI stubs.
// Calling getReference().resolve() will cause un-stubbing of the containing file
List<PsiElement> resolved = PyResolveUtil.resolveQualifiedNameInScope(qualifiedName, (PyFile)decorator.getContainingFile(), context);
for (PsiElement res : resolved) {
if (res instanceof PyFunction function) {
if (function.getTypeCommentAnnotation() != null || function.getAnnotation() != null) {
return false;
}
}
else if (res instanceof PyClass) {
return false;
}
}
return true;
}
@Nullable
private static Ref<PyType> evaluateType(@NotNull PyDecoratable referenceTarget,
@NotNull TypeEvalContext context,
@NotNull List<PyDecorator> decorators) {
PyType sourceType = null;
if (referenceTarget instanceof PyTypedElement typedElement) {
sourceType = context.getType(typedElement);
PyExpression fakeCallExpression = fakeCallExpression(referenceTarget, decorators, context);
if (fakeCallExpression == null) {
return null;
}
var annotatedDecorators = getAnnotatedDecorators(referenceTarget, decorators, context);
if (!annotatedDecorators.isEmpty()) {
PyExpression fakeCallExpression = fakeCallExpression(referenceTarget, annotatedDecorators, context);
if (fakeCallExpression == null) {
return null;
}
var fakeCallExpressionType = context.getType(fakeCallExpression);
return notNullToRef(fakeCallExpressionType);
}
return notNullToRef(sourceType);
}
@NotNull
private static List<PyDecorator> getAnnotatedDecorators(@NotNull PyDecoratable referenceTarget,
@NotNull List<PyDecorator> decorators,
@NotNull TypeEvalContext context) {
var result = new ArrayList<PyDecorator>();
var scopeOwner = ScopeUtil.getScopeOwner(referenceTarget);
if (scopeOwner == null) return Collections.emptyList();
for (var decorator : decorators) {
var qualifiedName = decorator.getQualifiedName();
if (qualifiedName == null) continue;
var resolved = PyResolveUtil.resolveQualifiedNameInScope(qualifiedName, scopeOwner, context);
for (var res : resolved) {
if (res instanceof PyFunction function) {
var annotation = PyTypingTypeProvider.getReturnTypeAnnotation(function, context);
if (annotation != null) {
result.add(decorator);
}
}
else if (res instanceof PyClass) {
result.add(decorator);
}
}
}
return result;
// TODO Don't ignore explicit return type Any on one of the decorators
return notNullToRef(context.getType(fakeCallExpression));
}
@Nullable
@@ -205,11 +205,6 @@ public class PyFunctionImpl extends PyBaseElementImpl<PyFunctionStub> implements
inferredType = getReturnStatementType(context);
}
}
if (getProperty() == null && PyKnownDecoratorUtil.hasUnknownOrChangingReturnTypeDecorator(this, context)) {
inferredType = PyUnionType.createWeakType(inferredType);
}
return PyTypingTypeProvider.toAsyncIfNeeded(this, inferredType);
}
@@ -1,9 +1,13 @@
import abc
from abc import abstractmethod
from typing import Callable
def decorator(f):
return f
def typed_decorator(f) -> Callable[..., int]:
return f
class C(object):
@@ -20,6 +24,10 @@ class C(object):
@decorator
def baz(self):
pass
@typed_decorator
def baz2(self):
pass
def quux(self):
pass
@@ -27,5 +35,6 @@ class C(object):
def test(self):
a = self.foo()
b = self.bar()
c = self.baz()
<weak_warning descr="Function 'baz' doesn't return anything">c1 = self.baz()</weak_warning>
c2 = self.baz2()
<weak_warning descr="Function 'quux' doesn't return anything">d = self.quux()</weak_warning>
@@ -5,4 +5,4 @@ def decorator(f):
def foo():
return 'foo'
print(<warning descr="Expected type 'int', got '() -> Union[str, Any]' instead">foo</warning> + 3) # we know type at least
print(<warning descr="Expected type 'int', got '() -> str' instead">foo</warning> + 3) # we know type at least
@@ -1 +1 @@
<span style="color:#808000;">@decorator1</span>, <span style="color:#808000;">@decorator2</span><br/><span style="color:#000080;font-weight:bold;">def </span><span style="color:#000000;">foo</span><span style="">(</span><span style="color:#000000;">param</span><span style="">: </span><span style="color:#000000;">Any</span><span style="">)</span> -&gt; <span style="color:#000000;">Any<span style=""> | </span><span style="color:#000080;font-weight:bold;">None</span></span>
<span style="color:#808000;">@decorator1</span>, <span style="color:#808000;">@decorator2</span><br/><span style="color:#000080;font-weight:bold;">def </span><span style="color:#000000;">foo</span><span style="">(</span><span style="color:#000000;">param</span><span style="">: </span><span style="color:#000000;">Any</span><span style="">)</span> -&gt; <span style="color:#000000;"><span style="color:#000080;font-weight:bold;">None</span></span>
@@ -1 +1 @@
<span style="color:#808000;">@decorator</span><br/><span style="color:#000080;font-weight:bold;">def </span><span style="color:#000000;">foo</span><span style="">(</span><span style="color:#000000;">param</span><span style="">: </span><span style="color:#000000;">Any</span><span style="">)</span> -&gt; <span style="color:#000000;">Any<span style=""> | </span><span style="color:#000080;font-weight:bold;">None</span></span>
<span style="color:#808000;">@decorator</span><br/><span style="color:#000080;font-weight:bold;">def </span><span style="color:#000000;">foo</span><span style="">(</span><span style="color:#000000;">param</span><span style="">: </span><span style="color:#000000;">Any</span><span style="">)</span> -&gt; <span style="color:#000000;"><span style="color:#000080;font-weight:bold;">None</span></span>
@@ -1 +1 @@
<html><body><div class="bottom"><icon src="AllIcons.Nodes.Package"/>&nbsp;<code><a href="psi_element://#module#ManyDecoratorsFunction">ManyDecoratorsFunction</a></code></div><div class="definition"><pre><span style="color:#808000;">@decorator1</span><br/><span style="color:#808000;">@decorator2</span><br/><span style="color:#000080;font-weight:bold;">def </span><span style="color:#000000;">foo</span><span style="">(</span><span style="color:#000000;">param</span><span style="">: </span><span style="color:#000000;">Any</span><span style="">)</span> -&gt; <span style="color:#000000;">Any<span style=""> | </span><span style="color:#000080;font-weight:bold;">None</span></span></pre></div></body></html>
<html><body><div class="bottom"><icon src="AllIcons.Nodes.Package"/>&nbsp;<code><a href="psi_element://#module#ManyDecoratorsFunction">ManyDecoratorsFunction</a></code></div><div class="definition"><pre><span style="color:#808000;">@decorator1</span><br/><span style="color:#808000;">@decorator2</span><br/><span style="color:#000080;font-weight:bold;">def </span><span style="color:#000000;">foo</span><span style="">(</span><span style="color:#000000;">param</span><span style="">: </span><span style="color:#000000;">Any</span><span style="">)</span> -&gt; <span style="color:#000000;"><span style="color:#000080;font-weight:bold;">None</span></span></pre></div></body></html>
+1 -1
View File
@@ -1 +1 @@
<html><body><div class="bottom"><icon src="AllIcons.Nodes.Class"/>&nbsp;<code><a href="psi_element://#typename#Method.Foo">Method.Foo</a></code></div><div class="definition"><pre><span style="color:#808000;">@deco</span><br/><span style="color:#000080;font-weight:bold;">def </span><span style="color:#000000;">meth</span><span style="">(</span><span style="color:#94558d;">self</span><span style="">)</span> -&gt; <span style="color:#000000;">Any<span style=""> | </span><span style="color:#000080;font-weight:bold;">None</span></span></pre></div><div class="content">Doc of meth.</div></body></html>
<html><body><div class="bottom"><icon src="AllIcons.Nodes.Class"/>&nbsp;<code><a href="psi_element://#typename#Method.Foo">Method.Foo</a></code></div><div class="definition"><pre><span style="color:#808000;">@deco</span><br/><span style="color:#000080;font-weight:bold;">def </span><span style="color:#000000;">meth</span><span style="">(</span><span style="color:#94558d;">self</span><span style="">)</span> -&gt; <span style="color:#000000;"><span style="color:#000080;font-weight:bold;">None</span></span></pre></div><div class="content">Doc of meth.</div></body></html>
@@ -1 +1 @@
<html><body><div class="bottom"><icon src="AllIcons.Nodes.Package"/>&nbsp;<code><a href="psi_element://#module#OneDecoratorFunction">OneDecoratorFunction</a></code></div><div class="definition"><pre><span style="color:#808000;">@decorator</span><br/><span style="color:#000080;font-weight:bold;">def </span><span style="color:#000000;">foo</span><span style="">(</span><span style="color:#000000;">param</span><span style="">: </span><span style="color:#000000;">Any</span><span style="">)</span> -&gt; <span style="color:#000000;">Any<span style=""> | </span><span style="color:#000080;font-weight:bold;">None</span></span></pre></div></body></html>
<html><body><div class="bottom"><icon src="AllIcons.Nodes.Package"/>&nbsp;<code><a href="psi_element://#module#OneDecoratorFunction">OneDecoratorFunction</a></code></div><div class="definition"><pre><span style="color:#808000;">@decorator</span><br/><span style="color:#000080;font-weight:bold;">def </span><span style="color:#000000;">foo</span><span style="">(</span><span style="color:#000000;">param</span><span style="">: </span><span style="color:#000000;">Any</span><span style="">)</span> -&gt; <span style="color:#000000;"><span style="color:#000080;font-weight:bold;">None</span></span></pre></div></body></html>
@@ -1563,7 +1563,7 @@ public class Py3TypeTest extends PyTestCase {
// PY-48338
public void testDecoratedFunctionHasOriginalFunctionType() {
doTest("(input_a: int, input_b: float) -> float | Any",
doTest("(input_a: int, input_b: float) -> float",
"import functools\n" +
"\n" +
"def decorator(func):\n" +
@@ -1585,7 +1585,7 @@ public class Py3TypeTest extends PyTestCase {
// PY-48338
public void testDecoratedFromOtherFileFunctionHasOriginalFunctionType() {
doMultiFileTest("(input_a: int, input_b: float) -> float | Any",
doMultiFileTest("(input_a: int, input_b: float) -> float",
"import functools\n" +
"from dec_mod import decorator\n" +
"\n" +
@@ -1601,7 +1601,7 @@ public class Py3TypeTest extends PyTestCase {
// PY-48338
public void testTwiceDecoratedFunctionHasUnionOfOriginalFunctionAndUnknownCallableType() {
doTest("(input_a: int, input_b: float) -> float | Any",
doTest("(input_a: int, input_b: float) -> float",
"import functools\n" +
"\n" +
"\n" +
@@ -1633,7 +1633,7 @@ public class Py3TypeTest extends PyTestCase {
// PY-48338
public void testDecoratedFunctionWithUnknownWrapperDecoratorHasUnionOriginalFunctionAndUnknownCallableType() {
doTest("(input_a: int, input_b: float) -> float | Any",
doTest("(input_a: int, input_b: float) -> float",
"import functools\n" +
"\n" +
"\n" +
@@ -1664,7 +1664,7 @@ public class Py3TypeTest extends PyTestCase {
// PY-48338
public void testDecoratedFunctionWithOtherWrapperParametersHasWrapperType() {
doTest("(str, input_a: int, input_b: float) -> float | Any",
doTest("(str, input_a: int, input_b: float) -> float",
"import functools\n" +
"from typing import Callable, ParamSpec, TypeVar, Concatenate\n" +
"P = ParamSpec(\"P\")\n" +
@@ -412,6 +412,24 @@ public class PyDecoratedFunctionTypeProviderTest extends PyTestCase {
""");
}
public void testUntypedFunctionDecoratedWithTypedDecorator() {
doTest("str", "() -> str", """
from typing import Callable, TypeVar
T = TypeVar('T')
def d(fn: Callable[[], T]) -> Callable[[], T]:
return fn
@d
def f():
return 'foo'
value = f()
dec_func = f
""");
}
private void doTest(@NotNull String expectedValueType, @NotNull String expectedFuncType, @NotNull String text) {
myFixture.configureByText(PythonFileType.INSTANCE, text);
checkTypes(expectedValueType, expectedFuncType, allContexts());