[groovy] fix highlighting of fields and local variables
authorDaniil Ovchinnikov <daniil.ovchinnikov@jetbrains.com>
Thu, 11 Aug 2016 14:25:47 +0000 (17:25 +0300)
committerDaniil Ovchinnikov <daniil.ovchinnikov@jetbrains.com>
Thu, 11 Aug 2016 14:47:17 +0000 (17:47 +0300)
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/GroovyBundle.properties
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/annotator/util.kt [new file with mode: 0644]
plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/bugs/GrRemoveModifierFix.java
plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GroovyHighlightingTest.groovy
plugins/groovy/testdata/highlighting/FieldModifiers.groovy [new file with mode: 0644]
plugins/groovy/testdata/highlighting/LocalVariableModifiers.groovy [new file with mode: 0644]

index 7a2bd26fa80b9a00d336e461097f8265d5981a6a..1ce2a6d362b36e4c69bca654c7d8d69500dc77e4 100644 (file)
@@ -187,8 +187,8 @@ interface.must.have.no.static.method=Interface must have no static method
 only.abstract.class.can.have.abstract.method=Only abstract class can have abstract method
 anonymous.class.cannot.have.abstract.method=Anonymous class cannot have abstract method
 illegal.combination.of.modifiers.volatile.and.final=Illegal combination of modifiers 'volatile' and 'final'
-variable.cannot.be.native=Variable cannot have modifier 'native'
-variable.cannot.be.abstract=Variable cannot have modifier 'abstract'
+variable.cannot.be=Variable cannot have modifier ''{0}''
+remove.modifier=Remove ''{0}''
 not.abstract.method.should.have.body=Not abstract method should have body
 cannot.create.class.error.text=Cannot Create Class ''{0}'': {1}
 cannot.create.class.error.title=Cannot Create Class
index 52f151fe6fe6b4320d68221ecd1d6d736310df82..41a2fcff169dbc394becbdf7a7a79593437cc769 100644 (file)
@@ -57,6 +57,7 @@ import org.jetbrains.plugins.groovy.annotator.checkers.AnnotationChecker;
 import org.jetbrains.plugins.groovy.annotator.checkers.CustomAnnotationChecker;
 import org.jetbrains.plugins.groovy.annotator.intentions.*;
 import org.jetbrains.plugins.groovy.codeInspection.bugs.GrModifierFix;
+import org.jetbrains.plugins.groovy.codeInspection.bugs.GrRemoveModifierFix;
 import org.jetbrains.plugins.groovy.config.GroovyConfigUtils;
 import org.jetbrains.plugins.groovy.findUsages.LiteralConstructorReference;
 import org.jetbrains.plugins.groovy.highlighter.GroovySyntaxHighlighter;
@@ -105,6 +106,10 @@ import org.jetbrains.plugins.groovy.lang.resolve.ast.InheritConstructorContribut
 
 import java.util.*;
 
+import static org.jetbrains.plugins.groovy.annotator.UtilKt.checkModifierIsNotAllowed;
+import static org.jetbrains.plugins.groovy.annotator.UtilKt.checkVariableModifiers;
+import static org.jetbrains.plugins.groovy.annotator.UtilKt.registerFix;
+
 /**
  * @author ven
  */
@@ -951,9 +956,12 @@ public class GroovyAnnotator extends GroovyElementVisitor {
     else if (parent instanceof GrTypeDefinition) {
       checkTypeDefinitionModifiers(myHolder, (GrTypeDefinition)parent);
     }
-    else if (parent instanceof GrVariableDeclaration && parent.getParent() instanceof GrTypeDefinition) {
+    else if (parent instanceof GrVariableDeclaration && parent.getParent() instanceof GrTypeDefinitionBody) {
       checkFieldModifiers(myHolder, (GrVariableDeclaration)parent);
     }
+    else if (parent instanceof GrVariableDeclaration) {
+      checkVariableModifiers(myHolder, ((GrVariableDeclaration)parent));
+    }
     else if (parent instanceof GrClassInitializer) {
       checkClassInitializerModifiers(myHolder, modifierList);
     }
@@ -993,9 +1001,6 @@ public class GroovyAnnotator extends GroovyElementVisitor {
       registerFix(annotation, new GrModifierFix(member, PsiModifier.FINAL, true, false, GrModifierFix.MODIFIER_LIST), modifierList);
     }
 
-    checkModifierIsNotAllowed(modifierList, PsiModifier.NATIVE, GroovyBundle.message("variable.cannot.be.native"), holder);
-    checkModifierIsNotAllowed(modifierList, PsiModifier.ABSTRACT, GroovyBundle.message("variable.cannot.be.abstract"), holder);
-
     if (member.getContainingClass() instanceof GrInterfaceDefinition) {
       checkModifierIsNotAllowed(modifierList,
                                 PsiModifier.PRIVATE, GroovyBundle.message("interface.members.are.not.allowed.to.be", PsiModifier.PRIVATE), holder);
@@ -1005,35 +1010,6 @@ public class GroovyAnnotator extends GroovyElementVisitor {
     }
   }
 
-  private static void registerFix(Annotation annotation, LocalQuickFix fix, PsiElement place) {
-    final InspectionManager manager = InspectionManager.getInstance(place.getProject());
-    assert !place.getTextRange().isEmpty() : place.getContainingFile().getName();
-
-    final ProblemDescriptor descriptor = manager.createProblemDescriptor(place, place, annotation.getMessage(),
-                                                                         annotation.getHighlightType(), true, LocalQuickFix.EMPTY_ARRAY);
-    final TextRange range = TextRange.create(annotation.getStartOffset(), annotation.getEndOffset());
-    annotation.registerFix(fix, range, null, descriptor);
-  }
-
-  private static void checkModifierIsNotAllowed(@NotNull GrModifierList modifierList,
-                                                @NotNull @GrModifier.GrModifierConstant String modifier,
-                                                @Nullable String message,
-                                                @NotNull AnnotationHolder holder) {
-    checkModifierIsNotAllowedImpl(modifierList, modifier, message, holder, false);
-  }
-
-  private static void checkModifierIsNotAllowedImpl(@NotNull GrModifierList modifierList,
-                                                    @NotNull @GrModifier.GrModifierConstant String modifier,
-                                                    @Nullable String message,
-                                                    @NotNull AnnotationHolder holder,
-                                                    final boolean explicit) {
-    if (explicit ? modifierList.hasModifierProperty(modifier) : modifierList.hasExplicitModifier(modifier)) {
-      PsiElement modifierOrList = getModifierOrList(modifierList, modifier);
-      final Annotation annotation = holder.createErrorAnnotation(modifierOrList, message);
-      registerFix(annotation, new GrModifierFix((PsiMember)modifierList.getParent(), modifier, true, false, GrModifierFix.MODIFIER_LIST), modifierList);
-    }
-  }
-
   private static void checkAnnotationAttributeType(GrTypeElement element, AnnotationHolder holder) {
     if (element instanceof GrBuiltInTypeElement) return;
 
diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/annotator/util.kt b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/annotator/util.kt
new file mode 100644 (file)
index 0000000..8031181
--- /dev/null
@@ -0,0 +1,57 @@
+/*
+ * Copyright 2000-2016 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jetbrains.plugins.groovy.annotator
+
+import com.intellij.codeInspection.InspectionManager
+import com.intellij.codeInspection.LocalQuickFix
+import com.intellij.lang.annotation.Annotation
+import com.intellij.lang.annotation.AnnotationHolder
+import com.intellij.openapi.util.TextRange
+import com.intellij.psi.PsiElement
+import org.jetbrains.plugins.groovy.GroovyBundle
+import org.jetbrains.plugins.groovy.codeInspection.bugs.GrRemoveModifierFix
+import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.GrModifier
+import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.GrModifier.GrModifierConstant
+import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.GrModifierList
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariableDeclaration
+
+val VARIABLE_MODIFIERS = setOf(GrModifier.DEF, GrModifier.FINAL)
+
+internal fun checkVariableModifiers(holder: AnnotationHolder, variableDeclaration: GrVariableDeclaration) {
+  val modifierList = variableDeclaration.modifierList
+  for (modifier in GrModifier.GROOVY_MODIFIERS) {
+    if (modifier in VARIABLE_MODIFIERS) continue
+    checkModifierIsNotAllowed(modifierList, modifier, GroovyBundle.message("variable.cannot.be", modifier), holder)
+  }
+}
+
+internal fun checkModifierIsNotAllowed(modifierList: GrModifierList,
+                                       @GrModifierConstant modifier: String,
+                                       message: String?,
+                                       holder: AnnotationHolder) {
+  val modifierElement = modifierList.getModifier(modifier) ?: return
+  val annotation = holder.createErrorAnnotation(modifierElement, message)
+  val fix = GrRemoveModifierFix(modifier, GroovyBundle.message("remove.modifier", modifier))
+  registerFix(annotation, fix, modifierElement)
+}
+
+internal fun registerFix(annotation: Annotation, fix: LocalQuickFix, place: PsiElement) {
+  val manager = InspectionManager.getInstance(place.project)
+  assert(!place.textRange.isEmpty) { place.containingFile.name }
+  val descriptor = manager.createProblemDescriptor(place, place, annotation.message, annotation.highlightType, true)
+  val range = TextRange.create(annotation.startOffset, annotation.endOffset)
+  annotation.registerFix(fix, range, null, descriptor)
+}
\ No newline at end of file
index ba672eb4bfb8f722375d82a3086fa2e4a5ff98cf..ce292191d85d53c80e473dc33cebfead996898a5 100644 (file)
@@ -19,13 +19,18 @@ import com.intellij.codeInspection.ProblemDescriptor;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.codeStyle.CodeStyleManager;
 import com.intellij.util.IncorrectOperationException;
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.plugins.groovy.codeInspection.GroovyInspectionBundle;
 import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.GrModifier;
 
 public class GrRemoveModifierFix extends GrModifierFix {
 
-  public GrRemoveModifierFix(@GrModifier.GrModifierConstant String modifier) {
-    super(GroovyInspectionBundle.message("unnecessary.modifier.remove", modifier), modifier, false, GrModifierFix.MODIFIER_LIST_CHILD);
+  public GrRemoveModifierFix(@NotNull @GrModifier.GrModifierConstant String modifier) {
+    this(modifier, GroovyInspectionBundle.message("unnecessary.modifier.remove", modifier));
+  }
+
+  public GrRemoveModifierFix(@NotNull @GrModifier.GrModifierConstant String modifier, @NotNull String text) {
+    super(text, modifier, false, GrModifierFix.MODIFIER_LIST_CHILD);
   }
 
   @Override
index 64469f0740a42d9a278654919d2fdc1ef02f003d..5ca01d3186cdba1606b232f344b49fd7531e77dd 100644 (file)
@@ -2007,4 +2007,8 @@ def doParse() {
 def foo = new <error descr="Cannot resolve symbol 'Rrrrrrrr'">Rrrrrrrr</error>() {}
 '''
   }
+
+  void testLocalVariableModifiers() { doTest() }
+
+  void testFieldModifiers() { doTest() }
 }
diff --git a/plugins/groovy/testdata/highlighting/FieldModifiers.groovy b/plugins/groovy/testdata/highlighting/FieldModifiers.groovy
new file mode 100644 (file)
index 0000000..e68c9e3
--- /dev/null
@@ -0,0 +1,30 @@
+class Simple {
+  def a0 = 1
+  protected a1 = 1
+  private a2 = 1
+  static a3 = 1
+  abstract a4 = 1
+  final a5 = 1
+  native a6 = 1
+  synchronized a7 = 1
+  strictfp a8 = 1
+  transient a9 = 1
+  volatile a10 = 1
+}
+
+class Combinations {
+  <error descr="Illegal combination of modifiers">public private</error> a
+  <error descr="Illegal combination of modifiers">private protected</error> b
+  <error descr="Illegal combination of modifiers">protected public</error> c
+  <error descr="Illegal combination of modifiers 'volatile' and 'final'">volatile final</error> g
+}
+
+class Duplicates {
+  <error descr="Duplicate modifier 'public'">public public</error> a
+}
+
+interface I {
+  <error descr="Interface members are not allowed to be private">private</error> a
+  public b
+  <error descr="Interface members are not allowed to be protected">protected</error> c
+}
diff --git a/plugins/groovy/testdata/highlighting/LocalVariableModifiers.groovy b/plugins/groovy/testdata/highlighting/LocalVariableModifiers.groovy
new file mode 100644 (file)
index 0000000..6c39fbf
--- /dev/null
@@ -0,0 +1,13 @@
+def foo() {
+  def a0 = 1
+  <error descr="Variable cannot have modifier 'protected'">protected</error> a1 = 1
+  <error descr="Variable cannot have modifier 'private'">private</error> a2 = 1
+  <error descr="Variable cannot have modifier 'static'">static</error> a3 = 1
+  <error descr="Variable cannot have modifier 'abstract'">abstract</error> a4 = 1
+  final a5 = 1
+  <error descr="Variable cannot have modifier 'native'">native</error> a6 = 1
+  <error descr="Variable cannot have modifier 'synchronized'">synchronized</error> a7 = 1
+  <error descr="Variable cannot have modifier 'strictfp'">strictfp</error> a8 = 1
+  <error descr="Variable cannot have modifier 'transient'">transient</error> a9 = 1
+  <error descr="Variable cannot have modifier 'volatile'">volatile</error> a10 = 1
+}