From 678ec7a127c6bfdf8660f36c535b5fd00411e4e6 Mon Sep 17 00:00:00 2001 From: Daniil Ovchinnikov Date: Tue, 29 Mar 2016 13:03:14 +0300 Subject: [PATCH] [groovy] method may be static: add option to ignore trait methods, ignore methods with explicit abstract modifier --- .../GroovyInspectionBundle.properties | 1 + .../GrMethodMayBeStaticInspection.java | 26 +++++++----- .../GrMethodMayBeStaticTest.groovy | 42 +++++++++++++++++-- 3 files changed, 55 insertions(+), 14 deletions(-) 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 a1f027de9947..c3c1d80d15a6 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 @@ -101,6 +101,7 @@ anonymous.class=anonymous class closure=closure other.scope=Other scope method.may.be.static=Method may be static +method.may.be.static.option.ignore.trait.methods=Ignore trait methods method.may.be.static.only.private.or.final.option=Only check final or private methods method.may.be.static.ignore.empty.method.option=Ignore empty methods have.instance.refs.in.closure=(have instance references inside closure) diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/declaration/GrMethodMayBeStaticInspection.java b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/declaration/GrMethodMayBeStaticInspection.java index 386c83019180..53f0cc87f7d5 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/declaration/GrMethodMayBeStaticInspection.java +++ b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/declaration/GrMethodMayBeStaticInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2014 JetBrains s.r.o. + * 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. @@ -28,7 +28,6 @@ import com.intellij.util.Function; 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.GroovyInspectionBundle; import org.jetbrains.plugins.groovy.codeInspection.bugs.GrModifierFix; import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement; import org.jetbrains.plugins.groovy.lang.psi.GroovyRecursiveElementVisitor; @@ -37,24 +36,27 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrOpenBlock; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.params.GrParameter; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTraitTypeDefinition; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrGdkMethod; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod; import org.jetbrains.plugins.groovy.lang.psi.api.types.GrCodeReferenceElement; -import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GroovyScriptClass; import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; import javax.swing.*; -@SuppressWarnings("JavaStylePropertiesInvocation") +import static org.jetbrains.plugins.groovy.codeInspection.GroovyInspectionBundle.message; + public class GrMethodMayBeStaticInspection extends BaseInspection { + public boolean myIgnoreTraitMethods = true; public boolean myOnlyPrivateOrFinal = false; public boolean myIgnoreEmptyMethods = true; @Override public JComponent createOptionsPanel() { final MultipleCheckboxOptionsPanel optionsPanel = new MultipleCheckboxOptionsPanel(this); - optionsPanel.addCheckbox(GroovyInspectionBundle.message("method.may.be.static.only.private.or.final.option"), "myOnlyPrivateOrFinal"); - optionsPanel.addCheckbox(GroovyInspectionBundle.message("method.may.be.static.ignore.empty.method.option"), "myIgnoreEmptyMethods"); + optionsPanel.addCheckbox(message("method.may.be.static.option.ignore.trait.methods"), "myIgnoreTraitMethods"); + optionsPanel.addCheckbox(message("method.may.be.static.only.private.or.final.option"), "myOnlyPrivateOrFinal"); + optionsPanel.addCheckbox(message("method.may.be.static.ignore.empty.method.option"), "myIgnoreEmptyMethods"); return optionsPanel; } @@ -74,7 +76,7 @@ public class GrMethodMayBeStaticInspection extends BaseInspection { return ((GrMethod)parent).getModifierList(); } }); - registerError(method.getNameIdentifierGroovy(), GroovyInspectionBundle.message("method.may.be.static"), new LocalQuickFix[]{modifierFix}, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); + registerError(method.getNameIdentifierGroovy(), message("method.may.be.static"), new LocalQuickFix[]{modifierFix}, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); } } }; @@ -83,8 +85,13 @@ public class GrMethodMayBeStaticInspection extends BaseInspection { private boolean checkMethod(final GrMethod method) { if (method.hasModifierProperty(PsiModifier.STATIC)) return false; if (method.hasModifierProperty(PsiModifier.SYNCHRONIZED)) return false; + if (method.getModifierList().hasExplicitModifier(PsiModifier.ABSTRACT)) return false; if (method.isConstructor()) return false; - if (method.getContainingClass() instanceof GroovyScriptClass) return false; + + PsiClass containingClass = method.getContainingClass(); + if (containingClass == null) return false; + + if (myIgnoreTraitMethods && containingClass instanceof GrTraitTypeDefinition) return false; if (SuperMethodsSearch.search(method, null, true, false).findFirst() != null) return false; if (OverridingMethodsSearch.search(method).findFirst() != null) return false; if (ignoreMethod(method)) return false; @@ -97,8 +104,6 @@ public class GrMethodMayBeStaticInspection extends BaseInspection { if (block == null) return false; if (myIgnoreEmptyMethods && block.getStatements().length == 0) return false; - PsiClass containingClass = method.getContainingClass(); - if (containingClass == null) return false; if (containingClass.getContainingClass() != null && !containingClass.hasModifierProperty(PsiModifier.STATIC)) { return false; } @@ -133,7 +138,6 @@ public class GrMethodMayBeStaticInspection extends BaseInspection { private static boolean isPrintOrPrintln(PsiElement element) { return element instanceof GrGdkMethod && - element instanceof PsiMethod && ("print".equals(((PsiMethod)element).getName()) || "println".equals(((PsiMethod)element).getName())); } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/inspections/GrMethodMayBeStaticTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/inspections/GrMethodMayBeStaticTest.groovy index 86111c721232..d295e957ec98 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/inspections/GrMethodMayBeStaticTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/inspections/GrMethodMayBeStaticTest.groovy @@ -1,5 +1,5 @@ /* - * Copyright 2000-2012 JetBrains s.r.o. + * 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. @@ -15,13 +15,21 @@ */ package org.jetbrains.plugins.groovy.inspections +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.codeInspection.declaration.GrMethodMayBeStaticInspection + /** * @author Max Medvedev */ +@CompileStatic public class GrMethodMayBeStaticTest extends LightGroovyTestCase { + final String basePath = null + final LightProjectDescriptor projectDescriptor = GroovyLightProjectDescriptor.GROOVY_2_3_9 + final GrMethodMayBeStaticInspection inspection = new GrMethodMayBeStaticInspection() void testSimple() { doTest('''\ @@ -68,10 +76,19 @@ class A { ''') } + void 'test abstract method with code block no error'() { + doTest ''' +abstract class A { + abstract foo() { + 1 + 2 + } +} +''' + } + private void doTest(final String text) { myFixture.configureByText('_.groovy', text) - - myFixture.enableInspections(GrMethodMayBeStaticInspection) + myFixture.enableInspections(inspection) myFixture.checkHighlighting(true, false, false) } @@ -141,4 +158,23 @@ class Bar { } ''') } + + void 'test trait methods'() { + doTest '''\ +trait A { + def foo() {1} + abstract bar() +} +''' + } + + void 'test trait methods with'() { + inspection.myIgnoreTraitMethods = false + doTest '''\ +trait A { + def foo() {1} + abstract bar() +} +''' + } }