From f2ca243d2e8de6e14df208ac8f7ff8c4a9a9df33 Mon Sep 17 00:00:00 2001 From: Semyon Proshev Date: Tue, 28 May 2019 20:11:35 +0300 Subject: [PATCH] Raise a warning on same name class and instance level `Final`s (PEP 591) (PY-34945) GitOrigin-RevId: e8eaad47c5a06cb8471e06d7c1e0890f02266820 --- .../python/inspections/PyFinalInspection.kt | 45 +++++++++++++------ .../inspections/PyFinalInspectionTest.java | 19 ++++++++ 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/python/src/com/jetbrains/python/inspections/PyFinalInspection.kt b/python/src/com/jetbrains/python/inspections/PyFinalInspection.kt index e49b7aab5b80..94c81b2b549d 100644 --- a/python/src/com/jetbrains/python/inspections/PyFinalInspection.kt +++ b/python/src/com/jetbrains/python/inspections/PyFinalInspection.kt @@ -54,7 +54,9 @@ class PyFinalInspection : PyInspection() { ) } else { - checkClassLevelFinalsAreInitialized(node) + val (classLevelFinals, initAttributes) = getClassLevelFinalsAndInitAttributes(node) + checkClassLevelFinalsAreInitialized(classLevelFinals, initAttributes) + checkSameNameClassAndInstanceFinals(classLevelFinals, initAttributes) } checkRedeclarationsInScope(node) @@ -140,24 +142,39 @@ class PyFinalInspection : PyInspection() { } } - private fun checkClassLevelFinalsAreInitialized(cls: PyClass) { - val notInitializedFinals = mutableMapOf() + private fun getClassLevelFinalsAndInitAttributes(cls: PyClass): Pair, Map> { + val classLevelFinals = mutableMapOf() + cls.classAttributes.forEach { if (isFinal(it)) classLevelFinals[it.name] = it } - cls.classAttributes.forEach { - if (!it.hasAssignedValue() && isFinal(it)) { - notInitializedFinals[it.name] = it + val initAttributes = mutableMapOf() + cls.findMethodByName(PyNames.INIT, false, myTypeEvalContext)?.let { PyClassImpl.collectInstanceAttributes(it, initAttributes) } + + return Pair(classLevelFinals, initAttributes) + } + + private fun checkClassLevelFinalsAreInitialized(classLevelFinals: Map, + initAttributes: Map) { + classLevelFinals.forEach { (name, psi) -> + if (!psi.hasAssignedValue() && name !in initAttributes) { + registerProblem(psi, "'Final' name should be initialized with a value") } } + } - if (notInitializedFinals.isNotEmpty()) { - cls.findMethodByName(PyNames.INIT, false, myTypeEvalContext)?.let { - val initializedAttributes = mutableMapOf() - PyClassImpl.collectInstanceAttributes(it, initializedAttributes) - notInitializedFinals -= initializedAttributes.keys - } + private fun checkSameNameClassAndInstanceFinals(classLevelFinals: Map, + initAttributes: Map) { + initAttributes.forEach { (name, initAttribute) -> + val sameNameClassLevelFinal = classLevelFinals[name] - notInitializedFinals.values.forEach { - registerProblem(it, "'Final' name should be initialized with a value") + if (sameNameClassLevelFinal != null && isFinal(initAttribute)) { + if (sameNameClassLevelFinal.hasAssignedValue()) { + registerProblem(initAttribute, "Already declared name could not be redefined as 'Final'") + } + else { + val message = "Either instance attribute or class attribute could be type hinted as 'Final'" + registerProblem(sameNameClassLevelFinal, message) + registerProblem(initAttribute, message) + } } } } diff --git a/python/testSrc/com/jetbrains/python/inspections/PyFinalInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyFinalInspectionTest.java index b8275ea4953e..6b9c4cc8dd90 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyFinalInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyFinalInspectionTest.java @@ -288,6 +288,25 @@ public class PyFinalInspectionTest extends PyInspectionTestCase { ); } + // PY-34945 + public void testSameNameClassAndInstanceLevelFinals() { + runWithLanguageLevel( + LanguageLevel.PYTHON36, + () -> doTestByText( + "from typing_extensions import Final\n" + + "\n" + + "class A:\n" + + " a: Final[int] = 1\n" + + " b: Final[str] = \"1\"\n" + + " c: Final[int]\n" + + "\n" + + " def __init__(self):\n" + + " self.a: Final[int] = 2\n" + + " self.b = \"2\"\n" + + " self.c: Final[int] = 2") + ); + } + @NotNull @Override protected Class getInspectionClass() {