Java: improve error message - repeated modifier (IDEA-330995)

GitOrigin-RevId: c2e4ddef81f2df01c0cd16ebc989f8130d255056

java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java

@@ -170,14 +170,11 @@ public final class HighlightUtil {
                                                 @NotNull PsiModifierList modifierList,
                                                 @NotNull Map<String, Set<String>> incompatibleModifiersHash) {
     // modifier is always incompatible with itself
-    PsiElement[] modifiers = modifierList.getChildren();
     int modifierCount = 0;
-    for (PsiElement otherModifier : modifiers) {
-      if (Comparing.equal(modifier, otherModifier.getText(), true)) modifierCount++;
-    }
-    if (modifierCount > 1) {
-      return modifier;
+    for (PsiElement otherModifier = modifierList.getFirstChild(); otherModifier != null; otherModifier = otherModifier.getNextSibling()) {
+      if (modifier.equals(otherModifier.getText())) modifierCount++;
     }
+    if (modifierCount > 1) return modifier;
 
     Set<String> incompatibles = incompatibleModifiersHash.get(modifier);
     if (incompatibles == null) return null;
@@ -1043,7 +1040,19 @@ public final class HighlightUtil {
     @PsiModifier.ModifierConstant String modifier = keyword.getText();
     String incompatible = getIncompatibleModifier(modifier, modifierList);
     if (incompatible != null) {
-      String message = JavaErrorBundle.message("incompatible.modifiers", modifier, incompatible);
+      String message;
+      if (incompatible.equals(modifier)) {
+        for (PsiElement child = modifierList.getFirstChild(); child != null; child = child.getNextSibling()) {
+          if (modifier.equals(child.getText())) {
+            if (child == keyword) return null;
+            else break;
+          }
+        }
+        message = JavaErrorBundle.message("repeated.modifier", incompatible);
+      }
+      else {
+        message = JavaErrorBundle.message("incompatible.modifiers", modifier, incompatible);
+      }
       HighlightInfo.Builder highlightInfo =
         HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(keyword).descriptionAndTooltip(message);
       IntentionAction action = getFixFactory().createModifierListFix(modifierList, modifier, false, false);
@@ -1217,8 +1226,7 @@ public final class HighlightUtil {
       String message = JavaErrorBundle.message("modifier.not.allowed", modifier);
       HighlightInfo.Builder highlightInfo =
         HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(keyword).descriptionAndTooltip(message);
-      IntentionAction action = fix != null ? fix : getFixFactory()
-        .createModifierListFix(modifierList, modifier, false, false);
+      IntentionAction action = fix != null ? fix : getFixFactory().createModifierListFix(modifierList, modifier, false, false);
       highlightInfo.registerFix(action, null, null, null, null);
       return highlightInfo;
     }

java/java-analysis-impl/src/messages/QuickFixBundle.properties

@@ -167,6 +167,7 @@ anonymous.class.presentation=Anonymous class derived from {0}
 class.initializer.presentation={0} class initializer
 add.modifier.fix=Make ''{0}'' {1}
 remove.modifier.fix=Make ''{0}'' not {1}
+remove.one.modifier.fix=Remove ''{0}'' modifier
 add.modifier.fix.family=Make {0}
 remove.modifier.fix.family=Make not {0}
 

java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ModifierFix.java

@@ -1,4 +1,4 @@
-// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.intellij.codeInsight.daemon.impl.quickfix;
 
 import com.intellij.codeInsight.daemon.QuickFixBundle;
@@ -93,6 +93,15 @@ public class ModifierFix extends PsiBasedModCommandAction<PsiModifierListOwner>
     }
 
     String modifierText = VisibilityUtil.toPresentableText(myModifier);
+    if (!myShouldHave && modifierList != null) {
+      int count = 0;
+      for (PsiElement child = modifierList.getFirstChild(); child != null; child = child.getNextSibling()) {
+        if (child.getText().equals(myModifier)) count++;
+      }
+      if (count > 1) {
+        return QuickFixBundle.message("remove.one.modifier.fix", modifierText);
+      }
+    }
     return QuickFixBundle.message(myShouldHave ? "add.modifier.fix" : "remove.modifier.fix", name, modifierText);
   }
 

java/java-psi-impl/src/messages/JavaErrorBundle.properties

@@ -223,7 +223,8 @@ continue.outside.loop=Continue outside of loop
 continue.outside.switch.expr=Continue outside of enclosing switch expression
 class.member.declared.outside=Class member declared outside of a class
 not.loop.label=Not a loop label: ''{0}''
-incompatible.modifiers=Illegal combination of modifiers: ''{0}'' and ''{1}''
+incompatible.modifiers=Illegal combination of modifiers ''{0}'' and ''{1}''
+repeated.modifier=Repeated modifier ''{0}''
 modifier.not.allowed=Modifier ''{0}'' not allowed here
 exception.never.thrown.try=Exception ''{0}'' is never thrown in the corresponding try block
 exception.already.caught.warn=Unreachable section: {1, choice, 0#exception|2#exceptions} ''{0}'' {1, choice, 0#has|2#have} already been caught

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/IllegalModifiersCombination.java

@@ -2,20 +2,20 @@
 
 abstract public class a {
   //////////////////// fields ////////////////////////////////
-  <error descr="Illegal combination of modifiers: 'public' and 'protected'">public</error> static
-  <error descr="Illegal combination of modifiers: 'protected' and 'public'">protected</error> int f1 = 0;
+  <error descr="Illegal combination of modifiers 'public' and 'protected'">public</error> static
+  <error descr="Illegal combination of modifiers 'protected' and 'public'">protected</error> int f1 = 0;
 
-  <error descr="Illegal combination of modifiers: 'public' and 'private'">public</error> volatile
-  <error descr="Illegal combination of modifiers: 'private' and 'public'">private</error> int f2 = 0;
+  <error descr="Illegal combination of modifiers 'public' and 'private'">public</error> volatile
+  <error descr="Illegal combination of modifiers 'private' and 'public'">private</error> int f2 = 0;
 
-  <error descr="Illegal combination of modifiers: 'protected' and 'private'">protected</error> final
-  <error descr="Illegal combination of modifiers: 'private' and 'protected'">private</error> int f3 = 0;
+  <error descr="Illegal combination of modifiers 'protected' and 'private'">protected</error> final
+  <error descr="Illegal combination of modifiers 'private' and 'protected'">private</error> int f3 = 0;
 
-  <error descr="Illegal combination of modifiers: 'final' and 'volatile'">final</error>
-  <error descr="Illegal combination of modifiers: 'volatile' and 'final'">volatile</error> private int f4 = 0;
+  <error descr="Illegal combination of modifiers 'final' and 'volatile'">final</error>
+  <error descr="Illegal combination of modifiers 'volatile' and 'final'">volatile</error> private int f4 = 0;
 
-  <error descr="Illegal combination of modifiers: 'public' and 'public'">public</error> 
-  <error descr="Illegal combination of modifiers: 'public' and 'public'">public</error>
+  public
+  <error descr="Repeated modifier 'public'">public</error>
      int f5 = 0;
 
   public static final int cf1 = 0;
@@ -28,23 +28,23 @@ abstract public class a {
 
   ///////////////////// methods ///////////////////////////////////
 
-  <error descr="Illegal combination of modifiers: 'abstract' and 'native'">abstract</error>  
-  <error descr="Illegal combination of modifiers: 'native' and 'abstract'">native</error>  void m1();
+  <error descr="Illegal combination of modifiers 'abstract' and 'native'">abstract</error>
+  <error descr="Illegal combination of modifiers 'native' and 'abstract'">native</error>  void m1();
 
-  <error descr="Illegal combination of modifiers: 'static' and 'abstract'">static</error>  public
-  <error descr="Illegal combination of modifiers: 'abstract' and 'static'">abstract</error>  void m2();
+  <error descr="Illegal combination of modifiers 'static' and 'abstract'">static</error>  public
+  <error descr="Illegal combination of modifiers 'abstract' and 'static'">abstract</error>  void m2();
 
-  <error descr="Illegal combination of modifiers: 'final' and 'abstract'">final</error>
-  <error descr="Illegal combination of modifiers: 'abstract' and 'final'">abstract</error>  void m3();
+  <error descr="Illegal combination of modifiers 'final' and 'abstract'">final</error>
+  <error descr="Illegal combination of modifiers 'abstract' and 'final'">abstract</error>  void m3();
 
-  <error descr="Illegal combination of modifiers: 'private' and 'public'">private</error> static
-  <error descr="Illegal combination of modifiers: 'public' and 'private'">public</error>  void m4() {}
+  <error descr="Illegal combination of modifiers 'private' and 'public'">private</error> static
+  <error descr="Illegal combination of modifiers 'public' and 'private'">public</error>  void m4() {}
 
-  <error descr="Illegal combination of modifiers: 'protected' and 'private'">protected</error> final
-  <error descr="Illegal combination of modifiers: 'private' and 'protected'">private</error>  void m5() {}
+  <error descr="Illegal combination of modifiers 'protected' and 'private'">protected</error> final
+  <error descr="Illegal combination of modifiers 'private' and 'protected'">private</error>  void m5() {}
 
-  <error descr="Illegal combination of modifiers: 'public' and 'public'">public</error> 
-  <error descr="Illegal combination of modifiers: 'public' and 'public'">public</error> void m6() {};
+  public
+  <error descr="Repeated modifier 'public'">public</error> void m6() {};
 
   public abstract void cm1();
   protected static synchronized native void cm2();
@@ -52,14 +52,14 @@ abstract public class a {
 
 
   ///////////////////////// classes //////////////////////////////////
-  <error descr="Illegal combination of modifiers: 'final' and 'abstract'">final</error>  static strictfp protected
-  <error descr="Illegal combination of modifiers: 'abstract' and 'final'">abstract</error>  class c1 {}
+  <error descr="Illegal combination of modifiers 'final' and 'abstract'">final</error>  static strictfp protected
+  <error descr="Illegal combination of modifiers 'abstract' and 'final'">abstract</error>  class c1 {}
 
-  <error descr="Illegal combination of modifiers: 'private' and 'public'">private</error> final
-  <error descr="Illegal combination of modifiers: 'public' and 'private'">public</error>  class c2 {}
+  <error descr="Illegal combination of modifiers 'private' and 'public'">private</error> final
+  <error descr="Illegal combination of modifiers 'public' and 'private'">public</error>  class c2 {}
 
-  <error descr="Illegal combination of modifiers: 'final' and 'final'">final</error> 
-  <error descr="Illegal combination of modifiers: 'final' and 'final'">final</error> class c3 {}
+  final
+  <error descr="Repeated modifier 'final'">final</error> class c3 {}
 
   abstract protected static strictfp class cc1 {}
   final private static class cc2 {}
@@ -68,7 +68,7 @@ abstract public class a {
 
   ///////////////////////// locals
   void f() {
-    <error descr="Illegal combination of modifiers: 'final' and 'final'">final</error>
-    <error descr="Illegal combination of modifiers: 'final' and 'final'">final</error> int loc;
+    final
+    <error descr="Repeated modifier 'final'">final</error> int loc;
   }
 }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting8/StrictfpInsideInterface.java

@@ -1,5 +1,5 @@
 interface A {
-   <error descr="Illegal combination of modifiers: 'strictfp' and 'abstract'">strictfp</error> void m();
+   <error descr="Illegal combination of modifiers 'strictfp' and 'abstract'">strictfp</error> void m();
    strictfp default void m1(){}
    strictfp static void m2() {}
 }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting9/PrivateInInterfaces.java

@@ -7,7 +7,7 @@ interface B {
 }
 
 interface C {
-    private <error descr="Illegal combination of modifiers: 'default' and 'private'">default</error> void m() {}
+    private <error descr="Illegal combination of modifiers 'default' and 'private'">default</error> void m() {}
 }
 
 interface D {
@@ -15,7 +15,7 @@ interface D {
 }
 
 interface E {
-    <error descr="Illegal combination of modifiers: 'private' and 'public'">private</error> class E1 {}
+    <error descr="Illegal combination of modifiers 'private' and 'public'">private</error> class E1 {}
 }
 
 interface F {

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingSealedTypes/SealedTypesBasics.java

@@ -4,9 +4,9 @@ sealed class B extends A permits C, D {}
 final class C extends B {}
 non-sealed class D extends B {}
 class E extends <error descr="'E' is not allowed in the sealed hierarchy">A</error> {}
-<error descr="Illegal combination of modifiers: 'sealed' and 'sealed'">sealed</error> <error descr="Illegal combination of modifiers: 'sealed' and 'sealed'">sealed</error> class SealedSealed {}
-<error descr="Illegal combination of modifiers: 'sealed' and 'non-sealed'">sealed</error> <error descr="Illegal combination of modifiers: 'non-sealed' and 'sealed'">non-sealed</error> class SealedNonSealed {}
-<error descr="Illegal combination of modifiers: 'sealed' and 'final'">sealed</error> <error descr="Illegal combination of modifiers: 'final' and 'sealed'">final</error> class SealedFinal {}
+sealed <error descr="Repeated modifier 'sealed'">sealed</error> class SealedSealed {}
+<error descr="Illegal combination of modifiers 'sealed' and 'non-sealed'">sealed</error> <error descr="Illegal combination of modifiers 'non-sealed' and 'sealed'">non-sealed</error> class SealedNonSealed {}
+<error descr="Illegal combination of modifiers 'sealed' and 'final'">sealed</error> <error descr="Illegal combination of modifiers 'final' and 'sealed'">final</error> class SealedFinal {}
 
 //interfaces
 sealed interface IA permits IB, IC {}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/interfaceMethods/ExtensionMethods.java

@@ -53,9 +53,9 @@ interface IllegalMods {
   void m1()<error descr="'{' or ';' expected"> </error>default<error descr="Identifier or type expected">;</error> <error descr="Not allowed in interface">{ }</error>
 
   <error descr="Static methods in interfaces should have a body">static void m2()</error>;
-  <error descr="Illegal combination of modifiers: 'static' and 'default'">static</error> <error descr="Illegal combination of modifiers: 'default' and 'static'">default</error> void m3() { }
+  <error descr="Illegal combination of modifiers 'static' and 'default'">static</error> <error descr="Illegal combination of modifiers 'default' and 'static'">default</error> void m3() { }
 
-  <error descr="Illegal combination of modifiers: 'abstract' and 'default'">abstract</error> <error descr="Illegal combination of modifiers: 'default' and 'abstract'">default</error> void m4() { }
+  <error descr="Illegal combination of modifiers 'abstract' and 'default'">abstract</error> <error descr="Illegal combination of modifiers 'default' and 'abstract'">default</error> void m4() { }
 
   <error descr="Extension method should have a body">default void m5()</error>;
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/interfaceMethods/StaticMethod.java

@@ -1,4 +1,4 @@
 interface Foo {
   public static void bar1() {}
-  public <error descr="Illegal combination of modifiers: 'abstract' and 'static'">abstract</error> static void bar2() {}
+  public <error descr="Illegal combination of modifiers 'abstract' and 'static'">abstract</error> static void bar2() {}
 }

java/java-tests/testData/ig/com/siyeh/igtest/classlayout/final_private_method/FinalPrivateMethod.java

@@ -10,7 +10,7 @@ public class FinalPrivateMethod {
   @java.lang.SafeVarargs
   private final <T> void foo(T... i) {}
 
-  <error descr="Illegal combination of modifiers: 'public' and 'private'">public</error> final
+  <error descr="Illegal combination of modifiers 'public' and 'private'">public</error> final
 
-  <error descr="Illegal combination of modifiers: 'private' and 'public'">private</error> void bar() {}
+  <error descr="Illegal combination of modifiers 'private' and 'public'">private</error> void bar() {}
 }

java/java-tests/testData/ig/com/siyeh/igtest/classlayout/final_static_method/FinalStaticMethod.java

@@ -9,7 +9,7 @@ public class X {
 
   // incomplete code: suppress
 
-  <error descr="Illegal combination of modifiers: 'private' and 'public'">private</error> final
+  <error descr="Illegal combination of modifiers 'private' and 'public'">private</error> final
 
-  <error descr="Illegal combination of modifiers: 'public' and 'private'">public</error> static void main(String[] args) {}
+  <error descr="Illegal combination of modifiers 'public' and 'private'">public</error> static void main(String[] args) {}
 }