Raise a warning on Finals reassignments (PEP 591) (PY-34945)

GitOrigin-RevId: 10729d211a99d3d1542e26752e2e103eb94396dc
This commit is contained in:
Semyon Proshev
2019-07-02 06:52:16 +03:00
committed by intellij-monorepo-bot
parent f2ca243d2e
commit b0e57d5d3a
10 changed files with 210 additions and 27 deletions
@@ -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) {
@@ -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<String?>()
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<String?, PyTargetExpression>, Map<String, PyTargetExpression>> {
val classLevelFinals = mutableMapOf<String?, PyTargetExpression>()
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<String, PyTargetExpression>()
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 {
@@ -0,0 +1,10 @@
from b import A
<warning descr="'a' is 'Final' and could not be reassigned">A.a</warning> = 4
class B(A):
@classmethod
def my_cls_method(cls):
<warning descr="'a' is 'Final' and could not be reassigned">cls.a</warning> = 6
<warning descr="'a' is 'Final' and could not be reassigned">B.a</warning> = 7
@@ -0,0 +1,4 @@
from typing_extensions import Final
class A:
a: Final[int] = 1
@@ -0,0 +1,14 @@
from b import A, B
<warning descr="'a' is 'Final' and could not be reassigned">A().a</warning> = 3
<warning descr="'b' is 'Final' and could not be reassigned">B().b</warning> = 3
class C(B):
def __init__(self):
super().__init__()
<warning descr="'B.b' is 'Final' and could not be reassigned">self.b</warning> = 4
def my_method(self):
<warning descr="'B.b' is 'Final' and could not be reassigned">self.b</warning> = 5
<warning descr="'B.b' is 'Final' and could not be reassigned">C().b</warning> = 6
@@ -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
@@ -0,0 +1,5 @@
import b
<warning descr="'a' is 'Final' and could not be reassigned">b.a</warning> = 2
from b import a
a = 3
@@ -0,0 +1,3 @@
from typing_extensions import Final
a: Final[int] = 1
@@ -2,6 +2,6 @@ from typing_extensions import Final
a: Final[int]
b: <warning descr="If assigned value is omitted, there should be an explicit type argument to 'Final'">Final</warning>
b = "10"
<warning descr="'b' is 'Final' and could not be reassigned">b</warning> = "10"
c: Final[str] = "10"
d: int
@@ -76,7 +76,7 @@ public class PyFinalInspectionTest extends PyInspectionTestCase {
"\n" +
"<warning descr=\"'Final' name should be initialized with a value\">a</warning>: Final[int]\n" +
"<warning descr=\"'Final' name should be initialized with a value\">b</warning>: Final\n" +
"b = \"10\"\n" +
"<warning descr=\"'b' is 'Final' and could not be reassigned\">b</warning> = \"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\"")
"<warning descr=\"'c' is 'Final' and could not be reassigned\">c</warning>: str = \"10\"")
);
}
@@ -247,7 +247,7 @@ public class PyFinalInspectionTest extends PyInspectionTestCase {
"\n" +
" c: Final[int] = 10\n" +
" print(c)\n" +
" c: str = \"10\"")
" <warning descr=\"'c' is 'Final' and could not be reassigned\">c</warning>: str = \"10\"")
);
}
@@ -268,7 +268,7 @@ public class PyFinalInspectionTest extends PyInspectionTestCase {
"\n" +
" c: Final[int] = 10\n" +
" print(c)\n" +
" c: str = \"10\"")
" <warning descr=\"'c' is 'Final' and could not be reassigned\">c</warning>: 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" +
"<warning descr=\"'a' is 'Final' and could not be reassigned\">a</warning> = 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" +
" <warning descr=\"'a' is 'Final' and could not be reassigned\">cls.a</warning> = 5\n" +
"\n" +
"<warning descr=\"'a' is 'Final' and could not be reassigned\">A.a</warning> = 4\n" +
"\n" +
"class B(A):\n" +
"\n" +
" @classmethod\n" +
" def my_cls_method(cls):\n" +
" <warning descr=\"'a' is 'Final' and could not be reassigned\">cls.a</warning> = 6\n" +
"\n" +
"<warning descr=\"'a' is 'Final' and could not be reassigned\">" +
"B.a</warning> = 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" +
" <warning descr=\"'a' is 'Final' and could not be reassigned\">self.a</warning> = 2\n" +
"\n" +
"<warning descr=\"'a' is 'Final' and could not be reassigned\">A().a</warning> = 3\n" +
"\n" +
"class B:\n" +
" b: Final[int]\n" +
"\n" +
" def __init__(self):\n" +
" self.b = 1\n" +
"\n" +
" def method(self):\n" +
" <warning descr=\"'b' is 'Final' and could not be reassigned\">self.b</warning> = 2\n" +
"\n" +
"<warning descr=\"'b' is 'Final' and could not be reassigned\">B().b</warning> = 3\n" +
"\n" +
"class C(B):\n" +
" def __init__(self):\n" +
" super().__init__()\n" +
" <warning descr=\"'B.b' is 'Final' and could not be reassigned\">self.b</warning> = 4\n" +
"\n" +
" def my_method(self):\n" +
" <warning descr=\"'B.b' is 'Final' and could not be reassigned\">self.b</warning> = 5\n" +
"\n" +
"<warning descr=\"'B.b' is 'Final' and could not be reassigned\">C().b</warning> = 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" +
" <warning descr=\"'a' is 'Final' and could not be reassigned\">a</warning> = 2")
);
}
@NotNull
@Override
protected Class<? extends PyInspection> getInspectionClass() {