diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties index 8f75274cd334..90f9afefe558 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties +++ b/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} diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/bugs/NewInstanceOfSingletonInspection.java b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/bugs/NewInstanceOfSingletonInspection.java index 08d42cb395f6..8790958809c6 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/bugs/NewInstanceOfSingletonInspection.java +++ b/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); + } } } diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/transformations/singleton/SingletonTransformationSupport.kt b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/transformations/singleton/SingletonTransformationSupport.kt index 6f2dda9e6b9a..a0329ea686e4 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/transformations/singleton/SingletonTransformationSupport.kt +++ b/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 { diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/transformations/singleton/impl.kt b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/transformations/singleton/impl.kt index c38b6cf3279f..171c548a927a 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/transformations/singleton/impl.kt +++ b/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 diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/transformations/singleton/SingletonNewInstanceInspectionTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/transformations/singleton/SingletonNewInstanceInspectionTest.groovy new file mode 100644 index 000000000000..8149ebc01b9e --- /dev/null +++ b/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 A() +''', '''\ +@Singleton +class A {} + +A.instance +''' + } + + void 'test fix custom property name'() { + doTest '''\ +@Singleton(property = "coolInstance") +class A {} + +new A() +''', '''\ +@Singleton(property = "coolInstance") +class A {} + +A.coolInstance +''' + } +}