From 171ec9fcac516bc2b83d505c5883c213fcc652f9 Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Tue, 18 Nov 2014 18:51:42 +0100 Subject: [PATCH 1/3] cleanup --- .../openapi/options/SchemesManager.java | 20 ++++---- .../options/AbstractSchemesManager.java | 46 +++++++++---------- .../InspectionProfileManagerImpl.java | 13 ++---- .../com/intellij/tools/BaseToolManager.java | 7 ++- .../openapi/options/CompoundScheme.java | 3 -- .../colors/impl/EditorColorsManagerImpl.java | 12 ++--- .../keymap/impl/KeymapManagerImpl.java | 7 ++- .../openapi/options/SchemesManagerImpl.java | 6 +-- 8 files changed, 51 insertions(+), 63 deletions(-) diff --git a/platform/core-api/src/com/intellij/openapi/options/SchemesManager.java b/platform/core-api/src/com/intellij/openapi/options/SchemesManager.java index 6b13e51385d1..e0a011445dc7 100644 --- a/platform/core-api/src/com/intellij/openapi/options/SchemesManager.java +++ b/platform/core-api/src/com/intellij/openapi/options/SchemesManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2013 JetBrains s.r.o. + * Copyright 2000-2014 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. @@ -73,7 +73,7 @@ public interface SchemesManager loadSchemes(); + @NotNull + Collection loadSchemes(); @Deprecated @SuppressWarnings({"unused", "deprecation"}) @@ -158,9 +157,10 @@ public interface SchemesManager getAllSchemeNames(); + @NotNull + Collection getAllSchemeNames(); File getRootDirectory(); } diff --git a/platform/core-impl/src/com/intellij/openapi/options/AbstractSchemesManager.java b/platform/core-impl/src/com/intellij/openapi/options/AbstractSchemesManager.java index a36968f3f251..4adb43543214 100644 --- a/platform/core-impl/src/com/intellij/openapi/options/AbstractSchemesManager.java +++ b/platform/core-impl/src/com/intellij/openapi/options/AbstractSchemesManager.java @@ -73,15 +73,16 @@ public abstract class AbstractSchemesManager getAllSchemes() { - return new ArrayList(mySchemes); + return Collections.unmodifiableList(mySchemes); } @Override @@ -112,37 +113,28 @@ public abstract class AbstractSchemesManager getAllSchemeNames() { - return getAllSchemeNames(mySchemes); - } - - public Collection getAllSchemeNames(@NotNull Collection schemes) { - Set names = new THashSet(schemes.size()); - for (T scheme : schemes) { + List names = new ArrayList(mySchemes.size()); + for (T scheme : mySchemes) { names.add(scheme.getName()); } return names; @@ -181,6 +173,10 @@ public abstract class AbstractSchemesManager profiles = mySchemesManager.getAllSchemes(); - + Collection profiles = mySchemesManager.getAllSchemes(); if (profiles.isEmpty()) { createDefaultProfile(); } @@ -337,8 +336,7 @@ public class InspectionProfileManagerImpl extends InspectionProfileManager imple @Override @NotNull public String[] getAvailableProfileNames() { - final Collection names = mySchemesManager.getAllSchemeNames(); - return ArrayUtil.toStringArray(names); + return ArrayUtil.toStringArray(mySchemesManager.getAllSchemeNames()); } @Override @@ -346,11 +344,6 @@ public class InspectionProfileManagerImpl extends InspectionProfileManager imple return getProfile(name, true); } - @NotNull - public SchemesManager getSchemesManager() { - return mySchemesManager; - } - public static void onProfilesChanged() { //cleanup caches blindly for all projects in case ide profile was modified for (final Project project : ProjectManager.getInstance().getOpenProjects()) { diff --git a/platform/lang-impl/src/com/intellij/tools/BaseToolManager.java b/platform/lang-impl/src/com/intellij/tools/BaseToolManager.java index 478f60eed51a..3db9e9206140 100644 --- a/platform/lang-impl/src/com/intellij/tools/BaseToolManager.java +++ b/platform/lang-impl/src/com/intellij/tools/BaseToolManager.java @@ -20,9 +20,12 @@ package com.intellij.tools; import com.intellij.openapi.actionSystem.ex.ActionManagerEx; import com.intellij.openapi.components.ExportableApplicationComponent; import com.intellij.openapi.components.RoamingType; -import com.intellij.openapi.options.*; +import com.intellij.openapi.options.SchemeProcessor; +import com.intellij.openapi.options.SchemesManager; +import com.intellij.openapi.options.SchemesManagerFactory; import com.intellij.openapi.util.Comparing; import com.intellij.util.ArrayUtil; +import com.intellij.util.SmartList; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -77,7 +80,7 @@ public abstract class BaseToolManager implements ExportableAppli } public List getTools() { - ArrayList result = new ArrayList(); + List result = new SmartList(); for (ToolsGroup group : mySchemesManager.getAllSchemes()) { result.addAll(group.getElements()); } diff --git a/platform/platform-api/src/com/intellij/openapi/options/CompoundScheme.java b/platform/platform-api/src/com/intellij/openapi/options/CompoundScheme.java index 9380f2780beb..93c45716dbdd 100644 --- a/platform/platform-api/src/com/intellij/openapi/options/CompoundScheme.java +++ b/platform/platform-api/src/com/intellij/openapi/options/CompoundScheme.java @@ -24,7 +24,6 @@ import java.util.Collections; import java.util.Iterator; import java.util.List; - public class CompoundScheme implements ExternalizableScheme { private static final Logger LOG = Logger.getInstance("#com.intellij.openapi.options.CompoundScheme"); @@ -49,8 +48,6 @@ public class CompoundScheme implements ExternalizableSc } } - - public List getElements() { return Collections.unmodifiableList(new ArrayList(myElements)); } diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/colors/impl/EditorColorsManagerImpl.java b/platform/platform-impl/src/com/intellij/openapi/editor/colors/impl/EditorColorsManagerImpl.java index 6a26c2350e0e..d2cae4a2d4f6 100644 --- a/platform/platform-impl/src/com/intellij/openapi/editor/colors/impl/EditorColorsManagerImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/colors/impl/EditorColorsManagerImpl.java @@ -51,8 +51,7 @@ import org.jetbrains.annotations.Nullable; import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.util.ArrayList; -import java.util.Collections; +import java.util.Arrays; import java.util.Comparator; import java.util.List; @@ -214,18 +213,17 @@ public class EditorColorsManagerImpl extends EditorColorsManager implements Name @NotNull @Override public EditorColorsScheme[] getAllSchemes() { - List schemes = new ArrayList(mySchemesManager.getAllSchemes()); - Collections.sort(schemes, new Comparator() { + List schemes = mySchemesManager.getAllSchemes(); + EditorColorsScheme[] result = schemes.toArray(new EditorColorsScheme[schemes.size()]); + Arrays.sort(result, new Comparator() { @Override public int compare(@NotNull EditorColorsScheme s1, @NotNull EditorColorsScheme s2) { if (isDefaultScheme(s1) && !isDefaultScheme(s2)) return -1; if (!isDefaultScheme(s1) && isDefaultScheme(s2)) return 1; - return s1.getName().compareToIgnoreCase(s2.getName()); } }); - - return schemes.toArray(new EditorColorsScheme[schemes.size()]); + return result; } @Override diff --git a/platform/platform-impl/src/com/intellij/openapi/keymap/impl/KeymapManagerImpl.java b/platform/platform-impl/src/com/intellij/openapi/keymap/impl/KeymapManagerImpl.java index 30956c28a412..0cdf59cd8d80 100644 --- a/platform/platform-impl/src/com/intellij/openapi/keymap/impl/KeymapManagerImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/keymap/impl/KeymapManagerImpl.java @@ -164,15 +164,18 @@ public class KeymapManagerImpl extends KeymapManagerEx implements PersistentStat } public void removeAllKeymapsExceptUnmodifiable() { - for (Keymap keymap : mySchemesManager.getAllSchemes()) { + List schemes = mySchemesManager.getAllSchemes(); + for (int i = schemes.size() - 1; i >= 0; i--) { + Keymap keymap = schemes.get(i); if (keymap.canModify()) { mySchemesManager.removeScheme(keymap); } } + mySchemesManager.setCurrentSchemeName(null); Collection keymaps = mySchemesManager.getAllSchemes(); - if (keymaps.size() > 0) { + if (!keymaps.isEmpty()) { mySchemesManager.setCurrentSchemeName(keymaps.iterator().next().getName()); } } diff --git a/platform/platform-impl/src/com/intellij/openapi/options/SchemesManagerImpl.java b/platform/platform-impl/src/com/intellij/openapi/options/SchemesManagerImpl.java index 0a571c9fdf0c..a3c07d4483a0 100644 --- a/platform/platform-impl/src/com/intellij/openapi/options/SchemesManagerImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/options/SchemesManagerImpl.java @@ -431,10 +431,6 @@ public class SchemesManagerImpl Date: Tue, 18 Nov 2014 18:03:11 +0100 Subject: [PATCH 2/3] support PSI changes in IndexTestGenerator --- .../intellij/index/IndexTestGenerator.scala | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/java/java-tests/testSrc/com/intellij/index/IndexTestGenerator.scala b/java/java-tests/testSrc/com/intellij/index/IndexTestGenerator.scala index 98d0268adfba..2df92087e6b8 100644 --- a/java/java-tests/testSrc/com/intellij/index/IndexTestGenerator.scala +++ b/java/java-tests/testSrc/com/intellij/index/IndexTestGenerator.scala @@ -20,7 +20,7 @@ import groovy.lang.GroovyClassLoader import junit.framework.{TestResult, TestCase} import org.scalacheck.Arbitrary.arbitrary import org.scalacheck.Gen._ -import org.scalacheck.Prop.forAll +import org.scalacheck.Prop.{forAll, BooleanOperators} import org.scalacheck._ import scala.collection.JavaConversions._ @@ -40,6 +40,7 @@ object IndexTestGenerator { const(Gc), const(Commit), const(Save), + const(PsiChange), for (withImport <- arbitrary[Boolean]; viaDocument <- arbitrary[Boolean]) yield TextChange(viaDocument, withImport), @@ -49,7 +50,13 @@ object IndexTestGenerator { arbitrary[Boolean] map UpdateDocumentRef ) val propIndexTest = forAll(Gen.nonEmptyListOf(genAction)) { actions => - new IndexTestSeq(actions).isSuccessful + containsChange(actions) ==> new IndexTestSeq(actions).isSuccessful + } + + def containsChange(actions: List[Action]) = actions.exists { + case TextChange(_, _) => true + case PsiChange => true + case _ => false } def main(args: Array[String]) { @@ -62,14 +69,13 @@ case class IndexTestSeq(actions: List[Action]) { val sb = StringBuilder.newBuilder sb.append(prefix) sb.append( - s""" + s""" |public void "$testName"() { - |def vFile = - | myFixture.addFileToProject("Foo.java", "class Foo {}").virtualFile + |def psiFile = myFixture.addFileToProject("Foo.java", "class Foo {}") + |def vFile = psiFile.virtualFile |def lastPsiName = "Foo" |long counterBefore |Document document - |PsiFile psiFile |ASTNode astNode |PsiClass psiClass |def scope = GlobalSearchScope.allScope(project) @@ -86,6 +92,15 @@ case class IndexTestSeq(actions: List[Action]) { s"""PsiDocumentManager.getInstance(project).commitAllDocuments() |lastPsiName = "$docClassName" |""".stripMargin) + case PsiChange => + sb.append( + s"""PsiDocumentManager.getInstance(project).commitAllDocuments() + |lastPsiName = "$docClassName" + |myFixture.findClass("$docClassName").add( + | elementFactory.createMethod("foo", PsiType.VOID)) + |PostprocessReformattingAspect.getInstance(getProject()). + | doPostponedFormatting() + |""".stripMargin) case Save => sb.append("FileDocumentManager.instance.saveAllDocuments()\n") case UpdatePsiClassRef(load) => @@ -144,6 +159,7 @@ case class IndexTestSeq(actions: List[Action]) { |import com.intellij.openapi.util.Ref |import com.intellij.openapi.vfs.VfsUtil |import com.intellij.psi.* + |import com.intellij.psi.impl.source.* |import com.intellij.psi.search.GlobalSearchScope |import com.intellij.testFramework.PlatformTestUtil |import com.intellij.testFramework.fixtures.JavaCodeInsightFixtureTestCase @@ -186,3 +202,4 @@ case class UpdatePsiClassRef(load: Boolean) extends Action case class UpdatePsiFileRef(load: Boolean) extends Action case class UpdateDocumentRef(load: Boolean) extends Action case class UpdateASTNodeRef(load: Boolean) extends Action +case object PsiChange extends Action From efaa42d0b3ea35431f72a6dbd0f86e5a2791908c Mon Sep 17 00:00:00 2001 From: peter Date: Tue, 18 Nov 2014 18:10:42 +0100 Subject: [PATCH 3/3] PsiReferenceExpressionImpl: when resolving qualifiers, don't let resolve results be gc-ed --- .../tree/java/PsiReferenceExpressionImpl.java | 34 +++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiReferenceExpressionImpl.java b/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiReferenceExpressionImpl.java index 8b10b6326df9..72935f26b026 100644 --- a/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiReferenceExpressionImpl.java +++ b/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiReferenceExpressionImpl.java @@ -45,7 +45,6 @@ import com.intellij.psi.tree.ChildRoleBase; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; -import com.intellij.psi.util.PsiUtilCore; import com.intellij.psi.util.TypeConversionUtil; import com.intellij.util.*; import gnu.trove.THashSet; @@ -191,34 +190,27 @@ public class PsiReferenceExpressionImpl extends PsiReferenceExpressionBase imple CompositeElement treeParent = expression.getTreeParent(); IElementType parentType = treeParent == null ? null : treeParent.getElementType(); - List qualifiers = resolveAllQualifiers(expression, containingFile); - try { - JavaResolveResult[] result = expression.resolve(parentType, containingFile); + List qualifiers = resolveAllQualifiers(expression, containingFile); + JavaResolveResult[] result = expression.resolve(parentType, containingFile); - if (result.length == 0 && incompleteCode && parentType != JavaElementType.REFERENCE_EXPRESSION) { - result = expression.resolve(JavaElementType.REFERENCE_EXPRESSION, containingFile); - } - - JavaResolveUtil.substituteResults(expression, result); - - return result; - } - finally { - PsiElement item = qualifiers.isEmpty() ? PsiUtilCore.NULL_PSI_ELEMENT : qualifiers.get(qualifiers.size()-1); - qualifiers.clear(); // hold qualifiers list until this moment to avoid psi elements inside to GC - if (item == null) { - throw new IncorrectOperationException(); - } + if (result.length == 0 && incompleteCode && parentType != JavaElementType.REFERENCE_EXPRESSION) { + result = expression.resolve(JavaElementType.REFERENCE_EXPRESSION, containingFile); } + + JavaResolveUtil.substituteResults(expression, result); + + qualifiers.clear(); // hold qualifier target list until this moment to avoid psi elements inside to GC + + return result; } @NotNull - private static List resolveAllQualifiers(@NotNull PsiReferenceExpressionImpl expression, @NotNull final PsiFile containingFile) { + private static List resolveAllQualifiers(@NotNull PsiReferenceExpressionImpl expression, @NotNull final PsiFile containingFile) { // to avoid SOE, resolve all qualifiers starting from the innermost PsiElement qualifier = expression.getQualifier(); if (qualifier == null) return Collections.emptyList(); - final List qualifiers = new SmartList(); + final List qualifiers = new SmartList(); final ResolveCache resolveCache = ResolveCache.getInstance(containingFile.getProject()); qualifier.accept(new JavaRecursiveElementWalkingVisitor() { @Override @@ -238,7 +230,7 @@ public class PsiReferenceExpressionImpl extends PsiReferenceExpressionBase imple if (!(element instanceof PsiReferenceExpressionImpl)) return; PsiReferenceExpressionImpl expression = (PsiReferenceExpressionImpl)element; resolveCache.resolveWithCaching(expression, INSTANCE, false, false, containingFile); - qualifiers.add(expression); + qualifiers.add(resolveCache.resolveWithCaching(expression, INSTANCE, false, false, containingFile)); } }); return qualifiers;