diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java index 38c112b47e1b..a9647f8e6640 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java @@ -417,7 +417,7 @@ public class HighlightControlFlowUtil { if (codeBlockProblems == null) { try { final ControlFlow controlFlow = getControlFlow(topBlock); - codeBlockProblems = ControlFlowUtil.getReadBeforeWrite(controlFlow); + codeBlockProblems = ControlFlowUtil.getReadBeforeWriteLocals(controlFlow); } catch (AnalysisCanceledException e) { codeBlockProblems = Collections.emptyList(); @@ -570,7 +570,7 @@ public class HighlightControlFlowUtil { } @NotNull - private static Collection getFinalVariableProblemsInBlock(Map> finalVarProblems, PsiElement codeBlock) { + public static Collection getFinalVariableProblemsInBlock(Map> finalVarProblems, PsiElement codeBlock) { Collection codeBlockProblems = finalVarProblems.get(codeBlock); if (codeBlockProblems == null) { try { diff --git a/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/BaseConvertToLocalQuickFix.java b/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/BaseConvertToLocalQuickFix.java new file mode 100644 index 000000000000..4e6f592e2d53 --- /dev/null +++ b/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/BaseConvertToLocalQuickFix.java @@ -0,0 +1,236 @@ +/* + * Copyright 2000-2012 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 com.intellij.codeInspection.varScopeCanBeNarrowed; + +import com.intellij.codeInsight.CodeInsightUtil; +import com.intellij.codeInspection.InspectionsBundle; +import com.intellij.codeInspection.LocalQuickFix; +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.editor.Editor; +import com.intellij.openapi.editor.ScrollType; +import com.intellij.openapi.fileEditor.FileEditorManager; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.search.searches.ReferencesSearch; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiUtil; +import com.intellij.util.IJSwingUtilities; +import com.intellij.util.IncorrectOperationException; +import com.intellij.util.containers.HashSet; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; +import java.util.Set; + +/** + * refactored from {@link com.intellij.codeInspection.varScopeCanBeNarrowed.FieldCanBeLocalInspection.MyQuickFix} + * + * @author Danila Ponomarenko + */ +public abstract class BaseConvertToLocalQuickFix implements LocalQuickFix { + private static final Logger LOG = Logger.getInstance(BaseConvertToLocalQuickFix.class); + + @NotNull + public final String getName() { + return InspectionsBundle.message("inspection.convert.to.local.quickfix"); + } + + public final void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + final T myVariable = getVariable(descriptor); + final PsiFile myFile = myVariable.getContainingFile(); + if (myVariable == null || !myVariable.isValid()) return; //weird. should not get here when field becomes invalid + + try { + final PsiElement newDeclaration = moveDeclaration(project, myVariable); + if (newDeclaration == null) return; + + positionCaretToDeclaration(project, myFile, newDeclaration); + } + catch (IncorrectOperationException e) { + LOG.error(e); + } + } + + @Nullable + protected abstract T getVariable(@NotNull ProblemDescriptor descriptor); + + private static void positionCaretToDeclaration(@NotNull Project project, @NotNull PsiFile psiFile, @NotNull PsiElement declaration) { + final Editor editor = FileEditorManager.getInstance(project).getSelectedTextEditor(); + if (editor != null && IJSwingUtilities.hasFocus(editor.getComponent())) { + final PsiFile openedFile = PsiDocumentManager.getInstance(project).getPsiFile(editor.getDocument()); + if (openedFile == psiFile) { + editor.getCaretModel().moveToOffset(declaration.getTextOffset()); + editor.getScrollingModel().scrollToCaret(ScrollType.RELATIVE); + } + } + } + + @Nullable + private PsiElement moveDeclaration(@NotNull Project project, @NotNull T variable){ + final PsiElement newDeclaration = addDeclaration(project, variable); + if (newDeclaration == null) return null; + + beforeDelete(project,variable,newDeclaration); + + variable.normalizeDeclaration(); + variable.delete(); + + return newDeclaration; + } + + protected void beforeDelete(@NotNull Project project, @NotNull T variable, @NotNull PsiElement newDeclaration){} + + @Nullable + private PsiElement addDeclaration(@NotNull Project project, @NotNull T myVariable) { + final Collection refs = ReferencesSearch.search(myVariable).findAll(); + if (refs.isEmpty()) return null; + + final PsiCodeBlock anchorBlock = findAnchorBlock(refs); + if (anchorBlock == null) return null; //was assert, but need to fix the case when obsolete inspection highlighting is left + if (!CodeInsightUtil.preparePsiElementsForWrite(anchorBlock)) return null; + + final PsiElement firstElement = getLowestOffsetElement(refs); + final String localName = suggestLocalName(project, myVariable, anchorBlock); + + final PsiElement anchor = getAnchorElement(anchorBlock, firstElement); + + final PsiElementFactory elementFactory = JavaPsiFacade.getElementFactory(project); + + final PsiAssignmentExpression anchorAssignmentExpression = searchAssignmentExpression(anchor); + if (anchorAssignmentExpression != null && isVariableAssignment(anchorAssignmentExpression, myVariable)) { + final PsiExpression initializer = anchorAssignmentExpression.getRExpression(); + final PsiDeclarationStatement declaration = elementFactory.createVariableDeclarationStatement(localName, myVariable.getType(), initializer); + if (!mayBeFinal(firstElement, refs)) { + PsiUtil.setModifierProperty((PsiModifierListOwner)declaration.getDeclaredElements()[0], PsiModifier.FINAL, false); + } + final PsiElement newDeclaration = anchor.replace(declaration); + + final Set refsSet = new HashSet(refs); + refsSet.remove(anchorAssignmentExpression.getLExpression()); + retargetReferences(elementFactory, localName, refsSet); + return newDeclaration; + } + + final PsiDeclarationStatement declaration = elementFactory.createVariableDeclarationStatement(localName, myVariable.getType(), myVariable.getInitializer()); + final PsiElement newDeclaration = anchorBlock.addBefore(declaration, anchor); + + retargetReferences(elementFactory, localName, refs); + return newDeclaration; + } + + @Nullable + private static PsiAssignmentExpression searchAssignmentExpression(@NotNull PsiElement anchor) { + if (!(anchor instanceof PsiExpressionStatement)) { + return null; + } + + final PsiExpression anchorExpression = ((PsiExpressionStatement)anchor).getExpression(); + + if (!(anchorExpression instanceof PsiAssignmentExpression)) { + return null; + } + + return (PsiAssignmentExpression)anchorExpression; + } + + private static boolean isVariableAssignment(@NotNull PsiAssignmentExpression expression, @NotNull PsiVariable variable) { + if (expression.getOperationTokenType() != JavaTokenType.EQ) { + return false; + } + + if (!(expression.getLExpression() instanceof PsiReferenceExpression)) { + return false; + } + + final PsiReferenceExpression leftExpression = (PsiReferenceExpression)expression.getLExpression(); + + if (!leftExpression.isReferenceTo(variable)) { + return false; + } + + return true; + } + + @NotNull + protected abstract String suggestLocalName(@NotNull Project project, @NotNull T variable, @NotNull PsiCodeBlock scope); + + private static boolean mayBeFinal(PsiElement firstElement, @NotNull Collection references) { + for (PsiReference reference : references) { + final PsiElement element = reference.getElement(); + if (element == firstElement) continue; + if (element instanceof PsiExpression && PsiUtil.isAccessedForWriting((PsiExpression)element)) return false; + } + return true; + } + + private static void retargetReferences(PsiElementFactory elementFactory, String localName, Collection refs) + throws IncorrectOperationException { + final PsiReferenceExpression refExpr = (PsiReferenceExpression)elementFactory.createExpressionFromText(localName, null); + for (PsiReference ref : refs) { + if (ref instanceof PsiReferenceExpression) { + ((PsiReferenceExpression)ref).replace(refExpr); + } + } + } + + @NotNull + public String getFamilyName() { + return getName(); + } + + private static PsiElement getAnchorElement(PsiCodeBlock anchorBlock, @NotNull PsiElement firstElement) { + PsiElement element = firstElement; + while (element != null && element.getParent() != anchorBlock) { + element = element.getParent(); + } + return element; + } + + @Nullable + private static PsiElement getLowestOffsetElement(@NotNull Collection refs) { + PsiElement firstElement = null; + for (PsiReference reference : refs) { + final PsiElement element = reference.getElement(); + if (firstElement == null || firstElement.getTextRange().getStartOffset() > element.getTextRange().getStartOffset()) { + firstElement = element; + } + } + return firstElement; + } + + private static PsiCodeBlock findAnchorBlock(final Collection refs) { + PsiCodeBlock result = null; + for (PsiReference psiReference : refs) { + final PsiElement element = psiReference.getElement(); + PsiCodeBlock block = PsiTreeUtil.getParentOfType(element, PsiCodeBlock.class); + if (result == null || block == null) { + result = block; + } + else { + final PsiElement commonParent = PsiTreeUtil.findCommonParent(result, block); + result = PsiTreeUtil.getParentOfType(commonParent, PsiCodeBlock.class, false); + } + } + return result; + } + + + public boolean runForWholeFile() { + return true; + } +} diff --git a/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/FieldCanBeLocalInspection.java b/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/FieldCanBeLocalInspection.java index d2966bcdca3f..52c4135e682d 100644 --- a/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/FieldCanBeLocalInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/FieldCanBeLocalInspection.java @@ -65,8 +65,6 @@ import java.util.Set; * @author ven */ public class FieldCanBeLocalInspection extends BaseLocalInspectionTool { - private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.varScopeCanBeNarrowed.FieldCanBeLocalInspection"); - @NonNls public static final String SHORT_NAME = "FieldCanBeLocal"; public final JDOMExternalizableStringList EXCLUDE_ANNOS = new JDOMExternalizableStringList(); @@ -110,13 +108,13 @@ public class FieldCanBeLocalInspection extends BaseLocalInspectionTool { @Override public void visitJavaFile(PsiJavaFile file) { for (PsiClass aClass : file.getClasses()) { - docheckClass(aClass, holder, EXCLUDE_ANNOS); + doCheckClass(aClass, holder, EXCLUDE_ANNOS); } } }; } - private static void docheckClass(final PsiClass aClass, ProblemsHolder holder, final List excludeAnnos) { + private static void doCheckClass(final PsiClass aClass, ProblemsHolder holder, final List excludeAnnos) { if (aClass.isInterface()) return; final PsiField[] fields = aClass.getFields(); final Set candidates = new LinkedHashSet(); @@ -129,6 +127,7 @@ public class FieldCanBeLocalInspection extends BaseLocalInspectionTool { } } + removeFieldsReferencedFromInitializers(aClass, candidates); if (candidates.isEmpty()) return; @@ -141,7 +140,7 @@ public class FieldCanBeLocalInspection extends BaseLocalInspectionTool { for (PsiField field : candidates) { if (usedFields.contains(field) && !hasImplicitReadOrWriteUsage(field, implicitUsageProviders)) { final String message = InspectionsBundle.message("inspection.field.can.be.local.problem.descriptor"); - holder.registerProblem(field.getNameIdentifier(), message, new MyQuickFix()); + holder.registerProblem(field.getNameIdentifier(), message, new ConvertFieldToLocalQuickFix()); } } } @@ -189,7 +188,7 @@ public class FieldCanBeLocalInspection extends BaseLocalInspectionTool { final PsiElement resolved = readBeforeWrite.resolve(); if (resolved instanceof PsiField) { final PsiField field = (PsiField)resolved; - if (!isImmutableState(field.getType()) || !PsiUtil.isConstantExpression(field.getInitializer()) || getWrittenVariables(controlFlow, writtenVariables).contains(field)){ + if (!isImmutableState(field.getType()) || !PsiUtil.isConstantExpression(field.getInitializer()) || getWrittenVariables(controlFlow, writtenVariables).contains(field)) { PsiElement parent = body.getParent(); if (!(parent instanceof PsiMethod) || !((PsiMethod)parent).isConstructor() || @@ -222,15 +221,18 @@ public class FieldCanBeLocalInspection extends BaseLocalInspectionTool { private static void removeFieldsReferencedFromInitializers(final PsiClass aClass, final Set candidates) { aClass.accept(new JavaRecursiveElementWalkingVisitor() { - @Override public void visitMethod(PsiMethod method) { + @Override + public void visitMethod(PsiMethod method) { //do not go inside method } - @Override public void visitClassInitializer(PsiClassInitializer initializer) { + @Override + public void visitClassInitializer(PsiClassInitializer initializer) { //do not go inside class initializer } - @Override public void visitReferenceExpression(PsiReferenceExpression expression) { + @Override + public void visitReferenceExpression(PsiReferenceExpression expression) { final PsiElement resolved = expression.resolve(); if (resolved instanceof PsiField) { final PsiField field = (PsiField)resolved; @@ -245,7 +247,7 @@ public class FieldCanBeLocalInspection extends BaseLocalInspectionTool { } private static boolean hasImplicitReadOrWriteUsage(final PsiField field, ImplicitUsageProvider[] implicitUsageProviders) { - for(ImplicitUsageProvider provider: implicitUsageProviders) { + for (ImplicitUsageProvider provider : implicitUsageProviders) { if (provider.isImplicitRead(field) || provider.isImplicitWrite(field)) { return true; } @@ -253,173 +255,41 @@ public class FieldCanBeLocalInspection extends BaseLocalInspectionTool { return false; } - private static class MyQuickFix implements LocalQuickFix { - @NotNull - public String getName() { - return InspectionsBundle.message("inspection.field.can.be.local.quickfix"); + private static class ConvertFieldToLocalQuickFix extends BaseConvertToLocalQuickFix { + + @Override + @Nullable + protected PsiField getVariable(@NotNull ProblemDescriptor descriptor) { + return PsiTreeUtil.getParentOfType(descriptor.getPsiElement(), PsiField.class); } - public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { - PsiElement element = descriptor.getPsiElement(); - PsiField myField = PsiTreeUtil.getParentOfType(element, PsiField.class); - if (myField == null || !myField.isValid()) return; //weird. should not get here when field becomes invalid + @Override + protected void beforeDelete(@NotNull Project project, @NotNull PsiField variable, @NotNull PsiElement newDeclaration) { + final PsiDocComment docComment = variable.getDocComment(); + if (docComment != null) moveDocCommentToDeclaration(project, docComment, newDeclaration); + } - final PsiDocComment docComment = myField.getDocComment(); - final Collection refs = ReferencesSearch.search(myField).findAll(); - if (refs.isEmpty()) return; - Set refsSet = new HashSet(refs); - PsiCodeBlock anchorBlock = findAnchorBlock(refs); - if (anchorBlock == null) return; //was assert, but need to fix the case when obsolete inspection highlighting is left - if (!CodeInsightUtil.preparePsiElementsForWrite(anchorBlock)) return; - final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(project).getElementFactory(); + @NotNull + @Override + protected String suggestLocalName(@NotNull Project project, @NotNull PsiField field, @NotNull PsiCodeBlock scope) { final JavaCodeStyleManager styleManager = JavaCodeStyleManager.getInstance(project); - final String propertyName = styleManager.variableNameToPropertyName(myField.getName(), VariableKind.FIELD); - String localName = styleManager.propertyNameToVariableName(propertyName, VariableKind.LOCAL_VARIABLE); - localName = RefactoringUtil.suggestUniqueVariableName(localName, anchorBlock, myField); - PsiElement firstElement = getFirstElement(refs); - boolean mayBeFinal = mayBeFinal(refsSet, firstElement); - PsiElement newDeclaration = null; - try { - final PsiElement anchor = getAnchorElement(anchorBlock, firstElement); - if (anchor instanceof PsiExpressionStatement && - ((PsiExpressionStatement) anchor).getExpression() instanceof PsiAssignmentExpression) { - final PsiAssignmentExpression expression = (PsiAssignmentExpression) ((PsiExpressionStatement) anchor).getExpression(); - if (expression.getOperationTokenType() == JavaTokenType.EQ && - expression.getLExpression() instanceof PsiReferenceExpression && - ((PsiReference)expression.getLExpression()).isReferenceTo(myField)) { - final PsiExpression initializer = expression.getRExpression(); - final PsiDeclarationStatement decl = elementFactory.createVariableDeclarationStatement(localName, myField.getType(), initializer); - if (!mayBeFinal) { - PsiUtil.setModifierProperty((PsiModifierListOwner)decl.getDeclaredElements()[0], PsiModifier.FINAL, false); - } - newDeclaration = anchor.replace(decl); - refsSet.remove(expression.getLExpression()); - retargetReferences(elementFactory, localName, refsSet); - } - else { - newDeclaration = addDeclarationWithFieldInitializerAndRetargetReferences(elementFactory, localName, anchorBlock, anchor, refsSet, - myField); - } - } - else { - newDeclaration = addDeclarationWithFieldInitializerAndRetargetReferences(elementFactory, localName, anchorBlock, anchor, refsSet, - myField); - } - } - catch (IncorrectOperationException e) { - LOG.error(e); - } - - if (newDeclaration != null) { - if (docComment != null) { - final StringBuilder buf = new StringBuilder(); - for (PsiElement psiElement : docComment.getDescriptionElements()) { - buf.append(psiElement.getText()); - } - if (buf.length() > 0) { - final JavaCommenter commenter = new JavaCommenter(); - final PsiComment comment = JavaPsiFacade.getElementFactory(project) - .createCommentFromText(commenter.getBlockCommentPrefix() + - buf.toString() + - commenter.getBlockCommentSuffix(), newDeclaration); - newDeclaration.getParent().addBefore(comment, newDeclaration); - } - } - final PsiFile psiFile = myField.getContainingFile(); - final Editor editor = FileEditorManager.getInstance(project).getSelectedTextEditor(); - if (editor != null && IJSwingUtilities.hasFocus(editor.getComponent())) { - final PsiFile file = PsiDocumentManager.getInstance(project).getPsiFile(editor.getDocument()); - if (file == psiFile) { - editor.getCaretModel().moveToOffset(newDeclaration.getTextOffset()); - editor.getScrollingModel().scrollToCaret(ScrollType.RELATIVE); - } - } - } - - try { - myField.normalizeDeclaration(); - myField.delete(); - } - catch (IncorrectOperationException e) { - LOG.error(e); - } + final String propertyName = styleManager.variableNameToPropertyName(field.getName(), VariableKind.FIELD); + final String localName = styleManager.propertyNameToVariableName(propertyName, VariableKind.LOCAL_VARIABLE); + return RefactoringUtil.suggestUniqueVariableName(localName, scope, field); } - private static boolean mayBeFinal(Set refsSet, PsiElement firstElement) { - for (PsiReference ref : refsSet) { - PsiElement element = ref.getElement(); - if (element == firstElement) continue; - if (element instanceof PsiExpression && PsiUtil.isAccessedForWriting((PsiExpression) element)) return false; + private static void moveDocCommentToDeclaration(@NotNull Project project, @NotNull PsiDocComment docComment, @NotNull PsiElement declaration) { + final StringBuilder buf = new StringBuilder(); + for (PsiElement psiElement : docComment.getDescriptionElements()) { + buf.append(psiElement.getText()); } - return true; - } - - private static void retargetReferences(final PsiElementFactory elementFactory, final String localName, final Set refs) - throws IncorrectOperationException { - final PsiReferenceExpression refExpr = (PsiReferenceExpression)elementFactory.createExpressionFromText(localName, null); - for (PsiReference ref : refs) { - if (ref instanceof PsiReferenceExpression) { - ((PsiReferenceExpression)ref).replace(refExpr); - } + if (buf.length() > 0) { + final PsiElementFactory elementFactory = JavaPsiFacade.getElementFactory(project); + final JavaCommenter commenter = new JavaCommenter(); + final PsiComment comment = elementFactory.createCommentFromText(commenter.getBlockCommentPrefix() + buf.toString() + commenter.getBlockCommentSuffix(), declaration); + declaration.getParent().addBefore(comment, declaration); } } - - private static PsiElement addDeclarationWithFieldInitializerAndRetargetReferences(final PsiElementFactory elementFactory, - final String localName, - final PsiCodeBlock anchorBlock, - final PsiElement anchor, - final Set refs, - PsiField myField) - throws IncorrectOperationException { - final PsiDeclarationStatement decl = elementFactory.createVariableDeclarationStatement(localName, myField.getType(), myField.getInitializer()); - final PsiElement newDeclaration = anchorBlock.addBefore(decl, anchor); - - retargetReferences(elementFactory, localName, refs); - return newDeclaration; - } - - @NotNull - public String getFamilyName() { - return getName(); - } - - private static PsiElement getAnchorElement(final PsiCodeBlock anchorBlock, @NotNull PsiElement firstElement) { - PsiElement element = firstElement; - while (element != null && element.getParent() != anchorBlock) { - element = element.getParent(); - } - return element; - } - - private static PsiElement getFirstElement(Collection refs) { - PsiElement firstElement = null; - for (PsiReference reference : refs) { - final PsiElement element = reference.getElement(); - if (firstElement == null || firstElement.getTextRange().getStartOffset() > element.getTextRange().getStartOffset()) { - firstElement = element; - } - } - return firstElement; - } - - private static PsiCodeBlock findAnchorBlock(final Collection refs) { - PsiCodeBlock result = null; - for (PsiReference psiReference : refs) { - final PsiElement element = psiReference.getElement(); - PsiCodeBlock block = PsiTreeUtil.getParentOfType(element, PsiCodeBlock.class); - if (result == null || block == null) { - result = block; - } - else { - final PsiElement commonParent = PsiTreeUtil.findCommonParent(result, block); - result = PsiTreeUtil.getParentOfType(commonParent, PsiCodeBlock.class, false); - } - } - return result; - } } - public boolean runForWholeFile() { - return true; - } -} +} \ No newline at end of file diff --git a/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/ParameterCanBeLocalInspection.java b/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/ParameterCanBeLocalInspection.java new file mode 100644 index 000000000000..9d11afac789f --- /dev/null +++ b/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/ParameterCanBeLocalInspection.java @@ -0,0 +1,148 @@ +/* + * Copyright 2000-2012 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 com.intellij.codeInspection.varScopeCanBeNarrowed; + +import com.intellij.codeInsight.daemon.GroupNames; +import com.intellij.codeInspection.*; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.controlFlow.*; +import com.intellij.psi.search.searches.SuperMethodsSearch; +import org.jetbrains.annotations.NonNls; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.*; + +/** + * @author Danila Ponomarenko + */ +public class ParameterCanBeLocalInspection extends BaseJavaLocalInspectionTool { + + @NonNls public static final String SHORT_NAME = "ParameterCanBeLocal"; + + @NotNull + public String getGroupDisplayName() { + return GroupNames.CLASS_LAYOUT_GROUP_NAME; + } + + @NotNull + public String getDisplayName() { + return InspectionsBundle.message("inspection.parameter.can.be.local.display.name"); + } + + @NotNull + public String getShortName() { + return SHORT_NAME; + } + + @Override + public ProblemDescriptor[] checkMethod(@NotNull PsiMethod method, @NotNull InspectionManager manager, boolean isOnTheFly) { + final Collection parameters = filterFinal(method.getParameterList().getParameters()); + final PsiCodeBlock body = method.getBody(); + if (body == null || parameters.isEmpty() || isOverrides(method)) { + return ProblemDescriptor.EMPTY_ARRAY; + } + + final List result = new ArrayList(); + for (PsiParameter parameter : getWriteBeforeRead(parameters, body)) { + final PsiIdentifier identifier = parameter.getNameIdentifier(); + if (identifier != null) { + result.add(createProblem(manager, identifier, isOnTheFly)); + } + } + return result.toArray(new ProblemDescriptor[result.size()]); + } + + @NotNull + private static List filterFinal(PsiParameter[] parameters) { + final List result = new ArrayList(parameters.length); + for (PsiParameter parameter : parameters) { + if (!parameter.hasModifierProperty(PsiModifier.FINAL)) { + result.add(parameter); + } + } + return result; + } + + + @NotNull + private static ProblemDescriptor createProblem(@NotNull InspectionManager manager, @NotNull PsiIdentifier identifier, boolean isOnTheFly) { + return manager.createProblemDescriptor( + identifier, + InspectionsBundle.message("inspection.parameter.can.be.local.problem.descriptor"), + true, + ProblemHighlightType.LIKE_UNUSED_SYMBOL, + isOnTheFly, + new ConvertParameterToLocalQuickFix() + ); + } + + private static Collection getWriteBeforeRead(@NotNull Collection parameters, + @NotNull PsiCodeBlock body) { + final ControlFlow controlFlow = getControlFlow(body); + if (controlFlow == null) return Collections.emptyList(); + + final Set result = filterParameters(controlFlow, parameters); + for (final PsiReferenceExpression readBeforeWrite : ControlFlowUtil.getReadBeforeWrite(controlFlow)) { + final PsiElement resolved = readBeforeWrite.resolve(); + if (resolved instanceof PsiParameter) { + result.remove((PsiParameter)resolved); + } + } + + return result; + } + + private static Set filterParameters(@NotNull ControlFlow controlFlow, @NotNull Collection parameters) { + final Set usedVars = new HashSet(ControlFlowUtil.getUsedVariables(controlFlow, 0, controlFlow.getSize())); + + final Set result = new HashSet(); + for (PsiParameter parameter : parameters) { + if (usedVars.contains(parameter)) { + result.add(parameter); + } + } + return result; + } + + private static boolean isOverrides(PsiMethod method) { + return SuperMethodsSearch.search(method, null, true, false).findFirst() != null; + } + + @Nullable + private static ControlFlow getControlFlow(final PsiElement context) { + try { + return ControlFlowFactory.getInstance(context.getProject()).getControlFlow(context, LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance()); + } + catch (AnalysisCanceledException e) { + return null; + } + } + + public static class ConvertParameterToLocalQuickFix extends BaseConvertToLocalQuickFix { + @Override + protected PsiParameter getVariable(@NotNull ProblemDescriptor descriptor) { + return (PsiParameter)descriptor.getPsiElement().getParent(); + } + + @NotNull + @Override + protected String suggestLocalName(@NotNull Project project, @NotNull PsiParameter parameter, @NotNull PsiCodeBlock scope) { + return parameter.getName(); + } + } +} diff --git a/java/java-impl/src/com/intellij/psi/controlFlow/DefUseUtil.java b/java/java-impl/src/com/intellij/psi/controlFlow/DefUseUtil.java index 8aa627ac4073..658164e9ec14 100644 --- a/java/java-impl/src/com/intellij/psi/controlFlow/DefUseUtil.java +++ b/java/java-impl/src/com/intellij/psi/controlFlow/DefUseUtil.java @@ -132,7 +132,6 @@ public class DefUseUtil { if (body == null) { return null; } - List unusedDefs = new ArrayList(); ControlFlow flow; try { @@ -223,6 +222,8 @@ public class DefUseUtil { } } + List unusedDefs = new ArrayList(); + for (int i = 0; i < instructions.size(); i++) { Instruction instruction = instructions.get(i); if (instruction instanceof WriteVariableInstruction) { diff --git a/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java b/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java index 4e0576c4641c..a6965e072b4d 100644 --- a/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java +++ b/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java @@ -669,7 +669,7 @@ public class ControlFlowUtil { InstructionClientVisitor[] visitors = new InstructionClientVisitor[]{ new ReturnPresentClientVisitor(flow), new UnreachableStatementClientVisitor(flow), - new ReadBeforeWriteClientVisitor(flow), + new ReadBeforeWriteClientVisitor(flow, true), new InitializedTwiceClientVisitor(flow, 0), }; CompositeInstructionClientVisitor visitor = new CompositeInstructionClientVisitor(visitors); @@ -1223,8 +1223,14 @@ public class ControlFlowUtil { /** * @return list of PsiReferenceExpression of usages of non-initialized local variables */ + public static List getReadBeforeWriteLocals(ControlFlow flow) { + final InstructionClientVisitor> visitor = new ReadBeforeWriteClientVisitor(flow, true); + depthFirstSearch(flow, visitor); + return visitor.getResult(); + } + public static List getReadBeforeWrite(ControlFlow flow) { - final InstructionClientVisitor> visitor = new ReadBeforeWriteClientVisitor(flow); + final InstructionClientVisitor> visitor = new ReadBeforeWriteClientVisitor(flow, false); depthFirstSearch(flow, visitor); return visitor.getResult(); } @@ -1233,9 +1239,11 @@ public class ControlFlowUtil { // map of variable->PsiReferenceExpressions for all read before written variables for this point and below in control flow private final CopyOnWriteList[] readVariables; private final ControlFlow myFlow; + private boolean localVariablesOnly; - public ReadBeforeWriteClientVisitor(ControlFlow flow) { + public ReadBeforeWriteClientVisitor(ControlFlow flow, boolean localVariablesOnly) { myFlow = flow; + this.localVariablesOnly = localVariablesOnly; readVariables = new CopyOnWriteList[myFlow.getSize() + 1]; } @@ -1243,7 +1251,7 @@ public class ControlFlowUtil { public void visitReadVariableInstruction(ReadVariableInstruction instruction, int offset, int nextOffset) { CopyOnWriteList readVars = readVariables[Math.min(nextOffset, myFlow.getSize())]; final PsiVariable variable = instruction.variable; - if (!isMethodParameter(variable)) { + if (!localVariablesOnly || !isMethodParameter(variable)) { final PsiReferenceExpression expression = getEnclosingReferenceExpression(myFlow.getElement(offset), variable); if (expression != null) { readVars = CopyOnWriteList.add(readVars, new VariableInfo(variable, expression)); @@ -1258,7 +1266,7 @@ public class ControlFlowUtil { if (readVars == null) return; final PsiVariable variable = instruction.variable; - if (!isMethodParameter(variable)) { + if (!localVariablesOnly || !isMethodParameter(variable)) { readVars = readVars.remove(new VariableInfo(variable, null)); } merge(offset, readVars, readVariables); @@ -1480,5 +1488,4 @@ public class ControlFlowUtil { int startOffset = flow.getStartOffset(assignmentExpression); return startOffset != -1 && isInstructionReachable(flow, startOffset, startOffset); } -} - +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/parameterCanBeLocal/for/expected.xml b/java/java-tests/testData/inspection/parameterCanBeLocal/for/expected.xml new file mode 100644 index 000000000000..ecdf1549b564 --- /dev/null +++ b/java/java-tests/testData/inspection/parameterCanBeLocal/for/expected.xml @@ -0,0 +1,8 @@ + + + + Test.java + 2 + Parameter can be converted to a variable + + diff --git a/java/java-tests/testData/inspection/parameterCanBeLocal/for/src/Test.java b/java/java-tests/testData/inspection/parameterCanBeLocal/for/src/Test.java new file mode 100644 index 000000000000..95467a612c6d --- /dev/null +++ b/java/java-tests/testData/inspection/parameterCanBeLocal/for/src/Test.java @@ -0,0 +1,8 @@ +class Temp { + public Temp(int p) { + for (int i = 0; i < 10; i++) { + p = i; + System.out.print(p); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/parameterCanBeLocal/if/expected.xml b/java/java-tests/testData/inspection/parameterCanBeLocal/if/expected.xml new file mode 100644 index 000000000000..06abec6d7bd7 --- /dev/null +++ b/java/java-tests/testData/inspection/parameterCanBeLocal/if/expected.xml @@ -0,0 +1,8 @@ + + + + Test.java + 4 + Parameter can be converted to a variable + + diff --git a/java/java-tests/testData/inspection/parameterCanBeLocal/if/src/Test.java b/java/java-tests/testData/inspection/parameterCanBeLocal/if/src/Test.java new file mode 100644 index 000000000000..4ae6800d5079 --- /dev/null +++ b/java/java-tests/testData/inspection/parameterCanBeLocal/if/src/Test.java @@ -0,0 +1,13 @@ +class Temp { + public boolean flag; + + void test(int p) { + if (flag) { + p = 1; + } + else { + p = 2; + } + System.out.print(p); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/parameterCanBeLocal/simple/expected.xml b/java/java-tests/testData/inspection/parameterCanBeLocal/simple/expected.xml new file mode 100644 index 000000000000..a83abd288af9 --- /dev/null +++ b/java/java-tests/testData/inspection/parameterCanBeLocal/simple/expected.xml @@ -0,0 +1,8 @@ + + + + Test.java + 3 + Parameter can be converted to a variable + + diff --git a/java/java-tests/testData/inspection/parameterCanBeLocal/simple/src/Test.java b/java/java-tests/testData/inspection/parameterCanBeLocal/simple/src/Test.java new file mode 100644 index 000000000000..33ec47a32bfa --- /dev/null +++ b/java/java-tests/testData/inspection/parameterCanBeLocal/simple/src/Test.java @@ -0,0 +1,7 @@ +class Temp { + + void test(int p) { + p = 1; + System.out.print(p); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterFor.java b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterFor.java new file mode 100644 index 000000000000..1635881830e0 --- /dev/null +++ b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterFor.java @@ -0,0 +1,9 @@ +// "Convert to local variable" "true" +class Temp { + public Temp() { + for (int i = 0; i < 10; i++) { + int p = i; + System.out.print(p); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterIf.java b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterIf.java new file mode 100644 index 000000000000..d5016105c40c --- /dev/null +++ b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterIf.java @@ -0,0 +1,15 @@ +// "Convert to local variable" "true" +class Temp { + public boolean flag; + + void test() { + int p; + if (flag) { + p = 1; + } + else { + p = 2; + } + System.out.print(p); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterSimple.java b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterSimple.java new file mode 100644 index 000000000000..1ace51e3ff57 --- /dev/null +++ b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterSimple.java @@ -0,0 +1,8 @@ +// "Convert to local variable" "true" +class Temp { + + void test() { + int p = 1; + System.out.print(p); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeFor.java b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeFor.java new file mode 100644 index 000000000000..598b23780897 --- /dev/null +++ b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeFor.java @@ -0,0 +1,9 @@ +// "Convert to local variable" "true" +class Temp { + public Temp(int p) { + for (int i = 0; i < 10; i++) { + p = i; + System.out.print(p); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeIf.java b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeIf.java new file mode 100644 index 000000000000..6dc3bb16c1ac --- /dev/null +++ b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeIf.java @@ -0,0 +1,14 @@ +// "Convert to local variable" "true" +class Temp { + public boolean flag; + + void test(int p) { + if (flag) { + p = 1; + } + else { + p = 2; + } + System.out.print(p); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeSimple.java b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeSimple.java new file mode 100644 index 000000000000..cf03ce27246c --- /dev/null +++ b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeSimple.java @@ -0,0 +1,8 @@ +// "Convert to local variable" "true" +class Temp { + + void test(int p) { + p = 1; + System.out.print(p); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/ConvertParameterToLocalVariableTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/ConvertParameterToLocalVariableTest.java new file mode 100644 index 000000000000..23d9c037a6a4 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInspection/ConvertParameterToLocalVariableTest.java @@ -0,0 +1,60 @@ +/* + * Copyright 2000-2012 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. + */ + +/* + * User: anna + * Date: 16-May-2007 + */ +package com.intellij.codeInspection; + +import com.intellij.JavaTestUtil; +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixTestCase; +import com.intellij.codeInspection.varScopeCanBeNarrowed.ParameterCanBeLocalInspection; +import com.intellij.psi.PsiElement; +import org.jetbrains.annotations.NonNls; + +public class ConvertParameterToLocalVariableTest extends LightQuickFixTestCase { + @Override + protected String getTestDataPath() { + return JavaTestUtil.getJavaTestDataPath() + "/inspection"; + } + + public void test() throws Exception { + doAllTests(); + } + + @Override + protected void doAction(final String text, final boolean actionShouldBeAvailable, final String testFullPath, final String testName) + throws Exception { + + final LocalQuickFix fix = new ParameterCanBeLocalInspection.ConvertParameterToLocalQuickFix(); + final int offset = getEditor().getCaretModel().getOffset(); + final PsiElement psiElement = getFile().findElementAt(offset); + assert psiElement != null; + final InspectionManager manager = InspectionManager.getInstance(getProject()); + final ProblemDescriptor descriptor = manager.createProblemDescriptor(psiElement, "", fix, ProblemHighlightType.LIKE_UNUSED_SYMBOL, true); + fix.applyFix(getProject(), descriptor); + final String expectedFilePath = getBasePath() + "/after" + testName; + checkResultByFile("In file :" + expectedFilePath, expectedFilePath, false); + } + + + @Override + @NonNls + protected String getBasePath() { + return "/quickFix/ConvertParameterToLocalVariable"; + } +} diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/ParameterCanBeLocalTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/ParameterCanBeLocalTest.java new file mode 100644 index 000000000000..10cc1f0f570f --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInspection/ParameterCanBeLocalTest.java @@ -0,0 +1,36 @@ +/* + * Copyright 2000-2012 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 com.intellij.codeInspection; + +import com.intellij.JavaTestUtil; +import com.intellij.codeInspection.varScopeCanBeNarrowed.FieldCanBeLocalInspection; +import com.intellij.codeInspection.varScopeCanBeNarrowed.ParameterCanBeLocalInspection; +import com.intellij.testFramework.InspectionTestCase; + +public class ParameterCanBeLocalTest extends InspectionTestCase { + @Override + protected String getTestDataPath() { + return JavaTestUtil.getJavaTestDataPath() + "/inspection"; + } + + private void doTest() throws Exception { + doTest("parameterCanBeLocal/" + getTestName(true), new ParameterCanBeLocalInspection()); + } + + public void testSimple () throws Exception { doTest(); } + public void testIf () throws Exception { doTest(); } + public void testFor () throws Exception { doTest(); } +} diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index d49cf30fc54c..934b3cf961f7 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -144,7 +144,9 @@ inspection.visibility.compose.suggestion=Can be {0} inspection.visibility.accept.quickfix=Accept Suggested Access Level inspection.field.can.be.local.display.name=Field can be local inspection.field.can.be.local.problem.descriptor=Field can be converted to a local variable -inspection.field.can.be.local.quickfix=Convert to local +inspection.parameter.can.be.local.display.name=Parameter can be local +inspection.parameter.can.be.local.problem.descriptor=Parameter can be converted to a local variable +inspection.convert.to.local.quickfix=Convert to local inspection.unused.return.value.display.name=Unused method return value inspection.unused.return.value.problem.descriptor=Return value of the method is never used @@ -648,4 +650,4 @@ inspection.javadoc.problem.pointing.to.itself=Javadoc pointing to itself inspection.redirect.template=Injected element has problem: {0} (in {3}). nothing.found=Nothing found -special.annotations.list.annotation.pattern=Add Annotations Pattern +special.annotations.list.annotation.pattern=Add Annotations Pattern \ No newline at end of file diff --git a/resources-en/src/inspectionDescriptions/ParameterCanBeLocal.html b/resources-en/src/inspectionDescriptions/ParameterCanBeLocal.html new file mode 100644 index 000000000000..81a3e513e6b6 --- /dev/null +++ b/resources-en/src/inspectionDescriptions/ParameterCanBeLocal.html @@ -0,0 +1,9 @@ + + + + This inspection searches for redundant method parameters that can be replaced with local variables. + If all local usages of a parameter are preceded by assignments to that parameter, the + parameter can be removed and its usages replaced with local variables. + + + diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index 82372a8498d0..30f80d8c791c 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -459,6 +459,9 @@ + @@ -535,6 +538,7 @@ groupName="Probable bugs" enabledByDefault="true" level="WARNING" implementationClass="com.intellij.codeInspection.magicConstant.MagicConstantInspection" /> + com.intellij.codeInsight.intention.impl.SplitIfAction Control Flow