Skip to content

Commit 31dd925

Browse files
alvov26intellij-monorepo-bot
alvov26
authored and
intellij-monorepo-bot
committed
PY-20611 Missing warning about functions implicitly returning None when return type is not Optional
Updated PyFunction to account for implicit 'return None' statements when inferring return statement types. It affected return type inference of PyFunction. Fixed a failing test related to formatted strings. Added a quick fix to make all return statements explicit. Updated the CFG to include PyPassStatements, enabling detection of exit points in empty functions. Simplified PyMakeFunctionReturnTypeQuickFix to independently infer function types and handle required imports. Currently, it does not support specifying custom suggested types. Merge-request: IJ-MR-148719 Merged-by: Aleksandr Govenko <aleksandr.govenko@jetbrains.com> GitOrigin-RevId: 9f58961f9eb70e4f9dbba7359f5aafdfd392b7e2
1 parent f2982c6 commit 31dd925

32 files changed

Lines changed: 429 additions & 300 deletions

‎python/python-ast/src/com/jetbrains/python/ast/PyAstElementVisitor.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ open class PyAstElementVisitor : PsiElementVisitor() {
101101
open fun visitPyReturnStatement(node: PyAstReturnStatement) {
102102
visitPyStatement(node)
103103
}
104+
105+
open fun visitPyPassStatement(node: PyAstPassStatement) {
106+
visitPyStatement(node)
107+
}
104108

105109
open fun visitPyYieldExpression(node: PyAstYieldExpression) {
106110
visitPyExpression(node)

‎python/python-ast/src/com/jetbrains/python/ast/PyAstPassStatement.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,8 @@
66

77
@ApiStatus.Experimental
88
public interface PyAstPassStatement extends PyAstStatement {
9+
@Override
10+
default void acceptPyVisitor(PyAstElementVisitor pyVisitor) {
11+
pyVisitor.visitPyPassStatement(this);
12+
}
913
}

‎python/python-psi-api/src/com/jetbrains/python/psi/PyElementVisitor.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,10 @@ public void visitPyAssertStatement(@NotNull PyAssertStatement node) {
274274
visitPyElement(node);
275275
}
276276

277+
public void visitPyPassStatement(@NotNull PyPassStatement node) {
278+
visitPyStatement(node);
279+
}
280+
277281
public void visitPyNoneLiteralExpression(@NotNull PyNoneLiteralExpression node) {
278282
visitPyElement(node);
279283
}

‎python/python-psi-api/src/com/jetbrains/python/psi/PyFunction.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@
88
import com.jetbrains.python.ast.*;
99
import com.jetbrains.python.ast.impl.PyPsiUtilsCore;
1010
import com.jetbrains.python.codeInsight.controlflow.ScopeOwner;
11+
import com.jetbrains.python.psi.impl.PyTypeProvider;
1112
import com.jetbrains.python.psi.stubs.PyFunctionStub;
1213
import com.jetbrains.python.psi.types.PyType;
1314
import com.jetbrains.python.psi.types.TypeEvalContext;
15+
import org.jetbrains.annotations.ApiStatus;
1416
import org.jetbrains.annotations.NotNull;
1517
import org.jetbrains.annotations.Nullable;
1618

@@ -28,9 +30,38 @@ public interface PyFunction extends PyAstFunction, StubBasedPsiElement<PyFunctio
2830
PyFunction[] EMPTY_ARRAY = new PyFunction[0];
2931
ArrayFactory<PyFunction> ARRAY_FACTORY = count -> count == 0 ? EMPTY_ARRAY : new PyFunction[count];
3032

33+
/**
34+
* Infers function's return type by analyzing <b>only return statements</b> (including implicit returns) in its control flow.
35+
* Does not consider yield statements or return type annotations.
36+
*
37+
* @see PyFunction#getInferredReturnType(TypeEvalContext)
38+
*/
3139
@Nullable
3240
PyType getReturnStatementType(@NotNull TypeEvalContext context);
3341

42+
/**
43+
* Infers function's return type by analyzing <b>return statements</b> (including implicit returns) and <b>yield expression</b>.
44+
* In contrast with {@link TypeEvalContext#getReturnType(PyCallable)} does not consider
45+
* return type annotations or any other {@link PyTypeProvider}.
46+
*
47+
* @apiNote Does not cache the result.
48+
*/
49+
@ApiStatus.Internal
50+
@Nullable
51+
PyType getInferredReturnType(@NotNull TypeEvalContext context);
52+
53+
/**
54+
* Returns a list of all function exit points that can return a value.
55+
* This includes explicit 'return' statements and statements that can complete
56+
* normally with an implicit 'return None', excluding statements that raise exceptions.
57+
*
58+
* @see PyFunction#getReturnStatementType(TypeEvalContext)
59+
* @return List of exit point statements, in control flow order
60+
*/
61+
@ApiStatus.Internal
62+
@NotNull
63+
List<PyStatement> getReturnPoints(@NotNull TypeEvalContext context);
64+
3465
/**
3566
* Checks whether the function contains a yield expression in its body.
3667
*/

‎python/python-psi-impl/resources/messages/PyPsiBundle.properties‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,9 @@ QFIX.remove.decorator=Remove decorator
460460
QFIX.NAME.make.function.return.type=Make function return inferred type
461461
QFIX.make.function.return.type=Make ''{0}'' return ''{1}''
462462

463+
# PyMakeReturnsExplicitQuickFix
464+
QFIX.NAME.make.return.stmts.explicit=Make 'return None' statements explicit
465+
463466
# Add method quick-fix
464467
QFIX.NAME.add.method.to.class=Add method to class
465468
QFIX.add.method.to.class=Add method {0}() to class {1}
@@ -1055,7 +1058,7 @@ INSP.NAME.type.checker=Incorrect type
10551058
INSP.type.checker.expected.type.got.type.instead=Expected type ''{0}'', got ''{1}'' instead
10561059
INSP.type.checker.typed.dict.extra.key=Extra key ''{0}'' for TypedDict ''{1}''
10571060
INSP.type.checker.typed.dict.missing.keys=TypedDict ''{0}'' has missing {1,choice,1#key|2#keys}: {2}
1058-
INSP.type.checker.expected.to.return.type.got.no.return=Expected to return ''{0}'', got no return
1061+
INSP.type.checker.returning.type.has.implicit.return=Function returning ''{0}'' has implicit ''return None''
10591062
INSP.type.checker.init.should.return.none=__init__ should return None
10601063
INSP.type.checker.type.does.not.have.expected.attribute=Type ''{0}'' doesn''t have expected {1,choice,1#attribute|2#attributes} {2}
10611064
INSP.type.checker.only.concrete.class.can.be.used.where.matched.protocol.expected=Only a concrete class can be used where ''{0}'' (matched generic type ''{1}'') protocol is expected

‎python/python-psi-impl/src/com/jetbrains/python/inspections/PyStringFormatInspection.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,7 @@ else if (callType instanceof PyUnionType) {
628628
allForSure = allForSure && elementsCount != -1;
629629
maxNumber = Math.max(maxNumber, elementsCount);
630630
}
631-
else {
631+
else if (!(member instanceof PyNoneType)) {
632632
allForSure = false;
633633
}
634634
}

‎python/python-psi-impl/src/com/jetbrains/python/inspections/PyTypeCheckerInspection.java‎

Lines changed: 49 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import com.jetbrains.python.codeInsight.typing.PyTypingTypeProvider;
1919
import com.jetbrains.python.documentation.PythonDocumentationProvider;
2020
import com.jetbrains.python.inspections.quickfix.PyMakeFunctionReturnTypeQuickFix;
21+
import com.jetbrains.python.inspections.quickfix.PyMakeReturnsExplicitFix;
2122
import com.jetbrains.python.psi.*;
2223
import com.jetbrains.python.psi.resolve.PyResolveContext;
2324
import com.jetbrains.python.psi.types.*;
@@ -27,6 +28,7 @@
2728

2829
import java.util.*;
2930

31+
import static com.intellij.util.containers.ContainerUtil.exists;
3032
import static com.jetbrains.python.psi.PyUtil.as;
3133
import static com.jetbrains.python.psi.impl.PyCallExpressionHelper.*;
3234

@@ -44,10 +46,17 @@ public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean is
4446
}
4547

4648
public static class Visitor extends PyInspectionVisitor {
47-
public Visitor(@Nullable ProblemsHolder holder, @NotNull TypeEvalContext context) {
49+
public Visitor(@NotNull ProblemsHolder holder, @NotNull TypeEvalContext context) {
4850
super(holder, context);
4951
}
5052

53+
@Override
54+
protected @NotNull ProblemsHolder getHolder() {
55+
var holder = super.getHolder();
56+
assert holder != null;
57+
return holder;
58+
}
59+
5160
// TODO: Visit decorators with arguments
5261
@Override
5362
public void visitPyCallExpression(@NotNull PyCallExpression node) {
@@ -83,32 +92,38 @@ public void visitPyReturnStatement(@NotNull PyReturnStatement node) {
8392
PyAnnotation annotation = function.getAnnotation();
8493
String typeCommentAnnotation = function.getTypeCommentAnnotation();
8594
if (annotation != null || typeCommentAnnotation != null) {
95+
PyType expected = getExpectedReturnType(function, myTypeEvalContext);
96+
if (expected == null) return;
97+
98+
// We cannot just match annotated and inferred types, as we cannot promote inferred to Literal
8699
PyExpression returnExpr = node.getExpression();
87-
PyType expected = getExpectedReturnType(function);
100+
if (returnExpr == null && !(expected instanceof PyNoneType) && PyTypeChecker.match(expected, PyNoneType.INSTANCE, myTypeEvalContext)) {
101+
final String expectedName = PythonDocumentationProvider.getVerboseTypeName(expected, myTypeEvalContext);
102+
getHolder()
103+
.problem(node, PyPsiBundle.message("INSP.type.checker.returning.type.has.implicit.return", expectedName))
104+
.fix(new PyMakeReturnsExplicitFix(function))
105+
.register();
106+
return;
107+
}
108+
88109
PyType actual = returnExpr != null ? tryPromotingType(returnExpr, expected) : PyNoneType.INSTANCE;
89110

90-
if (expected != null && actual instanceof PyTypedDictType) {
111+
if (actual instanceof PyTypedDictType) {
91112
if (reportTypedDictProblems(expected, (PyTypedDictType)actual, returnExpr)) return;
92113
}
93114

94115
if (!PyTypeChecker.match(expected, actual, myTypeEvalContext)) {
95-
String expectedName = PythonDocumentationProvider.getVerboseTypeName(expected, myTypeEvalContext);
96-
String actualName = PythonDocumentationProvider.getTypeName(actual, myTypeEvalContext);
97-
var localQuickFix = new PyMakeFunctionReturnTypeQuickFix(function, returnExpr, actual, myTypeEvalContext);
98-
var globalQuickFix = new PyMakeFunctionReturnTypeQuickFix(function, returnExpr, null, myTypeEvalContext);
99-
registerProblem(returnExpr != null ? returnExpr : node,
100-
PyPsiBundle.message("INSP.type.checker.expected.type.got.type.instead", expectedName, actualName),
101-
localQuickFix, globalQuickFix);
116+
final String expectedName = PythonDocumentationProvider.getVerboseTypeName(expected, myTypeEvalContext);
117+
final String actualName = PythonDocumentationProvider.getTypeName(actual, myTypeEvalContext);
118+
getHolder()
119+
.problem(returnExpr != null ? returnExpr : node, PyPsiBundle.message("INSP.type.checker.expected.type.got.type.instead", expectedName, actualName))
120+
.fix(new PyMakeFunctionReturnTypeQuickFix(function, myTypeEvalContext))
121+
.register();
102122
}
103123
}
104124
}
105125
}
106126

107-
@Nullable
108-
private PyType getExpectedReturnType(@NotNull PyFunction function) {
109-
return getExpectedReturnType(function, myTypeEvalContext);
110-
}
111-
112127
@Nullable
113128
public static PyType getExpectedReturnType(@NotNull PyFunction function, @NotNull TypeEvalContext typeEvalContext) {
114129
final PyType returnType = typeEvalContext.getReturnType(function);
@@ -120,13 +135,6 @@ public static PyType getExpectedReturnType(@NotNull PyFunction function, @NotNul
120135
return returnType;
121136
}
122137

123-
@Nullable
124-
public static PyType getActualReturnType(@NotNull PyFunction function, @Nullable PyExpression returnExpr,
125-
@NotNull TypeEvalContext context) {
126-
PyType returnTypeExpected = getExpectedReturnType(function, context);
127-
return returnExpr != null ? tryPromotingType(returnExpr, returnTypeExpected, context) : PyNoneType.INSTANCE;
128-
}
129-
130138
@Override
131139
public void visitPyTargetExpression(@NotNull PyTargetExpression node) {
132140
// TODO: Check types in class-level assignments
@@ -230,21 +238,28 @@ public void visitPyFunction(@NotNull PyFunction node) {
230238
final PyAnnotation annotation = node.getAnnotation();
231239
final String typeCommentAnnotation = node.getTypeCommentAnnotation();
232240
if (annotation != null || typeCommentAnnotation != null) {
233-
if (!PyUtil.isEmptyFunction(node)) {
234-
final ReturnVisitor visitor = new ReturnVisitor(node);
235-
node.getStatementList().accept(visitor);
236-
if (!visitor.myHasReturns) {
237-
final PyType expected = getExpectedReturnType(node);
238-
final String expectedName = PythonDocumentationProvider.getTypeName(expected, myTypeEvalContext);
239-
if (expected != null && !(expected instanceof PyNoneType)) {
240-
registerProblem(annotation != null ? annotation.getValue() : node.getTypeComment(),
241-
PyPsiBundle.message("INSP.type.checker.expected.to.return.type.got.no.return", expectedName));
241+
final PyType expected = getExpectedReturnType(node, myTypeEvalContext);
242+
final boolean returnsNone = expected instanceof PyNoneType;
243+
final boolean returnsOptional = PyTypeChecker.match(expected, PyNoneType.INSTANCE, myTypeEvalContext);
244+
245+
if (expected != null && !returnsOptional && !PyUtil.isEmptyFunction(node)) {
246+
final List<PyStatement> returnPoints = node.getReturnPoints(myTypeEvalContext);
247+
final boolean hasImplicitReturns = exists(returnPoints, it -> !(it instanceof PyReturnStatement));
248+
249+
if (hasImplicitReturns) {
250+
final String expectedName = PythonDocumentationProvider.getVerboseTypeName(expected, myTypeEvalContext);
251+
final String actualName = PythonDocumentationProvider.getTypeName(node.getReturnStatementType(myTypeEvalContext), myTypeEvalContext);
252+
final PsiElement annotationValue = annotation != null ? annotation.getValue() : node.getTypeComment();
253+
if (annotationValue != null) {
254+
getHolder()
255+
.problem(annotationValue, PyPsiBundle.message("INSP.type.checker.expected.type.got.type.instead", expectedName, actualName))
256+
.fix(new PyMakeFunctionReturnTypeQuickFix(node, myTypeEvalContext))
257+
.register();
242258
}
243259
}
244260
}
245261

246-
if (PyUtil.isInitMethod(node) && !(getExpectedReturnType(node) instanceof PyNoneType
247-
|| PyTypingTypeProvider.isNoReturn(node, myTypeEvalContext))) {
262+
if (PyUtil.isInitMethod(node) && !(returnsNone || PyTypingTypeProvider.isNoReturn(node, myTypeEvalContext))) {
248263
registerProblem(annotation != null ? annotation.getValue() : node.getTypeComment(),
249264
PyPsiBundle.message("INSP.type.checker.init.should.return.none"));
250265
}
@@ -260,29 +275,6 @@ public void visitPyComprehensionElement(@NotNull PyComprehensionElement node) {
260275
}
261276
}
262277

263-
private static class ReturnVisitor extends PyRecursiveElementVisitor {
264-
private final PyFunction myFunction;
265-
private boolean myHasReturns = false;
266-
267-
ReturnVisitor(PyFunction function) {
268-
myFunction = function;
269-
}
270-
271-
@Override
272-
public void visitPyYieldExpression(@NotNull PyYieldExpression node) {
273-
if (ScopeUtil.getScopeOwner(node) == myFunction) {
274-
myHasReturns = true;
275-
}
276-
}
277-
278-
@Override
279-
public void visitPyReturnStatement(@NotNull PyReturnStatement node) {
280-
if (ScopeUtil.getScopeOwner(node) == myFunction) {
281-
myHasReturns = true;
282-
}
283-
}
284-
}
285-
286278
private void checkCallSite(@NotNull PyCallSiteExpression callSite) {
287279
final List<AnalyzeCalleeResults> calleesResults = StreamEx
288280
.of(mapArguments(callSite, getResolveContext()))
@@ -513,7 +505,7 @@ private PyType substituteGenerics(@Nullable PyType expectedArgumentType, @NotNul
513505
}
514506

515507
private static boolean matchedCalleeResultsExist(@NotNull List<AnalyzeCalleeResults> calleesResults) {
516-
return ContainerUtil.exists(calleesResults, calleeResults ->
508+
return exists(calleesResults, calleeResults ->
517509
ContainerUtil.all(calleeResults.getResults(), AnalyzeArgumentResult::isMatched) &&
518510
calleeResults.getUnmatchedArguments().isEmpty() &&
519511
calleeResults.getUnmatchedParameters().isEmpty() &&

0 commit comments

Comments
 (0)