From 403de353d35a168e6e79923fa932db2cb61d184d Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Mon, 10 Sep 2012 14:32:24 +0400 Subject: [PATCH 1/7] Cleanup --- .../vfs/newvfs/RefreshSessionImpl.java | 3 +- .../newvfs/persistent/PersistentFSImpl.java | 29 ++++----- .../openapi/vfs/local/FileWatcherTest.java | 62 +++++++------------ 3 files changed, 37 insertions(+), 57 deletions(-) diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/RefreshSessionImpl.java b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/RefreshSessionImpl.java index 5da732d46532..fe12a1f3f21d 100644 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/RefreshSessionImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/RefreshSessionImpl.java @@ -96,9 +96,10 @@ public class RefreshSessionImpl extends RefreshSession { if (!workQueue.isEmpty()) { ((LocalFileSystemImpl)LocalFileSystem.getInstance()).markSuspiciousFilesDirty(workQueue); + final FileWatcher watcher = FileWatcher.getInstance(); for (VirtualFile file : workQueue) { final NewVirtualFile nvf = (NewVirtualFile)file; - if (!myIsRecursive && (!myIsAsync || !FileWatcher.getInstance().isWatched(nvf))) { // We're unable to definitely refresh synchronously by means of file watcher. + if (!myIsRecursive && (!myIsAsync || !watcher.isWatched(nvf))) { // We're unable to definitely refresh synchronously by means of file watcher. nvf.markDirty(); } diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java index ae7fe5944694..70785556a01f 100644 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java @@ -183,7 +183,7 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone final FakeVirtualFile child = new FakeVirtualFile(file, name); final FileAttributes attributes = fs.getAttributes(child); if (attributes != null) { - final int childId = createAndCopyRecord(fs, child, id, attributes); + final int childId = createAndFillRecord(fs, child, id, attributes); childrenIds[i] = childId; } else { @@ -285,11 +285,11 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone return FSRecords.getModCount(); } - private static boolean copyRecordFromDelegateFS(final int id, - final int parentId, - @NotNull VirtualFile file, - @NotNull NewVirtualFileSystem fs, - @NotNull FileAttributes attributes) { + private static boolean writeAttributesToRecord(final int id, + final int parentId, + @NotNull VirtualFile file, + @NotNull NewVirtualFileSystem fs, + @NotNull FileAttributes attributes) { String name = file.getName(); if (!name.isEmpty()) { if (namesEqual(fs, name, FSRecords.getName(id))) return false; // TODO: Handle root attributes change. @@ -396,7 +396,7 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone final VirtualFile fake = new FakeVirtualFile(parent, childName); final FileAttributes attributes = fs.getAttributes(fake); if (attributes != null) { - final int child = createAndCopyRecord(fs, fake, parentId, attributes); + final int child = createAndFillRecord(fs, fake, parentId, attributes); FSRecords.updateList(parentId, ArrayUtil.append(children, child)); return child; } @@ -778,7 +778,7 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone return null; } - final boolean newRoot = copyRecordFromDelegateFS(rootId, 0, root, fs, attributes); + final boolean newRoot = writeAttributesToRecord(rootId, 0, root, fs, attributes); if (!newRoot) { if (attributes.lastModified != FSRecords.getTimestamp(rootId)) { root.markDirtyRecursively(); @@ -937,7 +937,7 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone } else if (event instanceof VFileCopyEvent) { final VFileCopyEvent copyEvent = (VFileCopyEvent)event; - executeCopy(copyEvent.getFile(), copyEvent.getNewParent(), copyEvent.getNewChildName()); + executeCreateChild(copyEvent.getNewParent(), copyEvent.getNewChildName()); } else if (event instanceof VFileMoveEvent) { final VFileMoveEvent moveEvent = (VFileMoveEvent)event; @@ -971,7 +971,7 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone final FileAttributes attributes = delegate.getAttributes(fake); if (attributes != null) { final int parentId = getFileId(parent); - final int childId = createAndCopyRecord(delegate, fake, parentId, attributes); + final int childId = createAndFillRecord(delegate, fake, parentId, attributes); appendIdToParentList(parentId, childId); assert parent instanceof VirtualDirectoryImpl : parent; final VirtualDirectoryImpl dir = (VirtualDirectoryImpl)parent; @@ -979,12 +979,12 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone } } - private static int createAndCopyRecord(@NotNull NewVirtualFileSystem delegateSystem, + private static int createAndFillRecord(@NotNull NewVirtualFileSystem delegateSystem, @NotNull VirtualFile delegateFile, int parentId, @NotNull FileAttributes attributes) { final int childId = FSRecords.createRecord(); - copyRecordFromDelegateFS(childId, parentId, delegateFile, delegateSystem, attributes); + writeAttributesToRecord(childId, parentId, delegateFile, delegateSystem, attributes); return childId; } @@ -1097,11 +1097,6 @@ public class PersistentFSImpl extends PersistentFS implements ApplicationCompone ((VirtualFileSystemEntry)file).setModificationStamp(newModificationStamp); } - @SuppressWarnings({"UnusedDeclaration"}) - private static void executeCopy(VirtualFile from, @NotNull VirtualFile newParent, @NotNull String copyName) { - executeCreateChild(newParent, copyName); - } - private void executeMove(@NotNull VirtualFile file, @NotNull VirtualFile newParent) { clearIdCache(); diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/local/FileWatcherTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/local/FileWatcherTest.java index dba16f98f306..3ee3cfd5cf69 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/local/FileWatcherTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/local/FileWatcherTest.java @@ -130,7 +130,7 @@ public class FileWatcherTest extends PlatformLangTestCase { final LocalFileSystem.WatchRequest request = watch(file); try { myAccept = true; - writeToFile(file, "new content"); + FileUtil.writeToFile(file, "new content"); assertEvent(VFileContentChangeEvent.class, file.getAbsolutePath()); myAccept = true; @@ -138,7 +138,7 @@ public class FileWatcherTest extends PlatformLangTestCase { assertEvent(VFileDeleteEvent.class, file.getAbsolutePath()); myAccept = true; - writeToFile(file, "re-creation"); + FileUtil.writeToFile(file, "re-creation"); assertEvent(VFileCreateEvent.class, file.getAbsolutePath()); } finally { @@ -147,22 +147,6 @@ public class FileWatcherTest extends PlatformLangTestCase { } } - private static void writeToFile(File file, String text) throws IOException { - //try { - // Thread.sleep(1000); - //} - //catch (InterruptedException e) { - // LOG.error(e); - //} - FileUtil.writeToFile(file, text); - //try { - // Thread.sleep(1000); - //} - //catch (InterruptedException e) { - // LOG.error(e); - //} - } - public void testNonCanonicallyNamedFileRoot() throws Exception { if (SystemInfo.isFileSystemCaseSensitive) { System.err.println("Ignored: case-insensitive FS required"); @@ -176,7 +160,7 @@ public class FileWatcherTest extends PlatformLangTestCase { final LocalFileSystem.WatchRequest request = watch(new File(watchRoot)); try { myAccept = true; - writeToFile(file, "new content"); + FileUtil.writeToFile(file, "new content"); assertEvent(VFileContentChangeEvent.class, file.getAbsolutePath()); myAccept = true; @@ -184,7 +168,7 @@ public class FileWatcherTest extends PlatformLangTestCase { assertEvent(VFileDeleteEvent.class, file.getAbsolutePath()); myAccept = true; - writeToFile(file, "re-creation"); + FileUtil.writeToFile(file, "re-creation"); assertEvent(VFileCreateEvent.class, file.getAbsolutePath()); } finally { @@ -209,7 +193,7 @@ public class FileWatcherTest extends PlatformLangTestCase { assertEvent(VFileCreateEvent.class, file.getAbsolutePath()); myAccept = true; - writeToFile(file, "new content"); + FileUtil.writeToFile(file, "new content"); assertEvent(VFileContentChangeEvent.class, file.getAbsolutePath()); myAccept = true; @@ -217,7 +201,7 @@ public class FileWatcherTest extends PlatformLangTestCase { assertEvent(VFileDeleteEvent.class, file.getAbsolutePath()); myAccept = true; - writeToFile(file, "re-creation"); + FileUtil.writeToFile(file, "re-creation"); assertEvent(VFileCreateEvent.class, file.getAbsolutePath()); } finally { @@ -236,13 +220,13 @@ public class FileWatcherTest extends PlatformLangTestCase { final LocalFileSystem.WatchRequest request = watch(topDir, false); try { myAccept = true; - writeToFile(watchedFile, "new content"); + FileUtil.writeToFile(watchedFile, "new content"); assertEvent(VFileContentChangeEvent.class, watchedFile.getAbsolutePath()); myAccept = true; try { myTimeout = 10 * INTER_RESPONSE_DELAY; - writeToFile(unwatchedFile, "new content"); + FileUtil.writeToFile(unwatchedFile, "new content"); assertEvent(VFileEvent.class); } finally { @@ -269,9 +253,9 @@ public class FileWatcherTest extends PlatformLangTestCase { final LocalFileSystem.WatchRequest subRequest = watch(sub2Dir); try { myAccept = true; - writeToFile(watchedFile1, "new content"); - writeToFile(watchedFile2, "new content"); - writeToFile(unwatchedFile, "new content"); + FileUtil.writeToFile(watchedFile1, "new content"); + FileUtil.writeToFile(watchedFile2, "new content"); + FileUtil.writeToFile(unwatchedFile, "new content"); assertEvent(VFileContentChangeEvent.class, watchedFile1.getAbsolutePath(), watchedFile2.getAbsolutePath()); } finally { @@ -294,7 +278,7 @@ public class FileWatcherTest extends PlatformLangTestCase { refresh(subDir); myAccept = true; - writeToFile(file, "new content"); + FileUtil.writeToFile(file, "new content"); assertEvent(VFileCreateEvent.class, file.getAbsolutePath()); } finally { @@ -317,17 +301,17 @@ public class FileWatcherTest extends PlatformLangTestCase { final LocalFileSystem.WatchRequest requestForSideDir = watch(sideDir); try { myAccept = true; - writeToFile(fileInTopDir, "new content"); - writeToFile(fileInSubDir, "new content"); - writeToFile(fileInSideDir, "new content"); + FileUtil.writeToFile(fileInTopDir, "new content"); + FileUtil.writeToFile(fileInSubDir, "new content"); + FileUtil.writeToFile(fileInSideDir, "new content"); assertEvent(VFileContentChangeEvent.class, fileInSubDir.getAbsolutePath(), fileInSideDir.getAbsolutePath()); final LocalFileSystem.WatchRequest requestForTopDir = watch(topDir); try { myAccept = true; - writeToFile(fileInTopDir, "newer content"); - writeToFile(fileInSubDir, "newer content"); - writeToFile(fileInSideDir, "newer content"); + FileUtil.writeToFile(fileInTopDir, "newer content"); + FileUtil.writeToFile(fileInSubDir, "newer content"); + FileUtil.writeToFile(fileInSideDir, "newer content"); assertEvent(VFileContentChangeEvent.class, fileInTopDir.getAbsolutePath(), fileInSubDir.getAbsolutePath(), fileInSideDir.getAbsolutePath()); } finally { @@ -335,9 +319,9 @@ public class FileWatcherTest extends PlatformLangTestCase { } myAccept = true; - writeToFile(fileInTopDir, "newest content"); - writeToFile(fileInSubDir, "newest content"); - writeToFile(fileInSideDir, "newest content"); + FileUtil.writeToFile(fileInTopDir, "newest content"); + FileUtil.writeToFile(fileInSubDir, "newest content"); + FileUtil.writeToFile(fileInSideDir, "newest content"); assertEvent(VFileContentChangeEvent.class, fileInSubDir.getAbsolutePath(), fileInSideDir.getAbsolutePath()); myAccept = true; @@ -438,7 +422,7 @@ public class FileWatcherTest extends PlatformLangTestCase { final LocalFileSystem.WatchRequest request = watch(substDir); try { myAccept = true; - writeToFile(file, "new content"); + FileUtil.writeToFile(file, "new content"); assertEvent(VFileContentChangeEvent.class, substFile.getAbsolutePath()); final LocalFileSystem.WatchRequest request2 = watch(targetDir); @@ -452,7 +436,7 @@ public class FileWatcherTest extends PlatformLangTestCase { } myAccept = true; - writeToFile(file, "re-creation"); + FileUtil.writeToFile(file, "re-creation"); assertEvent(VFileCreateEvent.class, substFile.getAbsolutePath()); } finally { From 5cb12c39f1df7564746571e9c64681b345bdf33c Mon Sep 17 00:00:00 2001 From: peter Date: Mon, 10 Sep 2012 18:32:33 +0200 Subject: [PATCH 2/7] IDEA-88766 Nullable: include final fields into data flow --- .../AnnotationsAwareDataFlowRunner.java | 2 +- .../codeInspection/dataFlow/ControlFlow.java | 2 +- .../dataFlow/ControlFlowAnalyzer.java | 25 +++++--- .../dataFlow/DfaMemoryStateImpl.java | 11 +++- .../dataFlow/value/DfaUnboxedValue.java | 3 +- .../dataFlow/value/DfaValueFactory.java | 2 +- .../dataFlow/value/DfaVariableValue.java | 63 ++++++++++++++++--- .../fixture/ChainedFinalFieldsDfa.java | 40 ++++++++++++ .../DataFlowInspectionFixtureTest.java | 2 + 9 files changed, 127 insertions(+), 23 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldsDfa.java diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/AnnotationsAwareDataFlowRunner.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/AnnotationsAwareDataFlowRunner.java index 835a5cc811f9..203ea4265b8d 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/AnnotationsAwareDataFlowRunner.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/AnnotationsAwareDataFlowRunner.java @@ -43,7 +43,7 @@ public class AnnotationsAwareDataFlowRunner extends DataFlowRunner { //todo move out from generic runner for (PsiParameter parameter : method.getParameterList().getParameters()) { if (NullableNotNullManager.isNotNull(parameter)) { - final DfaVariableValue value = getFactory().getVarFactory().create(parameter, false); + final DfaVariableValue value = getFactory().getVarFactory().createVariableValue(parameter, false); for (final DfaMemoryState initialState : initialStates) { initialState.applyNotNull(value); } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlow.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlow.java index 10fc81b01787..537d6566839f 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlow.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlow.java @@ -67,7 +67,7 @@ public class ControlFlow { } public void removeVariable(PsiVariable variable) { - DfaVariableValue var = myFactory.getVarFactory().create(variable, false); + DfaVariableValue var = myFactory.getVarFactory().createVariableValue(variable, false); addInstruction(new FlushVariableInstruction(var)); } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 670f9a564734..1fb9cf75ef1b 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -232,7 +232,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } private void initializeVariable(PsiVariable variable, PsiExpression initializer) { - DfaVariableValue dfaVariable = myFactory.getVarFactory().create(variable, false); + DfaVariableValue dfaVariable = myFactory.getVarFactory().createVariableValue(variable, false); addInstruction(new PushInstruction(dfaVariable, initializer)); initializer.accept(this); generateBoxingUnboxingInstructionFor(initializer, variable.getType()); @@ -367,7 +367,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } int offset = myCurrentFlow.getInstructionCount(); - DfaVariableValue dfaVariable = myFactory.getVarFactory().create(parameter, false); + DfaVariableValue dfaVariable = myFactory.getVarFactory().createVariableValue(parameter, false); addInstruction(new PushInstruction(dfaVariable, null)); pushUnknown(); addInstruction(new AssignInstruction(null)); @@ -684,7 +684,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { * @param cd */ private void addGotoCatch(CatchDescriptor cd) { - addInstruction(new PushInstruction(myFactory.getVarFactory().create(cd.getParameter(), false), null)); + addInstruction(new PushInstruction(myFactory.getVarFactory().createVariableValue(cd.getParameter(), false), null)); addInstruction(new SwapInstruction()); myCurrentFlow.addInstruction(new AssignInstruction(null)); addInstruction(new PopInstruction()); @@ -1410,7 +1410,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { if (operand instanceof PsiReferenceExpression) { PsiVariable psiVariable = DfaValueFactory.resolveVariable((PsiReferenceExpression)expression.getOperand()); if (psiVariable != null) { - DfaVariableValue dfaVariable = myFactory.getVarFactory().create(psiVariable, false); + DfaVariableValue dfaVariable = myFactory.getVarFactory().createVariableValue(psiVariable, false); addInstruction(new FlushVariableInstruction(dfaVariable)); } } @@ -1443,7 +1443,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { if (operand instanceof PsiReferenceExpression) { PsiVariable psiVariable = DfaValueFactory.resolveVariable((PsiReferenceExpression)operand); if (psiVariable != null) { - DfaVariableValue dfaVariable = myFactory.getVarFactory().create(psiVariable, false); + DfaVariableValue dfaVariable = myFactory.getVarFactory().createVariableValue(psiVariable, false); addInstruction(new FlushVariableInstruction(dfaVariable)); } } @@ -1482,9 +1482,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } if (dfaValue == null && resolved instanceof PsiField) { - // Accessing a field from another instance - dfaValue = myFactory.getTypeFactory().create(((PsiField)resolved).getType(), - NullableNotNullManager.isNullable((PsiModifierListOwner)resolved)); + dfaValue = createDfaValueForAnotherInstanceMemberAccess(expression, (PsiField)resolved); } addInstruction(new PushInstruction(dfaValue, expression)); @@ -1492,6 +1490,17 @@ class ControlFlowAnalyzer extends JavaElementVisitor { finishElement(expression); } + private DfaValue createDfaValueForAnotherInstanceMemberAccess(PsiReferenceExpression expression, PsiField field) { + DfaValue dfaValue = null; + if (expression.getQualifierExpression() != null) { + dfaValue = myFactory.getVarFactory().createFromReference(expression, field); + } + if (dfaValue == null) { + return myFactory.getTypeFactory().create(field.getType(), NullableNotNullManager.isNullable(field)); + } + return dfaValue; + } + private void addField(DfaVariableValue field) { myFields.add(field); } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index 3b657884d886..3ba99012a81e 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -741,16 +741,23 @@ public class DfaMemoryStateImpl implements DfaMemoryState { return; } - doFlush(variable); + flushWithDependencies(variable); } @Override public void flushVariableOutOfScope(DfaVariableValue variable) { + flushWithDependencies(variable); + } + + private void flushWithDependencies(DfaVariableValue variable) { doFlush(variable); + for (DfaVariableValue dependent : myFactory.getVarFactory().getAllQualifiedBy(variable)) { + doFlush(dependent); + } } private void doFlush(DfaVariableValue varPlain) { - DfaVariableValue varNegated = (DfaVariableValue)varPlain.createNegated(); + DfaVariableValue varNegated = varPlain.createNegated(); final int idPlain = varPlain.getID(); final int idNegated = varNegated.getID(); diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaUnboxedValue.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaUnboxedValue.java index 43196431976a..e75ac44727e6 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaUnboxedValue.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaUnboxedValue.java @@ -40,7 +40,6 @@ public class DfaUnboxedValue extends DfaValue { } public DfaValue createNegated() { - DfaVariableValue negVar = myFactory.getVarFactory().create(myVariable.getPsiVariable(), !myVariable.isNegated()); - return myFactory.getBoxedFactory().createUnboxed(negVar); + return myFactory.getBoxedFactory().createUnboxed(myVariable.createNegated()); } } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java index 857ef1bf46c5..127484fdcfe0 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java @@ -90,7 +90,7 @@ public class DfaValueFactory { PsiVariable psiVariable = resolveVariable((PsiReferenceExpression)psiExpression); if (psiVariable != null) { - result = getVarFactory().create(psiVariable, false); + result = getVarFactory().createVariableValue(psiVariable, false); } } } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java index f11b36f1e144..7f5942a7590d 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java @@ -24,17 +24,21 @@ */ package com.intellij.codeInspection.dataFlow.value; -import com.intellij.psi.PsiVariable; +import com.intellij.psi.*; import com.intellij.util.containers.HashMap; +import com.intellij.util.containers.MultiMap; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.ArrayList; +import java.util.List; public class DfaVariableValue extends DfaValue { public static class Factory { private final DfaVariableValue mySharedInstance; private final HashMap> myStringToObject; private final DfaValueFactory myFactory; + private final MultiMap myQualifiersToChainedVariables = new MultiMap(); Factory(DfaValueFactory factory) { myFactory = factory; @@ -42,9 +46,13 @@ public class DfaVariableValue extends DfaValue { myStringToObject = new HashMap>(); } - public DfaVariableValue create(PsiVariable myVariable, boolean isNegated) { + public DfaVariableValue createVariableValue(PsiVariable myVariable, boolean isNegated) { + return createVariableValue(myVariable, isNegated, null); + } + private DfaVariableValue createVariableValue(PsiVariable myVariable, boolean isNegated, @Nullable DfaVariableValue qualifier) { mySharedInstance.myVariable = myVariable; mySharedInstance.myIsNegated = isNegated; + mySharedInstance.myQualifier = qualifier; String id = mySharedInstance.toString(); ArrayList conditions = myStringToObject.get(id); @@ -58,19 +66,51 @@ public class DfaVariableValue extends DfaValue { } } - DfaVariableValue result = new DfaVariableValue(myVariable, isNegated, myFactory); + DfaVariableValue result = new DfaVariableValue(myVariable, isNegated, myFactory, qualifier); + if (qualifier != null) { + myQualifiersToChainedVariables.putValue(qualifier, result); + } conditions.add(result); return result; } + + public List getAllQualifiedBy(DfaVariableValue value) { + ArrayList result = new ArrayList(); + for (DfaVariableValue directQualified : myQualifiersToChainedVariables.get(value)) { + result.add(directQualified); + result.addAll(getAllQualifiedBy(directQualified)); + } + return result; + } + + @Nullable + public DfaVariableValue createFromReference(@NotNull PsiReferenceExpression expression, @NotNull PsiVariable target) { + PsiExpression qualifier = expression.getQualifierExpression(); + if (qualifier == null) { + return createVariableValue(target, false, null); + } + + if (qualifier instanceof PsiReferenceExpression && target instanceof PsiField && target.hasModifierProperty(PsiModifier.FINAL)) { + PsiElement qTarget = ((PsiReferenceExpression)qualifier).resolve(); + if (qTarget instanceof PsiVariable) { + DfaVariableValue qualifierValue = createFromReference((PsiReferenceExpression)qualifier, (PsiVariable)qTarget); + return qualifierValue == null ? null : createVariableValue(target, false, qualifierValue); + } + } + + return null; + } } private PsiVariable myVariable; + @Nullable private DfaVariableValue myQualifier; private boolean myIsNegated; - private DfaVariableValue(PsiVariable variable, boolean isNegated, DfaValueFactory factory) { + private DfaVariableValue(PsiVariable variable, boolean isNegated, DfaValueFactory factory, @Nullable DfaVariableValue qualifier) { super(factory); myVariable = variable; myIsNegated = isNegated; + myQualifier = qualifier; } private DfaVariableValue(DfaValueFactory factory) { @@ -88,17 +128,24 @@ public class DfaVariableValue extends DfaValue { return myIsNegated; } - public DfaValue createNegated() { - return myFactory.getVarFactory().create(getPsiVariable(), !myIsNegated); + public DfaVariableValue createNegated() { + return myFactory.getVarFactory().createVariableValue(myVariable, !myIsNegated, myQualifier); } @SuppressWarnings({"HardCodedStringLiteral"}) public String toString() { if (myVariable == null) return "$currentException"; - return (myIsNegated ? "!" : "") + myVariable.getName(); + return (myIsNegated ? "!" : "") + myVariable.getName() + (myQualifier == null ? "" : "|" + myQualifier.toString()); } private boolean hardEquals(DfaVariableValue aVar) { - return aVar.myVariable == myVariable && aVar.myIsNegated == myIsNegated; + return aVar.myVariable == myVariable && + aVar.myIsNegated == myIsNegated && + (myQualifier == null ? aVar.myQualifier == null : myQualifier.hardEquals(aVar.myQualifier)); + } + + @Nullable + public DfaVariableValue getQualifier() { + return myQualifier; } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldsDfa.java b/java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldsDfa.java new file mode 100644 index 000000000000..8618bd0e1f66 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldsDfa.java @@ -0,0 +1,40 @@ +import org.jetbrains.annotations.Nullable; +public class BrokenAlignment { + + void main(Data data) { + if (data.text != null) { + System.out.println(data.text.hashCode()); + } + + data = new Data(null, null); + System.out.println(data.text.hashCode()); + + if (data.inner != null) { + System.out.println(data.inner.hashCode()); + System.out.println(data.inner.text.hashCode()); + if (data.inner != null) { + System.out.println(data.inner.hashCode()); + } + + data = new Data(null, null); + System.out.println(data.inner.hashCode()); + } + } + + void main2(Data data) { + if (data.inner != null && data.inner.text != null) { + System.out.println(data.inner.hashCode()); + System.out.println(data.inner.text.hashCode()); + } + } + + private static class Data { + @Nullable public final String text; + @Nullable public final Data inner; + + Data(@Nullable String text, Data inner) { + this.text = text; + this.inner = inner; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java index 491c10611917..cf55cb61a4f3 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java @@ -78,4 +78,6 @@ public class DataFlowInspectionFixtureTest extends JavaCodeInsightFixtureTestCas public void testFinalLoopVariableInstanceof() throws Throwable { doTest(); } public void testGreaterIsNotEquals() throws Throwable { doTest(); } + public void testChainedFinalFieldsDfa() throws Throwable { doTest(); } + } From d30abc644c58067fba4770f6ae892d2e496db01f Mon Sep 17 00:00:00 2001 From: peter Date: Mon, 10 Sep 2012 18:40:26 +0200 Subject: [PATCH 3/7] minor --- .../dataFlow/ControlFlowAnalyzer.java | 61 +++++++++++-------- .../dataFlow/StandardInstructionVisitor.java | 55 +++++++++-------- .../dataFlow/value/DfaVariableValue.java | 22 +------ 3 files changed, 68 insertions(+), 70 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 1fb9cf75ef1b..d37d2dd0ea17 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -1460,40 +1460,39 @@ class ControlFlowAnalyzer extends JavaElementVisitor { @Override public void visitReferenceExpression(PsiReferenceExpression expression) { startElement(expression); - DfaValue dfaValue = myFactory.create(expression); - PsiElement resolved = expression.resolve(); - if (dfaValue instanceof DfaVariableValue) { - DfaVariableValue dfaVariable = (DfaVariableValue)dfaValue; - PsiVariable psiVariable = dfaVariable.getPsiVariable(); - if (psiVariable instanceof PsiField) { - addField(dfaVariable); - } - } - final PsiExpression qualifierExpression = expression.getQualifierExpression(); if (qualifierExpression != null) { qualifierExpression.accept(this); - if (resolved instanceof PsiField) { - addInstruction(new FieldReferenceInstruction(expression, null)); - } - else { - addInstruction(new PopInstruction()); - } + addInstruction(expression.resolve() instanceof PsiField ? new FieldReferenceInstruction(expression, null) : new PopInstruction()); } - if (dfaValue == null && resolved instanceof PsiField) { - dfaValue = createDfaValueForAnotherInstanceMemberAccess(expression, (PsiField)resolved); - } - - addInstruction(new PushInstruction(dfaValue, expression)); + addInstruction(new PushInstruction(getExpressionDfaValue(expression), expression)); finishElement(expression); } + @Nullable + private DfaValue getExpressionDfaValue(PsiReferenceExpression expression) { + DfaValue dfaValue = myFactory.create(expression); + if (dfaValue instanceof DfaVariableValue) { + DfaVariableValue dfaVariable = (DfaVariableValue)dfaValue; + if (dfaVariable.getPsiVariable() instanceof PsiField) { + myFields.add(dfaVariable); + } + } + if (dfaValue == null) { + PsiElement resolved = expression.resolve(); + if (resolved instanceof PsiField) { + dfaValue = createDfaValueForAnotherInstanceMemberAccess(expression, (PsiField)resolved); + } + } + return dfaValue; + } + private DfaValue createDfaValueForAnotherInstanceMemberAccess(PsiReferenceExpression expression, PsiField field) { DfaValue dfaValue = null; if (expression.getQualifierExpression() != null) { - dfaValue = myFactory.getVarFactory().createFromReference(expression, field); + dfaValue = createChainedVariableValue(expression, field); } if (dfaValue == null) { return myFactory.getTypeFactory().create(field.getType(), NullableNotNullManager.isNullable(field)); @@ -1501,8 +1500,22 @@ class ControlFlowAnalyzer extends JavaElementVisitor { return dfaValue; } - private void addField(DfaVariableValue field) { - myFields.add(field); + @Nullable + private DfaVariableValue createChainedVariableValue(@NotNull PsiReferenceExpression expression, @NotNull PsiVariable target) { + PsiExpression qualifier = expression.getQualifierExpression(); + if (qualifier == null) { + return myFactory.getVarFactory().createVariableValue(target, false, null); + } + + if (qualifier instanceof PsiReferenceExpression && target instanceof PsiField && target.hasModifierProperty(PsiModifier.FINAL)) { + PsiElement qTarget = ((PsiReferenceExpression)qualifier).resolve(); + if (qTarget instanceof PsiVariable) { + DfaVariableValue qualifierValue = createChainedVariableValue((PsiReferenceExpression)qualifier, (PsiVariable)qTarget); + return qualifierValue == null ? null : myFactory.getVarFactory().createVariableValue(target, false, qualifierValue); + } + } + + return null; } @Override public void visitSuperExpression(PsiSuperExpression expression) { diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index f1df3c2e932b..f7de8aa2b4c5 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -189,48 +189,51 @@ public class StandardInstructionVisitor extends InstructionVisitor { return nextInstruction(instruction, runner, memState); } finally { - pushResult(instruction, memState, qualifier, runner.getFactory()); + memState.push(getMethodResultValue(instruction, qualifier, runner.getFactory())); if (instruction.shouldFlushFields()) { memState.flushFields(runner); } } } - private void pushResult(MethodCallInstruction instruction, DfaMemoryState state, final DfaValue oldValue, DfaValueFactory factory) { + @NotNull + private DfaValue getMethodResultValue(MethodCallInstruction instruction, @NotNull DfaValue qualifierValue, DfaValueFactory factory) { final PsiType type = instruction.getResultType(); final MethodCallInstruction.MethodType methodType = instruction.getMethodType(); - DfaValue dfaValue = null; if (type != null && (type instanceof PsiClassType || type.getArrayDimensions() > 0)) { @Nullable final Boolean nullability = myCalleeNullability.get(instruction); - dfaValue = nullability == Boolean.FALSE ? factory.getNotNullFactory().create(type) : factory.getTypeFactory().create(type, nullability == Boolean.TRUE); + return nullability == Boolean.FALSE ? factory.getNotNullFactory().create(type) : factory.getTypeFactory().create(type, nullability == Boolean.TRUE); } - else if (methodType == MethodCallInstruction.MethodType.UNBOXING) { - dfaValue = factory.getBoxedFactory().createUnboxed(oldValue); - } - else if (methodType == MethodCallInstruction.MethodType.BOXING) { - dfaValue = factory.getBoxedFactory().createBoxed(oldValue); - } - else if (methodType == MethodCallInstruction.MethodType.CAST) { - if (oldValue instanceof DfaConstValue) { - final DfaConstValue constValue = (DfaConstValue)oldValue; - Object o = constValue.getValue(); - if (o instanceof Double || o instanceof Float) { - double dbVal = o instanceof Double ? ((Double)o).doubleValue() : ((Float)o).doubleValue(); - // 5.0f == 5 - if (Math.floor(dbVal) == dbVal) o = TypeConversionUtil.computeCastTo(o, PsiType.LONG); - } - else { - o = TypeConversionUtil.computeCastTo(o, PsiType.LONG); - } - dfaValue = factory.getConstFactory().createFromValue(o, type); + if (methodType == MethodCallInstruction.MethodType.UNBOXING) { + return factory.getBoxedFactory().createUnboxed(qualifierValue); + } + + if (methodType == MethodCallInstruction.MethodType.BOXING) { + DfaValue boxed = factory.getBoxedFactory().createBoxed(qualifierValue); + return boxed == null ? DfaUnknownValue.getInstance() : boxed; + } + + if (methodType == MethodCallInstruction.MethodType.CAST) { + if (qualifierValue instanceof DfaConstValue) { + return factory.getConstFactory().createFromValue(castConstValue((DfaConstValue)qualifierValue), type); } - else { - dfaValue = oldValue; + return qualifierValue; + } + return DfaUnknownValue.getInstance(); + } + + private static Object castConstValue(DfaConstValue constValue) { + Object o = constValue.getValue(); + if (o instanceof Double || o instanceof Float) { + double dbVal = o instanceof Double ? ((Double)o).doubleValue() : ((Float)o).doubleValue(); + // 5.0f == 5 + if (Math.floor(dbVal) != dbVal) { + return o; } } - state.push(dfaValue == null ? DfaUnknownValue.getInstance() : dfaValue); + return TypeConversionUtil.computeCastTo(o, PsiType.LONG); } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java index 7f5942a7590d..2a2b9005d819 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java @@ -24,10 +24,9 @@ */ package com.intellij.codeInspection.dataFlow.value; -import com.intellij.psi.*; +import com.intellij.psi.PsiVariable; import com.intellij.util.containers.HashMap; import com.intellij.util.containers.MultiMap; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.ArrayList; @@ -49,7 +48,7 @@ public class DfaVariableValue extends DfaValue { public DfaVariableValue createVariableValue(PsiVariable myVariable, boolean isNegated) { return createVariableValue(myVariable, isNegated, null); } - private DfaVariableValue createVariableValue(PsiVariable myVariable, boolean isNegated, @Nullable DfaVariableValue qualifier) { + public DfaVariableValue createVariableValue(PsiVariable myVariable, boolean isNegated, @Nullable DfaVariableValue qualifier) { mySharedInstance.myVariable = myVariable; mySharedInstance.myIsNegated = isNegated; mySharedInstance.myQualifier = qualifier; @@ -83,23 +82,6 @@ public class DfaVariableValue extends DfaValue { return result; } - @Nullable - public DfaVariableValue createFromReference(@NotNull PsiReferenceExpression expression, @NotNull PsiVariable target) { - PsiExpression qualifier = expression.getQualifierExpression(); - if (qualifier == null) { - return createVariableValue(target, false, null); - } - - if (qualifier instanceof PsiReferenceExpression && target instanceof PsiField && target.hasModifierProperty(PsiModifier.FINAL)) { - PsiElement qTarget = ((PsiReferenceExpression)qualifier).resolve(); - if (qTarget instanceof PsiVariable) { - DfaVariableValue qualifierValue = createFromReference((PsiReferenceExpression)qualifier, (PsiVariable)qTarget); - return qualifierValue == null ? null : createVariableValue(target, false, qualifierValue); - } - } - - return null; - } } private PsiVariable myVariable; From 62b03bb715f00926bd6e80d09126112dccfd9f04 Mon Sep 17 00:00:00 2001 From: peter Date: Mon, 10 Sep 2012 20:47:59 +0200 Subject: [PATCH 4/7] include final field accessors into dfa (IDEA-89594) --- .../dataFlow/ControlFlowAnalyzer.java | 62 +++++++++--- .../dataFlow/DataFlowInspection.java | 21 ++-- .../dataFlow/StandardInstructionVisitor.java | 5 + .../instructions/MethodCallInstruction.java | 12 ++- .../refactoring/psi/PropertyUtils.java | 96 ++++++++----------- .../ChainedFinalFieldAccessorsDfa.java | 86 +++++++++++++++++ .../DataFlowInspectionFixtureTest.java | 1 + 7 files changed, 203 insertions(+), 80 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldAccessorsDfa.java diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index d37d2dd0ea17..405c6f637197 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -18,7 +18,10 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.ExceptionUtil; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInspection.dataFlow.instructions.*; -import com.intellij.codeInspection.dataFlow.value.*; +import com.intellij.codeInspection.dataFlow.value.DfaUnknownValue; +import com.intellij.codeInspection.dataFlow.value.DfaValue; +import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; +import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.ProgressManager; import com.intellij.psi.*; @@ -27,6 +30,7 @@ import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.RedundantCastUtil; import com.intellij.psi.util.TypeConversionUtil; +import com.intellij.refactoring.psi.PropertyUtils; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.Stack; import org.jetbrains.annotations.NonNls; @@ -1191,7 +1195,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } } - addInstruction(new MethodCallInstruction(expression)); + addInstruction(new MethodCallInstruction(expression, createChainedVariableValue(expression))); if (!myCatchStack.isEmpty()) { addMethodThrows(expression.resolveMethod()); @@ -1357,7 +1361,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { addInstruction(new PopInstruction()); } } - addInstruction(new MethodCallInstruction(expression)); + addInstruction(new MethodCallInstruction(expression, (DfaValue)null)); } else { final PsiExpressionList args = expression.getArgumentList(); @@ -1374,7 +1378,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } } - addInstruction(new MethodCallInstruction(expression)); + addInstruction(new MethodCallInstruction(expression, (DfaValue)null)); if (!myCatchStack.isEmpty()) { addMethodThrows(ctr); @@ -1492,7 +1496,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { private DfaValue createDfaValueForAnotherInstanceMemberAccess(PsiReferenceExpression expression, PsiField field) { DfaValue dfaValue = null; if (expression.getQualifierExpression() != null) { - dfaValue = createChainedVariableValue(expression, field); + dfaValue = createChainedVariableValue(expression); } if (dfaValue == null) { return myFactory.getTypeFactory().create(field.getType(), NullableNotNullManager.isNullable(field)); @@ -1501,20 +1505,50 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } @Nullable - private DfaVariableValue createChainedVariableValue(@NotNull PsiReferenceExpression expression, @NotNull PsiVariable target) { - PsiExpression qualifier = expression.getQualifierExpression(); - if (qualifier == null) { - return myFactory.getVarFactory().createVariableValue(target, false, null); + private DfaVariableValue createChainedVariableValue(@Nullable PsiExpression expression) { + if (expression instanceof PsiParenthesizedExpression) { + return createChainedVariableValue(((PsiParenthesizedExpression)expression).getExpression()); } - if (qualifier instanceof PsiReferenceExpression && target instanceof PsiField && target.hasModifierProperty(PsiModifier.FINAL)) { - PsiElement qTarget = ((PsiReferenceExpression)qualifier).resolve(); - if (qTarget instanceof PsiVariable) { - DfaVariableValue qualifierValue = createChainedVariableValue((PsiReferenceExpression)qualifier, (PsiVariable)qTarget); - return qualifierValue == null ? null : myFactory.getVarFactory().createVariableValue(target, false, qualifierValue); + PsiReferenceExpression refExpr; + if (expression instanceof PsiMethodCallExpression) { + refExpr = ((PsiMethodCallExpression)expression).getMethodExpression(); + } + else if (expression instanceof PsiReferenceExpression) { + refExpr = (PsiReferenceExpression)expression; + } + else { + return null; + } + + PsiVariable var = resolveToVariable(refExpr); + if (var == null) { + return null; + } + + PsiExpression qualifier = refExpr.getQualifierExpression(); + if (qualifier == null) { + return myFactory.getVarFactory().createVariableValue(var, false, null); + } + + if (var instanceof PsiField && var.hasModifierProperty(PsiModifier.FINAL)) { + DfaVariableValue qualifierValue = createChainedVariableValue(qualifier); + if (qualifierValue != null) { + return myFactory.getVarFactory().createVariableValue(var, false, qualifierValue); } } + return null; + } + @Nullable + private static PsiVariable resolveToVariable(PsiReferenceExpression refExpr) { + PsiElement target = refExpr.resolve(); + if (target instanceof PsiVariable) { + return (PsiVariable)target; + } + if (target instanceof PsiMethod) { + return PropertyUtils.getSimplyReturnedField((PsiMethod)target, PropertyUtils.getSingleReturnValue((PsiMethod)target)); + } return null; } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java index 2db2e72ecf1c..31fd7d552009 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java @@ -232,14 +232,12 @@ public class DataFlowInspection extends BaseLocalInspectionTool { createSimplifyToAssignmentFix() ); } - else { - boolean report = !(psiAnchor.getParent() instanceof PsiAssertStatement) || !DONT_REPORT_TRUE_ASSERT_STATEMENTS || !evaluatesToTrue; - if (report) { - final LocalQuickFix localQuickFix = createSimplifyBooleanExpressionFix(psiAnchor, evaluatesToTrue); - holder.registerProblem(psiAnchor, InspectionsBundle.message(underBinary ? "dataflow.message.constant.condition.whenriched" : "dataflow.message.constant.condition", - Boolean.toString(evaluatesToTrue)), - localQuickFix == null ? null : new LocalQuickFix[]{localQuickFix}); - } + else if (shouldReportConditionAlwaysTrueOrFalse(psiAnchor, evaluatesToTrue)) { + final LocalQuickFix fix = createSimplifyBooleanExpressionFix(psiAnchor, evaluatesToTrue); + String message = InspectionsBundle.message(underBinary ? + "dataflow.message.constant.condition.whenriched" : + "dataflow.message.constant.condition", Boolean.toString(evaluatesToTrue)); + holder.registerProblem(psiAnchor, message, fix == null ? null : new LocalQuickFix[]{fix}); } reportedAnchors.add(psiAnchor); } @@ -287,6 +285,13 @@ public class DataFlowInspection extends BaseLocalInspectionTool { } } + private boolean shouldReportConditionAlwaysTrueOrFalse(PsiElement psiAnchor, boolean evaluatesToTrue) { + if (psiAnchor.getParent() instanceof PsiAssertStatement && DONT_REPORT_TRUE_ASSERT_STATEMENTS && evaluatesToTrue) { + return false; + } + return true; + } + private static boolean isAtRHSOfBooleanAnd(PsiElement expr) { PsiElement cur = expr; diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index f7de8aa2b4c5..7d7d19813359 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -198,6 +198,11 @@ public class StandardInstructionVisitor extends InstructionVisitor { @NotNull private DfaValue getMethodResultValue(MethodCallInstruction instruction, @NotNull DfaValue qualifierValue, DfaValueFactory factory) { + DfaValue precalculated = instruction.getPrecalculatedReturnValue(); + if (precalculated != null) { + return precalculated; + } + final PsiType type = instruction.getResultType(); final MethodCallInstruction.MethodType methodType = instruction.getMethodType(); if (type != null && (type instanceof PsiClassType || type.getArrayDimensions() > 0)) { diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java index 079e0f0ba4e9..6643afd59e6a 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java @@ -28,6 +28,7 @@ import com.intellij.codeInspection.dataFlow.DataFlowRunner; import com.intellij.codeInspection.dataFlow.DfaInstructionState; import com.intellij.codeInspection.dataFlow.DfaMemoryState; import com.intellij.codeInspection.dataFlow.InstructionVisitor; +import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.psi.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -40,15 +41,17 @@ public class MethodCallInstruction extends Instruction { private boolean myShouldFlushFields; @NotNull private final PsiExpression myContext; private final MethodType myMethodType; + @Nullable private DfaValue myPrecalculatedReturnValue; public static enum MethodType { BOXING, UNBOXING, REGULAR_METHOD_CALL, CAST } - public MethodCallInstruction(@NotNull PsiCallExpression callExpression) { + public MethodCallInstruction(@NotNull PsiCallExpression callExpression, @Nullable DfaValue precalculatedReturnValue) { this(callExpression, MethodType.REGULAR_METHOD_CALL); + myPrecalculatedReturnValue = precalculatedReturnValue; } - public MethodCallInstruction(@NotNull PsiExpression context, MethodType methodType, PsiType resultType) { + public MethodCallInstruction(@NotNull PsiExpression context, MethodType methodType, @Nullable PsiType resultType) { this(context, methodType); myType = resultType; myShouldFlushFields = false; @@ -102,6 +105,11 @@ public class MethodCallInstruction extends Instruction { return myContext; } + @Nullable + public DfaValue getPrecalculatedReturnValue() { + return myPrecalculatedReturnValue; + } + public String toString() { return myMethodType == MethodType.UNBOXING ? "UNBOX" diff --git a/java/java-impl/src/com/intellij/refactoring/psi/PropertyUtils.java b/java/java-impl/src/com/intellij/refactoring/psi/PropertyUtils.java index 8de979f2166e..9d188b706b76 100644 --- a/java/java-impl/src/com/intellij/refactoring/psi/PropertyUtils.java +++ b/java/java-impl/src/com/intellij/refactoring/psi/PropertyUtils.java @@ -52,84 +52,68 @@ public class PropertyUtils { */ @Nullable public static PsiExpression getGetterReturnExpression(PsiMethod method) { - if (method == null) { - return null; - } - final PsiParameterList parameterList = method.getParameterList(); - if (parameterList.getParametersCount() != 0) { - return null; - } - @NonNls final String name = method.getName(); - if (!name.startsWith("get") && !name.startsWith("is")) { - return null; - } - if (method.hasModifierProperty(PsiModifier.SYNCHRONIZED)) { - return null; - } + return method != null && hasGetterSignature(method) ? getSingleReturnValue(method) : null; + } + + private static boolean hasGetterSignature(@NotNull PsiMethod method) { + return PropertyUtil.isSimplePropertyGetter(method) && !method.hasModifierProperty(PsiModifier.SYNCHRONIZED); + } + + @Nullable + public static PsiExpression getSingleReturnValue(@NotNull PsiMethod method) { final PsiCodeBlock body = method.getBody(); if (body == null) { return null; } final PsiStatement[] statements = body.getStatements(); - if (statements.length != 1) { - return null; - } - final PsiStatement statement = statements[0]; - if (!(statement instanceof PsiReturnStatement)) { - return null; - } - final PsiReturnStatement returnStatement = - (PsiReturnStatement)statement; - final PsiExpression value = returnStatement.getReturnValue(); - if (value == null) { - return null; - } - return value; + final PsiStatement statement = statements.length != 1 ? null : statements[0]; + return statement instanceof PsiReturnStatement ? ((PsiReturnStatement)statement).getReturnValue() : null; } @Nullable public static PsiField getFieldOfGetter(PsiMethod method) { - final PsiExpression value = getGetterReturnExpression(method); - if (value == null) return null; + PsiField field = getSimplyReturnedField(method, getGetterReturnExpression(method)); + if (field != null) { + final PsiType returnType = method.getReturnType(); + if (returnType != null && field.getType().equalsToText(returnType.getCanonicalText())) { + return field; + } + } + return null; + } + + @Nullable + public static PsiField getSimplyReturnedField(PsiMethod method, @Nullable PsiExpression value) { if (!(value instanceof PsiReferenceExpression)) { return null; } + final PsiReferenceExpression reference = (PsiReferenceExpression)value; - final PsiExpression qualifier = reference.getQualifierExpression(); - if (qualifier instanceof PsiReferenceExpression) { - final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifier; - final PsiElement target = referenceExpression.resolve(); - if (!(target instanceof PsiClass)) { - return null; - } - } - else if (qualifier != null && !(qualifier instanceof PsiThisExpression) && !(qualifier instanceof PsiSuperExpression)) { + if (hasSubstantialQualifier(reference)) { return null; } + final PsiElement referent = reference.resolve(); - if (referent == null) { - return null; - } if (!(referent instanceof PsiField)) { return null; } + final PsiField field = (PsiField)referent; - final PsiType fieldType = field.getType(); - final PsiType returnType = method.getReturnType(); - if (returnType == null) { - return null; + return InheritanceUtil.isInheritorOrSelf(method.getContainingClass(), field.getContainingClass(), true) ? field : null; + } + + private static boolean hasSubstantialQualifier(PsiReferenceExpression reference) { + final PsiExpression qualifier = reference.getQualifierExpression(); + if (qualifier == null) return false; + + if (qualifier instanceof PsiThisExpression || qualifier instanceof PsiSuperExpression) { + return false; } - if (!fieldType.equalsToText(returnType.getCanonicalText())) { - return null; - } - final PsiClass fieldContainingClass = field.getContainingClass(); - final PsiClass methodContainingClass = method.getContainingClass(); - if (InheritanceUtil.isInheritorOrSelf(methodContainingClass, fieldContainingClass, true)) { - return field; - } - else { - return null; + + if (qualifier instanceof PsiReferenceExpression) { + return !(((PsiReferenceExpression)qualifier).resolve() instanceof PsiClass); } + return true; } public static boolean isSimpleGetter(PsiMethod method) { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldAccessorsDfa.java b/java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldAccessorsDfa.java new file mode 100644 index 000000000000..5eb2ef10dcc4 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldAccessorsDfa.java @@ -0,0 +1,86 @@ +import org.jetbrains.annotations.Nullable; + +import java.lang.String; + +public class BrokenAlignment { + + void main(Data data) { + if (data.getText() != null) { + System.out.println(data.getText().hashCode()); + } + + data = new Data(null, null); + System.out.println(data.getText().hashCode()); + + if (data.inner() != null) { + System.out.println(data.inner().hashCode()); + System.out.println(data.inner().getText().hashCode()); + /* + if (data.inner() != null) { + System.out.println(data.inner().hashCode()); + } + */ + + data = new Data(null, null); + System.out.println(data.inner().hashCode()); + } + } + + void main2(Data data) { + if (data.inner() != null && data.inner().getText() != null) { + System.out.println(data.inner().hashCode()); + System.out.println(data.inner().getText().hashCode()); + } + } + + void main3(Data data) { + if (data.innerOverridden() != null) { + System.out.println(data.innerOverridden().hashCode()); + } + if (data.something() != null) { + System.out.println(data.something().hashCode()); + } + } + + private static class Data { + @Nullable final String text; + @Nullable final Data inner; + + Data(@Nullable String text, Data inner) { + this.text = text; + this.inner = inner; + } + + @Nullable + public String getText() { + return text; + } + + @Nullable + public Data inner() { + return inner; + } + + @Nullable + public Data innerOverridden() { + return inner; + } + + @Nullable + public String something() { + return new String(); + } + } + + class DataImpl extends Data { + DataImpl(@Nullable String text, Data inner) { + super(text, inner); + } + + @Nullable + @Override + public Data innerOverridden() { + return super.innerOverridden(); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java index cf55cb61a4f3..0e7f4ec099c0 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java @@ -79,5 +79,6 @@ public class DataFlowInspectionFixtureTest extends JavaCodeInsightFixtureTestCas public void testGreaterIsNotEquals() throws Throwable { doTest(); } public void testChainedFinalFieldsDfa() throws Throwable { doTest(); } + public void testChainedFinalFieldAccessorsDfa() throws Throwable { doTest(); } } From 65bf4558cbb029b1974cc1629391c84da05903c4 Mon Sep 17 00:00:00 2001 From: "Denis.Zhdanov" Date: Mon, 10 Sep 2012 23:34:21 +0400 Subject: [PATCH 5/7] IDEA-19061 Integrate the Rearranger-plugin into core-IDE Preserving range markers on arrangement --- .../openapi/editor/impl/DocumentImpl.java | 3 +- .../arrangement/engine/ArrangementEngine.java | 355 ++++++++++++------ .../engine/ArrangementEntryWrapper.java | 55 ++- 3 files changed, 302 insertions(+), 111 deletions(-) diff --git a/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java b/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java index 09425a675133..559d2da1486a 100644 --- a/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java +++ b/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java @@ -374,7 +374,8 @@ public class DocumentImpl extends UserDataHolderBase implements DocumentEx { if (dstOffset == srcEnd) return; assert !srcRange.containsOffset(dstOffset); - CharSequence replacement = getCharsSequence().subSequence(srcStart, srcEnd); + //CharSequence replacement = getCharsSequence().subSequence(srcStart, srcEnd); + String replacement = getCharsSequence().subSequence(srcStart, srcEnd).toString(); insertString(dstOffset, replacement); int shift = 0; diff --git a/platform/lang-impl/src/com/intellij/psi/codeStyle/arrangement/engine/ArrangementEngine.java b/platform/lang-impl/src/com/intellij/psi/codeStyle/arrangement/engine/ArrangementEngine.java index a4019083c9da..3023e42590e4 100644 --- a/platform/lang-impl/src/com/intellij/psi/codeStyle/arrangement/engine/ArrangementEngine.java +++ b/platform/lang-impl/src/com/intellij/psi/codeStyle/arrangement/engine/ArrangementEngine.java @@ -45,7 +45,7 @@ import java.util.*; *

* I.e. the general idea is to have a language-specific rules hidden by generic arrangement API and common arrangement * engine which works on top of that API and performs the arrangement. - * + * * @author Denis Zhdanov * @since 7/20/12 1:56 PM */ @@ -53,7 +53,7 @@ public class ArrangementEngine { /** * Arranges given PSI root contents that belong to the given ranges. - * + * * @param file target PSI root * @param ranges target ranges to use within the given root */ @@ -80,7 +80,7 @@ public class ArrangementEngine { if (arrangementRules.isEmpty()) { return; } - + final DocumentEx documentEx; if (document instanceof DocumentEx && !((DocumentEx)document).isInBulkUpdate()) { documentEx = (DocumentEx)document; @@ -196,7 +196,7 @@ public class ArrangementEngine { /** * Arranges (re-orders) given entries according to the given rules. - * + * * @param entries entries to arrange * @param rules rules to use for arrangement * @param arrangement entry type @@ -226,7 +226,7 @@ public class ArrangementEngine { } } } - + Set matched = new HashSet(); for (ArrangementRule rule : rules) { @@ -256,7 +256,7 @@ public class ArrangementEngine { return arranged; } - + @SuppressWarnings("unchecked") private static void doArrange(@NotNull List> wrappers, @NotNull Context context) { @@ -270,12 +270,12 @@ public class ArrangementEngine { List arranged = arrange(map.keySet(), context.rules); - context.prepare(wrappers.get(0).getParent()); + context.changer.prepare(wrappers, context); // We apply changes from the last position to the first position in order not to bother with offsets shifts. for (int i = arranged.size() - 1; i >= 0; i--) { ArrangementEntryWrapper arrangedWrapper = map.get(arranged.get(i)); ArrangementEntryWrapper initialWrapper = wrappers.get(i); - context.replace(arrangedWrapper, initialWrapper, i > 0 ? map.get(arranged.get(i - 1)) : null); + context.changer.replace(arrangedWrapper, initialWrapper, i > 0 ? map.get(arranged.get(i - 1)) : null, context); } } @@ -285,22 +285,21 @@ public class ArrangementEngine { @NotNull public final Collection> wrappers; @NotNull public final Document document; @NotNull public final List rules; - @NotNull public final CodeStyleSettings mySettings; - - @NotNull private String myParentText; - private int myParentShift; + @NotNull public final CodeStyleSettings settings; + @NotNull public final Changer changer; private Context(@NotNull Rearranger rearranger, @NotNull Collection> wrappers, @NotNull Document document, @NotNull List rules, - @NotNull CodeStyleSettings settings) + @NotNull CodeStyleSettings settings, @NotNull Changer changer) { this.rearranger = rearranger; this.wrappers = wrappers; this.document = document; this.rules = rules; - mySettings = settings; + this.settings = settings; + this.changer = changer; } public static Context from(@NotNull Rearranger rearranger, @@ -322,99 +321,17 @@ public class ArrangementEngine { wrappers.add(wrapper); previous = wrapper; } - return new Context(rearranger, wrappers, document, rules, settings); - } - - public void prepare(@Nullable ArrangementEntryWrapper parent) { - if (parent == null) { - myParentText = document.getText(); - myParentShift = 0; - } - else { - myParentText = document.getCharsSequence().subSequence(parent.getStartOffset(), parent.getEndOffset()).toString(); - myParentShift = parent.getStartOffset(); - } - } - - /** - * Replaces given 'old entry' by the given 'new entry'. - * - * @param newWrapper wrapper for an entry which text should replace given 'old entry' range - * @param oldWrapper wrapper for an entry which range should be replaced by the given 'new entry' - * @param previous wrapper which will be previous for the entry referenced via the given 'new wrapper' - */ - @SuppressWarnings("AssignmentToForLoopParameter") - public void replace(@NotNull ArrangementEntryWrapper newWrapper, - @NotNull ArrangementEntryWrapper oldWrapper, - @Nullable ArrangementEntryWrapper previous) - { - // Calculate blank lines before the arrangement. - int blankLinesBefore = 0; - TIntArrayList lineFeedOffsets = new TIntArrayList(); - int oldStartLine = document.getLineNumber(oldWrapper.getStartOffset()); - if (oldStartLine > 0) { - int lastLineFeed = document.getLineStartOffset(oldStartLine) - 1; - lineFeedOffsets.add(lastLineFeed); - for (int i = lastLineFeed - 1 - myParentShift; i >= 0; i--) { - i = CharArrayUtil.shiftBackward(myParentText, i, " \t"); - if (myParentText.charAt(i) == '\n') { - blankLinesBefore++; - lineFeedOffsets.add(i + myParentShift); - } - else { - break; - } - } - } - - ArrangementEntryWrapper parentWrapper = oldWrapper.getParent(); - int desiredBlankLinesNumber = rearranger.getBlankLines(mySettings, - parentWrapper == null ? null : parentWrapper.getEntry(), - previous == null ? null : previous.getEntry(), - newWrapper.getEntry()); - if (desiredBlankLinesNumber == blankLinesBefore && newWrapper.equals(oldWrapper)) { - return; - } - - String newEntryText = myParentText.substring(newWrapper.getStartOffset() - myParentShift, newWrapper.getEndOffset() - myParentShift); - int lineFeedsDiff = desiredBlankLinesNumber - blankLinesBefore; - if (lineFeedsDiff == 0 || desiredBlankLinesNumber < 0) { - document.replaceString(oldWrapper.getStartOffset(), oldWrapper.getEndOffset(), newEntryText); - return; - } - - if (lineFeedsDiff > 0) { - // Insert necessary number of blank lines. - StringBuilder buffer = new StringBuilder(StringUtil.repeat("\n", lineFeedsDiff)); - buffer.append(newEntryText); - document.replaceString(oldWrapper.getStartOffset(), oldWrapper.getEndOffset(), buffer); - } - else { - // Cut exceeding blank lines. - int replacementStartOffset = lineFeedOffsets.get(-lineFeedsDiff) + 1; - document.replaceString(replacementStartOffset, oldWrapper.getEndOffset(), newEntryText); - } - - // Update wrapper ranges. - ArrangementEntryWrapper parent = oldWrapper.getParent(); - if (parent == null) { - return; - } - - Deque> parents = new ArrayDeque>(); - do { - parents.add(parent); - parent.setEndOffset(parent.getEndOffset() + lineFeedsDiff); - parent = parent.getParent(); - } while (parent != null); - - - while (!parents.isEmpty()) { - - for (ArrangementEntryWrapper wrapper = parents.removeLast().getNext(); wrapper != null; wrapper = wrapper.getNext()) { - wrapper.applyShift(lineFeedsDiff); - } - } + Changer changer; + // TODO den remove + changer = new NormalChanger(); + // TODO den uncomment + //if (document instanceof DocumentEx) { + // changer = new RangeMarkerAwareChanger((DocumentEx)document); + //} + //else { + // changer = new NormalChanger(); + //} + return new Context(rearranger, wrappers, document, rules, settings, changer); } } @@ -430,4 +347,228 @@ public class ArrangementEngine { end = start + count; } } + + private interface Changer { + void prepare(@NotNull List> toArrange, @NotNull Context context); + + /** + * Replaces given 'old entry' by the given 'new entry'. + * + * @param newWrapper wrapper for an entry which text should replace given 'old entry' range + * @param oldWrapper wrapper for an entry which range should be replaced by the given 'new entry' + * @param previous wrapper which will be previous for the entry referenced via the given 'new wrapper' + * @param context current context + */ + void replace(@NotNull ArrangementEntryWrapper newWrapper, + @NotNull ArrangementEntryWrapper oldWrapper, + @Nullable ArrangementEntryWrapper previous, + @NotNull Context context); + } + + private static class NormalChanger implements Changer { + + @NotNull private String myParentText; + private int myParentShift; + + @Override + public void prepare(@NotNull List> toArrange, @NotNull Context context) { + ArrangementEntryWrapper parent = toArrange.get(0).getParent(); + if (parent == null) { + myParentText = context.document.getText(); + myParentShift = 0; + } + else { + myParentText = context.document.getCharsSequence().subSequence(parent.getStartOffset(), parent.getEndOffset()).toString(); + myParentShift = parent.getStartOffset(); + } + } + + @SuppressWarnings("AssignmentToForLoopParameter") + @Override + public void replace(@NotNull ArrangementEntryWrapper newWrapper, + @NotNull ArrangementEntryWrapper oldWrapper, + @Nullable ArrangementEntryWrapper previous, + @NotNull Context context) + { + // Calculate blank lines before the arrangement. + int blankLinesBefore = 0; + TIntArrayList lineFeedOffsets = new TIntArrayList(); + int oldStartLine = context.document.getLineNumber(oldWrapper.getStartOffset()); + if (oldStartLine > 0) { + int lastLineFeed = context.document.getLineStartOffset(oldStartLine) - 1; + lineFeedOffsets.add(lastLineFeed); + for (int i = lastLineFeed - 1 - myParentShift; i >= 0; i--) { + i = CharArrayUtil.shiftBackward(myParentText, i, " \t"); + if (myParentText.charAt(i) == '\n') { + blankLinesBefore++; + lineFeedOffsets.add(i + myParentShift); + } + else { + break; + } + } + } + + ArrangementEntryWrapper parentWrapper = oldWrapper.getParent(); + int desiredBlankLinesNumber = context.rearranger.getBlankLines(context.settings, + parentWrapper == null ? null : parentWrapper.getEntry(), + previous == null ? null : previous.getEntry(), + newWrapper.getEntry()); + if (desiredBlankLinesNumber == blankLinesBefore && newWrapper.equals(oldWrapper)) { + return; + } + + String newEntryText = myParentText.substring(newWrapper.getStartOffset() - myParentShift, newWrapper.getEndOffset() - myParentShift); + int lineFeedsDiff = desiredBlankLinesNumber - blankLinesBefore; + if (lineFeedsDiff == 0 || desiredBlankLinesNumber < 0) { + context.document.replaceString(oldWrapper.getStartOffset(), oldWrapper.getEndOffset(), newEntryText); + return; + } + + if (lineFeedsDiff > 0) { + // Insert necessary number of blank lines. + StringBuilder buffer = new StringBuilder(StringUtil.repeat("\n", lineFeedsDiff)); + buffer.append(newEntryText); + context.document.replaceString(oldWrapper.getStartOffset(), oldWrapper.getEndOffset(), buffer); + } + else { + // Cut exceeding blank lines. + int replacementStartOffset = lineFeedOffsets.get(-lineFeedsDiff) + 1; + context.document.replaceString(replacementStartOffset, oldWrapper.getEndOffset(), newEntryText); + } + + // Update wrapper ranges. + ArrangementEntryWrapper parent = oldWrapper.getParent(); + if (parent == null) { + return; + } + + Deque> parents = new ArrayDeque>(); + do { + parents.add(parent); + parent.setEndOffset(parent.getEndOffset() + lineFeedsDiff); + parent = parent.getParent(); + } + while (parent != null); + + + while (!parents.isEmpty()) { + + for (ArrangementEntryWrapper wrapper = parents.removeLast().getNext(); wrapper != null; wrapper = wrapper.getNext()) { + wrapper.applyShift(lineFeedsDiff); + } + } + } + } + + private static class RangeMarkerAwareChanger implements Changer { + + @NotNull private final Set> myNotMoved = new HashSet>(); + @NotNull private final DocumentEx myDocument; + + RangeMarkerAwareChanger(@NotNull DocumentEx document) { + myDocument = document; + } + + @Override + public void prepare(@NotNull List> toArrange, @NotNull Context context) { + myNotMoved.clear(); + myNotMoved.addAll(toArrange); + for (ArrangementEntryWrapper wrapper : toArrange) { + wrapper.updateBlankLines(myDocument); + } + } + + @SuppressWarnings("AssignmentToForLoopParameter") + @Override + public void replace(@NotNull ArrangementEntryWrapper newWrapper, + @NotNull ArrangementEntryWrapper oldWrapper, + @Nullable ArrangementEntryWrapper previous, + @NotNull Context context) + { + // Calculate blank lines before the arrangement. + int blankLinesBefore = oldWrapper.getBlankLinesBefore(); + + ArrangementEntryWrapper parentWrapper = oldWrapper.getParent(); + int desiredBlankLinesNumber = context.rearranger.getBlankLines(context.settings, + parentWrapper == null ? null : parentWrapper.getEntry(), + previous == null ? null : previous.getEntry(), + newWrapper.getEntry()); + if ((desiredBlankLinesNumber < 0 || desiredBlankLinesNumber == blankLinesBefore) && newWrapper.equals(oldWrapper)) { + return; + } + + int lineFeedsDiff = desiredBlankLinesNumber - blankLinesBefore; + int insertionOffset = oldWrapper.getStartOffset(); + if (oldWrapper.getStartOffset() > newWrapper.getStartOffset()) { + insertionOffset -= newWrapper.getEndOffset() - newWrapper.getStartOffset(); + } + myDocument.moveText(newWrapper.getStartOffset(), newWrapper.getEndOffset(), oldWrapper.getStartOffset()); + newWrapper.placeBefore(oldWrapper); + myNotMoved.remove(newWrapper); + for (ArrangementEntryWrapper w : myNotMoved) { + if (w.getStartOffset() >= oldWrapper.getStartOffset() && w.getStartOffset() < newWrapper.getStartOffset()) { + w.applyShift(newWrapper.getEndOffset() - newWrapper.getStartOffset()); + } + else if (w.getStartOffset() < oldWrapper.getStartOffset() && w.getStartOffset() > newWrapper.getStartOffset()) { + w.applyShift(newWrapper.getStartOffset() - newWrapper.getEndOffset()); + } + } + + if (desiredBlankLinesNumber >= 0 && lineFeedsDiff > 0) { + myDocument.insertString(insertionOffset, StringUtil.repeat("\n", lineFeedsDiff)); + shiftOffsets(newWrapper, null, lineFeedsDiff); + } + + if (desiredBlankLinesNumber >= 0 && lineFeedsDiff < 0) { + // Cut exceeding blank lines. + int replacementStartOffset = getBlankLineOffset(-lineFeedsDiff, insertionOffset); + myDocument.deleteString(replacementStartOffset, insertionOffset); + shiftOffsets(oldWrapper, null, lineFeedsDiff); + } + + // Update wrapper ranges. + if (desiredBlankLinesNumber < 0 || lineFeedsDiff == 0 || parentWrapper == null) { + return; + } + + Deque> parents = new ArrayDeque>(); + do { + parents.add(parentWrapper); + parentWrapper.setEndOffset(parentWrapper.getEndOffset() + lineFeedsDiff); + parentWrapper = parentWrapper.getParent(); + } + while (parentWrapper != null); + + + while (!parents.isEmpty()) { + for (ArrangementEntryWrapper wrapper = parents.removeLast().getNext(); wrapper != null; wrapper = wrapper.getNext()) { + wrapper.applyShift(lineFeedsDiff); + } + } + } + + private int getBlankLineOffset(int blankLinesNumber, int startOffset) { + int startLine = myDocument.getLineNumber(startOffset); + if (startLine <= 0) { + return 0; + } + CharSequence text = myDocument.getCharsSequence(); + for (int i = myDocument.getLineStartOffset(startLine - 1) - 1; i >= 0; i = CharArrayUtil.lastIndexOf(text, "\n", i - 1)) { + if (--blankLinesNumber <= 0) { + return i; + } + } + return 0; + } + + private static void shiftOffsets(@Nullable ArrangementEntryWrapper first, @Nullable ArrangementEntryWrapper last, int shift) { + if (first == null) { + return; + } + for (ArrangementEntryWrapper wrapper = first; wrapper != null && wrapper != last; wrapper = wrapper.getNext()) { + wrapper.applyShift(shift); + } + } + } } diff --git a/platform/lang-impl/src/com/intellij/psi/codeStyle/arrangement/engine/ArrangementEntryWrapper.java b/platform/lang-impl/src/com/intellij/psi/codeStyle/arrangement/engine/ArrangementEntryWrapper.java index 53239189beeb..ffa269459054 100644 --- a/platform/lang-impl/src/com/intellij/psi/codeStyle/arrangement/engine/ArrangementEntryWrapper.java +++ b/platform/lang-impl/src/com/intellij/psi/codeStyle/arrangement/engine/ArrangementEntryWrapper.java @@ -15,8 +15,10 @@ */ package com.intellij.psi.codeStyle.arrangement.engine; +import com.intellij.openapi.editor.Document; import com.intellij.psi.PsiFile; import com.intellij.psi.codeStyle.arrangement.ArrangementEntry; +import com.intellij.util.text.CharArrayUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -36,7 +38,7 @@ import java.util.List; * *

* Not thread-safe. - * + * * @author Denis Zhdanov * @since 8/31/12 12:06 PM */ @@ -51,15 +53,22 @@ public class ArrangementEntryWrapper { private int myStartOffset; private int myEndOffset; + private int myBlankLinesBefore; @SuppressWarnings("unchecked") public ArrangementEntryWrapper(@NotNull E entry) { myEntry = entry; myStartOffset = entry.getStartOffset(); myEndOffset = entry.getEndOffset(); + ArrangementEntryWrapper previous = null; for (ArrangementEntry child : entry.getChildren()) { ArrangementEntryWrapper childWrapper = new ArrangementEntryWrapper((E)child); childWrapper.setParent(this); + if (previous != null) { + previous.setNext(childWrapper); + childWrapper.setPrevious(previous); + } + previous = childWrapper; myChildren.add(childWrapper); } } @@ -104,6 +113,46 @@ public class ArrangementEntryWrapper { return myNext; } + public void placeBefore(@NotNull ArrangementEntryWrapper anchor) { + if (myNext != null) { + myNext.setPrevious(myPrevious); + } + if (myPrevious != null) { + myPrevious.setNext(myNext); + } + setPrevious(anchor.getPrevious()); + if (myPrevious != null) { + myPrevious.setNext(this); + } + setNext(anchor); + anchor.setPrevious(this); + } + + public int getBlankLinesBefore() { + return myBlankLinesBefore; + } + + @SuppressWarnings("AssignmentToForLoopParameter") + public void updateBlankLines(@NotNull Document document) { + int startLine = document.getLineNumber(getStartOffset()); + myBlankLinesBefore = 0; + if (startLine <= 0) { + return; + } + + CharSequence text = document.getCharsSequence(); + int lastLineFeed = document.getLineStartOffset(startLine) - 1; + for (int i = lastLineFeed - 1; i >= 0; i--) { + i = CharArrayUtil.shiftBackward(text, i, " \t"); + if (text.charAt(i) == '\n') { + ++myBlankLinesBefore; + } + else { + break; + } + } + } + public void setNext(@Nullable ArrangementEntryWrapper next) { myNext = next; } @@ -120,7 +169,7 @@ public class ArrangementEntryWrapper { child.applyShift(shift); } } - + @Override public int hashCode() { return myEntry.hashCode(); @@ -141,6 +190,6 @@ public class ArrangementEntryWrapper { @Override public String toString() { - return myEntry.toString(); + return String.format("range: [%d; %d), entry: %s", myStartOffset, myEndOffset, myEntry.toString()); } } From 93204abcaf001f830ad361b7a34a387b10cd162f Mon Sep 17 00:00:00 2001 From: Eugene Zhuravlev Date: Mon, 10 Sep 2012 22:44:43 +0200 Subject: [PATCH 6/7] do not store paths in symbol table => honor file system's case sensitivity when comparing files --- .../dependencyView/DependencyContext.java | 14 - .../java/dependencyView/IntObjectMaplet.java | 69 +++++ .../IntObjectPersistentMaplet.java | 138 ++++++++++ .../IntObjectTransientMaplet.java | 56 ++++ .../java/dependencyView/Mappings.java | 232 +++++++--------- .../ObjectObjectMultiMaplet.java | 105 ++++++++ .../ObjectObjectPersistentMultiMaplet.java | 248 ++++++++++++++++++ .../ObjectObjectTransientMultiMaplet.java | 130 +++++++++ .../incremental/storage/BuildDataManager.java | 2 +- .../storage/FileKeyDescriptor.java | 34 +++ .../incremental/storage/TimestampStorage.java | 23 -- 11 files changed, 872 insertions(+), 179 deletions(-) create mode 100644 jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectMaplet.java create mode 100644 jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectPersistentMaplet.java create mode 100644 jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectTransientMaplet.java create mode 100644 jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectMultiMaplet.java create mode 100644 jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectPersistentMultiMaplet.java create mode 100644 jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectTransientMultiMaplet.java create mode 100644 jps/jps-builders/src/org/jetbrains/jps/incremental/storage/FileKeyDescriptor.java diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/DependencyContext.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/DependencyContext.java index 75b842cbb7de..e374f74de44e 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/DependencyContext.java +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/DependencyContext.java @@ -84,20 +84,6 @@ class DependencyContext { } } - public int getFilePath(final String path) { - try { - if (StringUtil.isEmpty(path)) { - return myEmptyName; - } - final String _path = FileUtil.toSystemIndependentName(path); - //return myEnumerator.enumerate(SystemInfo.isFileSystemCaseSensitive ? _path : _path.toLowerCase(Locale.US)); - return myEnumerator.enumerate(_path); - } - catch (IOException e) { - throw new RuntimeException(e); - } - } - public void close() { try { myEnumerator.close(); diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectMaplet.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectMaplet.java new file mode 100644 index 000000000000..23782adaebf1 --- /dev/null +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectMaplet.java @@ -0,0 +1,69 @@ +/* + * Copyright 2000-2011 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jetbrains.jps.builders.java.dependencyView; + +import gnu.trove.TIntObjectProcedure; + +import java.io.PrintStream; + +/** + * Created by IntelliJ IDEA. + * User: db + * Date: 04.11.11 + * Time: 23:48 + * To change this template use File | Settings | File Templates. + */ +abstract class IntObjectMaplet implements Streamable { + abstract boolean containsKey(final int key); + + abstract V get(final int key); + + abstract void put(final int key, final V value); + + abstract void putAll(IntObjectMaplet m); + + abstract void remove(final int key); + + abstract void close(); + + abstract void forEachEntry(TIntObjectProcedure proc); + + abstract void flush(boolean memoryCachesOnly); + + public void toStream(final DependencyContext context, final PrintStream stream) { + final OrderProvider op = new OrderProvider(context); + + forEachEntry(new TIntObjectProcedure() { + @Override + public boolean execute(final int a, final V b) { + op.register(a); + return true; + } + }); + + final int[] keys = op.get(); + + for (final int a : keys) { + final V b = get(a); + + stream.print(" "); + stream.print(context.getValue(a)); + stream.print(" -> "); + + stream.print(b.toString()); + } + } +} diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectPersistentMaplet.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectPersistentMaplet.java new file mode 100644 index 000000000000..37d6d374bdb2 --- /dev/null +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectPersistentMaplet.java @@ -0,0 +1,138 @@ +package org.jetbrains.jps.builders.java.dependencyView; + +import com.intellij.util.Processor; +import com.intellij.util.containers.SLRUCache; +import com.intellij.util.io.DataExternalizer; +import com.intellij.util.io.IntInlineKeyDescriptor; +import com.intellij.util.io.PersistentHashMap; +import gnu.trove.TIntObjectProcedure; +import org.jetbrains.annotations.NotNull; + +import java.io.File; +import java.io.IOException; + +/** + * @author Eugene Zhuravlev + * Date: 9/10/12 + */ +public class IntObjectPersistentMaplet extends IntObjectMaplet{ + + private static final Object NULL_OBJ = new Object(); + private static final int CACHE_SIZE = 512; + private final PersistentHashMap myMap; + private final SLRUCache myCache; + + public IntObjectPersistentMaplet(final File file, final DataExternalizer externalizer) { + try { + myMap = new PersistentHashMap(file, new IntInlineKeyDescriptor(), externalizer); + myCache = new SLRUCache(CACHE_SIZE, CACHE_SIZE) { + @NotNull + @Override + public Object createValue(Integer key) { + try { + final V v1 = myMap.get(key); + return v1 == null? NULL_OBJ : v1; + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + }; + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public boolean containsKey(final int key) { + try { + return myMap.containsMapping(key); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public V get(final int key) { + final Object obj = myCache.get(key); + return obj == NULL_OBJ? null : (V)obj; + } + + @Override + public void put(final int key, final V value) { + try { + myCache.remove(key); + myMap.put(key, value); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public void putAll(final IntObjectMaplet m) { + m.forEachEntry(new TIntObjectProcedure() { + @Override + public boolean execute(int key, V value) { + put(key, value); + return true; + } + }); + } + + @Override + public void remove(final int key) { + try { + myCache.remove(key); + myMap.remove(key); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public void close() { + try { + myCache.clear(); + myMap.close(); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + public void flush(boolean memoryCachesOnly) { + if (memoryCachesOnly) { + if (myMap.isDirty()) { + myMap.dropMemoryCaches(); + } + } + else { + myMap.force(); + } + } + + @Override + public void forEachEntry(final TIntObjectProcedure proc) { + try { + myMap.processKeysWithExistingMapping(new Processor() { + @Override + public boolean process(Integer key) { + try { + final V value = myMap.get(key); + return value == null? proc.execute(key, null) : proc.execute(key, value); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + }); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } +} diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectTransientMaplet.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectTransientMaplet.java new file mode 100644 index 000000000000..40c13670e715 --- /dev/null +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/IntObjectTransientMaplet.java @@ -0,0 +1,56 @@ +package org.jetbrains.jps.builders.java.dependencyView; + +import gnu.trove.TIntObjectHashMap; +import gnu.trove.TIntObjectProcedure; + +/** + * @author Eugene Zhuravlev + * Date: 9/10/12 + */ +public class IntObjectTransientMaplet extends IntObjectMaplet{ + private final TIntObjectHashMap myMap = new TIntObjectHashMap(); + @Override + boolean containsKey(int key) { + return myMap.containsKey(key); + } + + @Override + V get(int key) { + return myMap.get(key); + } + + @Override + void put(int key, V value) { + myMap.put(key, value); + } + + @Override + void putAll(IntObjectMaplet m) { + m.forEachEntry(new TIntObjectProcedure() { + @Override + public boolean execute(int key, V value) { + myMap.put(key, value); + return true; + } + }); + } + + @Override + void remove(int key) { + myMap.remove(key); + } + + @Override + void close() { + myMap.clear(); + } + + @Override + void forEachEntry(TIntObjectProcedure proc) { + myMap.forEachEntry(proc); + } + + @Override + void flush(boolean memoryCachesOnly) { + } +} diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java index c63c0b29f996..ffda5c7e2280 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java @@ -5,14 +5,12 @@ import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.Ref; import com.intellij.openapi.util.io.FileUtil; import com.intellij.util.io.IntInlineKeyDescriptor; -import gnu.trove.TIntHashSet; -import gnu.trove.TIntIntProcedure; -import gnu.trove.TIntObjectProcedure; -import gnu.trove.TIntProcedure; +import gnu.trove.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.asm4.ClassReader; import org.jetbrains.asm4.Opcodes; +import org.jetbrains.jps.incremental.storage.FileKeyDescriptor; import java.io.File; import java.io.FileNotFoundException; @@ -36,8 +34,6 @@ public class Mappings { private final static String CLASS_TO_SUBCLASSES = "classToSubclasses.tab"; private final static String CLASS_TO_CLASS = "classToClass.tab"; private final static String SOURCE_TO_CLASS = "sourceToClass.tab"; - private final static String SOURCE_TO_ANNOTATIONS = "sourceToAnnotations.tab"; - private final static String SOURCE_TO_USAGES = "sourceToUsages.tab"; private final static String CLASS_TO_SOURCE = "classToSource.tab"; private static final IntInlineKeyDescriptor INT_KEY_DESCRIPTOR = new IntInlineKeyDescriptor(); private static final int DEFAULT_SET_CAPACITY = 32; @@ -54,7 +50,7 @@ public class Mappings { private boolean myIsRebuild = false; private final TIntHashSet myChangedClasses; - private final TIntHashSet myChangedFiles; + private final THashSet myChangedFiles; private final Set myDeletedClasses; private final Object myLock; private final File myRootDir; @@ -66,8 +62,8 @@ public class Mappings { private IntIntMultiMaplet myClassToSubclasses; private IntIntMultiMaplet myClassToClassDependency; - private IntObjectMultiMaplet mySourceFileToClasses; - private IntIntMaplet myClassToSourceFile; + private ObjectObjectMultiMaplet mySourceFileToClasses; + private IntObjectMaplet myClassToSourceFile; private IntIntTransientMultiMaplet myRemovedSuperClasses; private IntIntTransientMultiMaplet myAddedSuperClasses; @@ -79,7 +75,7 @@ public class Mappings { myLock = base.myLock; myIsDelta = true; myChangedClasses = new TIntHashSet(DEFAULT_SET_CAPACITY, DEFAULT_SET_LOAD_FACTOR); - myChangedFiles = new TIntHashSet(DEFAULT_SET_CAPACITY, DEFAULT_SET_LOAD_FACTOR); + myChangedFiles = new THashSet(FileUtil.FILE_HASHING_STRATEGY); myDeletedClasses = new HashSet(DEFAULT_SET_CAPACITY, DEFAULT_SET_LOAD_FACTOR); myDeltaIsTransient = base.myDeltaIsTransient; myRootDir = new File(FileUtil.toSystemIndependentName(base.myRootDir.getAbsolutePath()) + File.separatorChar + "myDelta"); @@ -115,8 +111,8 @@ public class Mappings { if (myIsDelta && myDeltaIsTransient) { myClassToSubclasses = new IntIntTransientMultiMaplet(); myClassToClassDependency = new IntIntTransientMultiMaplet(); - mySourceFileToClasses = new IntObjectTransientMultiMaplet(ourClassSetConstructor); - myClassToSourceFile = new IntIntTransientMaplet(); + mySourceFileToClasses = new ObjectObjectTransientMultiMaplet(FileUtil.FILE_HASHING_STRATEGY, ourClassSetConstructor); + myClassToSourceFile = new IntObjectTransientMaplet(); } else { if (myIsDelta) { @@ -124,11 +120,11 @@ public class Mappings { } myClassToSubclasses = new IntIntPersistentMultiMaplet(DependencyContext.getTableFile(myRootDir, CLASS_TO_SUBCLASSES), INT_KEY_DESCRIPTOR); myClassToClassDependency = new IntIntPersistentMultiMaplet(DependencyContext.getTableFile(myRootDir, CLASS_TO_CLASS), INT_KEY_DESCRIPTOR); - mySourceFileToClasses = new IntObjectPersistentMultiMaplet( - DependencyContext.getTableFile(myRootDir, SOURCE_TO_CLASS), INT_KEY_DESCRIPTOR, ClassRepr.externalizer(myContext), + mySourceFileToClasses = new ObjectObjectPersistentMultiMaplet( + DependencyContext.getTableFile(myRootDir, SOURCE_TO_CLASS), new FileKeyDescriptor(), ClassRepr.externalizer(myContext), ourClassSetConstructor ); - myClassToSourceFile = new IntIntPersistentMaplet(DependencyContext.getTableFile(myRootDir, CLASS_TO_SOURCE), INT_KEY_DESCRIPTOR); + myClassToSourceFile = new IntObjectPersistentMaplet(DependencyContext.getTableFile(myRootDir, CLASS_TO_SOURCE), new FileKeyDescriptor()); } } @@ -146,9 +142,8 @@ public class Mappings { private void compensateRemovedContent(final Collection compiled) { if (compiled != null) { for (final File file : compiled) { - final int fileName = myContext.getFilePath(file.getPath()); - if (!mySourceFileToClasses.containsKey(fileName)) { - mySourceFileToClasses.put(fileName, new HashSet()); + if (!mySourceFileToClasses.containsKey(file)) { + mySourceFileToClasses.put(file, new HashSet()); } } } @@ -156,9 +151,9 @@ public class Mappings { @Nullable private ClassRepr getReprByName(final int name) { - final int source = myClassToSourceFile.get(name); + final File source = myClassToSourceFile.get(name); - if (source > 0) { + if (source != null) { final Collection reprs = mySourceFileToClasses.get(source); if (reprs != null) { @@ -493,8 +488,8 @@ public class Mappings { private void affectSubclasses(final int className, final Collection affectedFiles, final Collection affectedUsages, final TIntHashSet dependants, final boolean usages) { debug("Affecting subclasses of class: ", className); - final int fileName = myClassToSourceFile.get(className); - if (fileName <= 0) { + final File fileName = myClassToSourceFile.get(className); + if (fileName == null) { debug("No source file detected for class ", className); debug("End of affectSubclasses"); return; @@ -516,7 +511,7 @@ public class Mappings { if (depClasses != null) { addAll(dependants, depClasses); } - affectedFiles.add(new File(myContext.getValue(fileName))); + affectedFiles.add(fileName); final TIntHashSet directSubclasses = myClassToSubclasses.get(className); if (directSubclasses != null) { @@ -631,18 +626,17 @@ public class Mappings { } private void affectAll(final int className, final Collection affectedFiles, @Nullable final DependentFilesFilter filter) { - final int sourceFile = myClassToSourceFile.get(className); - if (sourceFile > 0) { + final File sourceFile = myClassToSourceFile.get(className); + if (sourceFile != null) { final TIntHashSet dependants = myClassToClassDependency.get(className); if (dependants != null) { dependants.forEach(new TIntProcedure() { @Override public boolean execute(int depClass) { - final int depFile = myClassToSourceFile.get(depClass); - if (depFile > 0 && depFile != sourceFile) { - final File theFile = new File(myContext.getValue(depFile)); - if (filter == null || filter.accept(theFile)) { - affectedFiles.add(theFile); + final File depFile = myClassToSourceFile.get(depClass); + if (depFile != null && !FileUtil.filesEqual(depFile, sourceFile)) { + if (filter == null || filter.accept(depFile)) { + affectedFiles.add(depFile); } } return true; @@ -702,26 +696,17 @@ public class Mappings { debug("Root class: ", owner); final TIntHashSet propagated = self.propagateFieldAccess(isField ? member.name : myEmptyName, owner); - final TIntHashSet fileNames = new TIntHashSet(propagated.size()); propagated.forEach(new TIntProcedure() { @Override public boolean execute(int className) { - final int fileName = myClassToSourceFile.get(className); - if (fileName > 0) { - fileNames.add(fileName); + final File fileName = myClassToSourceFile.get(className); + if (fileName != null) { + debug("Adding ", fileName); + affectedFiles.add(fileName); } return true; } }); - fileNames.forEach(new TIntProcedure() { - @Override - public boolean execute(int file) { - final String fileName = myContext.getValue(file); - debug("Adding ", fileName); - affectedFiles.add(new File(fileName)); - return true; - } - }); } final String packageName = ClassRepr.getPackageName(myContext.getValue(isField ? owner : member.name)); @@ -730,24 +715,14 @@ public class Mappings { debug("Package name: ", packageName); // Package-local branch - final TIntHashSet fileNames = new TIntHashSet(DEFAULT_SET_CAPACITY, DEFAULT_SET_LOAD_FACTOR); - myClassToSourceFile.forEachEntry(new TIntIntProcedure() { + myClassToSourceFile.forEachEntry(new TIntObjectProcedure() { @Override - public boolean execute(int className, int fileName) { + public boolean execute(int className, File fileName) { if (ClassRepr.getPackageName(myContext.getValue(className)).equals(packageName)) { - fileNames.add(fileName); - } - return true; - } - }); - fileNames.forEach(new TIntProcedure() { - @Override - public boolean execute(int fileName) { - final String f = myContext.getValue(fileName); - final File file = new File(f); - if (filter == null || filter.accept(file)) { - debug("Adding: ", f); - affectedFiles.add(file); + if (filter == null || filter.accept(fileName)) { + debug("Adding: ", fileName); + affectedFiles.add(fileName); + } } return true; } @@ -859,10 +834,10 @@ public class Mappings { } private class FileClasses { - final int myFileName; + final File myFileName; final Set myFileClasses; - FileClasses(int fileName, Collection fileClasses) { + FileClasses(File fileName, Collection fileClasses) { this.myFileName = fileName; this.myFileClasses = new HashSet(fileClasses); } @@ -949,7 +924,7 @@ public class Mappings { if (removed != null) { for (final String file : removed) { - final Collection classes = mySourceFileToClasses.get(myContext.getFilePath(file)); + final Collection classes = mySourceFileToClasses.get(new File(file)); if (classes != null) { for (ClassRepr c : classes) { @@ -972,7 +947,6 @@ public class Mappings { debug("Class is annotation, skipping method analysis"); return; } - final TIntHashSet affectedFiles = new TIntHashSet(DEFAULT_SET_CAPACITY, DEFAULT_SET_LOAD_FACTOR); Ref oldItRef = null; for (final MethodRepr m : added) { debug("Method: ", m.name); @@ -1037,10 +1011,10 @@ public class Mappings { if (overrides.satisfy(method) && isInheritor) { debug("Current method overrides that found"); - final int file = myClassToSourceFile.get(methodClass.name); + final File file = myClassToSourceFile.get(methodClass.name); - if (file > 0) { - affectedFiles.add(file); + if (file != null) { + myAffectedFiles.add(file); debug("Affecting file ", file); } } @@ -1074,11 +1048,11 @@ public class Mappings { public boolean execute(int subClass) { final ClassRepr r = myFuture.reprByName(subClass); if (r != null) { - final int sourceFileName = myClassToSourceFile.get(subClass); - if (sourceFileName > 0) { + final File sourceFileName = myClassToSourceFile.get(subClass); + if (sourceFileName != null) { final int outerClass = r.getOuterClassName(); if (myFuture.isMethodVisible(outerClass, m)) { - affectedFiles.add(sourceFileName); + myAffectedFiles.add(sourceFileName); debug("Affecting file due to local overriding: ", sourceFileName); } } @@ -1089,13 +1063,6 @@ public class Mappings { } } } - affectedFiles.forEach(new TIntProcedure() { - @Override - public boolean execute(int file) { - myAffectedFiles.add(new File(myContext.getValue(file))); - return true; - } - }); debug("End of added methods processing"); } @@ -1105,7 +1072,6 @@ public class Mappings { return; } debug("Processing removed methods:"); - final TIntHashSet affectedFiles = new TIntHashSet(DEFAULT_SET_CAPACITY, DEFAULT_SET_LOAD_FACTOR); for (final MethodRepr m : removed) { debug("Method ", m.name); @@ -1140,8 +1106,8 @@ public class Mappings { myFuture.addOverridingMethods(m, it, MethodRepr.equalByJavaRules(m), overridingMethods); for (final Pair p : overridingMethods) { - final int fName = myClassToSourceFile.get(p.second.name); - affectedFiles.add(fName); + final File fName = myClassToSourceFile.get(p.second.name); + myAffectedFiles.add(fName); debug("Affecting file by overriding: ", fName); } @@ -1181,10 +1147,10 @@ public class Mappings { } if (allAbstract && visited) { - final int source = myClassToSourceFile.get(p); + final File source = myClassToSourceFile.get(p); - if (source > 0) { - affectedFiles.add(source); + if (source != null) { + myAffectedFiles.add(source); debug("Removed method is not abstract & overrides some abstract method which is not then over-overriden in subclass ", p); debug("Affecting subclass source file ", source); } @@ -1196,14 +1162,6 @@ public class Mappings { }); } } - affectedFiles.forEach(new TIntProcedure() { - @Override - public boolean execute(int file) { - final String f = myContext.getValue(file); - myAffectedFiles.add(new File(f)); - return true; - } - }); debug("End of removed methods processing"); } @@ -1264,10 +1222,9 @@ public class Mappings { final ClassRepr aClass = p.getSecond(); if (aClass != MOCK_CLASS) { - final int fileName = myClassToSourceFile.get(aClass.name); - - if (fileName > 0) { - myAffectedFiles.add(new File(myContext.getValue(fileName))); + final File fileName = myClassToSourceFile.get(aClass.name); + if (fileName != null) { + myAffectedFiles.add(fileName); } } } @@ -1340,17 +1297,17 @@ public class Mappings { public boolean execute(int subClass) { final ClassRepr r = myFuture.reprByName(subClass); if (r != null) { - final int sourceFileName = myClassToSourceFile.get(subClass); - if (sourceFileName > 0) { + final File sourceFileName = myClassToSourceFile.get(subClass); + if (sourceFileName != null) { if (r.isLocal()) { debug("Affecting local subclass (introduced field can potentially hide surrounding method parameters/local variables): ", sourceFileName); - myAffectedFiles.add(new File(myContext.getValue(sourceFileName))); + myAffectedFiles.add(sourceFileName); } else { final int outerClass = r.getOuterClassName(); if (!isEmpty(outerClass) && myFuture.isFieldVisible(outerClass, f)) { debug("Affecting inner subclass (introduced field can potentially hide surrounding class fields): ", sourceFileName); - myAffectedFiles.add(new File(myContext.getValue(sourceFileName))); + myAffectedFiles.add(sourceFileName); } } } @@ -1706,9 +1663,9 @@ public class Mappings { for (final ClassRepr c : removed) { myDelta.addDeletedClass(c); - final int fileName = myClassToSourceFile.get(c.name); + final File fileName = myClassToSourceFile.get(c.name); - if (fileName > 0) { + if (fileName != null) { myDelta.myChangedFiles.add(fileName); } @@ -1740,25 +1697,15 @@ public class Mappings { final TIntHashSet depClasses = myClassToClassDependency.get(c.name); if (depClasses != null) { - final TIntHashSet fileNames = new TIntHashSet(DEFAULT_SET_CAPACITY, DEFAULT_SET_LOAD_FACTOR); depClasses.forEach(new TIntProcedure() { @Override public boolean execute(int depClass) { - final int fName = myClassToSourceFile.get(depClass); - if (fName > 0) { - fileNames.add(fName); - } - return true; - } - }); - fileNames.forEach(new TIntProcedure() { - @Override - public boolean execute(int fName) { - final String f = myContext.getValue(fName); - final File theFile = new File(f); - if (myFilter == null || myFilter.accept(theFile)) { - debug("Adding dependent file ", f); - myAffectedFiles.add(theFile); + final File fName = myClassToSourceFile.get(depClass); + if (fName != null) { + if (myFilter == null || myFilter.accept(fName)) { + debug("Adding dependent file ", fName); + myAffectedFiles.add(fName); + } } return true; } @@ -1776,12 +1723,11 @@ public class Mappings { state.myDependants.forEach(new TIntProcedure() { @Override public boolean execute(final int depClass) { - final int depFile = myClassToSourceFile.get(depClass); + final File depFile = myClassToSourceFile.get(depClass); - if (depFile > 0) { - final File theFile = new File(myContext.getValue(depFile)); + if (depFile != null) { - if (myAffectedFiles.contains(theFile) || myCompiledFiles.contains(theFile)) { + if (myAffectedFiles.contains(depFile) || myCompiledFiles.contains(depFile)) { return true; } @@ -1804,7 +1750,7 @@ public class Mappings { for (final UsageRepr.AnnotationUsage query : state.myAnnotationQuery) { if (query.satisfies(usage)) { debug("Added file due to annotation query"); - myAffectedFiles.add(theFile); + myAffectedFiles.add(depFile); return true; } @@ -1815,14 +1761,14 @@ public class Mappings { if (constraint == null) { debug("Added file with no constraints"); - myAffectedFiles.add(theFile); + myAffectedFiles.add(depFile); return true; } else { if (constraint.checkResidence(depClass)) { debug("Added file with satisfied constraint"); - myAffectedFiles.add(theFile); + myAffectedFiles.add(depFile); return true; } @@ -1850,16 +1796,16 @@ public class Mappings { processDisappearedClasses(); final List newClasses = new ArrayList(); - myDelta.mySourceFileToClasses.forEachEntry(new TIntObjectProcedure>() { + myDelta.mySourceFileToClasses.forEachEntry(new TObjectObjectProcedure>() { @Override - public boolean execute(int fileName, Collection classes) { + public boolean execute(File fileName, Collection classes) { newClasses.add(new FileClasses(fileName, classes)); return true; } }); for (final FileClasses compiledFile : newClasses) { - final int fileName = compiledFile.myFileName; + final File fileName = compiledFile.myFileName; final Set classes = compiledFile.myFileClasses; final Set pastClasses = (Set)mySourceFileToClasses.get(fileName); final DiffState state = new DiffState(Difference.make(pastClasses, classes)); @@ -1965,7 +1911,7 @@ public class Mappings { if (removed != null) { for (final String file : removed) { - final int fileName = myContext.getFilePath(file); + final File fileName = new File(file); final Set fileClasses = (Set)mySourceFileToClasses.get(fileName); if (fileClasses != null) { @@ -2019,8 +1965,8 @@ public class Mappings { delta.getChangedClasses().forEach(new TIntProcedure() { @Override public boolean execute(final int className) { - final int sourceFile = delta.myClassToSourceFile.get(className); - if (sourceFile > 0) { + final File sourceFile = delta.myClassToSourceFile.get(className); + if (sourceFile != null) { myClassToSourceFile.put(className, sourceFile); } else { @@ -2033,9 +1979,9 @@ public class Mappings { } }); - delta.getChangedFiles().forEach(new TIntProcedure() { + delta.getChangedFiles().forEach(new TObjectProcedure() { @Override - public boolean execute(final int fileName) { + public boolean execute(final File fileName) { final Collection classes = delta.mySourceFileToClasses.get(fileName); mySourceFileToClasses.replace(fileName, classes); return true; @@ -2101,16 +2047,16 @@ public class Mappings { return new Callbacks.Backend() { public void associate(final String classFileName, final String sourceFileName, final ClassReader cr) { synchronized (myLock) { - final int classFileNameS = myContext.getFilePath(classFileName); + final int classFileNameS = myContext.get(classFileName); final Pair> result = new ClassfileAnalyzer(myContext).analyze(classFileNameS, cr); final ClassRepr repr = result.first; if (repr != null) { final Set localUsages = result.second; - final int sourceFileNameS = myContext.getFilePath(sourceFileName); + final File sourceFile = new File(sourceFileName); final int className = repr.name; - myClassToSourceFile.put(className, sourceFileNameS); - mySourceFileToClasses.put(sourceFileNameS, repr); + myClassToSourceFile.put(className, sourceFile); + mySourceFileToClasses.put(sourceFile, repr); for (final int s : repr.getSupers()) { myClassToSubclasses.put(s, className); @@ -2148,8 +2094,8 @@ public class Mappings { myPostPasses.offer(new Runnable() { public void run() { final int rootClassName = myContext.get(className.replace(".", "/")); - final int fileName = myClassToSourceFile.get(rootClassName); - final ClassRepr repr = fileName > 0? getReprByName(rootClassName) : null; + final File fileName = myClassToSourceFile.get(rootClassName); + final ClassRepr repr = fileName != null? getReprByName(rootClassName) : null; for (final String i : allImports) { final int iname = myContext.get(i.replace(".", "/")); @@ -2168,7 +2114,7 @@ public class Mappings { @Nullable public Set getClasses(final String sourceFileName) { synchronized (myLock) { - return (Set)mySourceFileToClasses.get(myContext.getFilePath(sourceFileName)); + return (Set)mySourceFileToClasses.get(new File(sourceFileName)); } } @@ -2272,9 +2218,9 @@ public class Mappings { assert (myChangedClasses != null && myChangedFiles != null); myChangedClasses.add(it); - final int file = myClassToSourceFile.get(it); + final File file = myClassToSourceFile.get(it); - if (file > 0) { + if (file != null) { myChangedFiles.add(file); } } @@ -2288,7 +2234,7 @@ public class Mappings { return myChangedClasses; } - private TIntHashSet getChangedFiles() { + private THashSet getChangedFiles() { return myChangedFiles; } @@ -2300,6 +2246,10 @@ public class Mappings { myDebugS.debug(comment, s); } + private void debug(final String comment, final File f) { + debug(comment, f.getPath()); + } + private void debug(final String comment, final String s) { myDebugS.debug(comment, s); } diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectMultiMaplet.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectMultiMaplet.java new file mode 100644 index 000000000000..e2bd3243e39f --- /dev/null +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectMultiMaplet.java @@ -0,0 +1,105 @@ +/* + * Copyright 2000-2011 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jetbrains.jps.builders.java.dependencyView; + +import com.intellij.openapi.util.Pair; +import gnu.trove.TObjectObjectProcedure; + +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.util.*; + +/** + * Created by IntelliJ IDEA. + * User: db + * Date: 03.11.11 + * Time: 21:01 + * To change this template use File | Settings | File Templates. + */ +abstract class ObjectObjectMultiMaplet implements Streamable { + abstract boolean containsKey(final K key); + + abstract Collection get(final K key); + + abstract void put(final K key, final V value); + + abstract void put(final K key, final Collection value); + + abstract void replace(final K key, final Collection value); + + abstract void putAll(ObjectObjectMultiMaplet m); + + abstract void replaceAll(ObjectObjectMultiMaplet m); + + abstract void remove(final K key); + + abstract void removeFrom(final K key, final V value); + + abstract void removeAll(final K key, final Collection value); + + abstract void close(); + + abstract void forEachEntry(TObjectObjectProcedure> procedure); + + abstract void flush(boolean memoryCachesOnly); + + public void toStream(final DependencyContext context, final PrintStream stream) { + + final List> keys = new ArrayList>(); + forEachEntry(new TObjectObjectProcedure>() { + @Override + public boolean execute(final K a, final Collection b) { + keys.add(new Pair(a, a.toString())); + return true; + } + }); + + Collections.sort(keys, new Comparator>() { + @Override + public int compare(Pair o1, Pair o2) { + return o1.second.compareTo(o2.second); + } + }); + + for (final Pair a: keys) { + final Collection b = get(a.first); + + stream.print(" Key: "); + stream.println(a.second); + stream.println(" Values:"); + + final List list = new LinkedList(); + + for (final V value : b) { + final ByteArrayOutputStream baos = new ByteArrayOutputStream(); + final PrintStream s = new PrintStream(baos); + + value.toStream(context, s); + + list.add(baos.toString()); + } + + Collections.sort(list); + + for (final String l : list) { + stream.print(l); + } + + stream.println(" End Of Values"); + } + } + +} diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectPersistentMultiMaplet.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectPersistentMultiMaplet.java new file mode 100644 index 000000000000..1bdd07751935 --- /dev/null +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectPersistentMultiMaplet.java @@ -0,0 +1,248 @@ +package org.jetbrains.jps.builders.java.dependencyView; + +import com.intellij.util.Processor; +import com.intellij.util.containers.SLRUCache; +import com.intellij.util.io.DataExternalizer; +import com.intellij.util.io.KeyDescriptor; +import com.intellij.util.io.PersistentHashMap; +import gnu.trove.TObjectObjectProcedure; +import org.jetbrains.annotations.NotNull; + +import java.io.*; +import java.util.Collection; +import java.util.Collections; + +/** + * @author Eugene Zhuravlev + * Date: 9/10/12 + */ +public class ObjectObjectPersistentMultiMaplet extends ObjectObjectMultiMaplet{ + private static final Collection NULL_COLLECTION = Collections.emptySet(); + private static final int CACHE_SIZE = 128; + private final PersistentHashMap> myMap; + private final DataExternalizer myValueExternalizer; + private final SLRUCache myCache; + + public ObjectObjectPersistentMultiMaplet(final File file, + final KeyDescriptor keyExternalizer, + final DataExternalizer valueExternalizer, + final CollectionFactory collectionFactory) throws IOException { + myValueExternalizer = valueExternalizer; + myMap = new PersistentHashMap>(file, keyExternalizer, new CollectionDataExternalizer(valueExternalizer, collectionFactory)); + myCache = new SLRUCache(CACHE_SIZE, CACHE_SIZE) { + @NotNull + @Override + public Collection createValue(K key) { + try { + final Collection collection = myMap.get(key); + return collection == null? NULL_COLLECTION : collection; + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + }; + } + + + @Override + public boolean containsKey(final K key) { + try { + return myMap.containsMapping(key); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public Collection get(final K key) { + final Collection collection = myCache.get(key); + return collection == NULL_COLLECTION? null : collection; + } + + @Override + public void replace(K key, Collection value) { + try { + myCache.remove(key); + if (value == null || value.isEmpty()) { + myMap.remove(key); + } + else { + myMap.put(key, value); + } + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public void put(final K key, final Collection value) { + try { + myCache.remove(key); + myMap.appendData(key, new PersistentHashMap.ValueDataAppender() { + public void append(DataOutput out) throws IOException { + for (V v : value) { + myValueExternalizer.save(out, v); + } + } + }); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public void put(final K key, final V value) { + put(key, Collections.singleton(value)); + } + + @Override + public void removeAll(K key, Collection values) { + try { + final Collection collection = myCache.get(key); + + if (collection != NULL_COLLECTION) { + if (collection.removeAll(values)) { + myCache.remove(key); + if (collection.isEmpty()) { + myMap.remove(key); + } + else { + myMap.put(key, (Collection)collection); + } + } + } + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public void removeFrom(final K key, final V value) { + try { + final Collection collection = myCache.get(key); + + if (collection != NULL_COLLECTION) { + if (collection.remove(value)) { + myCache.remove(key); + if (collection.isEmpty()) { + myMap.remove(key); + } + else { + myMap.put(key, (Collection)collection); + } + } + } + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public void remove(final K key) { + try { + myCache.remove(key); + myMap.remove(key); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public void putAll(ObjectObjectMultiMaplet m) { + m.forEachEntry(new TObjectObjectProcedure>() { + @Override + public boolean execute(K key, Collection value) { + put(key, value); + return true; + } + }); + } + + @Override + public void replaceAll(ObjectObjectMultiMaplet m) { + m.forEachEntry(new TObjectObjectProcedure>() { + @Override + public boolean execute(K key, Collection value) { + replace(key, value); + return true; + } + }); + } + + @Override + public void close() { + try { + myCache.clear(); + myMap.close(); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + public void flush(boolean memoryCachesOnly) { + if (memoryCachesOnly) { + if (myMap.isDirty()) { + myMap.dropMemoryCaches(); + } + } + else { + myMap.force(); + } + } + + @Override + public void forEachEntry(final TObjectObjectProcedure> procedure) { + try { + myMap.processKeysWithExistingMapping(new Processor() { + @Override + public boolean process(K key) { + try { + return procedure.execute(key, myMap.get(key)); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + }); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + + private static class CollectionDataExternalizer implements DataExternalizer> { + private final DataExternalizer myElementExternalizer; + private final CollectionFactory myCollectionFactory; + + public CollectionDataExternalizer(DataExternalizer elementExternalizer, + CollectionFactory collectionFactory) { + myElementExternalizer = elementExternalizer; + myCollectionFactory = collectionFactory; + } + + @Override + public void save(final DataOutput out, final Collection value) throws IOException { + for (V x : value) { + myElementExternalizer.save(out, x); + } + } + + @Override + public Collection read(final DataInput in) throws IOException { + final Collection result = myCollectionFactory.create(); + final DataInputStream stream = (DataInputStream)in; + while (stream.available() > 0) { + result.add(myElementExternalizer.read(in)); + } + return result; + } + } +} diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectTransientMultiMaplet.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectTransientMultiMaplet.java new file mode 100644 index 000000000000..dd9b8cf92033 --- /dev/null +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectTransientMultiMaplet.java @@ -0,0 +1,130 @@ +package org.jetbrains.jps.builders.java.dependencyView; + +import gnu.trove.THashMap; +import gnu.trove.TObjectHashingStrategy; +import gnu.trove.TObjectObjectProcedure; + +import java.util.Collection; + +/** + * @author Eugene Zhuravlev + * Date: 9/10/12 + */ +public class ObjectObjectTransientMultiMaplet extends ObjectObjectMultiMaplet{ + + private final THashMap> myMap; + private final CollectionFactory myCollectionFactory; + + public ObjectObjectTransientMultiMaplet(TObjectHashingStrategy hashingStrategy, CollectionFactory collectionFactory) { + myMap = new THashMap>(hashingStrategy); + myCollectionFactory = collectionFactory; + } + + @Override + public boolean containsKey(final K key) { + return myMap.containsKey(key); + } + + @Override + public Collection get(final K key) { + return myMap.get(key); + } + + @Override + public void putAll(ObjectObjectMultiMaplet m) { + m.forEachEntry(new TObjectObjectProcedure>() { + @Override + public boolean execute(K key, Collection value) { + put(key, value); + return true; + } + }); + } + + @Override + public void put(final K key, final Collection value) { + final Collection x = myMap.get(key); + if (x == null) { + myMap.put(key, value); + } + else { + x.addAll(value); + } + } + + @Override + public void replace(K key, Collection value) { + if (value == null || value.isEmpty()) { + myMap.remove(key); + } + else { + myMap.put(key, value); + } + } + + @Override + public void put(final K key, final V value) { + final Collection collection = myMap.get(key); + if (collection == null) { + final Collection x = myCollectionFactory.create(); + x.add(value); + myMap.put(key, x); + } + else { + collection.add(value); + } + } + + @Override + public void removeFrom(final K key, final V value) { + final Collection collection = myMap.get(key); + if (collection != null) { + if (collection.remove(value)) { + if (collection.isEmpty()) { + myMap.remove(key); + } + } + } + } + + @Override + public void removeAll(K key, Collection values) { + final Collection collection = myMap.get(key); + if (collection != null) { + if (collection.removeAll(values)) { + if (collection.isEmpty()) { + myMap.remove(key); + } + } + } + } + + @Override + public void remove(final K key) { + myMap.remove(key); + } + + @Override + public void replaceAll(ObjectObjectMultiMaplet m) { + m.forEachEntry(new TObjectObjectProcedure>() { + @Override + public boolean execute(K key, Collection value) { + replace(key, value); + return true; + } + }); + } + + @Override + public void forEachEntry(TObjectObjectProcedure> procedure) { + myMap.forEachEntry(procedure); + } + + @Override + public void close(){ + myMap.clear(); // free memory + } + + public void flush(boolean memoryCachesOnly) { + } +} diff --git a/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/BuildDataManager.java b/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/BuildDataManager.java index 5e72182d1c9a..47c4a251fba2 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/BuildDataManager.java +++ b/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/BuildDataManager.java @@ -20,7 +20,7 @@ import java.util.Map; * Date: 10/7/11 */ public class BuildDataManager implements StorageOwner { - private static final int VERSION = 9; + private static final int VERSION = 10; private static final Logger LOG = Logger.getInstance("#org.jetbrains.jps.incremental.storage.BuildDataManager"); private static final String SRC_TO_OUTPUTS_STORAGE = "src-out"; private static final String SRC_TO_FORM_STORAGE = "src-form"; diff --git a/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/FileKeyDescriptor.java b/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/FileKeyDescriptor.java new file mode 100644 index 000000000000..84953d12240d --- /dev/null +++ b/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/FileKeyDescriptor.java @@ -0,0 +1,34 @@ +package org.jetbrains.jps.incremental.storage; + +import com.intellij.openapi.util.io.FileUtil; +import com.intellij.util.io.IOUtil; +import com.intellij.util.io.KeyDescriptor; + +import java.io.DataInput; +import java.io.DataOutput; +import java.io.File; +import java.io.IOException; + +/** +* @author Eugene Zhuravlev +* Date: 9/10/12 +*/ +public final class FileKeyDescriptor implements KeyDescriptor { + private final byte[] buffer = IOUtil.allocReadWriteUTFBuffer(); + + public void save(DataOutput out, File value) throws IOException { + IOUtil.writeUTFFast(buffer, out, value.getPath()); + } + + public File read(DataInput in) throws IOException { + return new File(IOUtil.readUTFFast(buffer, in)); + } + + public int getHashCode(File value) { + return FileUtil.fileHashCode(value); + } + + public boolean isEqual(File val1, File val2) { + return FileUtil.filesEqual(val1, val2); + } +} diff --git a/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/TimestampStorage.java b/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/TimestampStorage.java index f1493c45187f..0820f3308a7e 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/TimestampStorage.java +++ b/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/TimestampStorage.java @@ -1,9 +1,6 @@ package org.jetbrains.jps.incremental.storage; -import com.intellij.openapi.util.io.FileUtil; import com.intellij.util.io.DataExternalizer; -import com.intellij.util.io.IOUtil; -import com.intellij.util.io.KeyDescriptor; import java.io.DataInput; import java.io.DataOutput; @@ -45,26 +42,6 @@ public class TimestampStorage extends AbstractStateStorage { - private final byte[] buffer = IOUtil.allocReadWriteUTFBuffer(); - - public void save(DataOutput out, File value) throws IOException { - IOUtil.writeUTFFast(buffer, out, value.getPath()); - } - - public File read(DataInput in) throws IOException { - return new File(IOUtil.readUTFFast(buffer, in)); - } - - public int getHashCode(File value) { - return FileUtil.fileHashCode(value); - } - - public boolean isEqual(File val1, File val2) { - return FileUtil.filesEqual(val1, val2); - } - } - private static class StateExternalizer implements DataExternalizer { public void save(DataOutput out, TimestampValidityState value) throws IOException { From 615d7ccd4b9f3c65eac6acc2c83d6a4c663cfbee Mon Sep 17 00:00:00 2001 From: Eugene Zhuravlev Date: Mon, 10 Sep 2012 23:03:17 +0200 Subject: [PATCH 7/7] adapt tests to case-insensitive file systems --- .../java/dependencyView/ObjectObjectMultiMaplet.java | 8 +++++++- .../testSrc/org/jetbrains/ether/ClassRenameTest.java | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectMultiMaplet.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectMultiMaplet.java index e2bd3243e39f..df5b139b4446 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectMultiMaplet.java +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ObjectObjectMultiMaplet.java @@ -16,9 +16,11 @@ package org.jetbrains.jps.builders.java.dependencyView; import com.intellij.openapi.util.Pair; +import com.intellij.openapi.util.SystemInfo; import gnu.trove.TObjectObjectProcedure; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.PrintStream; import java.util.*; @@ -62,7 +64,11 @@ abstract class ObjectObjectMultiMaplet implements Strea forEachEntry(new TObjectObjectProcedure>() { @Override public boolean execute(final K a, final Collection b) { - keys.add(new Pair(a, a.toString())); + // on case-insensitive file systems save paths in normalized (lowercase) format in order to make tests run deterministically + final String keyStr = a instanceof File && !SystemInfo.isFileSystemCaseSensitive? + ((File)a).getPath().toLowerCase(Locale.US) : + a.toString(); + keys.add(new Pair(a, keyStr)); return true; } }); diff --git a/jps/jps-builders/testSrc/org/jetbrains/ether/ClassRenameTest.java b/jps/jps-builders/testSrc/org/jetbrains/ether/ClassRenameTest.java index a24513a30a9f..b9b29422f803 100644 --- a/jps/jps-builders/testSrc/org/jetbrains/ether/ClassRenameTest.java +++ b/jps/jps-builders/testSrc/org/jetbrains/ether/ClassRenameTest.java @@ -16,7 +16,7 @@ public class ClassRenameTest extends IncrementalTestCase { doTest().assertSuccessful(); } - public void _testChangeCaseOfName() { + public void testChangeCaseOfName() { doTest().assertSuccessful(); } }