PY-49165 PY-86123 PY-86124 Improve after review

GitOrigin-RevId: 581c80229ca4f9e3a1faab057d0a3c689e2ce96e

python/python-psi-api/src/com/jetbrains/python/psi/PsiReferenceEx.java

@@ -33,7 +33,7 @@ public interface PsiReferenceEx extends PsiReference {
   @Nullable
   @Nls
   String getUnresolvedDescription();
-  
+
   @NotNull
   default List<@NotNull LocalQuickFix> getQuickFixes(TypeEvalContext context) {
     return Collections.emptyList();

python/python-psi-impl/resources/messages/PyPsiBundle.properties

@@ -162,6 +162,7 @@ ANN.patterns.pattern.makes.remaining.case.clauses.unreachable=Pattern makes rema
 ANN.patterns.pattern.does.not.bind.names=Pattern does not bind {0,choice,1#name|2#names} {1}
 ANN.patterns.attribute.name.is.repeated=Attribute name ''{0}'' is repeated
 ANN.patterns.name.already.bound=Name ''{0}'' is already bound
+ANN.patterns.positional.pattern.must.appear.before.keyword.pattern=Positional pattern must appear before keyword patterns
 
 ANN.try.except.can.not.have.except.and.star.except=Try statement cannot contain both except and except*
 ANN.exception.group.in.star.except=ExceptionGroup cannot be used in except*
@@ -1373,7 +1374,7 @@ INSP.patterns.class.does.not.support.pattern.matching.with.positional.arguments=
 INSP.patterns.too.many.positional.patterns.expected=Too many positional patterns, expected {0}
 INSP.patterns.attribute.already.specified.as.positional.pattern.at.position=Attribute ''{0}'' is already specified as positional pattern at position {1}
 QFIX.simplify.as.pattern=Simplify 'as' pattern
-QFIX.NAME.remove.list.member=Remove list member
+QFIX.NAME.remove.element=Remove element
 QFIX.remove.pattern=Remove pattern
 QFIX.NAME.add.match.args.to.class=Add __match_args__ to class
 QFIX.add.match.args.to.class=Add __match_args__ to class {0}

python/python-psi-impl/src/com/jetbrains/python/codeInsight/PyDunderMatchArgsReference.kt

@@ -21,7 +21,7 @@ import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiReferenceBase
 import com.intellij.psi.util.parentOfType
 import com.jetbrains.python.PyNames
-import com.jetbrains.python.inspections.RemoveListMemberFix
+import com.jetbrains.python.inspections.PyRemoveElementFix
 import com.jetbrains.python.inspections.quickfix.AddFieldQuickFix
 import com.jetbrains.python.psi.AccessDirection
 import com.jetbrains.python.psi.PsiReferenceEx
@@ -37,14 +37,14 @@ import com.jetbrains.python.psi.types.TypeEvalContext
  * Similar to [PyDunderSlotsReference], but resolves using [PyType.resolveMember]
  * to account for properties, descriptors, class members, etc.
  */
-class PyDunderMatchArgsReference(element: PyStringLiteralExpression) 
-  : PsiReferenceBase<PyStringLiteralExpression>(element, element.getStringValueTextRanges().firstOrNull()),
-    PsiReferenceEx {
-  
+class PyDunderMatchArgsReference(element: PyStringLiteralExpression) :
+  PsiReferenceBase<PyStringLiteralExpression>(element, element.getStringValueTextRanges().firstOrNull()),
+  PsiReferenceEx {
+
   override fun resolve(): PsiElement? {
     val referenceClass = myElement?.parentOfType<PyClass>() ?: return null
     val typeContext = TypeEvalContext.codeAnalysis(myElement.project, myElement.containingFile)
-    
+
     return referenceClass.getType(typeContext)
       ?.toInstance()
       ?.resolveMember(myElement.stringValue, null, AccessDirection.READ, PyResolveContext.defaultContext(typeContext))
@@ -55,12 +55,12 @@ class PyDunderMatchArgsReference(element: PyStringLiteralExpression)
   override fun getUnresolvedHighlightSeverity(context: TypeEvalContext?): HighlightSeverity = HighlightSeverity.WARNING
 
   override fun getUnresolvedDescription(): String? = null
-  
+
   override fun getQuickFixes(context: TypeEvalContext): List<LocalQuickFix> {
     val clazz = myElement?.parentOfType<PyClass>() ?: return emptyList()
     return listOf(
       AddFieldQuickFix(myElement.stringValue, PyNames.NONE, clazz.name, true),
-      LocalQuickFix.from(RemoveListMemberFix(myElement))!!
+      LocalQuickFix.from(PyRemoveElementFix(myElement))!!
     )
   }
 }

python/python-psi-impl/src/com/jetbrains/python/inspections/PyPatternInspection.kt

@@ -8,10 +8,8 @@ import com.intellij.modcommand.ModPsiUpdater
 import com.intellij.modcommand.Presentation
 import com.intellij.modcommand.PsiUpdateModCommandAction
 import com.intellij.psi.PsiElementVisitor
-import com.intellij.psi.util.PsiTreeUtil
 import com.jetbrains.python.PyNames
 import com.jetbrains.python.PyPsiBundle
-import com.jetbrains.python.PyTokenTypes
 import com.jetbrains.python.documentation.PythonDocumentationProvider
 import com.jetbrains.python.psi.*
 import com.jetbrains.python.psi.impl.PyBuiltinCache
@@ -38,10 +36,10 @@ private class PyPatternInspectionVisitor(holder: ProblemsHolder, context: TypeEv
     val pattern = element.getPattern()
     if (element.getTarget() == null) return
 
-    if (pattern is PyClassPattern && 
-        pattern.classNameReference.name in PyClassPattern.SPECIAL_BUILTINS && 
+    if (pattern is PyClassPattern &&
+        pattern.classNameReference.name in PyClassPattern.SPECIAL_BUILTINS &&
         pattern.argumentList.patterns.isEmpty()
-      ) {
+    ) {
       holder.problem(element, PyPsiBundle.message("INSP.patterns.pattern.can.be.simplified"))
         .highlight(ProblemHighlightType.WEAK_WARNING)
         .fix(SimplifyAsPatternFix(element))
@@ -49,31 +47,32 @@ private class PyPatternInspectionVisitor(holder: ProblemsHolder, context: TypeEv
     }
   }
 
-  
+
   override fun visitPyClassPattern(node: PyClassPattern) {
     val classType = myTypeEvalContext.getType(node.classNameReference) as? PyClassType ?: return
     val pyClass = classType.pyClass
     if (pyClass.name in PyClassPattern.SPECIAL_BUILTINS) return
-    
+
     val matchArgs = PyClassPatternImpl.getMatchArgs(classType, myTypeEvalContext) ?: run {
       node.argumentList.patterns.filterNot { it is PyKeywordPattern }.forEach { pattern ->
-        holder.problem(pattern, PyPsiBundle.message("INSP.patterns.class.does.not.support.pattern.matching.with.positional.arguments", pyClass.name))
+        holder.problem(pattern,
+                       PyPsiBundle.message("INSP.patterns.class.does.not.support.pattern.matching.with.positional.arguments", pyClass.name))
           .fix(AddMatchArgsFix(pyClass))
           .register()
       }
       return
     }
-    
+
     val (positionalPatterns, keywordPatterns) = node.argumentList.patterns.partition { it !is PyKeywordPattern }
-    
+
     for (pattern in positionalPatterns.drop(matchArgs.size)) {
       holder.problem(pattern, PyPsiBundle.message("INSP.patterns.too.many.positional.patterns.expected", matchArgs.size))
-        .fix(RemoveListMemberFix(pattern))
+        .fix(PyRemoveElementFix(pattern))
         .register()
     }
-    
+
     if (positionalPatterns.isEmpty() || keywordPatterns.isEmpty()) return
-    
+
     // Map positional patterns to their corresponding attribute names
     val positionalAttributeNames = positionalPatterns.indices.map { index ->
       if (index < matchArgs.size) matchArgs[index] else null
@@ -84,8 +83,11 @@ private class PyPatternInspectionVisitor(holder: ProblemsHolder, context: TypeEv
       val keywordName = (keywordPattern as PyKeywordPattern).keyword
       val positionalIndex = positionalAttributeNames.indexOf(keywordName)
       if (positionalIndex >= 0) {
-        holder.problem(keywordPattern, PyPsiBundle.message("INSP.patterns.attribute.already.specified.as.positional.pattern.at.position", keywordName, positionalIndex + 1))
-          .fix(RemoveListMemberFix(keywordPattern))
+        holder.problem(keywordPattern,
+                       PyPsiBundle.message("INSP.patterns.attribute.already.specified.as.positional.pattern.at.position",
+                                           keywordName,
+                                           positionalIndex + 1))
+          .fix(PyRemoveElementFix(keywordPattern))
           .register()
       }
     }
@@ -93,20 +95,19 @@ private class PyPatternInspectionVisitor(holder: ProblemsHolder, context: TypeEv
 
   override fun visitPyClass(node: PyClass) {
     val matchArgs = node
-      .findClassAttribute(PyNames.MATCH_ARGS, false, myTypeEvalContext)
-      ?.findAssignedValue()
-      ?.let { PyPsiUtils.flattenParens(it) } ?: return
-    
+                      .findClassAttribute(PyNames.MATCH_ARGS, false, myTypeEvalContext)
+                      ?.findAssignedValue()
+                      ?.let { PyPsiUtils.flattenParens(it) } ?: return
+
     val matchArgsType = myTypeEvalContext.getType(matchArgs) ?: return
     val strType = PyBuiltinCache.getInstance(matchArgs).strType ?: return
     val goodTuple = PyTupleType.createHomogeneous(matchArgs, strType) ?: return
     if (PyTypeChecker.match(goodTuple, matchArgsType, myTypeEvalContext)) return
     // __match_args__ must be a tuple[str, ...]
     holder.problem(matchArgs, PyPsiBundle.message(
-      "INSP.type.checker.expected.type.got.type.instead", 
+      "INSP.type.checker.expected.type.got.type.instead",
       PythonDocumentationProvider.getTypeName(goodTuple, myTypeEvalContext),
-      PythonDocumentationProvider.getTypeName(matchArgsType, myTypeEvalContext))
-    ).register()
+      PythonDocumentationProvider.getTypeName(matchArgsType, myTypeEvalContext))).register()
   }
 }
 
@@ -116,61 +117,50 @@ private class SimplifyAsPatternFix(element: PyAsPattern) : PsiUpdateModCommandAc
   override fun invoke(context: ActionContext, element: PyAsPattern, updater: ModPsiUpdater) {
     val pattern = element.getPattern() as PyClassPattern
     val target = element.getTarget() ?: return
-    
+
     val generator = PyElementGenerator.getInstance(element.project)
     val newPattern = generator.createPatternFromText(
       LanguageLevel.forElement(element),
       "${pattern.classNameReference.text}(${target.name})"
     )
-    
+
     element.replace(newPattern)
   }
 }
 
-// Almost identical to PyRemoveDictKeyQuickFix
-class RemoveListMemberFix(element: PyElement) : PsiUpdateModCommandAction<PyElement>(element) {
-  override fun getFamilyName(): String = PyPsiBundle.message("QFIX.NAME.remove.list.member")
+class PyRemoveElementFix(element: PyElement) : PsiUpdateModCommandAction<PyElement>(element) {
+  override fun getFamilyName(): String = PyPsiBundle.message("QFIX.NAME.remove.element")
   override fun getPresentation(context: ActionContext, element: PyElement): Presentation? = when (element) {
     is PyPattern -> Presentation.of(PyPsiBundle.message("QFIX.remove.pattern"))
     else -> super.getPresentation(context, element)
   }
 
   override fun invoke(context: ActionContext, element: PyElement, updater: ModPsiUpdater) {
-    val nextSibling = PsiTreeUtil.skipWhitespacesForward(element)
-    val prevSibling = PsiTreeUtil.skipWhitespacesBackward(element)
     element.delete()
-    if (nextSibling != null && nextSibling.getNode().getElementType() == PyTokenTypes.COMMA) {
-      nextSibling.delete()
-      return
-    }
-    if (prevSibling != null && prevSibling.getNode().getElementType() == PyTokenTypes.COMMA) {
-      prevSibling.delete()
-    }
   }
 }
 
 
 class AddMatchArgsFix(element: PyClass) : PsiUpdateModCommandAction<PyClass>(element) {
-
   override fun getFamilyName(): String = PyPsiBundle.message("QFIX.NAME.add.match.args.to.class")
   override fun getPresentation(context: ActionContext, element: PyClass): Presentation {
     return Presentation.of(PyPsiBundle.message("QFIX.add.match.args.to.class", element.name))
   }
-  
+
   /**
    * Take positional arguments from `__init__`, check whether the class has an attribute with the same name,
    * and if so, add it to `__match_args__`.
    */
   override fun invoke(context: ActionContext, pyClass: PyClass, updater: ModPsiUpdater) {
     val typeEvalContext = TypeEvalContext.userInitiated(pyClass.project, pyClass.containingFile)
-    
+
     val initMethod = pyClass.findMethodByName(PyNames.INIT, false, typeEvalContext)
     val positionalArgs = initMethod?.parameterList?.parameters
-      ?.drop(1)
-      ?.mapNotNull { it.name }
-      ?.filter { pyClass.findInstanceAttribute(it, true) != null }
-      ?.toList() ?: emptyList()
-    
+                           ?.drop(1)
+                           ?.mapNotNull { it.name }
+                           ?.filter { pyClass.findInstanceAttribute(it, true) != null }
+                           ?.toList() ?: emptyList()
+
     val generator = PyElementGenerator.getInstance(pyClass.project)
     val matchArgsValue = positionalArgs.joinToString(
       prefix = "(",
@@ -186,5 +176,6 @@ class AddMatchArgsFix(element: PyClass) : PsiUpdateModCommandAction<PyClass>(ele
     val anchor = pyClass.statementList.statements.firstOrNull()
     val result = pyClass.statementList.addBefore(matchArgsAssignment, anchor) as PyAssignmentStatement
     updater.moveCaretTo(result.textRange.endOffset - 1)
+    PyPsiUtils.removeRedundantPass(pyClass.statementList)
   }
 }

python/python-psi-impl/src/com/jetbrains/python/inspections/PyProtocolInspection.kt

@@ -100,7 +100,7 @@ class PyProtocolInspection : PyInspection() {
 
       val type = myTypeEvalContext.getType(base)
       if (type is PyClassType && type.isProtocol(myTypeEvalContext) && !type.isRuntimeCheckable(myTypeEvalContext)) {
-        registerProblem(base, 
+        registerProblem(base,
                         PyPsiBundle.message("INSP.protocol.only.runtime.checkable.protocols.can.be.used.with.instance.class.checks"),
                         GENERIC_ERROR)
       }

python/python-psi-impl/src/com/jetbrains/python/psi/impl/PyClassPatternImpl.java

@@ -139,7 +139,7 @@ public class PyClassPatternImpl extends PyElementImpl implements PyClassPattern,
   public static @Nullable List<@NotNull String> getMatchArgs(@NotNull PyClassType type, @NotNull TypeEvalContext context) {
     final PyClass cls = type.getPyClass();
     // TODO: change to getMemberType, when PyLiteralType can be created without PyExpression
-    
+
     List<String> matchArgs = cls.getOwnMatchArgs();
     if (matchArgs != null) return matchArgs;
 

python/python-psi-impl/src/com/jetbrains/python/psi/impl/PyPatternArgumentListImpl.java

@@ -23,4 +23,12 @@ public class PyPatternArgumentListImpl extends PyElementImpl implements PyPatter
   public @NotNull List<? extends PsiElement> getComponents() {
     return getPatterns();
   }
+
+  @Override
+  public void deleteChildInternal(@NotNull ASTNode child) {
+    if (getPatterns().contains(child.getPsi())) {
+      PyPsiUtils.deleteAdjacentCommaWithWhitespaces(this, child.getPsi());
+    }
+    super.deleteChildInternal(child);
+  }
 }

python/python-psi-impl/src/com/jetbrains/python/validation/PyPatternAnnotatorVisitor.java

@@ -9,7 +9,7 @@ import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.containers.Stack;
 import com.jetbrains.python.PyPsiBundle;
 import com.jetbrains.python.PyTokenTypes;
-import com.jetbrains.python.inspections.RemoveListMemberFix;
+import com.jetbrains.python.inspections.PyRemoveElementFix;
 import com.jetbrains.python.psi.*;
 import org.jetbrains.annotations.NotNull;
 
@@ -116,15 +116,17 @@ final class PyPatternAnnotatorVisitor extends PyElementVisitor {
       PyKeywordPattern keywordPattern = as(attrPattern, PyKeywordPattern.class);
       if (keywordPattern == null) {
         if (seenKeywordPattern) {
-          myHolder.newAnnotation(HighlightSeverity.ERROR, "Positional pattern must appear before keyword patterns")
-            .range(attrPattern).withFix(new RemoveListMemberFix(attrPattern)).create();
+          myHolder.newAnnotation(HighlightSeverity.ERROR,
+                                 PyPsiBundle.message("ANN.patterns.positional.pattern.must.appear.before.keyword.pattern"))
+            .range(attrPattern).withFix(new PyRemoveElementFix(attrPattern)).create();
         }
         continue;
       }
       seenKeywordPattern = true;
       if (!usedAttrNames.add(keywordPattern.getKeyword())) {
-        myHolder.newAnnotation(HighlightSeverity.ERROR, PyPsiBundle.message("ANN.patterns.attribute.name.is.repeated", keywordPattern.getKeyword()))
-            .range(keywordPattern.getKeywordElement()).withFix(new RemoveListMemberFix(keywordPattern)).create();
+        myHolder.newAnnotation(HighlightSeverity.ERROR,
+                               PyPsiBundle.message("ANN.patterns.attribute.name.is.repeated", keywordPattern.getKeyword()))
+          .range(keywordPattern.getKeywordElement()).withFix(new PyRemoveElementFix(keywordPattern)).create();
       }
     }
   }

python/testData/quickFixes/PyAddMatchArgsFixTest/addMatchArgsNoInit_after.py

@@ -1,6 +1,6 @@
 class Empty:
     __match_args__ = ()
-    pass
+
 
 def f(obj):
     match obj:

python/testSrc/com/jetbrains/python/inlayHints/PyPseudoKeywordPatternInlayHintsProviderTest.kt

@@ -20,7 +20,7 @@ def f(p: Point):
     match p:
         case Point(/*<# x= #>*/1, /*<# y= #>*/b):
             pass
-      """.trimIndent()
+      """
     )
   }
 
@@ -34,7 +34,7 @@ def f(p: Point):
     match p:
         case Point(/*<# x= #>*/1, y=2):  # second is keyword, no inlay expected there
             pass
-      """.trimIndent()
+      """
     )
   }
 
@@ -49,7 +49,7 @@ def f(p: Point):
     )
   }
 
-  override fun getProjectDescriptor(): LightProjectDescriptor? {
+  override fun getProjectDescriptor(): LightProjectDescriptor {
     return PyLightProjectDescriptor(LanguageLevel.getLatest())
   }
 }

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

@@ -102,7 +102,7 @@ def f(z):
     match z:
         case Z():
             pass
-        case Z(<warning descr=\"Too many positional patterns, expected 0\">1</warning>):
+        case Z(<warning descr="Too many positional patterns, expected 0">1</warning>):
             pass
     """);
   }
@@ -117,7 +117,7 @@ class Point:
 
 def f(p):
     match p:
-        case Point(0, 0, <warning descr=\"Attribute 'y' is already specified as positional pattern at position 2\">y=0</warning>):
+        case Point(0, 0, <warning descr="Attribute 'y' is already specified as positional pattern at position 2">y=0</warning>):
             pass
     """);
   }
@@ -194,7 +194,7 @@ class P:
 
 def f(p):
     match p:
-        case P(1, 2, <warning descr=\"Too many positional patterns, expected 2\">3</warning>, z=0):
+        case P(1, 2, <warning descr="Too many positional patterns, expected 2">3</warning>, z=0):
             pass
     """);
   }