From b0e57d5d3aaf4b109efb7e3855ba4f46a009e8d7 Mon Sep 17 00:00:00 2001 From: Semyon Proshev Date: Tue, 28 May 2019 21:28:16 +0300 Subject: [PATCH] Raise a warning on `Final`s reassignments (PEP 591) (PY-34945) GitOrigin-RevId: 10729d211a99d3d1542e26752e2e103eb94396dc --- .../python/psi/types/PyClassType.java | 3 +- .../python/inspections/PyFinalInspection.kt | 69 +++++++---- .../ImportedClassFinalReassignment/a.py | 10 ++ .../ImportedClassFinalReassignment/b.py | 4 + .../ImportedInstanceFinalReassignment/a.py | 14 +++ .../ImportedInstanceFinalReassignment/b.py | 11 ++ .../ImportedModuleFinalReassignment/a.py | 5 + .../ImportedModuleFinalReassignment/b.py | 3 + ...mittedAssignedValueInStubOnModuleLevel.pyi | 2 +- .../inspections/PyFinalInspectionTest.java | 116 +++++++++++++++++- 10 files changed, 210 insertions(+), 27 deletions(-) create mode 100644 python/testData/inspections/PyFinalInspection/ImportedClassFinalReassignment/a.py create mode 100644 python/testData/inspections/PyFinalInspection/ImportedClassFinalReassignment/b.py create mode 100644 python/testData/inspections/PyFinalInspection/ImportedInstanceFinalReassignment/a.py create mode 100644 python/testData/inspections/PyFinalInspection/ImportedInstanceFinalReassignment/b.py create mode 100644 python/testData/inspections/PyFinalInspection/ImportedModuleFinalReassignment/a.py create mode 100644 python/testData/inspections/PyFinalInspection/ImportedModuleFinalReassignment/b.py diff --git a/python/psi-api/src/com/jetbrains/python/psi/types/PyClassType.java b/python/psi-api/src/com/jetbrains/python/psi/types/PyClassType.java index 4d40ee1eae4a..0706c968f85b 100644 --- a/python/psi-api/src/com/jetbrains/python/psi/types/PyClassType.java +++ b/python/psi-api/src/com/jetbrains/python/psi/types/PyClassType.java @@ -27,9 +27,10 @@ public interface PyClassType extends PyClassLikeType, UserDataHolder { PyClass getPyClass(); /** - * @param name name to check + * @param name name to check * @param context type evaluation context * @return true if attribute with the specified name could be created or updated. + * Does not take `typing.Final` into account. * @see PyClass#getSlots(TypeEvalContext) */ default boolean isAttributeWritable(@NotNull String name, @NotNull TypeEvalContext context) { diff --git a/python/src/com/jetbrains/python/inspections/PyFinalInspection.kt b/python/src/com/jetbrains/python/inspections/PyFinalInspection.kt index 94c81b2b549d..7f56f47d0a99 100644 --- a/python/src/com/jetbrains/python/inspections/PyFinalInspection.kt +++ b/python/src/com/jetbrains/python/inspections/PyFinalInspection.kt @@ -6,8 +6,6 @@ import com.intellij.codeInspection.ProblemsHolder import com.intellij.psi.PsiElementVisitor import com.intellij.psi.impl.source.resolve.FileContextUtil import com.jetbrains.python.PyNames -import com.jetbrains.python.codeInsight.controlflow.ControlFlowCache -import com.jetbrains.python.codeInsight.controlflow.ScopeOwner import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil import com.jetbrains.python.codeInsight.functionTypeComments.psi.PyParameterTypeList import com.jetbrains.python.codeInsight.typing.PyTypingTypeProvider @@ -58,8 +56,6 @@ class PyFinalInspection : PyInspection() { checkClassLevelFinalsAreInitialized(classLevelFinals, initAttributes) checkSameNameClassAndInstanceFinals(classLevelFinals, initAttributes) } - - checkRedeclarationsInScope(node) } override fun visitPyFunction(node: PyFunction) { @@ -90,8 +86,6 @@ class PyFinalInspection : PyInspection() { registerProblem(node.typeComment, "'Final' could not be used in annotations for function parameters") } } - - checkRedeclarationsInScope(node) } override fun visitPyTargetExpression(node: PyTargetExpression) { @@ -111,6 +105,19 @@ class PyFinalInspection : PyInspection() { } } } + else if (!isFinal(node)) { + val qualifierType = node.qualifier?.let { myTypeEvalContext.getType(it) } + if (qualifierType is PyClassType && !qualifierType.isDefinition) { + checkInstanceFinalReassignment(node, qualifierType.pyClass) + } + else if (PyUtil.multiResolveTopPriority(node, resolveContext).any { it != node && it is PyTargetExpression && isFinal(it) }) { + registerProblem(node, "'${node.name}' is 'Final' and could not be reassigned") + } + } + + if (isFinal(node) && PyUtil.multiResolveTopPriority(node, resolveContext).any { it != node }) { + registerProblem(node, "Already declared name could not be redefined as 'Final'") + } } override fun visitPyNamedParameter(node: PyNamedParameter) { @@ -127,21 +134,6 @@ class PyFinalInspection : PyInspection() { checkFinalIsOuterMost(node) } - override fun visitPyFile(node: PyFile) { - super.visitPyFile(node) - - checkRedeclarationsInScope(node) - } - - private fun checkRedeclarationsInScope(scopeOwner: ScopeOwner) { - val visitedNames = mutableSetOf() - ControlFlowCache.getScope(scopeOwner).targetExpressions.forEach { - if (!visitedNames.add(it.name) && isFinal(it)) { - registerProblem(it, "Already declared name could not be redefined as 'Final'") - } - } - } - private fun getClassLevelFinalsAndInitAttributes(cls: PyClass): Pair, Map> { val classLevelFinals = mutableMapOf() cls.classAttributes.forEach { if (isFinal(it)) classLevelFinals[it.name] = it } @@ -189,6 +181,41 @@ class PyFinalInspection : PyInspection() { } } + private fun checkInstanceFinalReassignment(target: PyTargetExpression, cls: PyClass) { + val name = target.name ?: return + + val classAttribute = cls.findClassAttribute(name, false, myTypeEvalContext) + if (classAttribute != null && !classAttribute.hasAssignedValue() && isFinal(classAttribute)) { + val scopeOwner = ScopeUtil.getScopeOwner(target) + val insideClsInit = scopeOwner is PyFunction && PyUtil.isInit(scopeOwner) && cls == scopeOwner.containingClass + if (!insideClsInit) { + registerProblem(target, "'$name' is 'Final' and could not be reassigned") + } + return + } + + for (ancestor in cls.getAncestorClasses(myTypeEvalContext)) { + val inheritedClassAttribute = ancestor.findClassAttribute(name, false, myTypeEvalContext) + if (inheritedClassAttribute != null && !inheritedClassAttribute.hasAssignedValue() && isFinal(inheritedClassAttribute)) { + registerProblem(target, "'${ancestor.name}.$name' is 'Final' and could not be reassigned") + return + } + } + + for (current in (sequenceOf(cls) + cls.getAncestorClasses(myTypeEvalContext).asSequence())) { + val init = current.findMethodByName(PyNames.INIT, false, myTypeEvalContext) + if (init != null) { + val attributesInInit = mutableMapOf() + PyClassImpl.collectInstanceAttributes(init, attributesInInit) + if (attributesInInit[name]?.let { it != target && isFinal(it) } == true) { + val qualifier = if (cls == current) "" else "${current.name}." + registerProblem(target, "'$qualifier$name' is 'Final' and could not be reassigned") + break + } + } + } + } + private fun checkFinalIsOuterMost(node: PyReferenceExpression) { if (isTopLevelInAnnotationOrTypeComment(node)) return (node.parent as? PySubscriptionExpression)?.let { diff --git a/python/testData/inspections/PyFinalInspection/ImportedClassFinalReassignment/a.py b/python/testData/inspections/PyFinalInspection/ImportedClassFinalReassignment/a.py new file mode 100644 index 000000000000..50d82886c862 --- /dev/null +++ b/python/testData/inspections/PyFinalInspection/ImportedClassFinalReassignment/a.py @@ -0,0 +1,10 @@ +from b import A + +A.a = 4 + +class B(A): + @classmethod + def my_cls_method(cls): + cls.a = 6 + +B.a = 7 \ No newline at end of file diff --git a/python/testData/inspections/PyFinalInspection/ImportedClassFinalReassignment/b.py b/python/testData/inspections/PyFinalInspection/ImportedClassFinalReassignment/b.py new file mode 100644 index 000000000000..639e612dc326 --- /dev/null +++ b/python/testData/inspections/PyFinalInspection/ImportedClassFinalReassignment/b.py @@ -0,0 +1,4 @@ +from typing_extensions import Final + +class A: + a: Final[int] = 1 \ No newline at end of file diff --git a/python/testData/inspections/PyFinalInspection/ImportedInstanceFinalReassignment/a.py b/python/testData/inspections/PyFinalInspection/ImportedInstanceFinalReassignment/a.py new file mode 100644 index 000000000000..a8b406edbb41 --- /dev/null +++ b/python/testData/inspections/PyFinalInspection/ImportedInstanceFinalReassignment/a.py @@ -0,0 +1,14 @@ +from b import A, B + +A().a = 3 +B().b = 3 + +class C(B): + def __init__(self): + super().__init__() + self.b = 4 + + def my_method(self): + self.b = 5 + +C().b = 6 \ No newline at end of file diff --git a/python/testData/inspections/PyFinalInspection/ImportedInstanceFinalReassignment/b.py b/python/testData/inspections/PyFinalInspection/ImportedInstanceFinalReassignment/b.py new file mode 100644 index 000000000000..632146fea392 --- /dev/null +++ b/python/testData/inspections/PyFinalInspection/ImportedInstanceFinalReassignment/b.py @@ -0,0 +1,11 @@ +from typing_extensions import Final + +class A: + def __init__(self): + self.a: Final[int] = 1 + +class B: + b: Final[int] + + def __init__(self): + self.b = 1 \ No newline at end of file diff --git a/python/testData/inspections/PyFinalInspection/ImportedModuleFinalReassignment/a.py b/python/testData/inspections/PyFinalInspection/ImportedModuleFinalReassignment/a.py new file mode 100644 index 000000000000..a42972ee18ed --- /dev/null +++ b/python/testData/inspections/PyFinalInspection/ImportedModuleFinalReassignment/a.py @@ -0,0 +1,5 @@ +import b +b.a = 2 + +from b import a +a = 3 \ No newline at end of file diff --git a/python/testData/inspections/PyFinalInspection/ImportedModuleFinalReassignment/b.py b/python/testData/inspections/PyFinalInspection/ImportedModuleFinalReassignment/b.py new file mode 100644 index 000000000000..77693bd8887f --- /dev/null +++ b/python/testData/inspections/PyFinalInspection/ImportedModuleFinalReassignment/b.py @@ -0,0 +1,3 @@ +from typing_extensions import Final + +a: Final[int] = 1 \ No newline at end of file diff --git a/python/testData/inspections/PyFinalInspection/omittedAssignedValueInStubOnModuleLevel.pyi b/python/testData/inspections/PyFinalInspection/omittedAssignedValueInStubOnModuleLevel.pyi index 9064a055c4eb..1ddbcff53d19 100644 --- a/python/testData/inspections/PyFinalInspection/omittedAssignedValueInStubOnModuleLevel.pyi +++ b/python/testData/inspections/PyFinalInspection/omittedAssignedValueInStubOnModuleLevel.pyi @@ -2,6 +2,6 @@ from typing_extensions import Final a: Final[int] b: Final -b = "10" +b = "10" c: Final[str] = "10" d: int \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/inspections/PyFinalInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyFinalInspectionTest.java index 6b9c4cc8dd90..a6b202d7dd00 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyFinalInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyFinalInspectionTest.java @@ -76,7 +76,7 @@ public class PyFinalInspectionTest extends PyInspectionTestCase { "\n" + "a: Final[int]\n" + "b: Final\n" + - "b = \"10\"\n" + + "b = \"10\"\n" + "c: Final[str] = \"10\"\n" + "d: int\n") ); @@ -226,7 +226,7 @@ public class PyFinalInspectionTest extends PyInspectionTestCase { "\n" + "c: Final[int] = 10\n" + "print(c)\n" + - "c: str = \"10\"") + "c: str = \"10\"") ); } @@ -247,7 +247,7 @@ public class PyFinalInspectionTest extends PyInspectionTestCase { "\n" + " c: Final[int] = 10\n" + " print(c)\n" + - " c: str = \"10\"") + " c: str = \"10\"") ); } @@ -268,7 +268,7 @@ public class PyFinalInspectionTest extends PyInspectionTestCase { "\n" + " c: Final[int] = 10\n" + " print(c)\n" + - " c: str = \"10\"") + " c: str = \"10\"") ); } @@ -307,6 +307,114 @@ public class PyFinalInspectionTest extends PyInspectionTestCase { ); } + // PY-34945 + public void testModuleFinalReassignment() { + runWithLanguageLevel( + LanguageLevel.PYTHON36, + () -> doTestByText("from typing_extensions import Final\n" + + "\n" + + "a: Final[int] = 1\n" + + "a = 2") + ); + } + + // PY-34945 + public void testImportedModuleFinalReassignment() { + runWithLanguageLevel(LanguageLevel.PYTHON36, this::doMultiFileTest); + } + + // PY-34945 + public void testClassFinalReassignment() { + runWithLanguageLevel( + LanguageLevel.PYTHON36, + () -> doTestByText("from typing_extensions import Final\n" + + "\n" + + "class A:\n" + + " a: Final[int] = 1\n" + + "\n" + + " def __init__(self):\n" + + " self.a = 2\n" + + "\n" + + " def method(self):\n" + + " self.a = 3\n" + + "\n" + + " @classmethod\n" + + " def cls_method(cls):\n" + + " cls.a = 5\n" + + "\n" + + "A.a = 4\n" + + "\n" + + "class B(A):\n" + + "\n" + + " @classmethod\n" + + " def my_cls_method(cls):\n" + + " cls.a = 6\n" + + "\n" + + "" + + "B.a = 7") + ); + } + + // PY-34945 + public void testImportedClassFinalReassignment() { + runWithLanguageLevel(LanguageLevel.PYTHON36, this::doMultiFileTest); + } + + // PY-34945 + public void testInstanceFinalReassignment() { + runWithLanguageLevel( + LanguageLevel.PYTHON36, + () -> doTestByText("from typing_extensions import Final\n" + + "\n" + + "class A:\n" + + " def __init__(self):\n" + + " self.a: Final[int] = 1\n" + + "\n" + + " def method(self):\n" + + " self.a = 2\n" + + "\n" + + "A().a = 3\n" + + "\n" + + "class B:\n" + + " b: Final[int]\n" + + "\n" + + " def __init__(self):\n" + + " self.b = 1\n" + + "\n" + + " def method(self):\n" + + " self.b = 2\n" + + "\n" + + "B().b = 3\n" + + "\n" + + "class C(B):\n" + + " def __init__(self):\n" + + " super().__init__()\n" + + " self.b = 4\n" + + "\n" + + " def my_method(self):\n" + + " self.b = 5\n" + + "\n" + + "C().b = 6") + ); + } + + // PY-34945 + public void testImportedInstanceFinalReassignment() { + runWithLanguageLevel(LanguageLevel.PYTHON36, this::doMultiFileTest); + } + + // PY-34945 + public void testFunctionLevelFinalReassignment() { + runWithLanguageLevel( + LanguageLevel.PYTHON36, + () -> doTestByText("from typing_extensions import Final\n" + + "\n" + + "def foo():\n" + + " a: Final[int] = 1\n" + + " a = 2") + ); + } + @NotNull @Override protected Class getInspectionClass() {