Highlight list literal passed as generic parameter to typing member (PY-20530)

python/src/com/jetbrains/python/inspections/PyTypeHintsInspection.kt

@@ -8,10 +8,12 @@ import com.intellij.openapi.util.TextRange
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiElementVisitor
 import com.intellij.psi.PsiFileFactory
+import com.intellij.psi.util.PsiTreeUtil
 import com.intellij.psi.util.QualifiedName
 import com.jetbrains.python.PyNames
 import com.jetbrains.python.codeInsight.controlflow.ControlFlowCache
 import com.jetbrains.python.codeInsight.controlflow.ReadWriteInstruction
+import com.jetbrains.python.codeInsight.controlflow.ScopeOwner
 import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil
 import com.jetbrains.python.codeInsight.functionTypeComments.PyFunctionTypeAnnotationDialect
 import com.jetbrains.python.codeInsight.typing.PyTypingTypeProvider
@@ -72,17 +74,7 @@ class PyTypeHintsInspection : PyInspection() {
     override fun visitPySubscriptionExpression(node: PySubscriptionExpression) {
       super.visitPySubscriptionExpression(node)
 
-      val operand = node.operand as? PyReferenceExpression ?: return
-      val index = node.indexExpression ?: return
-
-      val callableQName = QualifiedName.fromDottedString(PyTypingTypeProvider.CALLABLE)
-
-      PyResolveUtil.resolveImportedElementQNameLocally(operand).forEach {
-        when (it) {
-          genericQName -> checkGenericParameters(index)
-          callableQName -> checkCallableParameters(index)
-        }
-      }
+      checkParameters(node)
     }
 
     override fun visitPyReferenceExpression(node: PyReferenceExpression) {
@@ -438,6 +430,33 @@ class PyTypeHintsInspection : PyInspection() {
       return Pair(if (seenGeneric) genericTypeVars else null, nonGenericTypeVars)
     }
 
+    private fun checkParameters(node: PySubscriptionExpression) {
+      val operand = node.operand as? PyReferenceExpression ?: return
+      val index = node.indexExpression ?: return
+
+      val callableQName = QualifiedName.fromDottedString(PyTypingTypeProvider.CALLABLE)
+      val qNames = PyResolveUtil.resolveImportedElementQNameLocally(operand)
+
+      var typingOnly = true
+      var callableExists = false
+
+      qNames.forEach {
+        when (it) {
+          genericQName -> checkGenericParameters(index)
+          callableQName -> {
+            callableExists = true
+            checkCallableParameters(index)
+          }
+        }
+
+        typingOnly = typingOnly && it.firstComponent == PyTypingTypeProvider.TYPING
+      }
+
+      if (qNames.isNotEmpty() && typingOnly) {
+        checkTypingMemberParameters(index, callableExists)
+      }
+    }
+
     private fun checkGenericParameters(index: PyExpression) {
       val parameters = (index as? PyTupleExpression)?.elements ?: arrayOf(index)
       val typeVars = mutableSetOf<PsiElement>()
@@ -498,6 +517,26 @@ class PyTypeHintsInspection : PyInspection() {
       }
     }
 
+    private fun checkTypingMemberParameters(index: PyExpression, isCallable: Boolean) {
+      val parameters = if (index is PyTupleExpression) index.elements else arrayOf(index)
+
+      parameters
+        .asSequence()
+        .drop(if (isCallable) 1 else 0)
+        .forEach {
+          if (it is PyListLiteralExpression) {
+            registerProblem(it,
+                            "Parameters to generic types must be types",
+                            ProblemHighlightType.GENERIC_ERROR,
+                            null,
+                            RemoveSquareBracketsQuickFix())
+          }
+          else if (it is PyReferenceExpression && multiFollowAssignmentsChain(it).any { it is PyListLiteralExpression }) {
+            registerProblem(it, "Parameters to generic types must be types", ProblemHighlightType.GENERIC_ERROR)
+          }
+        }
+    }
+
     private fun checkTupleMatching(expression: PyExpression) {
       if (expression !is PyTupleExpression) return
 
@@ -692,5 +731,36 @@ class PyTypeHintsInspection : PyInspection() {
         element.replace(list)
       }
     }
+
+    private class RemoveSquareBracketsQuickFix : LocalQuickFix {
+
+      override fun getFamilyName() = "Remove square brackets"
+
+      override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+        val element = descriptor.psiElement as? PyListLiteralExpression ?: return
+
+        val subscription = PsiTreeUtil.getParentOfType(element, PySubscriptionExpression::class.java, true, ScopeOwner::class.java)
+        val index = subscription?.indexExpression ?: return
+
+        val newIndexElements = if (index is PyTupleExpression) {
+          index.elements.flatMap { if (it == element) element.elements.asList() else listOf(it) }
+        }
+        else {
+          element.elements.asList()
+        }
+
+        if (newIndexElements.size == 1) {
+          index.replace(newIndexElements.first())
+        }
+        else {
+          val newIndexText = newIndexElements.joinToString(prefix = "(", postfix = ")") { it.text }
+
+          val expression = PyElementGenerator.getInstance(project).createExpressionFromText(LanguageLevel.forElement(element), newIndexText)
+          val newIndex = (expression as? PyParenthesizedExpression)?.containedExpression as? PyTupleExpression ?: return
+
+          index.replace(newIndex)
+        }
+      }
+    }
   }
 }

python/testData/quickFixes/PyTypeHintsQuickFixTest/multipleElementListAsTypingMemberParameter.py

@@ -0,0 +1,3 @@
+from typing import List
+
+foo4: List[<error descr="Parameters to generic types must be types">[int,<caret> str]</error>]

python/testData/quickFixes/PyTypeHintsQuickFixTest/multipleElementListAsTypingMemberParameter_after.py

@@ -0,0 +1,3 @@
+from typing import List
+
+foo4: List[int, str]

python/testData/quickFixes/PyTypeHintsQuickFixTest/multipleElementListInTupleAsTypingMemberParameter.py

@@ -0,0 +1,3 @@
+from typing import Callable
+
+foo2: Callable[[int], <error descr="Parameters to generic types must be types">[int,<caret> str]</error>] = None

python/testData/quickFixes/PyTypeHintsQuickFixTest/multipleElementListInTupleAsTypingMemberParameter_after.py

@@ -0,0 +1,3 @@
+from typing import Callable
+
+foo2: Callable[[int], int, str] = None

python/testData/quickFixes/PyTypeHintsQuickFixTest/oneElementListAsTypingMemberParameter.py

@@ -0,0 +1,3 @@
+from typing import List
+
+foo3: List[<error descr="Parameters to generic types must be types">[in<caret>t]</error>]

python/testData/quickFixes/PyTypeHintsQuickFixTest/oneElementListAsTypingMemberParameter_after.py

@@ -0,0 +1,3 @@
+from typing import List
+
+foo3: List[int]

python/testData/quickFixes/PyTypeHintsQuickFixTest/oneElementListInTupleAsTypingMemberParameter.py

@@ -0,0 +1,3 @@
+from typing import Callable
+
+foo1: Callable[[int], <error descr="Parameters to generic types must be types">[in<caret>t]</error>] = None

python/testData/quickFixes/PyTypeHintsQuickFixTest/oneElementListInTupleAsTypingMemberParameter_after.py

@@ -0,0 +1,3 @@
+from typing import Callable
+
+foo1: Callable[[int], int] = None

python/testData/quickFixes/PyTypeHintsQuickFixTest/referenceToListAsTypingMemberParameter.py

@@ -0,0 +1,4 @@
+from typing import List
+
+l1 = [int]
+foo7: List[<error descr="Parameters to generic types must be types">l<caret>1</error>]

python/testData/quickFixes/PyTypeHintsQuickFixTest/referenceToListInTupleAsTypingMemberParameter.py

@@ -0,0 +1,4 @@
+from typing import Callable
+
+l1 = [int]
+foo5: Callable[[int], <error descr="Parameters to generic types must be types">l<caret>1</error>] = None

python/testSrc/com/jetbrains/python/inspections/PyTypeHintsInspectionTest.java

@@ -763,6 +763,29 @@ public class PyTypeHintsInspectionTest extends PyInspectionTestCase {
                  "        pass");
   }
 
+  // PY-20530
+  public void testTypingMemberParameters() {
+    runWithLanguageLevel(
+      LanguageLevel.PYTHON36,
+      () -> doTestByText(
+        "from typing import Callable, List\n" +
+        "\n" +
+        "foo1: Callable[[int], <error descr=\"Parameters to generic types must be types\">[int]</error>] = None\n" +
+        "foo2: Callable[[int], <error descr=\"Parameters to generic types must be types\">[int, str]</error>] = None\n" +
+        "foo3: List[<error descr=\"Parameters to generic types must be types\">[int]</error>]\n" +
+        "foo4: List[<error descr=\"Parameters to generic types must be types\">[int, str]</error>]\n" +
+        "\n" +
+        "l1 = [int]\n" +
+        "l2 = [int, str]\n" +
+        "\n" +
+        "foo5: Callable[[int], <error descr=\"Parameters to generic types must be types\">l1</error>] = None\n" +
+        "foo6: Callable[[int], <error descr=\"Parameters to generic types must be types\">l2</error>] = None\n" +
+        "foo7: List[<error descr=\"Parameters to generic types must be types\">l1</error>]\n" +
+        "foo8: List[<error descr=\"Parameters to generic types must be types\">l2</error>]"
+      )
+    );
+  }
+
   @NotNull
   @Override
   protected Class<? extends PyInspection> getInspectionClass() {

python/testSrc/com/jetbrains/python/quickFixes/PyTypeHintsQuickFixTest.kt

@@ -100,4 +100,52 @@ class PyTypeHintsQuickFixTest : PyQuickFixTestCase() {
   fun testFunctionAnnotationAndTypeComment() {
     doQuickFixTest(PyTypeHintsInspection::class.java, "Remove function annotations", LanguageLevel.PYTHON35)
   }
+
+  // PY-20530
+  fun testOneElementListAsTypingMemberParameter() {
+    doQuickFixTest(PyTypeHintsInspection::class.java, "Remove square brackets", LanguageLevel.PYTHON37)
+  }
+
+  // PY-20530
+  fun testMultipleElementListAsTypingMemberParameter() {
+    doQuickFixTest(PyTypeHintsInspection::class.java, "Remove square brackets", LanguageLevel.PYTHON37)
+  }
+
+  // PY-20530
+  fun testOneElementListInTupleAsTypingMemberParameter() {
+    doQuickFixTest(PyTypeHintsInspection::class.java, "Remove square brackets", LanguageLevel.PYTHON37)
+  }
+
+  // PY-20530
+  fun testMultipleElementListInTupleAsTypingMemberParameter() {
+    doQuickFixTest(PyTypeHintsInspection::class.java, "Remove square brackets", LanguageLevel.PYTHON37)
+  }
+
+  // PY-20530
+  fun testReferenceToListAsTypingMemberParameter() {
+    runWithLanguageLevel(
+      LanguageLevel.PYTHON37,
+      {
+        myFixture.enableInspections(PyTypeHintsInspection::class.java)
+        myFixture.configureByFile("${getTestName(true)}.py")
+        myFixture.checkHighlighting(true, false, false)
+
+        assertEmpty(myFixture.filterAvailableIntentions("Remove square brackets"))
+      }
+    )
+  }
+
+  // PY-20530
+  fun testReferenceToListInTupleAsTypingMemberParameter() {
+    runWithLanguageLevel(
+      LanguageLevel.PYTHON37,
+      {
+        myFixture.enableInspections(PyTypeHintsInspection::class.java)
+        myFixture.configureByFile("${getTestName(true)}.py")
+        myFixture.checkHighlighting(true, false, false)
+
+        assertEmpty(myFixture.filterAvailableIntentions("Remove square brackets"))
+      }
+    )
+  }
 }