From 2dbb1bca33c39ff2fd6a2ab501076c276aeb451d Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Thu, 27 Jun 2013 21:28:01 +0200 Subject: [PATCH] IDEA-100664 (Small false positive in "class may be static" inspection for inner classes extending enclosing class) --- .../InnerClassMayBeStaticInspection.java | 35 +++---- .../InnerClassReferenceVisitor.java | 91 +++++++------------ .../InnerClassMayBeStaticInspection.java | 10 ++ 3 files changed, 56 insertions(+), 80 deletions(-) diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/InnerClassMayBeStaticInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/InnerClassMayBeStaticInspection.java index b175055b51f6..aac5783f0eb3 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/InnerClassMayBeStaticInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/InnerClassMayBeStaticInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2011 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2013 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,7 +20,6 @@ import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.search.SearchScope; import com.intellij.psi.search.searches.ReferencesSearch; -import com.intellij.util.IncorrectOperationException; import com.intellij.util.Query; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; @@ -35,15 +34,13 @@ public class InnerClassMayBeStaticInspection extends BaseInspection { @Override @NotNull public String getDisplayName() { - return InspectionGadgetsBundle.message( - "inner.class.may.be.static.display.name"); + return InspectionGadgetsBundle.message("inner.class.may.be.static.display.name"); } @Override @NotNull protected String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message( - "inner.class.may.be.static.problem.descriptor"); + return InspectionGadgetsBundle.message("inner.class.may.be.static.problem.descriptor"); } @Override @@ -65,15 +62,12 @@ public class InnerClassMayBeStaticInspection extends BaseInspection { } @Override - public void doFix(Project project, ProblemDescriptor descriptor) - throws IncorrectOperationException { - final PsiJavaToken classNameToken = - (PsiJavaToken)descriptor.getPsiElement(); + public void doFix(Project project, ProblemDescriptor descriptor) { + final PsiJavaToken classNameToken = (PsiJavaToken)descriptor.getPsiElement(); final PsiClass innerClass = (PsiClass)classNameToken.getParent(); assert innerClass != null; final SearchScope useScope = innerClass.getUseScope(); - final Query query = - ReferencesSearch.search(innerClass, useScope); + final Query query = ReferencesSearch.search(innerClass, useScope); final Collection references = query.findAll(); for (final PsiReference reference : references) { final PsiElement element = reference.getElement(); @@ -81,10 +75,8 @@ public class InnerClassMayBeStaticInspection extends BaseInspection { if (!(parent instanceof PsiNewExpression)) { continue; } - final PsiNewExpression newExpression = - (PsiNewExpression)parent; - final PsiExpression qualifier = - newExpression.getQualifier(); + final PsiNewExpression newExpression = (PsiNewExpression)parent; + final PsiExpression qualifier = newExpression.getQualifier(); if (qualifier == null) { continue; } @@ -103,15 +95,11 @@ public class InnerClassMayBeStaticInspection extends BaseInspection { return new InnerClassMayBeStaticVisitor(); } - private static class InnerClassMayBeStaticVisitor - extends BaseInspectionVisitor { + private static class InnerClassMayBeStaticVisitor extends BaseInspectionVisitor { @Override public void visitClass(@NotNull PsiClass aClass) { - // no call to super, so that it doesn't drill down to inner classes - if (aClass.getContainingClass() != null && - !aClass.hasModifierProperty(PsiModifier.STATIC)) { - // inner class cannot have static declarations + if (aClass.getContainingClass() != null && !aClass.hasModifierProperty(PsiModifier.STATIC)) { return; } if (aClass instanceof PsiAnonymousClass) { @@ -122,8 +110,7 @@ public class InnerClassMayBeStaticInspection extends BaseInspection { if (innerClass.hasModifierProperty(PsiModifier.STATIC)) { continue; } - final InnerClassReferenceVisitor visitor = - new InnerClassReferenceVisitor(innerClass); + final InnerClassReferenceVisitor visitor = new InnerClassReferenceVisitor(innerClass); innerClass.accept(visitor); if (!visitor.canInnerClassBeStatic()) { continue; diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/InnerClassReferenceVisitor.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/InnerClassReferenceVisitor.java index 7a515d0baf98..46aca8e6b3b8 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/InnerClassReferenceVisitor.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/InnerClassReferenceVisitor.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2011 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2013 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,8 +35,7 @@ public class InnerClassReferenceVisitor extends JavaRecursiveElementVisitor { } private boolean isClassStaticallyAccessible(PsiClass aClass) { - if (aClass.getContainingClass() != null && - aClass.hasModifierProperty(PsiModifier.STATIC)) { + if (aClass.getContainingClass() != null && aClass.hasModifierProperty(PsiModifier.STATIC)) { if (!PsiTreeUtil.isAncestor(aClass, innerClass, false)) { return true; } @@ -45,11 +44,9 @@ public class InnerClassReferenceVisitor extends JavaRecursiveElementVisitor { return true; } PsiClass classScope = aClass; - final PsiClass outerClass = - ClassUtils.getContainingClass(innerClass); + final PsiClass outerClass = ClassUtils.getContainingClass(innerClass); while (classScope != null) { - if (InheritanceUtil.isInheritorOrSelf(outerClass, classScope, - true)) { + if (InheritanceUtil.isInheritorOrSelf(outerClass, classScope, true)) { return false; } final PsiElement scope = classScope.getScope(); @@ -64,33 +61,29 @@ public class InnerClassReferenceVisitor extends JavaRecursiveElementVisitor { } @Override - public void visitThisExpression( - @NotNull PsiThisExpression expression) { + public void visitThisExpression(@NotNull PsiThisExpression expression) { if (!referencesStaticallyAccessible) { return; } super.visitThisExpression(expression); - final PsiJavaCodeReferenceElement qualifier = expression.getQualifier(); - if (isContainingClassQualifier(qualifier)) { + if (hasContainingClassQualifier(expression)) { referencesStaticallyAccessible = false; } } @Override - public void visitSuperExpression( - @NotNull PsiSuperExpression expression) { + public void visitSuperExpression(@NotNull PsiSuperExpression expression) { if (!referencesStaticallyAccessible) { return; } super.visitSuperExpression(expression); - final PsiJavaCodeReferenceElement qualifier = expression.getQualifier(); - if (isContainingClassQualifier(qualifier)) { + if (hasContainingClassQualifier(expression)) { referencesStaticallyAccessible = false; } } - private boolean isContainingClassQualifier( - PsiJavaCodeReferenceElement qualifier) { + private boolean hasContainingClassQualifier(PsiQualifiedExpression expression) { + final PsiJavaCodeReferenceElement qualifier = expression.getQualifier(); if (qualifier == null) { return false; } @@ -103,72 +96,58 @@ public class InnerClassReferenceVisitor extends JavaRecursiveElementVisitor { } @Override - public void visitReferenceElement( - @NotNull PsiJavaCodeReferenceElement reference) { + public void visitReferenceElement(@NotNull PsiJavaCodeReferenceElement reference) { if (!referencesStaticallyAccessible) { return; } final PsiElement parent = reference.getParent(); - if (parent instanceof PsiThisExpression || - parent instanceof PsiSuperExpression) { + if (parent instanceof PsiThisExpression || parent instanceof PsiSuperExpression) { return; } super.visitReferenceElement(reference); - final PsiElement element = reference.resolve(); - if (!(element instanceof PsiClass)) { - return; - } - final PsiClass aClass = (PsiClass)element; - final PsiElement scope = aClass.getScope(); - if (!(scope instanceof PsiClass)) { - return; - } - referencesStaticallyAccessible &= - aClass.hasModifierProperty(PsiModifier.STATIC); - } - @Override - public void visitReferenceExpression( - @NotNull PsiReferenceExpression expression) { - if (!referencesStaticallyAccessible) { - return; - } - super.visitReferenceExpression(expression); - final PsiExpression qualifier = - expression.getQualifierExpression(); + final PsiElement qualifier = reference.getQualifier(); if (qualifier instanceof PsiSuperExpression) { return; } if (qualifier instanceof PsiReferenceExpression) { - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)qualifier; + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifier; final PsiElement resolvedExpression = referenceExpression.resolve(); - if (!(resolvedExpression instanceof PsiField) && - !(resolvedExpression instanceof PsiMethod)) { + if (!(resolvedExpression instanceof PsiField) && !(resolvedExpression instanceof PsiMethod)) { return; } } - final PsiElement element = expression.resolve(); + final PsiElement element = reference.resolve(); if (element instanceof PsiMethod || element instanceof PsiField) { final PsiMember member = (PsiMember)element; if (member.hasModifierProperty(PsiModifier.STATIC)) { return; } final PsiClass containingClass = member.getContainingClass(); - referencesStaticallyAccessible &= - isClassStaticallyAccessible(containingClass); + if (innerClass.equals(containingClass)) { + return; + } + if (member.hasModifierProperty(PsiModifier.PRIVATE)) { + referencesStaticallyAccessible = false; + return; + } + referencesStaticallyAccessible &= isClassStaticallyAccessible(containingClass); } - if (element instanceof PsiLocalVariable || - element instanceof PsiParameter) { - final PsiElement containingMethod = - PsiTreeUtil.getParentOfType(expression, - PsiMethod.class); - final PsiElement referencedMethod = - PsiTreeUtil.getParentOfType(element, PsiMethod.class); + else if (element instanceof PsiLocalVariable || element instanceof PsiParameter) { + final PsiElement containingMethod = PsiTreeUtil.getParentOfType(reference, PsiMethod.class); + final PsiElement referencedMethod = PsiTreeUtil.getParentOfType(element, PsiMethod.class); if (containingMethod != null && referencedMethod != null && !containingMethod.equals(referencedMethod)) { referencesStaticallyAccessible = false; } } + else if ((element instanceof PsiClass)) { + final PsiClass aClass = (PsiClass)element; + final PsiElement scope = aClass.getScope(); + if (!(scope instanceof PsiClass)) { + return; + } + referencesStaticallyAccessible &= aClass.hasModifierProperty(PsiModifier.STATIC); + } } } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/inner_class_may_be_static/InnerClassMayBeStaticInspection.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/inner_class_may_be_static/InnerClassMayBeStaticInspection.java index aae6f943217a..7b14d866331f 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/inner_class_may_be_static/InnerClassMayBeStaticInspection.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/inner_class_may_be_static/InnerClassMayBeStaticInspection.java @@ -46,4 +46,14 @@ class D { class Y {} } } +} +class StaticInnerClass { + + private int foo; + + public class Baz extends StaticInnerClass { + Baz() { + foo = -1; + } + } } \ No newline at end of file