PY-52477 PY-78913 Add inspection for inconsistent returns

Merge-request: IJ-MR-154297
Merged-by: Aleksandr Govenko <aleksandr.govenko@jetbrains.com>

GitOrigin-RevId: e89225212cc981a5b256fca4ee23d77659b3ce5e
This commit is contained in:
Aleksandr.Govenko
2025-02-14 17:21:51 +00:00
committed by intellij-monorepo-bot
parent 6c8675e758
commit e8f5b1c631
14 changed files with 151 additions and 42 deletions
@@ -185,6 +185,7 @@
<localInspection language="Python" shortName="PyFromFutureImportInspection" suppressId="PyFromFutureImport" bundle="messages.PyPsiBundle" key="INSP.NAME.from.future.import" groupKey="INSP.GROUP.python" enabledByDefault="true" level="WARNING" implementationClass="com.jetbrains.python.inspections.PyFromFutureImportInspection"/>
<localInspection language="Python" shortName="PyGlobalUndefinedInspection" suppressId="PyGlobalUndefined" bundle="messages.PyPsiBundle" key="INSP.NAME.global.undefined" groupKey="INSP.GROUP.python" enabledByDefault="true" level="WEAK WARNING" implementationClass="com.jetbrains.python.inspections.PyGlobalUndefinedInspection"/>
<localInspection language="Python" shortName="PyInconsistentIndentationInspection" suppressId="PyInconsistentIndentation" bundle="messages.PyPsiBundle" key="INSP.NAME.inconsistent.indentation" groupKey="INSP.GROUP.python" enabledByDefault="true" level="WARNING" implementationClass="com.jetbrains.python.inspections.PyInconsistentIndentationInspection"/>
<localInspection language="Python" shortName="PyInconsistentReturnsInspection" suppressId="PyInconsistentReturns" bundle="messages.PyPsiBundle" key="INSP.NAME.inconsistent.returns" groupKey="INSP.GROUP.python" enabledByDefault="true" level="WEAK WARNING" implementationClass="com.jetbrains.python.inspections.PyInconsistentReturnsInspection"/>
<localInspection language="Python" shortName="PyIncorrectDocstringInspection" suppressId="PyIncorrectDocstring" bundle="messages.PyPsiBundle" key="INSP.NAME.incorrect.docstring" groupKey="INSP.GROUP.python" enabledByDefault="true" level="WEAK WARNING" implementationClass="com.jetbrains.python.inspections.PyIncorrectDocstringInspection"/>
<localInspection language="Python" shortName="PyMissingOrEmptyDocstringInspection" suppressId="PyMissingOrEmptyDocstring" bundle="messages.PyPsiBundle" key="INSP.NAME.missing.or.empty.docstring" groupKey="INSP.GROUP.python" enabledByDefault="false" level="WEAK WARNING" implementationClass="com.jetbrains.python.inspections.PyMissingOrEmptyDocstringInspection"/>
<localInspection language="Python" shortName="PyNamedTupleInspection" suppressId="PyNamedTuple" bundle="messages.PyPsiBundle" key="INSP.named.tuple" groupKey="INSP.GROUP.python" enabledByDefault="true" level="WARNING" implementationClass="com.jetbrains.python.inspections.PyNamedTupleInspection"/>
@@ -0,0 +1,40 @@
<html>
<body>
Highlights inconsistent return statements in functions.
According to PEP8, either all return statements in a function should return an expression, or none of them should.
<p>
PEP8's recommendation:
Either all return statements in a function should return an expression, or none of them should.
If any return statement returns an expression, any return statements where no value is returned
should explicitly state this as return None, and an explicit return statement should be present
at the end of the function (if reachable):
</p>
<pre><code>
# Correct:
def foo(x):
if x >= 0:
return math.sqrt(x)
else:
return None
def bar(x):
if x < 0:
return None
return math.sqrt(x)
</code></pre>
<pre><code>
# Wrong:
def foo(x):
if x >= 0:
return math.sqrt(x)
def bar(x):
if x < 0:
return
return math.sqrt(x)
</code></pre>
<!-- tooltip end -->
</body>
</html>
@@ -1010,6 +1010,12 @@ INSP.inconsistent.indentation.mix.tabs.spaces=Inconsistent indentation: mix of t
INSP.inconsistent.indentation.previous.line.used.tabs.this.line.uses.spaces=Inconsistent indentation: previous line used tabs, this line uses spaces
INSP.inconsistent.indentation.previous.line.used.spaces.this.line.uses.tabs=Inconsistent indentation: previous line used spaces, this line uses tabs
# PyInconsistentReturnsInspection
INSP.NAME.inconsistent.returns=Inconsistent return statements
INSP.inconsistent.returns.stmt.expected=Explicit return statement expected
INSP.inconsistent.returns.value.expected=Explicit return value expected
# PyMissingTypeHintsInspection
INSP.NAME.missing.type.hints=Missing type hinting for function definition
INSP.missing.type.hints.type.hinting.missing.for.function.definition=Type hinting is missing for a function definition
@@ -1064,7 +1070,6 @@ INSP.type.checker.yield.from.send.type.mismatch=Expected send type ''{0}'', got
INSP.type.checker.yield.from.async.generator=Cannot yield from ''{0}'', use 'async for' instead
INSP.type.checker.typed.dict.extra.key=Extra key ''{0}'' for TypedDict ''{1}''
INSP.type.checker.typed.dict.missing.keys=TypedDict ''{0}'' has missing {1,choice,1#key|2#keys}: {2}
INSP.type.checker.returning.type.has.implicit.return=Function returning ''{0}'' has implicit ''return None''
INSP.type.checker.init.should.return.none=__init__ should return None
INSP.type.checker.type.does.not.have.expected.attribute=Type ''{0}'' doesn''t have expected {1,choice,1#attribute|2#attributes} {2}
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
@@ -811,10 +811,11 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
// assert False
if (args.length >= 1) {
if (!PyEvaluator.evaluateAsBooleanNoResolve(args[0], true)) {
myBuilder.addNode(new PyRaiseInstruction(myBuilder, node));
myBuilder.addPendingEdge(null, myBuilder.prevInstruction);
myBuilder.flowAbrupted();
return;
}
myBuilder.flowAbrupted();
return;
}
TransparentInstruction trueNode = addTransparentInstruction();
TransparentInstruction falseNode = addTransparentInstruction();
@@ -0,0 +1,37 @@
package com.jetbrains.python.inspections
import com.intellij.codeInspection.LocalInspectionToolSession
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.psi.PsiElementVisitor
import com.jetbrains.python.PyPsiBundle
import com.jetbrains.python.inspections.quickfix.PyMakeReturnsExplicitFix
import com.jetbrains.python.psi.PyFunction
import com.jetbrains.python.psi.PyReturnStatement
/**
* PEP8:
* Be consistent in return statements. Either all return statements in a function should return an expression,
* or none of them should. If any return statement returns an expression, any return statements where no value
* is returned should explicitly state this as return None, and an explicit return statement should be present
* at the end of the function (if reachable).
*/
class PyInconsistentReturnsInspection : PyInspection() {
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
return object : PyInspectionVisitor(holder, getContext(session)) {
override fun visitPyFunction(node: PyFunction) {
val returnPoints = node.getReturnPoints(myTypeEvalContext)
val hasExplicitReturns = returnPoints.any { (it as? PyReturnStatement)?.expression != null }
if (hasExplicitReturns) {
returnPoints
.filter { (it !is PyReturnStatement || it.expression == null) }
.forEach {
val message =
if (it is PyReturnStatement) PyPsiBundle.message("INSP.inconsistent.returns.value.expected")
else PyPsiBundle.message("INSP.inconsistent.returns.stmt.expected")
this.holder!!.problem(it, message).fix(PyMakeReturnsExplicitFix(node)).register()
}
}
}
}
}
}
@@ -109,15 +109,6 @@ public class PyTypeCheckerInspection extends PyInspection {
// We cannot just match annotated and inferred types, as we cannot promote inferred to Literal
PyExpression returnExpr = node.getExpression();
if (returnExpr == null && !(expected instanceof PyNoneType) && PyTypeChecker.match(expected, PyNoneType.INSTANCE, myTypeEvalContext)) {
final String expectedName = PythonDocumentationProvider.getVerboseTypeName(expected, myTypeEvalContext);
getHolder()
.problem(node, PyPsiBundle.message("INSP.type.checker.returning.type.has.implicit.return", expectedName))
.fix(new PyMakeReturnsExplicitFix(function))
.register();
return;
}
if (expected instanceof PyTypedDictType expectedTypedDictType) {
if (returnExpr != null && PyTypedDictType.isDictExpression(returnExpr, myTypeEvalContext)) {
reportTypedDictProblems(expectedTypedDictType, returnExpr);
@@ -1,5 +1,6 @@
0(1) element: null
1(2) element: PyAssertStatement
2(4) READ ACCESS: False
3(4) element: PyPrintStatement
4() element: null
2(3) READ ACCESS: False
3(5) raise: PyAssertStatement
4(5) element: PyPrintStatement
5() element: null
@@ -1,10 +1,12 @@
0(1) element: null
1(2) element: PyAssertStatement
2(9) READ ACCESS: False
3(4) element: PyPrintStatement
4(5) element: PyAssertStatement
5(6) READ ACCESS: False
6(7) READ ACCESS: f
7(9) element: PyCallExpression: f
8(9) element: PyPrintStatement
9() element: null
2(3) READ ACCESS: False
3(11) raise: PyAssertStatement
4(5) element: PyPrintStatement
5(6) element: PyAssertStatement
6(7) READ ACCESS: False
7(8) READ ACCESS: f
8(9) element: PyCallExpression: f
9(11) raise: PyAssertStatement
10(11) element: PyPrintStatement
11() element: null
@@ -1,11 +1,12 @@
0(1) element: null
1(2) element: PyWithStatement
2(4) READ ACCESS: context_manager
3(9) exit context manager: context_manager
3(10) exit context manager: context_manager
4(5,3) element: PyAssertStatement
5(6,3) READ ACCESS: False
6(7,3) READ ACCESS: f
7(10,3) element: PyCallExpression: f
8(9,3) element: PyPrintStatement
9(10) element: PyPrintStatement
10() element: null
7(8,3) element: PyCallExpression: f
8(11,3) raise: PyAssertStatement
9(10,3) element: PyPrintStatement
10(11) element: PyPrintStatement
11() element: null
@@ -1,15 +1,16 @@
0(1) element: null
1(2) element: PyWithStatement
2(4) READ ACCESS: cm1
3(13) exit context manager: cm1
3(14) exit context manager: cm1
4(3,6) READ ACCESS: cm2
5(13) exit context manager: cm2
5(14) exit context manager: cm2
6(3,5,8) READ ACCESS: cm3
7(13) exit context manager: cm3
7(14) exit context manager: cm3
8(9,3,5,7) element: PyAssertStatement
9(10,3,5,7) READ ACCESS: False
10(11,3,5,7) READ ACCESS: f
11(14,3,5,7) element: PyCallExpression: f
12(13,3,5,7) element: PyPrintStatement
13(14) element: PyPrintStatement
14() element: null
11(12,3,5,7) element: PyCallExpression: f
12(15,3,5,7) raise: PyAssertStatement
13(14,3,5,7) element: PyPrintStatement
14(15) element: PyPrintStatement
15() element: null
@@ -22,7 +22,7 @@ def f() -> Optional[str]:
elif x == 0:
return 'abc'
else:
<warning descr="Function returning 'str | None' has implicit 'return None'">return</warning>
return
def g(x) -> int:
if x:
@@ -1,7 +1,15 @@
def f(x) -> int | None:
if x == 1:
y = 42
print(y)
<weak_warning descr="Explicit return statement expected">if x == 1:
return 42
elif x == 2:
<warning descr="Function returning 'int | None' has implicit 'return None'">return<caret></warning>
<weak_warning descr="Explicit return value expected">return<caret></weak_warning>
elif x == 3:
pass
raise Exception()
elif x == 4:
assert False
elif x == 5:
<weak_warning descr="Explicit return statement expected">assert x</weak_warning>
elif x == 4:
<weak_warning descr="Explicit return statement expected">pass</weak_warning></weak_warning>
@@ -1,8 +1,17 @@
def f(x) -> int | None:
y = 42
print(y)
if x == 1:
return 42
elif x == 2:
return None
elif x == 3:
raise Exception()
elif x == 4:
assert False
elif x == 5:
assert x
return None
elif x == 4:
return None
return None
@@ -4,12 +4,24 @@ package com.jetbrains.python.quickFixes;
import com.intellij.testFramework.TestDataPath;
import com.jetbrains.python.PyPsiBundle;
import com.jetbrains.python.PyQuickFixTestCase;
import com.jetbrains.python.inspections.PyTypeCheckerInspection;
import com.jetbrains.python.inspections.PyInconsistentReturnsInspection;
@TestDataPath("$CONTENT_ROOT/../testData/quickFixes/PyMakeReturnsExplicitFixTest/")
public class PyMakeReturnsExplicitFixTest extends PyQuickFixTestCase {
public void testAddReturnsFromReturnStmt() {
doQuickFixTest(PyTypeCheckerInspection.class, PyPsiBundle.message("QFIX.NAME.make.return.stmts.explicit"));
doQuickFixTest(PyInconsistentReturnsInspection.class, PyPsiBundle.message("QFIX.NAME.make.return.stmts.explicit"));
}
@Override
protected void doQuickFixTest(final Class inspectionClass, final String hint) {
final String testFileName = getTestName(true);
myFixture.enableInspections(inspectionClass);
myFixture.configureByFile(testFileName + ".py");
myFixture.checkHighlighting(true, false, true);
final var intentionAction = myFixture.findSingleIntention(hint);
assertNotNull(intentionAction);
myFixture.launchAction(intentionAction);
myFixture.checkResultByFile(testFileName + "_after.py", true);
}
}