[groovy] @Singleton: make new instance inspection aware of property name

plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties

@@ -80,7 +80,7 @@ check.labeled.statement=Labeled statement inspection
 unnecessary.qualified.reference=Unnecessary qualified reference
 rtype.cannot.contain.ltype=''{1}'' cannot contain ''{0}''
 new.instance.of.singleton=New instance of class annotated with @groovy.lang.Singleton
-replace.new.expression.with.0.instance=Replace with ''{0}.instance''
+replace.new.expression.with.instance.access=Replace with instance access
 getter.0.clashes.with.getter.1={0} clashes with {1}
 unused.0=Unused {0}
 remove.0=Remove {0}

plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/bugs/NewInstanceOfSingletonInspection.java

@@ -15,31 +15,30 @@
  */
 package org.jetbrains.plugins.groovy.codeInspection.bugs;
 
+import com.intellij.codeInsight.AnnotationUtil;
 import com.intellij.codeInspection.ProblemDescriptor;
-import com.intellij.openapi.diagnostic.Logger;
+import com.intellij.codeInspection.ProblemHighlightType;
 import com.intellij.openapi.project.Project;
+import com.intellij.psi.PsiAnnotation;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.codeStyle.JavaCodeStyleManager;
 import com.intellij.util.IncorrectOperationException;
-import org.jetbrains.annotations.Nls;
+import com.intellij.util.containers.ContainerUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.plugins.groovy.codeInspection.BaseInspection;
 import org.jetbrains.plugins.groovy.codeInspection.BaseInspectionVisitor;
 import org.jetbrains.plugins.groovy.codeInspection.GroovyFix;
 import org.jetbrains.plugins.groovy.codeInspection.GroovyInspectionBundle;
-import org.jetbrains.plugins.groovy.dsl.psi.PsiClassCategory;
 import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElementFactory;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrNewExpression;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition;
 import org.jetbrains.plugins.groovy.lang.psi.api.types.GrCodeReferenceElement;
-import org.jetbrains.plugins.groovy.lang.psi.util.GroovyCommonClassNames;
 
-/**
- * @author Max Medvedev
- */
+import static org.jetbrains.plugins.groovy.lang.psi.util.GroovyCommonClassNames.GROOVY_LANG_SINGLETON;
+import static org.jetbrains.plugins.groovy.transformations.singleton.ImplKt.getPropertyName;
+
 public class NewInstanceOfSingletonInspection extends BaseInspection {
-  private static final Logger LOG = Logger.getInstance(NewInstanceOfSingletonInspection.class);
 
   @NotNull
   @Override
@@ -47,67 +46,61 @@ public class NewInstanceOfSingletonInspection extends BaseInspection {
     return new BaseInspectionVisitor() {
       @Override
       public void visitNewExpression(@NotNull GrNewExpression newExpression) {
-        super.visitNewExpression(newExpression);
-
-        final GrCodeReferenceElement refElement = newExpression.getReferenceElement();
-        if (refElement == null) return;
         if (newExpression.getArrayDeclaration() != null) return;
 
-        final PsiElement resolved = refElement.resolve();
-        if (resolved instanceof GrTypeDefinition &&
-            PsiClassCategory.hasAnnotation((GrTypeDefinition)resolved, GroovyCommonClassNames.GROOVY_LANG_SINGLETON)) {
-          registerError(newExpression, GroovyInspectionBundle.message("new.instance.of.singleton"));
-        }
+        GrCodeReferenceElement refElement = newExpression.getReferenceElement();
+        if (refElement == null) return;
+
+        PsiElement resolved = refElement.resolve();
+        if (!(resolved instanceof GrTypeDefinition)) return;
+
+        PsiAnnotation annotation = AnnotationUtil.findAnnotation((GrTypeDefinition)resolved, GROOVY_LANG_SINGLETON);
+        if (annotation == null) return;
+
+        registerError(
+          newExpression,
+          GroovyInspectionBundle.message("new.instance.of.singleton"),
+          ContainerUtil.ar(new ReplaceWithInstanceAccessFix()),
+          ProblemHighlightType.GENERIC_ERROR_OR_WARNING
+        );
       }
     };
   }
 
-  @Override
-  public boolean isEnabledByDefault() {
-    return true;
-  }
+  private static class ReplaceWithInstanceAccessFix extends GroovyFix {
 
-  @Override
-  protected GroovyFix buildFix(@NotNull final PsiElement location) {
-    final GrCodeReferenceElement refElement = ((GrNewExpression)location).getReferenceElement();
-    LOG.assertTrue(refElement != null);
-    final GrTypeDefinition singleton = (GrTypeDefinition)refElement.resolve();
-    LOG.assertTrue(singleton != null);
+    @NotNull
+    @Override
+    public String getFamilyName() {
+      return GroovyInspectionBundle.message("replace.new.expression.with.instance.access");
+    }
 
-    return new GroovyFix() {
-      @Override
-      protected void doFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) throws IncorrectOperationException {
-        final GrExpression instanceRef =
-          GroovyPsiElementFactory.getInstance(project).createExpressionFromText(singleton.getQualifiedName() + ".instance");
+    @Override
+    protected void doFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) throws IncorrectOperationException {
+      PsiElement element = descriptor.getPsiElement();
+      if (!(element instanceof GrNewExpression)) return;
 
-        final GrExpression replaced = ((GrNewExpression)location).replaceWithExpression(instanceRef, true);
-        JavaCodeStyleManager.getInstance(project).shortenClassReferences(replaced);
-      }
+      GrNewExpression newExpression = (GrNewExpression)element;
 
-      @NotNull
-      @Override
-      public String getName() {
-        return GroovyInspectionBundle.message("replace.new.expression.with.0.instance", singleton.getName());
-      }
-    };
-  }
+      GrCodeReferenceElement refElement = newExpression.getReferenceElement();
+      if (refElement == null) return;
 
-  @Nls
-  @NotNull
-  @Override
-  public String getGroupDisplayName() {
-    return CONFUSING_CODE_CONSTRUCTS;
-  }
+      PsiElement resolved = refElement.resolve();
+      if (!(resolved instanceof GrTypeDefinition)) return;
 
-  @Override
-  protected String buildErrorString(Object... args) {
-    return (String)args[0];
-  }
+      GrTypeDefinition singleton = (GrTypeDefinition)resolved;
 
-  @Nls
-  @NotNull
-  @Override
-  public String getDisplayName() {
-    return "New instance of class annotated with @groovy.lang.Singleton";
+      PsiAnnotation annotation = AnnotationUtil.findAnnotation(singleton, GROOVY_LANG_SINGLETON);
+      if (annotation == null) return;
+
+      String qualifiedName = singleton.getQualifiedName();
+      if (qualifiedName == null) return;
+
+      String propertyName = getPropertyName(annotation);
+      GrExpression instanceRef = GroovyPsiElementFactory.getInstance(project).createExpressionFromText(qualifiedName + "." + propertyName);
+
+      final GrExpression replaced = newExpression.replaceWithExpression(instanceRef, true);
+      JavaCodeStyleManager.getInstance(project).shortenClassReferences(replaced);
+    }
   }
 }

plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/transformations/singleton/SingletonTransformationSupport.kt

@@ -15,11 +15,9 @@
  */
 package org.jetbrains.plugins.groovy.transformations.singleton
 
-import com.intellij.util.text.nullize
 import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.GrModifierFlags.*
 import org.jetbrains.plugins.groovy.lang.psi.impl.booleanValue
 import org.jetbrains.plugins.groovy.lang.psi.impl.findDeclaredDetachedValue
-import org.jetbrains.plugins.groovy.lang.psi.impl.stringValue
 import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GrLightField
 import org.jetbrains.plugins.groovy.transformations.AstTransformationSupport
 import org.jetbrains.plugins.groovy.transformations.TransformationContext
@@ -29,7 +27,7 @@ class SingletonTransformationSupport : AstTransformationSupport {
 
   override fun applyTransformation(context: TransformationContext) {
     val annotation = context.getAnnotation(singletonFqn) ?: return
-    val name = annotation.findDeclaredDetachedValue("property").stringValue().nullize(true) ?: "instance"
+    val name = annotation.getPropertyName()
     val lazy = annotation.findDeclaredDetachedValue("lazy").booleanValue() ?: false
 
     context += GrLightField(context.codeClass, name, context.classType, annotation).apply {

plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/transformations/singleton/impl.kt

@@ -16,15 +16,21 @@
 package org.jetbrains.plugins.groovy.transformations.singleton
 
 import com.intellij.codeInsight.AnnotationUtil
+import com.intellij.psi.PsiAnnotation
 import com.intellij.psi.PsiElement
+import com.intellij.util.text.nullize
 import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.annotation.GrAnnotation
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod
+import org.jetbrains.plugins.groovy.lang.psi.impl.findDeclaredDetachedValue
+import org.jetbrains.plugins.groovy.lang.psi.impl.stringValue
 import org.jetbrains.plugins.groovy.lang.psi.util.GroovyCommonClassNames
 
 internal val singletonFqn = GroovyCommonClassNames.GROOVY_LANG_SINGLETON
 internal val singletonOriginInfo = "by @Singleton"
 
+fun PsiAnnotation.getPropertyName() = findDeclaredDetachedValue("property").stringValue().nullize(true) ?: "instance"
+
 internal fun getAnnotation(identifier: PsiElement?): GrAnnotation? {
   val parent = identifier?.parent as? GrMethod ?: return null
   if (!parent.isConstructor) return null

plugins/groovy/test/org/jetbrains/plugins/groovy/transformations/singleton/SingletonNewInstanceInspectionTest.groovy

@@ -0,0 +1,70 @@
+/*
+ * 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.transformations.singleton
+
+import com.intellij.testFramework.LightProjectDescriptor
+import org.jetbrains.plugins.groovy.GroovyLightProjectDescriptor
+import org.jetbrains.plugins.groovy.LightGroovyTestCase
+import org.jetbrains.plugins.groovy.codeInspection.GroovyInspectionBundle
+import org.jetbrains.plugins.groovy.codeInspection.bugs.NewInstanceOfSingletonInspection
+
+class SingletonNewInstanceInspectionTest extends LightGroovyTestCase {
+
+  final LightProjectDescriptor projectDescriptor = GroovyLightProjectDescriptor.GROOVY_LATEST
+
+  void setUp() {
+    super.setUp()
+    fixture.enableInspections NewInstanceOfSingletonInspection
+  }
+
+  private void doTest(String before, String after) {
+    fixture.with {
+      configureByText '_.groovy', before
+      def action = findSingleIntention(GroovyInspectionBundle.message("replace.new.expression.with.instance.access"))
+      assert action
+      launchAction action
+      checkResult after
+    }
+  }
+
+  void 'test fix simple'() {
+    doTest '''\
+@Singleton
+class A {}
+
+new <caret>A()
+''', '''\
+@Singleton
+class A {}
+
+A.instance
+'''
+  }
+
+  void 'test fix custom property name'() {
+    doTest '''\
+@Singleton(property = "coolInstance")
+class A {}
+
+new <caret>A()
+''', '''\
+@Singleton(property = "coolInstance")
+class A {}
+
+A.coolInstance
+'''
+  }
+}