[groovy] @Singleton: add constructor inspection

with ability to remove inappropriate constructor or mark @Singleton as non-strict
2026-09-27 10:03:11 +07:00 · 2016-11-08 19:58:30 +03:00
parent fac5e5d6de
commit 7470920b2b
8 changed files with 291 additions and 1 deletions
@@ -0,0 +1,5 @@
+<html>
+<body>
+Checks that classes annotated by <code>@Singleton</code> do not have constructors unless it is declared non strict.
+</body>
+</html>
@@ -0,0 +1,53 @@
+/*
+ * 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.codeInspection.fixes;
+
+import com.intellij.codeInspection.LocalQuickFix;
+import com.intellij.codeInspection.ProblemDescriptor;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.PsiElement;
+import com.intellij.util.Function;
+import org.jetbrains.annotations.Nls;
+import org.jetbrains.annotations.NotNull;
+
+public class RemoveElementQuickFix implements LocalQuickFix {
+
+  private final String myName;
+  private final Function<PsiElement, PsiElement> myElementFunction;
+
+  public RemoveElementQuickFix(@NotNull String name, @NotNull Function<PsiElement, PsiElement> function) {
+    myName = name;
+    myElementFunction = function;
+  }
+
+  @Nls
+  @NotNull
+  @Override
+  public String getFamilyName() {
+    return myName;
+  }
+
+  @Override
+  public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
+    PsiElement descriptorElement = descriptor.getPsiElement();
+    if (descriptorElement == null) return;
+
+    PsiElement elementToRemove = myElementFunction.fun(descriptorElement);
+    if (elementToRemove == null) return;
+
+    elementToRemove.delete();
+  }
+}
@@ -0,0 +1,5 @@
+# @Singleton
+singleton.constructor.inspection=@Singleton constructors
+singleton.constructor.found=@Singleton class should not have constructors
+singleton.constructor.remove=Remove constructor
+singleton.constructor.makeNonStrict=Make @Singleton non-strict
@@ -0,0 +1,45 @@
+/*
+ * 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.codeInsight.AnnotationUtil
+import com.intellij.codeInspection.LocalQuickFix
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.openapi.project.Project
+import org.jetbrains.annotations.Nls
+import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElementFactory
+import org.jetbrains.plugins.groovy.transformations.message
+
+internal class MakeNonStrictQuickFix : LocalQuickFix {
+
+  @Nls
+  override fun getFamilyName() = message("singleton.constructor.makeNonStrict")
+
+  override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+    val annotation = getAnnotation(descriptor.psiElement) ?: return
+    val existingValue = AnnotationUtil.findDeclaredAttribute(annotation, "strict")
+    val newValue = GroovyPsiElementFactory.getInstance(project)
+      .createAnnotationFromText("@A(strict=false)")
+      .parameterList
+      .attributes[0]
+    if (existingValue == null) {
+      annotation.parameterList.add(newValue)
+    }
+    else {
+      existingValue.replace(newValue)
+    }
+  }
+}
@@ -0,0 +1,49 @@
+/*
+ * 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.codeInspection.LocalInspectionTool
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.PsiElement
+import com.intellij.psi.PsiElementVisitor
+import org.jetbrains.plugins.groovy.codeInspection.fixes.RemoveElementQuickFix
+import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes
+import org.jetbrains.plugins.groovy.lang.psi.impl.booleanValue
+import org.jetbrains.plugins.groovy.lang.psi.impl.findDeclaredDetachedValue
+import org.jetbrains.plugins.groovy.transformations.message
+
+class SingletonConstructorInspection : LocalInspectionTool() {
+
+  override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) = object : PsiElementVisitor() {
+
+    override fun visitElement(element: PsiElement) {
+      if (element.node.elementType !== GroovyTokenTypes.mIDENT) return
+      val annotation = getAnnotation(element) ?: return
+      val strict = annotation.findDeclaredDetachedValue("strict").booleanValue() ?: true
+      if (!strict) return
+
+      holder.registerProblem(
+        element,
+        message("singleton.constructor.found"),
+        ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+        RemoveElementQuickFix(message("singleton.constructor.remove")) { e -> e.parent },
+        MakeNonStrictQuickFix()
+      )
+    }
+  }
+}
@@ -15,7 +15,19 @@
 */
 package org.jetbrains.plugins.groovy.transformations.singleton

+import com.intellij.codeInsight.AnnotationUtil
+import com.intellij.psi.PsiElement
+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.util.GroovyCommonClassNames

 internal val singletonFqn = GroovyCommonClassNames.GROOVY_LANG_SINGLETON
-internal val singletonOriginInfo = "by @Singleton"
+internal val singletonOriginInfo = "by @Singleton"
+
+internal fun getAnnotation(identifier: PsiElement?): GrAnnotation? {
+  val parent = identifier?.parent as? GrMethod ?: return null
+  if (!parent.isConstructor) return null
+  val clazz = parent.containingClass as? GrTypeDefinition ?: return null
+  return AnnotationUtil.findAnnotation(clazz, singletonFqn) as? GrAnnotation
+}
@@ -1148,6 +1148,10 @@
                     enabledByDefault="true" level="WEAK WARNING"
                     implementationClass="org.jetbrains.plugins.groovy.codeInspection.untypedUnresolvedAccess.GrUnresolvedAccessInspection"/>

+    <localInspection language="Groovy" groupPath="Groovy" groupName="Annotations verifying"
+                     bundle="org.jetbrains.plugins.groovy.transformations.GroovyTransformationsBundle"
+                     key="singleton.constructor.inspection" enabledByDefault="true" level="ERROR"
+                     implementationClass="org.jetbrains.plugins.groovy.transformations.singleton.SingletonConstructorInspection"/>
    <localInspection language="Groovy" groupPath="Groovy" shortName="DelegatesTo" displayName="@DelegatesTo inspection"
                     groupName="Annotations verifying" enabledByDefault="true" level="WARNING"
                     implementationClass="org.jetbrains.plugins.groovy.codeInspection.confusing.DelegatesToInspection"/>
@@ -0,0 +1,117 @@
+/*
+ * 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 groovy.transform.CompileStatic
+import org.jetbrains.plugins.groovy.GroovyLightProjectDescriptor
+import org.jetbrains.plugins.groovy.LightGroovyTestCase
+import org.jetbrains.plugins.groovy.transformations.GroovyTransformationsBundle
+
+@CompileStatic
+class SingletonConstructorInspectionTest extends LightGroovyTestCase {
+
+  final LightProjectDescriptor projectDescriptor = GroovyLightProjectDescriptor.GROOVY_LATEST
+
+  void setUp() {
+    super.setUp()
+    fixture.enableInspections(SingletonConstructorInspection)
+  }
+
+  void 'test highlighting'() {
+    fixture.with {
+      configureByText '_.groovy', '''\
+@Singleton
+class A {
+    <error descr="@Singleton class should not have constructors">A</error>() {}
+    <error descr="@Singleton class should not have constructors">A</error>(a) {}
+}
+
+@Singleton(strict=true)
+class ExplicitStrict {
+  <error descr="@Singleton class should not have constructors">ExplicitStrict</error>() {}
+  <error descr="@Singleton class should not have constructors">ExplicitStrict</error>(a) {}
+}
+
+@Singleton(strict = false)
+class NonStrict {
+  NonStrict() {}
+  NonStrict(a) {}
+}
+'''
+      checkHighlighting()
+    }
+  }
+
+  void 'test make non strict fix'() {
+    fixture.with {
+      configureByText '_.groovy', '''\
+@Singleton
+class A {
+  <caret>A() {}
+}
+'''
+      def intention = findSingleIntention(GroovyTransformationsBundle.message("singleton.constructor.makeNonStrict"))
+      assert intention
+      launchAction(intention)
+      checkResult '''\
+@Singleton(strict = false)
+class A {
+  <caret>A() {}
+}
+'''
+    }
+  }
+
+  void 'test make non strict fix existing'() {
+    fixture.with {
+      configureByText '_.groovy', '''\
+@Singleton(strict = true, property = "lol")
+class A {
+  <caret>A() {}
+}
+'''
+      def intention = findSingleIntention(GroovyTransformationsBundle.message("singleton.constructor.makeNonStrict"))
+      assert intention
+      launchAction(intention)
+      checkResult '''\
+@Singleton(strict = false, property = "lol")
+class A {
+  <caret>A() {}
+}
+'''
+    }
+  }
+
+  void 'test remove constructor fix'() {
+    fixture.with {
+      configureByText '_.groovy', '''\
+@Singleton
+class A {
+  <caret>A() {}
+}
+'''
+      def intention = findSingleIntention(GroovyTransformationsBundle.message("singleton.constructor.remove"))
+      assert intention
+      launchAction(intention)
+      checkResult '''\
+@Singleton
+class A {
+}
+'''
+    }
+  }
+}