From 34bd4f81baf5044da09f4a677c906ef473211c55 Mon Sep 17 00:00:00 2001 From: Eugene Zhuravlev Date: Fri, 18 May 2012 16:56:52 +0200 Subject: [PATCH 01/14] release already allocated resources in case of initialization problems --- .../util/io/PersistentEnumeratorDelegate.java | 6 ++- .../intellij/util/io/PersistentHashMap.java | 47 +++++++++++++------ 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/platform/util/src/com/intellij/util/io/PersistentEnumeratorDelegate.java b/platform/util/src/com/intellij/util/io/PersistentEnumeratorDelegate.java index 34957b1fb0e5..1e2df377e4f2 100644 --- a/platform/util/src/com/intellij/util/io/PersistentEnumeratorDelegate.java +++ b/platform/util/src/com/intellij/util/io/PersistentEnumeratorDelegate.java @@ -39,7 +39,11 @@ public class PersistentEnumeratorDelegate implements Closeable, Forceable @Override public void close() throws IOException { - myEnumerator.close(); + final PersistentEnumeratorBase enumerator = myEnumerator; + //noinspection ConstantConditions + if (enumerator != null) { + enumerator.close(); + } } public boolean isClosed() { diff --git a/platform/util/src/com/intellij/util/io/PersistentHashMap.java b/platform/util/src/com/intellij/util/io/PersistentHashMap.java index 161c173ba62c..825f419166d4 100644 --- a/platform/util/src/com/intellij/util/io/PersistentHashMap.java +++ b/platform/util/src/com/intellij/util/io/PersistentHashMap.java @@ -191,10 +191,22 @@ public class PersistentHashMap extends PersistentEnumeratorDelegate< } } catch (IOException e) { + try { + // attempt to close already opened resources + close(); + } + catch (Throwable ignored) { + } throw e; // rethrow } catch (Throwable t) { LOG.error(t); + try { + // attempt to close already opened resources + close(); + } + catch (Throwable ignored) { + } throw new PersistentEnumerator.CorruptedException(file); } } @@ -472,7 +484,10 @@ public class PersistentHashMap extends PersistentEnumeratorDelegate< try { myAppendCacheFlusher.stop(); myAppendCache.clear(); - myValueStorage.dispose(); + final PersistentHashMapValueStorage valueStorage = myValueStorage; + if (valueStorage != null) { + valueStorage.dispose(); + } } finally { super.close(); @@ -490,22 +505,26 @@ public class PersistentHashMap extends PersistentEnumeratorDelegate< myLiveAndGarbageKeysCounter = 0; myReadCompactionGarbageSize = 0; - traverseAllRecords(new PersistentEnumerator.RecordsProcessor() { - @Override - public boolean process(final int keyId) throws IOException { - final long record = readValueId(keyId); - if (record != NULL_ADDR) { - PersistentHashMapValueStorage.ReadResult readResult = myValueStorage.readBytes(record); - long value = newStorage.appendBytes(readResult.buffer, 0, readResult.buffer.length, 0); - updateValueId(keyId, value, record, null, getCurrentKey()); - myLiveAndGarbageKeysCounter += LIVE_KEY_MASK; + try { + traverseAllRecords(new PersistentEnumerator.RecordsProcessor() { + @Override + public boolean process(final int keyId) throws IOException { + final long record = readValueId(keyId); + if (record != NULL_ADDR) { + PersistentHashMapValueStorage.ReadResult readResult = myValueStorage.readBytes(record); + long value = newStorage.appendBytes(readResult.buffer, 0, readResult.buffer.length, 0); + updateValueId(keyId, value, record, null, getCurrentKey()); + myLiveAndGarbageKeysCounter += LIVE_KEY_MASK; + } + return true; } - return true; - } - }); + }); + } + finally { + newStorage.dispose(); + } myValueStorage.dispose(); - newStorage.dispose(); FileUtil.rename(new File(newPath), getDataFile(myEnumerator.myFile)); From 9e1f37939e508623507a98b4a70b034db18a6c3d Mon Sep 17 00:00:00 2001 From: Danila Ponomarenko Date: Thu, 17 May 2012 16:11:26 +0400 Subject: [PATCH 02/14] getInitializedTwice and getReadBeforeWrite refactored --- .../psi/controlFlow/ControlFlowUtil.java | 364 +++++++++++------- 1 file changed, 217 insertions(+), 147 deletions(-) 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 b9938c174104..4e0576c4641c 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 @@ -26,6 +26,7 @@ import com.intellij.util.containers.IntArrayList; import gnu.trove.THashSet; import gnu.trove.TIntHashSet; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.*; @@ -73,7 +74,7 @@ public class ControlFlowUtil { } public static List getSSAVariables(ControlFlow flow, int from, int to, - boolean reportVarsIfNonInitializingPathExists) { + boolean reportVarsIfNonInitializingPathExists) { List instructions = flow.getInstructions(); Collection writtenVariables = getWrittenVariables(flow, from, to, false); ArrayList result = new ArrayList(1); @@ -147,23 +148,25 @@ public class ControlFlowUtil { private static boolean needVariableValueAt(final PsiVariable variable, final ControlFlow flow, final int offset) { InstructionClientVisitor visitor = new InstructionClientVisitor() { - final boolean[] neededBelow = new boolean[flow.getSize()+1]; + final boolean[] neededBelow = new boolean[flow.getSize() + 1]; @Override public void procedureEntered(int startOffset, int endOffset) { for (int i = startOffset; i < endOffset; i++) neededBelow[i] = false; } - @Override public void visitReadVariableInstruction(ReadVariableInstruction instruction, int offset, int nextOffset) { + @Override + public void visitReadVariableInstruction(ReadVariableInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); boolean needed = neededBelow[nextOffset]; if (instruction.variable.equals(variable)) { - needed = true; + needed = true; } neededBelow[offset] |= needed; } - @Override public void visitWriteVariableInstruction(WriteVariableInstruction instruction, int offset, int nextOffset) { + @Override + public void visitWriteVariableInstruction(WriteVariableInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); boolean needed = neededBelow[nextOffset]; if (instruction.variable.equals(variable)) { @@ -172,7 +175,8 @@ public class ControlFlowUtil { neededBelow[offset] = needed; } - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); boolean needed = neededBelow[nextOffset]; neededBelow[offset] |= needed; @@ -182,7 +186,7 @@ public class ControlFlowUtil { public Boolean getResult() { return neededBelow[offset]; } - }; + }; depthFirstSearch(flow, visitor, offset, flow.getSize()); return visitor.getResult().booleanValue(); } @@ -267,26 +271,33 @@ public class ControlFlowUtil { } final Collection exitStatements = new THashSet(); InstructionClientVisitor visitor = new InstructionClientVisitor() { - @Override public void visitThrowToInstruction(ThrowToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitThrowToInstruction(ThrowToInstruction instruction, int offset, int nextOffset) { //[ven]This is a hack since Extract Method doesn't want to see throw's exit points processGotoStatement(classesFilter, exitStatements, findStatement(flow, offset)); } - @Override public void visitBranchingInstruction(BranchingInstruction instruction, int offset, int nextOffset) { + @Override + public void visitBranchingInstruction(BranchingInstruction instruction, int offset, int nextOffset) { processGoto(flow, start, end, exitPoints, exitStatements, instruction, classesFilter, findStatement(flow, offset)); } // call/return do not incur exit points - @Override public void visitReturnInstruction(ReturnInstruction instruction, int offset, int nextOffset) { - } - @Override public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { + @Override + public void visitReturnInstruction(ReturnInstruction instruction, int offset, int nextOffset) { } - @Override public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { + } + + @Override + public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { visitInstruction(instruction, offset, nextOffset); } - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { if (offset >= end - 1) { int exitOffset = end; exitOffset = promoteThroughGotoChain(flow, exitOffset); @@ -319,7 +330,8 @@ public class ControlFlowUtil { } if (gotoOffset >= end || gotoOffset < start) { processGotoStatement(classesFilter, exitStatements, statement); - } else { + } + else { boolean isReturn = instruction instanceof GoToInstruction && ((GoToInstruction)instruction).isReturn; final Instruction gotoInstruction = flow.getInstructions().get(gotoOffset); isReturn |= gotoInstruction instanceof GoToInstruction && ((GoToInstruction)gotoInstruction).isReturn; @@ -357,7 +369,7 @@ public class ControlFlowUtil { return offset; } - public static final Class[] DEFAULT_EXIT_STATEMENTS_CLASSES = new Class[] {PsiReturnStatement.class, PsiBreakStatement.class, PsiContinueStatement.class}; + public static final Class[] DEFAULT_EXIT_STATEMENTS_CLASSES = new Class[]{PsiReturnStatement.class, PsiBreakStatement.class, PsiContinueStatement.class}; private static PsiStatement findStatement(ControlFlow flow, int offset) { PsiElement element = flow.getElement(offset); @@ -401,22 +413,23 @@ public class ControlFlowUtil { /** * Checks possibility of extracting code fragment outside containing anonymous (local) class. * Also collects variables to be passed as additional parameters. - * @return true if code fragement can be extracted outside - * @param array Vector to collect variables to be passed as additional parameters - * @param scope scope to be scanned (part of code fragement to be extracted) - * @param member member containing the code to be extracted + * + * @param array Vector to collect variables to be passed as additional parameters + * @param scope scope to be scanned (part of code fragement to be extracted) + * @param member member containing the code to be extracted * @param targetClassMember member in target class containing code fragement + * @return true if code fragement can be extracted outside */ public static boolean collectOuterLocals(List array, PsiElement scope, PsiElement member, PsiElement targetClassMember) { if (scope instanceof PsiMethodCallExpression) { final PsiMethodCallExpression call = (PsiMethodCallExpression)scope; - if (!checkReferenceExpressionScope (call.getMethodExpression(), targetClassMember)) { + if (!checkReferenceExpressionScope(call.getMethodExpression(), targetClassMember)) { return false; } } else if (scope instanceof PsiReferenceExpression) { - if (!checkReferenceExpressionScope ((PsiReferenceExpression)scope, targetClassMember)) { + if (!checkReferenceExpressionScope((PsiReferenceExpression)scope, targetClassMember)) { return false; } } @@ -483,10 +496,10 @@ public class ControlFlowUtil { depthFirstSearch(flow, visitor); return visitor.getResult().booleanValue(); } - + public static boolean processReturns(final ControlFlow flow, final ReturnStatementsVisitor afterVisitor) throws IncorrectOperationException { final ConvertReturnClientVisitor instructionsVisitor = new ConvertReturnClientVisitor(flow, afterVisitor); - + depthFirstSearch(flow, instructionsVisitor); instructionsVisitor.afterProcessing(); @@ -510,7 +523,7 @@ public class ControlFlowUtil { if (instruction.isReturn) { final PsiElement element = myFlow.getElement(offset); if (element instanceof PsiReturnStatement) { - final PsiReturnStatement returnStatement = (PsiReturnStatement) element; + final PsiReturnStatement returnStatement = (PsiReturnStatement)element; myAffectedReturns.add(returnStatement); } } @@ -532,26 +545,30 @@ public class ControlFlowUtil { isNormalCompletion[myFlow.getSize()] = true; } - @Override public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { if (nextOffset > myFlow.getSize()) nextOffset = myFlow.getSize(); boolean isNormal = instruction.offset == nextOffset && nextOffset != offset + 1 ? - !isLeaf(nextOffset) && isNormalCompletion[nextOffset] : - isLeaf(nextOffset) || isNormalCompletion[nextOffset]; + !isLeaf(nextOffset) && isNormalCompletion[nextOffset] : + isLeaf(nextOffset) || isNormalCompletion[nextOffset]; isNormalCompletion[offset] |= isNormal; } - @Override public void visitThrowToInstruction(ThrowToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitThrowToInstruction(ThrowToInstruction instruction, int offset, int nextOffset) { if (nextOffset > myFlow.getSize()) nextOffset = myFlow.getSize(); isNormalCompletion[offset] |= !isLeaf(nextOffset) && isNormalCompletion[nextOffset]; } - @Override public void visitGoToInstruction(GoToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitGoToInstruction(GoToInstruction instruction, int offset, int nextOffset) { if (nextOffset > myFlow.getSize()) nextOffset = myFlow.getSize(); isNormalCompletion[offset] |= !instruction.isReturn && isNormalCompletion[nextOffset]; } - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { if (nextOffset > myFlow.getSize()) nextOffset = myFlow.getSize(); boolean isNormal = isLeaf(nextOffset) || isNormalCompletion[nextOffset]; @@ -580,7 +597,8 @@ public class ControlFlowUtil { } } - @Override public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); if (offset > endOffset) return; int throwToOffset = instruction.offset; @@ -599,7 +617,8 @@ public class ControlFlowUtil { isNormalCompletion[offset] |= isNormal; } - @Override public void visitThrowToInstruction(ThrowToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitThrowToInstruction(ThrowToInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); if (offset > endOffset) return; if (nextOffset <= endOffset) { @@ -608,7 +627,8 @@ public class ControlFlowUtil { } } - @Override public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { + @Override + public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); if (offset > endOffset) return; if (nextOffset > endOffset && nextOffset != offset + 1) { @@ -618,15 +638,17 @@ public class ControlFlowUtil { isNormalCompletion[offset] |= isNormal; } - @Override public void visitGoToInstruction(GoToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitGoToInstruction(GoToInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); if (offset > endOffset) return; - boolean isRethrowFromFinally = instruction instanceof ReturnInstruction && ((ReturnInstruction) instruction).isRethrowFromFinally(); + boolean isRethrowFromFinally = instruction instanceof ReturnInstruction && ((ReturnInstruction)instruction).isRethrowFromFinally(); boolean isNormal = !instruction.isReturn && isNormalCompletion[nextOffset] && !isRethrowFromFinally; isNormalCompletion[offset] |= isNormal; } - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); if (offset > endOffset) return; final boolean isNormal = isLeaf(nextOffset) || isNormalCompletion[nextOffset]; @@ -664,10 +686,13 @@ public class ControlFlowUtil { // false if control flow at this offset terminates abruptly final boolean[] canCompleteNormally = new boolean[flow.getSize() + 1]; - @Override public void visitConditionalGoToInstruction(ConditionalGoToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitConditionalGoToInstruction(ConditionalGoToInstruction instruction, int offset, int nextOffset) { checkInstruction(offset, nextOffset, false); } - @Override public void visitGoToInstruction(GoToInstruction instruction, int offset, int nextOffset) { + + @Override + public void visitGoToInstruction(GoToInstruction instruction, int offset, int nextOffset) { checkInstruction(offset, nextOffset, instruction.isReturn); } @@ -684,7 +709,8 @@ public class ControlFlowUtil { canCompleteNormally[offset] |= isNormal; } - @Override public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); if (offset > endOffset) return; int throwToOffset = instruction.offset; @@ -698,7 +724,8 @@ public class ControlFlowUtil { canCompleteNormally[offset] |= isNormal; } - @Override public void visitThrowToInstruction(ThrowToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitThrowToInstruction(ThrowToInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); if (offset > endOffset) return; if (nextOffset <= endOffset) { @@ -707,7 +734,8 @@ public class ControlFlowUtil { } } - @Override public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { + @Override + public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); if (offset > endOffset) return; if (nextOffset > endOffset && nextOffset != offset + 1) { @@ -717,7 +745,8 @@ public class ControlFlowUtil { canCompleteNormally[offset] |= isNormal; } - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { checkInstruction(offset, nextOffset, false); } @@ -739,6 +768,7 @@ public class ControlFlowUtil { depthFirstSearch(flow, visitor); return visitor.getResult(); } + private static class UnreachableStatementClientVisitor extends InstructionClientVisitor { private final ControlFlow myFlow; @@ -760,7 +790,7 @@ public class ControlFlowUtil { } if (element instanceof PsiStatement && element.getParent() instanceof PsiForStatement - && element == ((PsiForStatement) element.getParent()).getUpdate()) { + && element == ((PsiForStatement)element.getParent()).getUpdate()) { continue; } //filter out generated stmts @@ -809,11 +839,13 @@ public class ControlFlowUtil { class MyVisitor extends InstructionClientVisitor { // true if from this point below there may be branch with no variable assignment final boolean[] maybeUnassigned = new boolean[flow.getSize() + 1]; + { - maybeUnassigned[maybeUnassigned.length-1] = true; + maybeUnassigned[maybeUnassigned.length - 1] = true; } - @Override public void visitWriteVariableInstruction(WriteVariableInstruction instruction, int offset, int nextOffset) { + @Override + public void visitWriteVariableInstruction(WriteVariableInstruction instruction, int offset, int nextOffset) { if (instruction.variable == variable) { maybeUnassigned[offset] = false; } @@ -822,7 +854,8 @@ public class ControlFlowUtil { } } - @Override public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); boolean unassigned = offset == flow.getSize() - 1 || !isLeaf(nextOffset) && maybeUnassigned[nextOffset]; @@ -830,21 +863,24 @@ public class ControlFlowUtil { maybeUnassigned[offset] |= unassigned; } - @Override public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { + @Override + public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { visitInstruction(instruction, offset, nextOffset); // clear return statements after procedure as well - for (int i = instruction.procBegin; i flow.getSize()) nextOffset = flow.getSize(); boolean unassigned = !isLeaf(nextOffset) && maybeUnassigned[nextOffset]; maybeUnassigned[offset] |= unassigned; } - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); boolean unassigned = isLeaf(nextOffset) || maybeUnassigned[nextOffset]; @@ -867,27 +903,31 @@ public class ControlFlowUtil { // true if from this point below there may be branch with variable assignment final boolean[] maybeAssigned = new boolean[flow.getSize() + 1]; - @Override public void visitWriteVariableInstruction(WriteVariableInstruction instruction, int offset, int nextOffset) { + @Override + public void visitWriteVariableInstruction(WriteVariableInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); boolean assigned = instruction.variable == variable || maybeAssigned[nextOffset]; maybeAssigned[offset] |= assigned; } - @Override public void visitThrowToInstruction(ThrowToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitThrowToInstruction(ThrowToInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); boolean assigned = !isLeaf(nextOffset) && maybeAssigned[nextOffset]; maybeAssigned[offset] |= assigned; } - @Override public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { + @Override + public void visitConditionalThrowToInstruction(ConditionalThrowToInstruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); int throwToOffset = instruction.offset; boolean assigned = throwToOffset == nextOffset ? !isLeaf(nextOffset) && maybeAssigned[nextOffset] : - maybeAssigned[nextOffset]; + maybeAssigned[nextOffset]; maybeAssigned[offset] |= assigned; } - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); boolean assigned = maybeAssigned[nextOffset]; @@ -914,7 +954,8 @@ public class ControlFlowUtil { // set of exit posint reached from this offset final TIntHashSet[] exitPoints = new TIntHashSet[flow.getSize()]; - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { if (nextOffset > flow.getSize()) nextOffset = flow.getSize(); if (exitPoints[offset] == null) { @@ -978,7 +1019,8 @@ public class ControlFlowUtil { synchronized (instructions) { final IntArrayList currentProcedureReturnOffsets = new IntArrayList(); ControlFlowInstructionVisitor getNextOffsetVisitor = new ControlFlowInstructionVisitor() { - @Override public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { + @Override + public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { instruction.execute(offset + 1); int newOffset = instruction.offset; // 'procedure' pointed by call instruction should be processed regardless of whether it was already visited or not @@ -997,7 +1039,8 @@ public class ControlFlowUtil { currentProcedureReturnOffsets.add(offset + 1); } - @Override public void visitReturnInstruction(ReturnInstruction instruction, int offset, int nextOffset) { + @Override + public void visitReturnInstruction(ReturnInstruction instruction, int offset, int nextOffset) { int newOffset = instruction.execute(false); if (newOffset != -1) { oldOffsets.add(offset); @@ -1008,7 +1051,8 @@ public class ControlFlowUtil { } } - @Override public void visitBranchingInstruction(BranchingInstruction instruction, int offset, int nextOffset) { + @Override + public void visitBranchingInstruction(BranchingInstruction instruction, int offset, int nextOffset) { int newOffset = instruction.offset; oldOffsets.add(offset); newOffsets.add(newOffset); @@ -1017,7 +1061,8 @@ public class ControlFlowUtil { newOffsets.add(-1); } - @Override public void visitConditionalBranchingInstruction(ConditionalBranchingInstruction instruction, int offset, int nextOffset) { + @Override + public void visitConditionalBranchingInstruction(ConditionalBranchingInstruction instruction, int offset, int nextOffset) { int newOffset = instruction.offset; oldOffsets.add(offset); @@ -1033,7 +1078,8 @@ public class ControlFlowUtil { newOffsets.add(-1); } - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { int newOffset = offset + 1; oldOffsets.add(offset); newOffsets.add(newOffset); @@ -1063,7 +1109,7 @@ public class ControlFlowUtil { } if (!currentProcedureReturnOffsets.isEmpty()) { int returnOffset = currentProcedureReturnOffsets.get(currentProcedureReturnOffsets.size() - 1); - CallInstruction callInstruction = (CallInstruction) instructions.get(returnOffset - 1); + CallInstruction callInstruction = (CallInstruction)instructions.get(returnOffset - 1); // check if we inside procedure but 'return offset' stack is empty, so // we should push back to 'return offset' stack synchronized (callInstruction.stack) { @@ -1099,6 +1145,7 @@ public class ControlFlowUtil { newList.list.add(value); return newList; } + public CopyOnWriteList remove(VariableInfo value) { CopyOnWriteList newList = new CopyOnWriteList(); List list = getList(); @@ -1116,8 +1163,17 @@ public class ControlFlowUtil { } public CopyOnWriteList() { - list = new LinkedList(); + this(Collections.emptyList()); } + + public CopyOnWriteList(VariableInfo... infos) { + this(Arrays.asList(infos)); + } + + public CopyOnWriteList(Collection infos) { + list = new LinkedList(infos); + } + public CopyOnWriteList addAll(CopyOnWriteList addList) { CopyOnWriteList newList = new CopyOnWriteList(); List list = getList(); @@ -1133,7 +1189,12 @@ public class ControlFlowUtil { } return newList; } + + public static CopyOnWriteList add(@Nullable CopyOnWriteList list, @NotNull VariableInfo value) { + return list == null ? new CopyOnWriteList(value) : list.add(value); + } } + public static class VariableInfo { private final PsiVariable variable; public final PsiElement expression; @@ -1151,20 +1212,23 @@ public class ControlFlowUtil { return variable.hashCode(); } } - private static void merge(int offset, CopyOnWriteList readVars, CopyOnWriteList[] readVariables) { - if (readVars != null) { - CopyOnWriteList existing = readVariables[offset]; - readVariables[offset] = existing == null ? readVars : existing.addAll(readVars); + + private static void merge(int offset, CopyOnWriteList source, CopyOnWriteList[] target) { + if (source != null) { + CopyOnWriteList existing = target[offset]; + target[offset] = existing == null ? source : existing.addAll(source); } } + /** - * @return list of PsiReferenceExpression of usages of non-initialized variables + * @return list of PsiReferenceExpression of usages of non-initialized local variables */ - public static List getReadBeforeWrite(final ControlFlow flow) { - InstructionClientVisitor> visitor = new ReadBeforeWriteClientVisitor(flow); + public static List getReadBeforeWrite(ControlFlow flow) { + final InstructionClientVisitor> visitor = new ReadBeforeWriteClientVisitor(flow); depthFirstSearch(flow, visitor); return visitor.getResult(); } + private static class ReadBeforeWriteClientVisitor extends InstructionClientVisitor> { // map of variable->PsiReferenceExpressions for all read before written variables for this point and below in control flow private final CopyOnWriteList[] readVariables; @@ -1172,47 +1236,49 @@ public class ControlFlowUtil { public ReadBeforeWriteClientVisitor(ControlFlow flow) { myFlow = flow; - readVariables = new CopyOnWriteList[myFlow.getSize()+1]; + readVariables = new CopyOnWriteList[myFlow.getSize() + 1]; } - @Override public void visitReadVariableInstruction(ReadVariableInstruction instruction, int offset, int nextOffset) { - if (nextOffset > myFlow.getSize()) nextOffset = myFlow.getSize(); - CopyOnWriteList readVars = readVariables[nextOffset]; - PsiElement element = myFlow.getElement(offset); + @Override + public void visitReadVariableInstruction(ReadVariableInstruction instruction, int offset, int nextOffset) { + CopyOnWriteList readVars = readVariables[Math.min(nextOffset, myFlow.getSize())]; final PsiVariable variable = instruction.variable; - if (!(variable instanceof PsiParameter) || ((PsiParameter)variable).getDeclarationScope() instanceof PsiForeachStatement) { - PsiReferenceExpression expression = getEnclosingReferenceExpression(element, variable); + if (!isMethodParameter(variable)) { + final PsiReferenceExpression expression = getEnclosingReferenceExpression(myFlow.getElement(offset), variable); if (expression != null) { - VariableInfo variableInfo = new VariableInfo(variable, expression); - if (readVars == null) { - readVars = new CopyOnWriteList(); - readVars.list.add(variableInfo); - } - else { - readVars = readVars.add(variableInfo); - } + readVars = CopyOnWriteList.add(readVars, new VariableInfo(variable, expression)); } } merge(offset, readVars, readVariables); } - @Override public void visitWriteVariableInstruction(WriteVariableInstruction instruction, int offset, int nextOffset) { - if (nextOffset > myFlow.getSize()) nextOffset = myFlow.getSize(); - CopyOnWriteList readVars = readVariables[nextOffset]; + @Override + public void visitWriteVariableInstruction(WriteVariableInstruction instruction, int offset, int nextOffset) { + CopyOnWriteList readVars = readVariables[Math.min(nextOffset, myFlow.getSize())]; + if (readVars == null) return; + final PsiVariable variable = instruction.variable; - if (readVars != null && (!(variable instanceof PsiParameter) || ((PsiParameter)variable).getDeclarationScope() instanceof PsiForeachStatement)) { + if (!isMethodParameter(variable)) { readVars = readVars.remove(new VariableInfo(variable, null)); } merge(offset, readVars, readVariables); } - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { - if (nextOffset > myFlow.getSize()) nextOffset = myFlow.getSize(); - CopyOnWriteList readVars = readVariables[nextOffset]; - merge(offset, readVars, readVariables); + private static boolean isMethodParameter(@NotNull PsiVariable variable) { + if (variable instanceof PsiParameter) { + final PsiParameter parameter = (PsiParameter)variable; + return !(parameter instanceof PsiForeachStatement); + } + return false; } - @Override public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + merge(offset, readVariables[Math.min(nextOffset, myFlow.getSize())], readVariables); + } + + @Override + public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) { visitInstruction(instruction, offset, nextOffset); for (int i = instruction.procBegin; i <= instruction.procEnd; i++) { readVariables[i] = null; @@ -1221,30 +1287,32 @@ public class ControlFlowUtil { @Override public List getResult() { - List problemsFound = new ArrayList(); - CopyOnWriteList topReadVariables = readVariables[0]; - if (topReadVariables != null) { - List list = topReadVariables.getList(); - for (final VariableInfo variableInfo : list) { - problemsFound.add((PsiReferenceExpression)variableInfo.expression); - } + final CopyOnWriteList topReadVariables = readVariables[0]; + if (topReadVariables == null) return Collections.emptyList(); + + final List result = new ArrayList(); + List list = topReadVariables.getList(); + for (final VariableInfo variableInfo : list) { + result.add((PsiReferenceExpression)variableInfo.expression); } - return problemsFound; + return result; } } public static final int NORMAL_COMPLETION_REASON = 1; public static final int RETURN_COMPLETION_REASON = 2; + /** * return reasons.normalCompletion when block can complete normally - * reasons.returnCalled when block can complete abruptly because of return statement executed + * reasons.returnCalled when block can complete abruptly because of return statement executed */ public static int getCompletionReasons(final ControlFlow flow, final int offset, final int endOffset) { class MyVisitor extends InstructionClientVisitor { final boolean[] normalCompletion = new boolean[endOffset]; final boolean[] returnCalled = new boolean[endOffset]; - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { boolean ret = nextOffset < endOffset && returnCalled[nextOffset]; boolean normal = nextOffset < endOffset && normalCompletion[nextOffset]; final PsiElement element = flow.getElement(offset); @@ -1302,62 +1370,63 @@ public class ControlFlowUtil { writtenTwiceVariables = new CopyOnWriteList[myFlow.getSize() + 1]; } - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { - if (nextOffset > myFlow.getSize()) nextOffset = myFlow.getSize(); + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + final int safeNextOffset = Math.min(nextOffset, myFlow.getSize()); - CopyOnWriteList writeVars = writtenVariables[nextOffset]; - CopyOnWriteList writeTwiceVars = writtenTwiceVariables[nextOffset]; + CopyOnWriteList writeVars = writtenVariables[safeNextOffset]; + CopyOnWriteList writeTwiceVars = writtenTwiceVariables[safeNextOffset]; if (instruction instanceof WriteVariableInstruction) { - final WriteVariableInstruction writeVariableInstruction = (WriteVariableInstruction)instruction; - final PsiVariable variable = writeVariableInstruction.variable; - final PsiElement element = myFlow.getElement(offset); + final PsiVariable variable = ((WriteVariableInstruction)instruction).variable; + + final PsiElement latestWriteVarExpression = getLatestWriteVarExpression(writeVars, variable); - PsiElement latestWriteVarExpression = null; - if (writeVars != null) { - List list = writeVars.getList(); - for (final VariableInfo variableInfo : list) { - if (variableInfo.variable == variable) { - latestWriteVarExpression = variableInfo.expression; - break; - } - } - } if (latestWriteVarExpression == null) { - PsiElement expression = null; - if (element instanceof PsiAssignmentExpression - && ((PsiAssignmentExpression)element).getLExpression() instanceof PsiReferenceExpression) { - expression = ((PsiAssignmentExpression)element).getLExpression(); - } - else if (element instanceof PsiPostfixExpression) { - expression = ((PsiPostfixExpression)element).getOperand(); - } - else if (element instanceof PsiPrefixExpression) { - expression = ((PsiPrefixExpression)element).getOperand(); - } - else if (element instanceof PsiDeclarationStatement) { - //should not happen - expression = element; - } - if (writeVars == null) { - writeVars = new CopyOnWriteList(); - } - writeVars = writeVars.add(new VariableInfo(variable, expression)); + final PsiElement expression = getExpression(myFlow.getElement(offset)); + writeVars = CopyOnWriteList.add(writeVars, new VariableInfo(variable, expression)); } else { - if (writeTwiceVars == null) { - writeTwiceVars = new CopyOnWriteList(); - } - writeTwiceVars = writeTwiceVars.add(new VariableInfo(variable, latestWriteVarExpression)); + writeTwiceVars = CopyOnWriteList.add(writeTwiceVars, new VariableInfo(variable, latestWriteVarExpression)); } } merge(offset, writeVars, writtenVariables); merge(offset, writeTwiceVars, writtenTwiceVariables); } + @Nullable + private static PsiElement getExpression(@NotNull PsiElement element) { + if (element instanceof PsiAssignmentExpression && ((PsiAssignmentExpression)element).getLExpression() instanceof PsiReferenceExpression) { + return ((PsiAssignmentExpression)element).getLExpression(); + } + else if (element instanceof PsiPostfixExpression) { + return ((PsiPostfixExpression)element).getOperand(); + } + else if (element instanceof PsiPrefixExpression) { + return ((PsiPrefixExpression)element).getOperand(); + } + else if (element instanceof PsiDeclarationStatement) { + //should not happen + return element; + } + return null; + } + + @Nullable + private static PsiElement getLatestWriteVarExpression(@Nullable CopyOnWriteList writeVars, @Nullable PsiVariable variable) { + if (writeVars == null) return null; + + for (final VariableInfo variableInfo : writeVars.getList()) { + if (variableInfo.variable == variable) { + return variableInfo.expression; + } + } + return null; + } + @Override @NotNull public Collection getResult() { - CopyOnWriteList writtenTwiceVariable = writtenTwiceVariables[myStartOffset]; + final CopyOnWriteList writtenTwiceVariable = writtenTwiceVariables[myStartOffset]; if (writtenTwiceVariable == null) return Collections.emptyList(); return writtenTwiceVariable.getList(); } @@ -1370,7 +1439,8 @@ public class ControlFlowUtil { class MyVisitor extends InstructionClientVisitor { boolean reachable; - @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { + @Override + public void visitInstruction(Instruction instruction, int offset, int nextOffset) { if (nextOffset == instructionOffset) reachable = true; } From 2968482da480cebbe892f3d08244705bc07cff5c Mon Sep 17 00:00:00 2001 From: Danila Ponomarenko Date: Fri, 18 May 2012 14:53:27 +0400 Subject: [PATCH 03/14] IDEA-15281 A method/constructor parameter which is written (before read) should be treated as redundant implemented --- .../analysis/HighlightControlFlowUtil.java | 4 +- .../BaseConvertToLocalQuickFix.java | 236 ++++++++++++++++++ .../FieldCanBeLocalInspection.java | 208 +++------------ .../ParameterCanBeLocalInspection.java | 148 +++++++++++ .../intellij/psi/controlFlow/DefUseUtil.java | 3 +- .../psi/controlFlow/ControlFlowUtil.java | 21 +- .../parameterCanBeLocal/for/expected.xml | 8 + .../parameterCanBeLocal/for/src/Test.java | 8 + .../parameterCanBeLocal/if/expected.xml | 8 + .../parameterCanBeLocal/if/src/Test.java | 13 + .../parameterCanBeLocal/simple/expected.xml | 8 + .../parameterCanBeLocal/simple/src/Test.java | 7 + .../afterFor.java | 9 + .../afterIf.java | 15 ++ .../afterSimple.java | 8 + .../beforeFor.java | 9 + .../beforeIf.java | 14 ++ .../beforeSimple.java | 8 + .../ConvertParameterToLocalVariableTest.java | 60 +++++ .../ParameterCanBeLocalTest.java | 36 +++ .../src/messages/InspectionsBundle.properties | 6 +- .../ParameterCanBeLocal.html | 9 + resources/src/META-INF/IdeaPlugin.xml | 4 + 23 files changed, 669 insertions(+), 181 deletions(-) create mode 100644 java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/BaseConvertToLocalQuickFix.java create mode 100644 java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/ParameterCanBeLocalInspection.java create mode 100644 java/java-tests/testData/inspection/parameterCanBeLocal/for/expected.xml create mode 100644 java/java-tests/testData/inspection/parameterCanBeLocal/for/src/Test.java create mode 100644 java/java-tests/testData/inspection/parameterCanBeLocal/if/expected.xml create mode 100644 java/java-tests/testData/inspection/parameterCanBeLocal/if/src/Test.java create mode 100644 java/java-tests/testData/inspection/parameterCanBeLocal/simple/expected.xml create mode 100644 java/java-tests/testData/inspection/parameterCanBeLocal/simple/src/Test.java create mode 100644 java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterFor.java create mode 100644 java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterIf.java create mode 100644 java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterSimple.java create mode 100644 java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeFor.java create mode 100644 java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeIf.java create mode 100644 java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/beforeSimple.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInspection/ConvertParameterToLocalVariableTest.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInspection/ParameterCanBeLocalTest.java create mode 100644 resources-en/src/inspectionDescriptions/ParameterCanBeLocal.html 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 From fc69d362428cbdac7b4c786863ecc5efb4b052e3 Mon Sep 17 00:00:00 2001 From: Danila Ponomarenko Date: Fri, 18 May 2012 19:00:55 +0400 Subject: [PATCH 04/14] IDEA-15281 tests fixed. All modifications moved inside writeAction --- .../BaseConvertToLocalQuickFix.java | 119 +++++++++++------- .../FieldCanBeLocalInspection.java | 10 -- .../ParameterCanBeLocalInspection.java | 7 +- .../afterFor.java | 2 +- .../afterIf.java | 4 +- .../afterSimple.java | 2 +- 6 files changed, 85 insertions(+), 59 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/BaseConvertToLocalQuickFix.java b/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/BaseConvertToLocalQuickFix.java index 4e6f592e2d53..149beec691dc 100644 --- a/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/BaseConvertToLocalQuickFix.java +++ b/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/BaseConvertToLocalQuickFix.java @@ -19,17 +19,20 @@ import com.intellij.codeInsight.CodeInsightUtil; import com.intellij.codeInspection.InspectionsBundle; import com.intellij.codeInspection.LocalQuickFix; import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.openapi.application.ApplicationManager; 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.openapi.util.Computable; 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.NotNullFunction; import com.intellij.util.containers.HashSet; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -42,7 +45,7 @@ import java.util.Set; * * @author Danila Ponomarenko */ -public abstract class BaseConvertToLocalQuickFix implements LocalQuickFix { +public abstract class BaseConvertToLocalQuickFix implements LocalQuickFix { private static final Logger LOG = Logger.getInstance(BaseConvertToLocalQuickFix.class); @NotNull @@ -51,12 +54,12 @@ public abstract class BaseConvertToLocalQuickFix implemen } 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 + final V variable = getVariable(descriptor); + final PsiFile myFile = variable.getContainingFile(); + if (variable == null || !variable.isValid()) return; //weird. should not get here when field becomes invalid try { - final PsiElement newDeclaration = moveDeclaration(project, myVariable); + final PsiElement newDeclaration = moveDeclaration(project, variable); if (newDeclaration == null) return; positionCaretToDeclaration(project, myFile, newDeclaration); @@ -67,11 +70,11 @@ public abstract class BaseConvertToLocalQuickFix implemen } @Nullable - protected abstract T getVariable(@NotNull ProblemDescriptor descriptor); + protected abstract V 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())) { + if (editor != null && (IJSwingUtilities.hasFocus(editor.getComponent()) || ApplicationManager.getApplication().isUnitTestMode())) { final PsiFile openedFile = PsiDocumentManager.getInstance(project).getPsiFile(editor.getDocument()); if (openedFile == psiFile) { editor.getCaretModel().moveToOffset(declaration.getTextOffset()); @@ -80,57 +83,87 @@ public abstract class BaseConvertToLocalQuickFix implemen } } - @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 V variable, @NotNull PsiElement 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; + private PsiElement moveDeclaration(@NotNull Project project, @NotNull V variable) { + final Collection references = ReferencesSearch.search(variable).findAll(); + if (references.isEmpty()) return null; - final PsiCodeBlock anchorBlock = findAnchorBlock(refs); + final PsiCodeBlock anchorBlock = findAnchorBlock(references); 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 firstElement = getLowestOffsetElement(references); + final String localName = suggestLocalName(project, variable, 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); + if (anchorAssignmentExpression != null && isVariableAssignment(anchorAssignmentExpression, variable)) { + final Set refsSet = new HashSet(references); refsSet.remove(anchorAssignmentExpression.getLExpression()); - retargetReferences(elementFactory, localName, refsSet); - return newDeclaration; + return applyChanges( + project, + localName, + anchorAssignmentExpression.getRExpression(), + variable, + refsSet, + new NotNullFunction() { + @NotNull + @Override + public PsiElement fun(PsiDeclarationStatement declaration) { + if (!mayBeFinal(firstElement, references)) { + PsiUtil.setModifierProperty((PsiModifierListOwner)declaration.getDeclaredElements()[0], PsiModifier.FINAL, false); + } + return anchor.replace(declaration); + } + } + ); } - final PsiDeclarationStatement declaration = elementFactory.createVariableDeclarationStatement(localName, myVariable.getType(), myVariable.getInitializer()); - final PsiElement newDeclaration = anchorBlock.addBefore(declaration, anchor); + return applyChanges( + project, + localName, + variable.getInitializer(), + variable, + references, + new NotNullFunction() { + @NotNull + @Override + public PsiElement fun(PsiDeclarationStatement declaration) { + return anchorBlock.addBefore(declaration, anchor); + } + } + ); + } - retargetReferences(elementFactory, localName, refs); - return newDeclaration; + @NotNull + private PsiElement applyChanges(final @NotNull Project project, + final @NotNull String localName, + final @Nullable PsiExpression initializer, + final @NotNull V variable, + final @NotNull Collection references, + final @NotNull NotNullFunction action) { + final PsiElementFactory elementFactory = JavaPsiFacade.getElementFactory(project); + + return ApplicationManager.getApplication().runWriteAction( + new Computable() { + @Override + public PsiElement compute() { + final PsiDeclarationStatement declaration = + elementFactory.createVariableDeclarationStatement(localName, variable.getType(), initializer); + final PsiElement newDeclaration = action.fun(declaration); + retargetReferences(elementFactory, localName, references); + beforeDelete(project, variable, newDeclaration); + variable.normalizeDeclaration(); + variable.delete(); + return newDeclaration; + } + } + ); } @Nullable @@ -167,7 +200,7 @@ public abstract class BaseConvertToLocalQuickFix implemen } @NotNull - protected abstract String suggestLocalName(@NotNull Project project, @NotNull T variable, @NotNull PsiCodeBlock scope); + protected abstract String suggestLocalName(@NotNull Project project, @NotNull V variable, @NotNull PsiCodeBlock scope); private static boolean mayBeFinal(PsiElement firstElement, @NotNull Collection references) { for (PsiReference reference : references) { 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 52c4135e682d..f9e84420a033 100644 --- a/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/FieldCanBeLocalInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/FieldCanBeLocalInspection.java @@ -16,21 +16,15 @@ package com.intellij.codeInspection.varScopeCanBeNarrowed; import com.intellij.codeInsight.AnnotationUtil; -import com.intellij.codeInsight.CodeInsightUtil; import com.intellij.codeInsight.daemon.GroupNames; import com.intellij.codeInsight.daemon.ImplicitUsageProvider; import com.intellij.codeInspection.InspectionsBundle; -import com.intellij.codeInspection.LocalQuickFix; import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.codeInspection.ProblemsHolder; import com.intellij.codeInspection.ex.BaseLocalInspectionTool; import com.intellij.codeInspection.util.SpecialAnnotationsUtil; import com.intellij.lang.java.JavaCommenter; -import com.intellij.openapi.diagnostic.Logger; -import com.intellij.openapi.editor.Editor; -import com.intellij.openapi.editor.ScrollType; import com.intellij.openapi.extensions.Extensions; -import com.intellij.openapi.fileEditor.FileEditorManager; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Comparing; import com.intellij.openapi.util.JDOMExternalizableStringList; @@ -41,13 +35,9 @@ import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.controlFlow.*; import com.intellij.psi.javadoc.PsiDocComment; -import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.refactoring.util.RefactoringUtil; -import com.intellij.util.IJSwingUtilities; -import com.intellij.util.IncorrectOperationException; -import com.intellij.util.containers.HashSet; import gnu.trove.THashSet; import org.jdom.Element; import org.jetbrains.annotations.NonNls; diff --git a/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/ParameterCanBeLocalInspection.java b/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/ParameterCanBeLocalInspection.java index 9d11afac789f..97b9855402fa 100644 --- a/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/ParameterCanBeLocalInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/ParameterCanBeLocalInspection.java @@ -80,7 +80,9 @@ public class ParameterCanBeLocalInspection extends BaseJavaLocalInspectionTool { @NotNull - private static ProblemDescriptor createProblem(@NotNull InspectionManager manager, @NotNull PsiIdentifier identifier, boolean isOnTheFly) { + 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"), @@ -126,7 +128,8 @@ public class ParameterCanBeLocalInspection extends BaseJavaLocalInspectionTool { @Nullable private static ControlFlow getControlFlow(final PsiElement context) { try { - return ControlFlowFactory.getInstance(context.getProject()).getControlFlow(context, LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance()); + return ControlFlowFactory.getInstance(context.getProject()) + .getControlFlow(context, LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance()); } catch (AnalysisCanceledException e) { return null; diff --git a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterFor.java b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterFor.java index 1635881830e0..ce8711eb6de5 100644 --- a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterFor.java +++ b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterFor.java @@ -2,7 +2,7 @@ class Temp { public Temp() { for (int i = 0; i < 10; i++) { - int p = i; + int p = i; System.out.print(p); } } diff --git a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterIf.java b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterIf.java index d5016105c40c..e6d8f7518700 100644 --- a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterIf.java +++ b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterIf.java @@ -3,8 +3,8 @@ class Temp { public boolean flag; void test() { - int p; - if (flag) { + int p; + if (flag) { p = 1; } else { diff --git a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterSimple.java b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterSimple.java index 1ace51e3ff57..f692ff1cafbc 100644 --- a/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterSimple.java +++ b/java/java-tests/testData/inspection/quickFix/ConvertParameterToLocalVariable/afterSimple.java @@ -2,7 +2,7 @@ class Temp { void test() { - int p = 1; + int p = 1; System.out.print(p); } } \ No newline at end of file From 52107cb04aa23aecb5a75fc31e2899a1e63d85aa Mon Sep 17 00:00:00 2001 From: "Maxim.Medvedev" Date: Fri, 18 May 2012 17:22:45 +0400 Subject: [PATCH 05/14] IDEA-86229 Groovy: good code is red: diamond operator is error highlighted (regression) --- .../plugins/groovy/annotator/GroovyAnnotator.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java index 1d0368b1bfff..08e9f15fe863 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java @@ -1001,16 +1001,17 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator { } private static boolean checkDiamonds(GrCodeReferenceElement refElement, AnnotationHolder holder) { - final GroovyConfigUtils configUtils = GroovyConfigUtils.getInstance(); - if (configUtils.isVersionAtLeast(refElement, GroovyConfigUtils.GROOVY1_8)) return true; - GrTypeArgumentList typeArgumentList = refElement.getTypeArgumentList(); - if (typeArgumentList != null && typeArgumentList.isDiamond()) { + if (typeArgumentList == null) return true; + + if (!typeArgumentList.isDiamond()) return true; + + final GroovyConfigUtils configUtils = GroovyConfigUtils.getInstance(); + if (!configUtils.isVersionAtLeast(refElement, GroovyConfigUtils.GROOVY1_8)) { final String message = GroovyBundle.message("diamonds.are.not.allowed.in.groovy.0", configUtils.getSDKVersion(refElement)); holder.createErrorAnnotation(typeArgumentList, message); - return false; } - return true; + return false; } @Override From 03b1227e050a684ccf29786442fefeb470fd790d Mon Sep 17 00:00:00 2001 From: "Maxim.Medvedev" Date: Fri, 18 May 2012 17:42:24 +0400 Subject: [PATCH 06/14] IDEA-86225 Groovy: Introduce/Create Variable in case label --- .../editor/actions/GStringTypedActionHandler.java | 8 ++++---- .../introduce/GrIntroduceHandlerBase.java | 6 +++++- .../introduceVariable/IntroduceVariableTest.java | 1 + .../refactoring/introduceVariable/caseLabel.test | 14 ++++++++++++++ 4 files changed, 24 insertions(+), 5 deletions(-) create mode 100644 plugins/groovy/testdata/groovy/refactoring/introduceVariable/caseLabel.test diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/editor/actions/GStringTypedActionHandler.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/editor/actions/GStringTypedActionHandler.java index 351b404cb936..21810b5bb013 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/editor/actions/GStringTypedActionHandler.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/editor/actions/GStringTypedActionHandler.java @@ -23,9 +23,9 @@ import com.intellij.openapi.editor.highlighter.EditorHighlighter; import com.intellij.openapi.editor.highlighter.HighlighterIterator; import com.intellij.openapi.project.Project; import com.intellij.psi.PsiFile; +import org.jetbrains.annotations.NotNull; import org.jetbrains.plugins.groovy.lang.editor.HandlerUtils; import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes; -import org.jetbrains.plugins.groovy.lang.parser.GroovyElementTypes; import org.jetbrains.plugins.groovy.lang.psi.GroovyFile; /** @@ -33,7 +33,7 @@ import org.jetbrains.plugins.groovy.lang.psi.GroovyFile; */ public class GStringTypedActionHandler extends TypedHandlerDelegate { @Override - public Result charTyped(char c, Project project, Editor editor, PsiFile file) { + public Result charTyped(char c, Project project, Editor editor, @NotNull PsiFile file) { if (c != '{' || project == null || !HandlerUtils.canBeInvoked(editor, project)) { return Result.CONTINUE; } @@ -45,9 +45,9 @@ public class GStringTypedActionHandler extends TypedHandlerDelegate { if (caret < 1) return Result.CONTINUE; HighlighterIterator iterator = highlighter.createIterator(caret - 1); - if (iterator.getTokenType() != GroovyElementTypes.mLCURLY) return Result.CONTINUE; + if (iterator.getTokenType() != GroovyTokenTypes.mLCURLY) return Result.CONTINUE; iterator.retreat(); - if (iterator.atEnd() || iterator.getTokenType() != GroovyElementTypes.mDOLLAR) return Result.CONTINUE; + if (iterator.atEnd() || iterator.getTokenType() != GroovyTokenTypes.mDOLLAR) return Result.CONTINUE; iterator.advance(); if (iterator.atEnd()) return Result.CONTINUE; iterator.advance(); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/GrIntroduceHandlerBase.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/GrIntroduceHandlerBase.java index 61cd64f3e1c8..01f208973634 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/GrIntroduceHandlerBase.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/GrIntroduceHandlerBase.java @@ -47,6 +47,7 @@ import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement; import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult; import org.jetbrains.plugins.groovy.lang.psi.api.statements.*; import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.clauses.GrCaseLabel; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.*; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.literals.GrStringInjection; import org.jetbrains.plugins.groovy.lang.psi.api.statements.params.GrParameter; @@ -412,7 +413,10 @@ public abstract class GrIntroduceHandlerBase1: println "111" + case xxx: println "xxx" + } +} +----- +def test(int p) { + def preved = 1 + switch (p) { + case preved: println "111" + case xxx: println "xxx" + } +} \ No newline at end of file From f5d1ee6b5706e02c2c753ac071cbc4f8dec654cd Mon Sep 17 00:00:00 2001 From: "Maxim.Medvedev" Date: Fri, 18 May 2012 18:47:19 +0400 Subject: [PATCH 07/14] IDEA-86221 Create parameter from usage in a context with no expected type --- .../CreateParameterFromUsageFix.java | 14 +++++---- .../GroovyExpectedTypesProvider.java | 31 +++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateParameterFromUsageFix.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateParameterFromUsageFix.java index 8d745b479c01..33d5876beb45 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateParameterFromUsageFix.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateParameterFromUsageFix.java @@ -21,10 +21,7 @@ import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; import com.intellij.openapi.ui.popup.JBPopup; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiFile; -import com.intellij.psi.PsiMethod; -import com.intellij.psi.PsiType; +import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiTypesUtil; import com.intellij.refactoring.RefactoringBundle; @@ -141,8 +138,13 @@ public class CreateParameterFromUsageFix implements IntentionAction, MethodOrClo final String name = ref.getName(); final Set types = GroovyExpectedTypesProvider.getDefaultExpectedTypes(ref); - PsiType _type = types.iterator().next(); - final PsiType type = TypesUtil.unboxPrimitiveTypeWrapper(_type); + final PsiType type; + if (types.isEmpty()) { + type = PsiType.getJavaLangObject(PsiManager.getInstance(project), ref.getResolveScope()); + } + else { + type = TypesUtil.unboxPrimitiveTypeWrapper(types.iterator().next()); + } if (method instanceof GrMethod) { new GrChangeSignatureDialog(project, (GrMethod)method) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/expectedTypes/GroovyExpectedTypesProvider.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/expectedTypes/GroovyExpectedTypesProvider.java index 50b5bac05291..e33b7c2684c0 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/expectedTypes/GroovyExpectedTypesProvider.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/expectedTypes/GroovyExpectedTypesProvider.java @@ -36,6 +36,8 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlo import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrOpenBlock; import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrReturnStatement; import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrThrowStatement; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.clauses.GrCaseLabel; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.clauses.GrCaseSection; import org.jetbrains.plugins.groovy.lang.psi.api.statements.clauses.GrTraditionalForClause; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.*; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrMethodCallExpression; @@ -391,6 +393,35 @@ public class GroovyExpectedTypesProvider { } } + @Override + public void visitCaseLabel(GrCaseLabel caseLabel) { + final PsiElement parent = caseLabel.getParent().getParent(); + assert parent instanceof GrSwitchStatement; + final GrExpression condition = ((GrSwitchStatement)parent).getCondition(); + if (condition == null) return; + + final PsiType type = condition.getType(); + if (type == null) return; + + myResult = new TypeConstraint[]{SubtypeConstraint.create(type)}; + } + + @Override + public void visitSwitchStatement(GrSwitchStatement switchStatement) { + final GrCaseSection[] sections = switchStatement.getCaseSections(); + List types = new ArrayList(sections.length); + for (GrCaseSection section : sections) { + final GrExpression value = section.getCaseLabel().getValue(); + final PsiType type = value != null ? value.getType() : null; + if (type != null) types.add(type); + } + + final PsiType upperBoundNullable = TypesUtil.getLeastUpperBoundNullable(types, switchStatement.getManager()); + if (upperBoundNullable == null) return; + + myResult = new TypeConstraint[]{SubtypeConstraint.create(upperBoundNullable)}; + } + public TypeConstraint[] getResult() { return myResult; } From 404cdeb0cd8151d12dd8cc31c2e7bcbc8796937c Mon Sep 17 00:00:00 2001 From: "Maxim.Medvedev" Date: Fri, 18 May 2012 19:34:40 +0400 Subject: [PATCH 08/14] IDEA-86216 Groovy: Create From Usage: 'Create Interface' quick fix is missing for unresolved data type --- .../plugins/groovy/GroovyBundle.properties | 6 ++- .../groovy/annotator/GroovyAnnotator.java | 24 ++++++++++- .../intentions/CreateClassActionBase.java | 42 ++++++++++++------- .../annotator/intentions/CreateClassFix.java | 28 +++++++++---- .../intentions/GroovyCreateClassDialog.java | 2 +- .../api/statements/typedef/GrMemberOwner.java | 3 +- 6 files changed, 77 insertions(+), 28 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties b/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties index 8e7de64218df..21fc4b593aaa 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties @@ -122,8 +122,8 @@ field.already.defined=Field ''{0}'' already defined import.what=Import ''{0}'' import.class=Import Class create.class.family.name=Create Class -create.class.text=Create Class ''{0}'' -create.interface.text=Create Interface ''{0}'' +create.class.text=Create Class {0} +create.interface.text=Create Interface {0} static.declaration.in.inner.class=Inner classes cannot have static declarations constructors.are.not.allowed.in.anonymous.class=Constructors are not allowed in anonymous class no.such.property=Property ''{0}'' does not exist @@ -311,3 +311,5 @@ type.argument.0.is.not.in.its.bound.should.extend.1=Type parameter ''{0}'' is no catch.statement.parameter.type.should.be.a.subclass.of.throwable=Catch statement parameter type should be a subclass of Throwable exception.0.has.already.been.caught=Exception ''{0}'' has already been caught unnecessary.type=Unnecessary exception ''{0}''. ''{1}'' is already declared +create.enum=Create Enum {0} +create.inner.class=Create Inner Class {0} diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java index 08e9f15fe863..1bcc53574892 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java @@ -1952,11 +1952,33 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator { annotation.registerFix(CreateClassFix.createClassFromNewAction((GrNewExpression)parent)); } else { - annotation.registerFix(CreateClassFix.createClassFixAction(refElement)); + if (shouldBeInterface(refElement)) { + annotation.registerFix(CreateClassFix.createClassFixAction(refElement, CreateClassActionBase.Type.INTERFACE)); + } + else if (shouldBeClass(refElement)) { + annotation.registerFix(CreateClassFix.createClassFixAction(refElement, CreateClassActionBase.Type.CLASS)); + annotation.registerFix(CreateClassFix.createClassFixAction(refElement, CreateClassActionBase.Type.ENUM)); + } + else { + annotation.registerFix(CreateClassFix.createClassFixAction(refElement, CreateClassActionBase.Type.CLASS)); + annotation.registerFix(CreateClassFix.createClassFixAction(refElement, CreateClassActionBase.Type.INTERFACE)); + annotation.registerFix(CreateClassFix.createClassFixAction(refElement, CreateClassActionBase.Type.ENUM)); + } } } } + private static boolean shouldBeInterface(GrReferenceElement myRefElement) { + PsiElement parent = myRefElement.getParent(); + return parent instanceof GrImplementsClause || parent instanceof GrExtendsClause && parent.getParent() instanceof GrInterfaceDefinition; + } + + private static boolean shouldBeClass(GrReferenceElement myRefElement) { + PsiElement parent = myRefElement.getParent(); + return parent instanceof GrExtendsClause && !(parent.getParent() instanceof GrInterfaceDefinition); + } + + private static void highlightMember(AnnotationHolder holder, GrMember member) { if (member instanceof GrField) { GrField field = (GrField)member; diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateClassActionBase.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateClassActionBase.java index 5d6d8c86d687..07822de94efb 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateClassActionBase.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateClassActionBase.java @@ -34,26 +34,35 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.GroovyBundle; import org.jetbrains.plugins.groovy.actions.GroovyTemplatesFactory; import org.jetbrains.plugins.groovy.lang.psi.GrReferenceElement; -import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrExtendsClause; -import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrImplementsClause; -import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrInterfaceDefinition; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition; /** * @author ilyas */ public abstract class CreateClassActionBase implements IntentionAction { + private Type myType; + protected final GrReferenceElement myRefElement; private static final Logger LOG = Logger.getInstance("#org.jetbrains.plugins.groovy.annotator.intentions.CreateClassActionBase"); - public CreateClassActionBase(GrReferenceElement refElement) { + public CreateClassActionBase(Type type, GrReferenceElement refElement) { + myType = type; myRefElement = refElement; } @NotNull public String getText() { String referenceName = myRefElement.getReferenceName(); - return shouldCreateInterface() ? GroovyBundle.message("create.interface.text", referenceName) : GroovyBundle.message("create.class.text", referenceName); + switch (getType()) { + case ENUM: + return GroovyBundle.message("create.enum", referenceName); + case CLASS: + return GroovyBundle.message("create.class.text", referenceName); + case INTERFACE: + return GroovyBundle.message("create.interface.text", referenceName); + default: + return ""; + } } @NotNull @@ -69,17 +78,17 @@ public abstract class CreateClassActionBase implements IntentionAction { return true; } - protected boolean shouldCreateInterface() { - PsiElement parent = myRefElement.getParent(); - return parent instanceof GrImplementsClause || parent instanceof GrExtendsClause && parent.getParent() instanceof GrInterfaceDefinition; + + protected Type getType() { + return myType; } @Nullable - public static GrTypeDefinition createClassByType(@NotNull final PsiDirectory directory, - @NotNull final String name, - @NotNull final PsiManager manager, - @Nullable final PsiElement contextElement, - @NotNull final String templateName) { + public static GrTypeDefinition createClassByType(@NotNull final PsiDirectory directory, + @NotNull final String name, + @NotNull final PsiManager manager, + @Nullable final PsiElement contextElement, + @NotNull final String templateName) { AccessToken accessToken = WriteAction.start(); try { @@ -130,8 +139,13 @@ public abstract class CreateClassActionBase implements IntentionAction { if (virtualFile != null) { OpenFileDescriptor descriptor = new OpenFileDescriptor(project, virtualFile, textOffset); return FileEditorManager.getInstance(project).openTextEditor(descriptor, true); - } else { + } + else { return null; } } + + public static enum Type { + ENUM, CLASS, INTERFACE + } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateClassFix.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateClassFix.java index dcc285a8e05b..c7a44098fe4b 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateClassFix.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/CreateClassFix.java @@ -27,7 +27,6 @@ import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.IncorrectOperationException; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.plugins.groovy.GroovyBundle; import org.jetbrains.plugins.groovy.actions.NewGroovyClassAction; import org.jetbrains.plugins.groovy.intentions.base.IntentionUtils; import org.jetbrains.plugins.groovy.lang.editor.template.expressions.ChooseTypeExpression; @@ -50,7 +49,7 @@ import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; public abstract class CreateClassFix { public static IntentionAction createClassFromNewAction(final GrNewExpression expression) { - return new CreateClassActionBase(expression.getReferenceElement()) { + return new CreateClassActionBase(CreateClassActionBase.Type.CLASS, expression.getReferenceElement()) { public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException { if (!(file instanceof GroovyFileBase)) return; @@ -60,7 +59,7 @@ public abstract class CreateClassFix { final String name = myRefElement.getReferenceName(); assert name != null; final Module module = ModuleUtil.findModuleForPsiElement(file); - PsiDirectory targetDirectory = getTargetDirectory(project, qualifier, name, module); + PsiDirectory targetDirectory = getTargetDirectory(project, qualifier, name, module, getText()); if (targetDirectory == null) return; GrTypeDefinition targetClass = createClassByType(targetDirectory, name, manager, myRefElement, NewGroovyClassAction.GROOVY_CLASS); @@ -96,8 +95,8 @@ public abstract class CreateClassFix { }; } - public static IntentionAction createClassFixAction(final GrReferenceElement refElement) { - return new CreateClassActionBase(refElement) { + public static IntentionAction createClassFixAction(final GrReferenceElement refElement, CreateClassActionBase.Type type) { + return new CreateClassActionBase(type, refElement) { public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException { if (!(file instanceof GroovyFileBase)) return; @@ -106,10 +105,22 @@ public abstract class CreateClassFix { final PsiManager manager = PsiManager.getInstance(project); final String name = myRefElement.getReferenceName(); final Module module = ModuleUtil.findModuleForPsiElement(file); - PsiDirectory targetDirectory = getTargetDirectory(project, qualifier, name, module); + PsiDirectory targetDirectory = getTargetDirectory(project, qualifier, name, module, getText()); if (targetDirectory == null) return; - String templateName = shouldCreateInterface() ? NewGroovyClassAction.GROOVY_INTERFACE : NewGroovyClassAction.GROOVY_CLASS; + + String templateName = null; + switch (getType()) { + case ENUM: + templateName = NewGroovyClassAction.GROOVY_ENUM; + break; + case CLASS: + templateName = NewGroovyClassAction.GROOVY_CLASS; + break; + case INTERFACE: + templateName = NewGroovyClassAction.GROOVY_INTERFACE; + break; + } assert name != null; PsiClass targetClass = createClassByType(targetDirectory, name, manager, myRefElement, templateName); if (targetClass != null) { @@ -122,8 +133,7 @@ public abstract class CreateClassFix { } @Nullable - private static PsiDirectory getTargetDirectory(Project project, String qualifier, String name, Module module) { - String title = GroovyBundle.message("create.class.family.name"); + private static PsiDirectory getTargetDirectory(Project project, String qualifier, String name, Module module, String title) { GroovyCreateClassDialog dialog = new GroovyCreateClassDialog(project, title, name, qualifier, module); dialog.show(); if (dialog.getExitCode() != DialogWrapper.OK_EXIT_CODE) return null; diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/GroovyCreateClassDialog.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/GroovyCreateClassDialog.java index 7caa247467ce..781e1963eac4 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/GroovyCreateClassDialog.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/intentions/GroovyCreateClassDialog.java @@ -67,7 +67,7 @@ public class GroovyCreateClassDialog extends DialogWrapper { setModal(true); setTitle(title); - myInformationLabel.setText(GroovyInspectionBundle.message("dialog.create.class.label.0", targetClassName)); + myInformationLabel.setText(title); myPackageTextField.setText(targetPackageName != null ? targetPackageName : ""); init(); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/statements/typedef/GrMemberOwner.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/statements/typedef/GrMemberOwner.java index f2256fc8fa5f..dd269d9f46c2 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/statements/typedef/GrMemberOwner.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/statements/typedef/GrMemberOwner.java @@ -19,12 +19,13 @@ package org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef; import com.intellij.psi.PsiClass; import com.intellij.psi.PsiElement; import com.intellij.util.IncorrectOperationException; +import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMembersDeclaration; /** * @author ilyas */ public interface GrMemberOwner extends PsiClass { - T addMemberDeclaration(T decl, PsiElement anchorBefore) throws IncorrectOperationException ; + T addMemberDeclaration(T decl, @Nullable PsiElement anchorBefore) throws IncorrectOperationException ; } \ No newline at end of file From 3742031b2547005eae06b90c1beae00b16108cf6 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Fri, 18 May 2012 19:45:29 +0400 Subject: [PATCH 09/14] IDEA-55075 git annotate: ignore whitespace change. Also change annotate to blame: they are equal, but blame is preferable. --- .../git4idea/src/git4idea/annotate/GitAnnotationProvider.java | 4 ++-- plugins/git4idea/src/git4idea/commands/GitCommand.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/git4idea/src/git4idea/annotate/GitAnnotationProvider.java b/plugins/git4idea/src/git4idea/annotate/GitAnnotationProvider.java index d9490caf578d..82a261cdc3af 100644 --- a/plugins/git4idea/src/git4idea/annotate/GitAnnotationProvider.java +++ b/plugins/git4idea/src/git4idea/annotate/GitAnnotationProvider.java @@ -145,11 +145,11 @@ public class GitAnnotationProvider implements AnnotationProvider, VcsCacheableAn final VcsFileRevision revision, final List revisions, final VirtualFile file) throws VcsException { - GitSimpleHandler h = new GitSimpleHandler(myProject, GitUtil.getGitRoot(repositoryFilePath), GitCommand.ANNOTATE); + GitSimpleHandler h = new GitSimpleHandler(myProject, GitUtil.getGitRoot(repositoryFilePath), GitCommand.BLAME); h.setNoSSH(true); h.setStdoutSuppressed(true); h.setCharset(file.getCharset()); - h.addParameters("-p", "-l", "-t"); + h.addParameters("-p", "-l", "-t", "-w"); if (revision == null) { h.addParameters("HEAD"); } diff --git a/plugins/git4idea/src/git4idea/commands/GitCommand.java b/plugins/git4idea/src/git4idea/commands/GitCommand.java index 302caeccc76b..2528424c8d1a 100644 --- a/plugins/git4idea/src/git4idea/commands/GitCommand.java +++ b/plugins/git4idea/src/git4idea/commands/GitCommand.java @@ -35,7 +35,7 @@ import org.jetbrains.annotations.NotNull; public class GitCommand { public static final GitCommand ADD = write("add"); - public static final GitCommand ANNOTATE = read("annotate"); + public static final GitCommand BLAME = read("blame"); public static final GitCommand BRANCH = read("branch"); public static final GitCommand CHECKOUT = write("checkout"); public static final GitCommand COMMIT = write("commit"); From 51127d1b1cff30083f1e1d27e703a8aaee81847b Mon Sep 17 00:00:00 2001 From: Maxim Shafirov Date: Fri, 18 May 2012 19:49:19 +0400 Subject: [PATCH 10/14] IDEA-86208 --- .../openapi/keymap/impl/ui/ActionsTree.java | 46 +++++++++---------- .../openapi/keymap/impl/ui/Group.java | 27 +++++++++-- 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/platform/platform-impl/src/com/intellij/openapi/keymap/impl/ui/ActionsTree.java b/platform/platform-impl/src/com/intellij/openapi/keymap/impl/ui/ActionsTree.java index f01b53da5585..c8dc8ad532d9 100644 --- a/platform/platform-impl/src/com/intellij/openapi/keymap/impl/ui/ActionsTree.java +++ b/platform/platform-impl/src/com/intellij/openapi/keymap/impl/ui/ActionsTree.java @@ -28,13 +28,13 @@ import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Comparing; import com.intellij.openapi.util.IconLoader; import com.intellij.openapi.util.registry.Registry; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.ui.ColoredTreeCellRenderer; import com.intellij.ui.Gray; import com.intellij.ui.LayeredIcon; import com.intellij.ui.ScrollPaneFactory; import com.intellij.ui.treeStructure.Tree; import com.intellij.ui.treeStructure.treetable.TreeTableModel; -import com.intellij.util.Alarm; import com.intellij.util.ui.EmptyIcon; import com.intellij.util.ui.UIUtil; import com.intellij.util.ui.tree.MacTreeUI; @@ -301,14 +301,8 @@ public class ActionsTree { if (node == null) { return; } - tree.expandPath(new TreePath(((DefaultMutableTreeNode)node.getParent()).getPath())); - Alarm alarm = new Alarm(); - alarm.addRequest(new Runnable() { - public void run() { - TreeUtil.selectPath(myTree, new TreePath(node.getPath())); - } - }, 100); + TreeUtil.selectInTree(node, true, tree); } @Nullable @@ -383,24 +377,31 @@ public class ActionsTree { TreePath path = new TreePath(root.getPath()); if (myTree.isPathSelected(path)){ - mySelectionPaths.add(getPath(root)); + addPathToList(root, mySelectionPaths); } if (myTree.isExpanded(path) || root.getChildCount() == 0){ - myPathsToExpand.add(getPath(root)); + addPathToList(root, myPathsToExpand); _storePaths(root); } } + private void addPathToList(DefaultMutableTreeNode root, ArrayList list) { + String path = getPath(root); + if (!StringUtil.isEmpty(path)) { + list.add(path); + } + } + private void _storePaths(DefaultMutableTreeNode root) { ArrayList childNodes = childrenToArray(root); for (final Object childNode1 : childNodes) { DefaultMutableTreeNode childNode = (DefaultMutableTreeNode)childNode1; TreePath path = new TreePath(childNode.getPath()); if (myTree.isPathSelected(path)) { - mySelectionPaths.add(getPath(childNode)); + addPathToList(childNode, mySelectionPaths); } if ((myTree.isExpanded(path) || childNode.getChildCount() == 0) && !childNode.isLeaf()) { - myPathsToExpand.add(getPath(childNode)); + addPathToList(childNode, myPathsToExpand); _storePaths(childNode); } } @@ -412,20 +413,17 @@ public class ActionsTree { myTree.expandPath(new TreePath(node.getPath())); } - Alarm alarm = new Alarm().setActivationComponent(myComponent); - alarm.addComponentRequest(new Runnable() { - public void run() { - final ArrayList nodesToSelect = getNodesByPaths(mySelectionPaths); - if (!nodesToSelect.isEmpty()) { - for (DefaultMutableTreeNode node : nodesToSelect) { - TreeUtil.selectNode(myTree, node); - } - } - else { - myTree.setSelectionRow(0); + if (myTree.getSelectionModel().getSelectionCount() == 0) { + final ArrayList nodesToSelect = getNodesByPaths(mySelectionPaths); + if (!nodesToSelect.isEmpty()) { + for (DefaultMutableTreeNode node : nodesToSelect) { + TreeUtil.selectInTree(node, false, myTree); } } - }, 100); + else { + myTree.setSelectionRow(0); + } + } } diff --git a/platform/platform-impl/src/com/intellij/openapi/keymap/impl/ui/Group.java b/platform/platform-impl/src/com/intellij/openapi/keymap/impl/ui/Group.java index 0ad016aa14aa..cc2202cb9dd0 100644 --- a/platform/platform-impl/src/com/intellij/openapi/keymap/impl/ui/Group.java +++ b/platform/platform-impl/src/com/intellij/openapi/keymap/impl/ui/Group.java @@ -17,8 +17,8 @@ package com.intellij.openapi.keymap.impl.ui; import com.intellij.openapi.actionSystem.*; import com.intellij.openapi.actionSystem.ex.QuickList; -import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.keymap.KeymapGroup; +import com.intellij.openapi.util.text.StringUtil; import javax.swing.*; import java.util.ArrayList; @@ -136,6 +136,24 @@ public class Group implements KeymapGroup { } public String getActionQualifiedPath(String id) { + Group cur = myParent; + StringBuilder answer = new StringBuilder(); + + while (cur != null && !cur.isRoot()) { + answer.insert(0, cur.getName() + " | "); + + cur = cur.myParent; + } + + String suffix = calcActionQualifiedPath(id); + if (StringUtil.isEmpty(suffix)) return null; + + answer.append(suffix); + + return answer.toString(); + } + + private String calcActionQualifiedPath(String id) { for (Object child : myChildren) { if (child instanceof QuickList) { child = ((QuickList)child).getActionId(); @@ -154,7 +172,7 @@ public class Group implements KeymapGroup { } } else if (child instanceof Group) { - String path = ((Group)child).getActionQualifiedPath(id); + String path = ((Group)child).calcActionQualifiedPath(id); if (path != null) { return !isRoot() ? getName() + " | " + path : path; } @@ -168,10 +186,11 @@ public class Group implements KeymapGroup { } public String getQualifiedPath() { - StringBuffer path = new StringBuffer(64); + StringBuilder path = new StringBuilder(64); Group group = this; while (group != null && !group.isRoot()) { - path.insert(0, group.getName() + " | "); + if (path.length() > 0) path.insert(0, " | "); + path.insert(0, group.getName()); group = group.myParent; } return path.toString(); From 39a2ca4115d67ce700dafcb8fbc15c029f1123ed Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Fri, 18 May 2012 19:58:05 +0400 Subject: [PATCH 11/14] ability to create psitype with jvmfactory --- .../codeInsight/generation/GenerateMembersUtil.java | 2 +- .../src/com/intellij/psi/JVMElementFactory.java | 9 +++++++++ .../src/com/intellij/psi/PsiElementFactory.java | 8 -------- .../lang/psi/impl/GroovyPsiElementFactoryImpl.java | 6 ++++++ 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInsight/generation/GenerateMembersUtil.java b/java/java-impl/src/com/intellij/codeInsight/generation/GenerateMembersUtil.java index 13f3d21348dd..337db3857199 100644 --- a/java/java-impl/src/com/intellij/codeInsight/generation/GenerateMembersUtil.java +++ b/java/java-impl/src/com/intellij/codeInsight/generation/GenerateMembersUtil.java @@ -320,7 +320,7 @@ public class GenerateMembersUtil { super.visitReferenceElement(reference); final PsiElement resolve = reference.resolve(); if (resolve instanceof PsiTypeParameter) { - replacementMap.put(reference, factory.createReferenceElementByType((PsiClassType)substitutor.substitute((PsiTypeParameter)resolve))); + replacementMap.put(reference, factory.createReferenceElementByType((PsiClassType)substituteType(substitutor, factory.createType((PsiTypeParameter)resolve)))); } } }); diff --git a/java/java-psi-api/src/com/intellij/psi/JVMElementFactory.java b/java/java-psi-api/src/com/intellij/psi/JVMElementFactory.java index 3e6487879d67..ff2dcc2b1ec9 100644 --- a/java/java-psi-api/src/com/intellij/psi/JVMElementFactory.java +++ b/java/java-psi-api/src/com/intellij/psi/JVMElementFactory.java @@ -136,4 +136,13 @@ public interface JVMElementFactory { @NotNull PsiTypeParameter createTypeParameter(String name, PsiClassType[] superTypes); + + /** + * Creates a class type for the specified class. + * + * @param aClass the class for which the class type is created. + * @return the class type instance. + */ + @NotNull + PsiClassType createType(@NotNull PsiClass aClass); } diff --git a/java/java-psi-api/src/com/intellij/psi/PsiElementFactory.java b/java/java-psi-api/src/com/intellij/psi/PsiElementFactory.java index 333b7b9e94ab..c90c49489137 100644 --- a/java/java-psi-api/src/com/intellij/psi/PsiElementFactory.java +++ b/java/java-psi-api/src/com/intellij/psi/PsiElementFactory.java @@ -154,14 +154,6 @@ public interface PsiElementFactory extends PsiJavaParserFacade, JVMElementFactor */ @NotNull PsiCodeBlock createCodeBlock(); - /** - * Creates a class type for the specified class. - * - * @param aClass the class for which the class type is created. - * @return the class type instance. - */ - @NotNull PsiClassType createType(@NotNull PsiClass aClass); - /** * Creates a class type for the specified class, using the specified substitutor * to replace generic type parameters on the class. diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyPsiElementFactoryImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyPsiElementFactoryImpl.java index eb22d3caf49b..f9a4f0e8e667 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyPsiElementFactoryImpl.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyPsiElementFactoryImpl.java @@ -396,6 +396,12 @@ public class GroovyPsiElementFactoryImpl extends GroovyPsiElementFactory { return createTypeElement(typeText); } + @NotNull + @Override + public PsiClassType createType(@NotNull PsiClass aClass) { + return JavaPsiFacade.getElementFactory(myProject).createType(aClass); + } + public GrParenthesizedExpression createParenthesizedExpr(GrExpression newExpr) { return ((GrParenthesizedExpression) getInstance(myProject).createExpressionFromText("(" + newExpr.getText() + ")")); } From 5c4a140ef5076c9e7235f9a5ca244e600cc9b91b Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Fri, 18 May 2012 16:18:04 +0400 Subject: [PATCH 12/14] deprecated --- .../src/com/intellij/openapi/application/Application.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/platform/core-api/src/com/intellij/openapi/application/Application.java b/platform/core-api/src/com/intellij/openapi/application/Application.java index 78c74cf74b3e..f14d3344675a 100644 --- a/platform/core-api/src/com/intellij/openapi/application/Application.java +++ b/platform/core-api/src/com/intellij/openapi/application/Application.java @@ -324,11 +324,13 @@ public interface Application extends ComponentManager { boolean isActive(); /** + * @deprecated use {@link #runReadAction(Runnable)} instead * Returns lock used for read operations, should be closed in finally block */ AccessToken acquireReadActionLock(); /** + * @deprecated use {@link #runWriteAction(Runnable)} instead * Returns lock used for write operations, should be closed in finally block */ AccessToken acquireWriteActionLock(@Nullable Class marker); From a539eab1c00efd1f946b1afb1d6d5b87cf17e347 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Fri, 18 May 2012 16:19:15 +0400 Subject: [PATCH 13/14] cleanup --- .../roots/impl/ModuleLibraryTable.java | 20 +++++++++++++++++++ .../conversion/EclipseClasspathReader.java | 9 +++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ModuleLibraryTable.java b/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ModuleLibraryTable.java index 537e1607f1ec..4dbeeb3d5199 100644 --- a/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ModuleLibraryTable.java +++ b/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ModuleLibraryTable.java @@ -44,14 +44,17 @@ public class ModuleLibraryTable implements LibraryTable, LibraryTableBase.Modifi private final RootModelImpl myRootModel; private final ProjectRootManagerImpl myProjectRootManager; public static final LibraryTablePresentation MODULE_LIBRARY_TABLE_PRESENTATION = new LibraryTablePresentation() { + @Override public String getDisplayName(boolean plural) { return ProjectBundle.message("module.library.display.name", plural ? 2 : 1); } + @Override public String getDescription() { return ProjectBundle.message("libraries.node.text.module"); } + @Override public String getLibraryTableEditorTitle() { return ProjectBundle.message("library.configure.module.title"); } @@ -62,6 +65,7 @@ public class ModuleLibraryTable implements LibraryTable, LibraryTableBase.Modifi myProjectRootManager = projectRootManager; } + @Override @NotNull public Library[] getLibraries() { final ArrayList result = new ArrayList(); @@ -70,10 +74,12 @@ public class ModuleLibraryTable implements LibraryTable, LibraryTableBase.Modifi return result.toArray(new Library[result.size()]); } + @Override public Library createLibrary() { return createLibrary(null); } + @Override public Library createLibrary(String name) { return createLibrary(name, null); } @@ -85,6 +91,7 @@ public class ModuleLibraryTable implements LibraryTable, LibraryTableBase.Modifi return orderEntry.getLibrary(); } + @Override public void removeLibrary(@NotNull Library library) { final Iterator orderIterator = myRootModel.getOrderIterator(); while (orderIterator.hasNext()) { @@ -102,6 +109,7 @@ public class ModuleLibraryTable implements LibraryTable, LibraryTableBase.Modifi } } + @Override @NotNull public Iterator getLibraryIterator() { FilteringIterator filteringIterator = @@ -109,18 +117,22 @@ public class ModuleLibraryTable implements LibraryTable, LibraryTableBase.Modifi return new ConvertingIterator(filteringIterator, ORDER_ENTRY_TO_LIBRARY_CONVERTOR); } + @Override public String getTableLevel() { return LibraryTableImplUtil.MODULE_LEVEL; } + @Override public LibraryTablePresentation getPresentation() { return MODULE_LIBRARY_TABLE_PRESENTATION; } + @Override public boolean isEditable() { return true; } + @Override @Nullable public Library getLibraryByName(@NotNull String name) { final Iterator libraryIterator = getLibraryIterator(); @@ -131,14 +143,17 @@ public class ModuleLibraryTable implements LibraryTable, LibraryTableBase.Modifi return null; } + @Override public void addListener(Listener listener) { throw new UnsupportedOperationException(); } + @Override public void addListener(Listener listener, Disposable parentDisposable) { throw new UnsupportedOperationException("Method addListener is not yet implemented in " + getClass().getName()); } + @Override public void removeListener(Listener listener) { throw new UnsupportedOperationException(); } @@ -149,24 +164,29 @@ public class ModuleLibraryTable implements LibraryTable, LibraryTableBase.Modifi private static class ModuleLibraryOrderEntryCondition implements Condition { + @Override public boolean value(OrderEntry entry) { return entry instanceof LibraryOrderEntry && ((LibraryOrderEntry)entry).isModuleLevel() && ((LibraryOrderEntry)entry).getLibrary() != null; } } private static class OrderEntryToLibraryConvertor implements Convertor { + @Override public Library convert(LibraryOrderEntry o) { return o.getLibrary(); } } + @Override public void commit() { } + @Override public boolean isChanged() { return myRootModel.isChanged(); } + @Override public ModifiableModel getModifiableModel() { return this; } diff --git a/plugins/eclipse/src/org/jetbrains/idea/eclipse/conversion/EclipseClasspathReader.java b/plugins/eclipse/src/org/jetbrains/idea/eclipse/conversion/EclipseClasspathReader.java index 75d676089ade..93e93177fbd7 100644 --- a/plugins/eclipse/src/org/jetbrains/idea/eclipse/conversion/EclipseClasspathReader.java +++ b/plugins/eclipse/src/org/jetbrains/idea/eclipse/conversion/EclipseClasspathReader.java @@ -168,8 +168,9 @@ public class EclipseClasspathReader { srcUrl = VfsUtil.pathToUrl(linked); eclipseModuleManager.registerEclipseLinkedSrcVarPath(srcUrl, path); rootModel.addContentEntry(srcUrl).addSourceFolder(srcUrl, isTestFolder); - } else { - getContentEntry().addSourceFolder(srcUrl, isTestFolder); + } + else { + myContentEntry.addSourceFolder(srcUrl, isTestFolder); } eclipseModuleManager.setExpectedModuleSourcePlace(rearrangeOrderEntryOfType(rootModel, ModuleSourceOrderEntry.class)); eclipseModuleManager.registerSrcPlace(srcUrl, idx); @@ -438,10 +439,6 @@ public class EclipseClasspathReader { return idx < 0 || idx == path.length() - 1 ? null : path.substring(idx + 1); } - private ContentEntry getContentEntry() { - return myContentEntry; - } - private static String getVariableRelatedPath(String var, String path) { return var == null ? null : ("$" + var + "$" + (path == null ? "" : ("/" + path))); } From 971a6a66ebadd83f691c0a8f86942a65e9387e76 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Fri, 18 May 2012 16:29:08 +0400 Subject: [PATCH 14/14] EA-36207 - IAE: ObjectUtils._assertNotNull --- .../CreateNSDeclarationIntentionFix.java | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/xml/impl/src/com/intellij/codeInsight/daemon/impl/analysis/CreateNSDeclarationIntentionFix.java b/xml/impl/src/com/intellij/codeInsight/daemon/impl/analysis/CreateNSDeclarationIntentionFix.java index 2fd21d165ffe..8d98f7fe61f9 100644 --- a/xml/impl/src/com/intellij/codeInsight/daemon/impl/analysis/CreateNSDeclarationIntentionFix.java +++ b/xml/impl/src/com/intellij/codeInsight/daemon/impl/analysis/CreateNSDeclarationIntentionFix.java @@ -51,7 +51,6 @@ import com.intellij.psi.xml.XmlToken; import com.intellij.ui.components.JBList; import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; -import com.intellij.util.ObjectUtils; import com.intellij.xml.XmlElementDescriptor; import com.intellij.xml.XmlExtension; import com.intellij.xml.impl.schema.AnyXmlElementDescriptor; @@ -102,6 +101,7 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi myToken = token == null ? null : PsiAnchor.create(token); } + @Override @NotNull public String getText() { final String alias = StringUtil.capitalize(getXmlExtension().getNamespaceAlias(getFile())); @@ -112,16 +112,19 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi return XmlExtension.getExtension(getFile()); } + @Override @NotNull public String getName() { return getFamilyName(); } + @Override @NotNull public String getFamilyName() { return getText(); } + @Override public void applyFix(@NotNull final Project project, @NotNull final ProblemDescriptor descriptor) { final PsiFile containingFile = descriptor.getPsiElement().getContainingFile(); Editor editor = FileEditorManager.getInstance(project).getSelectedTextEditor(); @@ -133,15 +136,18 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi } } + @Override public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) { PsiElement element = myElement.retrieve(); return element != null && element.isValid(); } + @Override public void invoke(@NotNull final Project project, final Editor editor, final PsiFile file) throws IncorrectOperationException { if (!CodeInsightUtilBase.prepareFileForWrite(file)) return; - final PsiElement element = ObjectUtils.assertNotNull(myElement.retrieve()); + final PsiElement element = myElement.retrieve(); + if (element == null) return; final Set set = getXmlExtension().guessUnboundNamespaces(element, getFile()); final String[] namespaces = ArrayUtil.toStringArray(set); Arrays.sort(namespaces); @@ -150,6 +156,7 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi namespaces, project, new StringToAttributeProcessor() { + @Override public void doSomethingWithGivenStringToProduceXmlAttributeNowPlease(@NotNull final String namespace) throws IncorrectOperationException { String prefix = myNamespacePrefix; if (StringUtil.isEmpty(prefix)) { @@ -173,8 +180,9 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi final RangeMarker marker = editor.getDocument().createRangeMarker(offset, offset); final XmlExtension extension = XmlExtension.getExtension(file); extension.insertNamespaceDeclaration((XmlFile)file, editor, Collections.singleton(namespace), prefix, new XmlExtension.Runner() { + @Override public void run(final String param) throws IncorrectOperationException { - if (namespace.length() > 0) { + if (!namespace.isEmpty()) { editor.getCaretModel().moveToOffset(marker.getStartOffset()); } } @@ -189,18 +197,21 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi return XmlErrorMessages.message("select.namespace.title", StringUtil.capitalize(getXmlExtension().getNamespaceAlias(getFile()))); } + @Override public boolean startInWriteAction() { return true; } + @Override public boolean showHint(final Editor editor) { if (myToken == null) return false; XmlToken token = (XmlToken)myToken.retrieve(); if (token == null) return false; - if (!XmlSettings.getInstance().SHOW_XML_ADD_IMPORT_HINTS || myNamespacePrefix.length() == 0) { + if (!XmlSettings.getInstance().SHOW_XML_ADD_IMPORT_HINTS || myNamespacePrefix.isEmpty()) { return false; } - PsiElement element = ObjectUtils.assertNotNull(myElement.retrieve()); + final PsiElement element = myElement.retrieve(); + if (element == null) return false; final Set namespaces = getXmlExtension().guessUnboundNamespaces(element, getFile()); if (!namespaces.isEmpty()) { final String message = ShowAutoImportPass.getMessage(namespaces.size() > 1, namespaces.iterator().next()); @@ -224,13 +235,14 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi } private static boolean checkIfGivenXmlHasTheseWords(final String name, final XmlFile tldFileByUri) { - if (name == null || name.length() == 0) return true; + if (name == null || name.isEmpty()) return true; final List list = StringUtil.getWordsIn(name); final String[] words = ArrayUtil.toStringArray(list); final boolean[] wordsFound = new boolean[words.length]; final int[] wordsFoundCount = new int[1]; IdTableBuilding.ScanWordProcessor wordProcessor = new IdTableBuilding.ScanWordProcessor() { + @Override public void run(final CharSequence chars, @Nullable char[] charsArray, int start, int end) { if (wordsFoundCount[0] == words.length) return; final int foundWordLen = end - start; @@ -263,7 +275,7 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi } - public static void runActionOverSeveralAttributeValuesAfterLettingUserSelectTheNeededOne(final @NotNull String[] namespacesToChooseFrom, + public static void runActionOverSeveralAttributeValuesAfterLettingUserSelectTheNeededOne(@NotNull final String[] namespacesToChooseFrom, final Project project, final StringToAttributeProcessor onSelection, String title, final IntentionAction requestor, @@ -273,6 +285,7 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi final JList list = new JBList(namespacesToChooseFrom); list.setCellRenderer(XmlNSRenderer.INSTANCE); Runnable runnable = new Runnable() { + @Override public void run() { final int index = list.getSelectedIndex(); if (index < 0) return; @@ -281,9 +294,11 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi CommandProcessor.getInstance().executeCommand( project, new Runnable() { + @Override public void run() { ApplicationManager.getApplication().runWriteAction( new Runnable() { + @Override public void run() { try { onSelection.doSomethingWithGivenStringToProduceXmlAttributeNowPlease(namespacesToChooseFrom[index]); @@ -321,6 +336,7 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi else { ProgressManager.getInstance().runProcessWithProgressSynchronously( new Runnable() { + @Override public void run() { processExternalUrisImpl(metaHandler, file, processor); } @@ -345,6 +361,7 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi myName = name; } + @Override public boolean isAcceptableMetaData(final PsiMetaData metaData, final String url) { if (metaData instanceof XmlNSDescriptorImpl) { final XmlNSDescriptorImpl nsDescriptor = (XmlNSDescriptorImpl)metaData; @@ -355,6 +372,7 @@ public class CreateNSDeclarationIntentionFix implements HintAction, LocalQuickFi return false; } + @Override public String searchFor() { return myName; }