From 19a418802382ecb7fdda5c59c1a9417af3602389 Mon Sep 17 00:00:00 2001 From: Konstantin Bulenkov Date: Fri, 11 Mar 2011 13:45:09 +0300 Subject: [PATCH 01/14] Registry -> util --- .../src/com/intellij/openapi/ui/CheckBoxWithDescription.java | 0 .../src/com/intellij/openapi/ui/DescriptionLabel.java | 0 .../src/com/intellij/openapi/util/registry/Registry.java | 0 .../src/com/intellij/openapi/util/registry/RegistryValue.java | 0 .../com/intellij/openapi/util/registry/RegistryValueListener.java | 0 .../com/intellij/openapi/util/registry/ui/RegistryCheckBox.java | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename platform/{platform-api => util}/src/com/intellij/openapi/ui/CheckBoxWithDescription.java (100%) rename platform/{platform-api => util}/src/com/intellij/openapi/ui/DescriptionLabel.java (100%) rename platform/{platform-api => util}/src/com/intellij/openapi/util/registry/Registry.java (100%) rename platform/{platform-api => util}/src/com/intellij/openapi/util/registry/RegistryValue.java (100%) rename platform/{platform-api => util}/src/com/intellij/openapi/util/registry/RegistryValueListener.java (100%) rename platform/{platform-api => util}/src/com/intellij/openapi/util/registry/ui/RegistryCheckBox.java (100%) diff --git a/platform/platform-api/src/com/intellij/openapi/ui/CheckBoxWithDescription.java b/platform/util/src/com/intellij/openapi/ui/CheckBoxWithDescription.java similarity index 100% rename from platform/platform-api/src/com/intellij/openapi/ui/CheckBoxWithDescription.java rename to platform/util/src/com/intellij/openapi/ui/CheckBoxWithDescription.java diff --git a/platform/platform-api/src/com/intellij/openapi/ui/DescriptionLabel.java b/platform/util/src/com/intellij/openapi/ui/DescriptionLabel.java similarity index 100% rename from platform/platform-api/src/com/intellij/openapi/ui/DescriptionLabel.java rename to platform/util/src/com/intellij/openapi/ui/DescriptionLabel.java diff --git a/platform/platform-api/src/com/intellij/openapi/util/registry/Registry.java b/platform/util/src/com/intellij/openapi/util/registry/Registry.java similarity index 100% rename from platform/platform-api/src/com/intellij/openapi/util/registry/Registry.java rename to platform/util/src/com/intellij/openapi/util/registry/Registry.java diff --git a/platform/platform-api/src/com/intellij/openapi/util/registry/RegistryValue.java b/platform/util/src/com/intellij/openapi/util/registry/RegistryValue.java similarity index 100% rename from platform/platform-api/src/com/intellij/openapi/util/registry/RegistryValue.java rename to platform/util/src/com/intellij/openapi/util/registry/RegistryValue.java diff --git a/platform/platform-api/src/com/intellij/openapi/util/registry/RegistryValueListener.java b/platform/util/src/com/intellij/openapi/util/registry/RegistryValueListener.java similarity index 100% rename from platform/platform-api/src/com/intellij/openapi/util/registry/RegistryValueListener.java rename to platform/util/src/com/intellij/openapi/util/registry/RegistryValueListener.java diff --git a/platform/platform-api/src/com/intellij/openapi/util/registry/ui/RegistryCheckBox.java b/platform/util/src/com/intellij/openapi/util/registry/ui/RegistryCheckBox.java similarity index 100% rename from platform/platform-api/src/com/intellij/openapi/util/registry/ui/RegistryCheckBox.java rename to platform/util/src/com/intellij/openapi/util/registry/ui/RegistryCheckBox.java From 227e2129a93c7c6baffbf110f985b4495d887447 Mon Sep 17 00:00:00 2001 From: Konstantin Bulenkov Date: Fri, 11 Mar 2011 14:19:01 +0300 Subject: [PATCH 02/14] Registry -> util --- .../completion/JavaCompletionContributor.java | 2 +- .../src/com/intellij/ui/SpeedSearchBase.java | 6 +++--- .../src/misc/registry.properties | 3 +++ .../src/com/intellij/psi/codeStyle/NameUtil.java | 13 +++++++------ .../completion/GroovyCompletionContributor.java | 2 +- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionContributor.java b/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionContributor.java index fbe329a42c91..ade0d69873f6 100644 --- a/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionContributor.java +++ b/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionContributor.java @@ -330,7 +330,7 @@ public class JavaCompletionContributor extends CompletionContributor { return false; } - if (NameUtil.useMinusculeHumpMatcher) return true; + if (NameUtil.isUseMinusculeHumpMatcher()) return true; final String s = result.getPrefixMatcher().getPrefix(); if (StringUtil.isEmpty(s) || !Character.isUpperCase(s.charAt(0))) return false; diff --git a/platform/platform-impl/src/com/intellij/ui/SpeedSearchBase.java b/platform/platform-impl/src/com/intellij/ui/SpeedSearchBase.java index ae7ec1fc4582..758eb022556d 100644 --- a/platform/platform-impl/src/com/intellij/ui/SpeedSearchBase.java +++ b/platform/platform-impl/src/com/intellij/ui/SpeedSearchBase.java @@ -104,7 +104,7 @@ public abstract class SpeedSearchBase extends SpeedSear if (!isPopupActive()) return null; final SpeedSearchComparator comparator = getComparator(); final String recentSearchText = comparator.getRecentSearchText(); - return recentSearchText != null && recentSearchText.length() > 0 && comparator.doCompare(recentSearchText, text) && !NameUtil.useMinusculeHumpMatcher ? comparator.getRecentSearchMatcher() : null; + return recentSearchText != null && recentSearchText.length() > 0 && comparator.doCompare(recentSearchText, text) && !NameUtil.isUseMinusculeHumpMatcher() ? comparator.getRecentSearchMatcher() : null; } /** @@ -183,7 +183,7 @@ public abstract class SpeedSearchBase extends SpeedSear if (myRecentSearchText != null && myRecentSearchText.equals(pattern) ) { - if (NameUtil.useMinusculeHumpMatcher) { + if (NameUtil.isUseMinusculeHumpMatcher()) { return myMinusculeMatcher.matches(text); } @@ -202,7 +202,7 @@ public abstract class SpeedSearchBase extends SpeedSear final Pattern recentSearchPattern = Pattern.compile(buf.toString(), allLowercase ? Pattern.CASE_INSENSITIVE : 0); myRecentSearchMatcher = recentSearchPattern.matcher(text); - if (NameUtil.useMinusculeHumpMatcher) { + if (NameUtil.isUseMinusculeHumpMatcher()) { myMinusculeMatcher = new NameUtil.MinusculeMatcher(myShouldMatchFromTheBeginning ? pattern : "*" + pattern); return myMinusculeMatcher.matches(text); } diff --git a/platform/platform-resources-en/src/misc/registry.properties b/platform/platform-resources-en/src/misc/registry.properties index 405a73b0a55b..d5bdd55266cc 100644 --- a/platform/platform-resources-en/src/misc/registry.properties +++ b/platform/platform-resources-en/src/misc/registry.properties @@ -94,6 +94,7 @@ debugger.mayBringFrameToFrontOnBreakpoint=true filesystem.useNative=true analyze.exceptions.on.the.fly=false +analyze.exceptions.on.the.fly.description=Automatically analyze clipboard on frame activation, and if there is a stacktrace calls Analyze Stacktrace compiler.perform.outputs.refresh.on.start=false compiler.perform.outputs.refresh.on.start.description=Whether to perform initial FS refresh before compilation starts. Need this to detect external changes to output dirs @@ -119,3 +120,5 @@ navbar.userActivityMergeTime=500 navbar.newpopup=true inspectionGadgets.telemetry.enabled=false +minuscule.humps.matching=false +minuscule.humps.matching.description=Camel Case without holding Shift in Ctrl+N/Ctrl+Shift+N etc diff --git a/platform/util/src/com/intellij/psi/codeStyle/NameUtil.java b/platform/util/src/com/intellij/psi/codeStyle/NameUtil.java index 653fc14bb630..6711afe7272a 100644 --- a/platform/util/src/com/intellij/psi/codeStyle/NameUtil.java +++ b/platform/util/src/com/intellij/psi/codeStyle/NameUtil.java @@ -15,6 +15,7 @@ */ package com.intellij.psi.codeStyle; +import com.intellij.openapi.util.registry.Registry; import com.intellij.openapi.util.text.StringUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.Function; @@ -39,7 +40,6 @@ public class NameUtil { } }; private static final int MAX_LENGTH = 40; - public static final boolean useMinusculeHumpMatcher = "true".equals(System.getProperty("minuscule.humps.matching")); private NameUtil() {} @@ -390,12 +390,13 @@ public class NameUtil { return buildMatcher(pattern, buildRegexp(pattern, exactPrefixLen, allowToUpper, allowToLower, lowerCaseWords, false)); } - private static Matcher buildMatcher(final String pattern, String regexp) { - if (useMinusculeHumpMatcher) { - return new MinusculeMatcher(pattern); - } + public static boolean isUseMinusculeHumpMatcher() { + return Registry.is("minuscule.humps.matching"); + } - return new OptimizedMatcher(pattern, regexp); + private static Matcher buildMatcher(final String pattern, String regexp) { + return isUseMinusculeHumpMatcher() ? new MinusculeMatcher(pattern) + : new OptimizedMatcher(pattern, regexp); } private static class OptimizedMatcher implements Matcher { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/completion/GroovyCompletionContributor.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/completion/GroovyCompletionContributor.java index b86e1ad8fb08..4aec6a74bf94 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/completion/GroovyCompletionContributor.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/completion/GroovyCompletionContributor.java @@ -454,7 +454,7 @@ public class GroovyCompletionContributor extends CompletionContributor { }); final String s = result.getPrefixMatcher().getPrefix(); - if (NameUtil.useMinusculeHumpMatcher || !StringUtil.isEmpty(s) && Character.isUpperCase(s.charAt(0))) { + if (NameUtil.isUseMinusculeHumpMatcher() || !StringUtil.isEmpty(s) && Character.isUpperCase(s.charAt(0))) { addAllClasses(parameters, result, inheritors); } } From 92fe151d547d020b5f215a91a772b4e3704a1a37 Mon Sep 17 00:00:00 2001 From: Maxim Medvedev Date: Fri, 11 Mar 2011 11:27:57 +0300 Subject: [PATCH 03/14] titles for introduce param & field dialogs --- .../refactoring/introduce/field/GrIntroduceFieldDialog.java | 2 ++ .../introduce/parameter/GrIntroduceParameterDialog.java | 3 +++ 2 files changed, 5 insertions(+) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/field/GrIntroduceFieldDialog.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/field/GrIntroduceFieldDialog.java index c0ded18ee43f..5dafb7fc3a8f 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/field/GrIntroduceFieldDialog.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/field/GrIntroduceFieldDialog.java @@ -22,6 +22,7 @@ import com.intellij.psi.PsiElement; import com.intellij.psi.PsiMethod; import com.intellij.psi.PsiType; import com.intellij.refactoring.RefactoringBundle; +import com.intellij.refactoring.introduceField.IntroduceFieldHandler; import com.intellij.refactoring.ui.NameSuggestionsField; import com.intellij.refactoring.util.RadioUpDownListener; import com.intellij.util.IncorrectOperationException; @@ -165,6 +166,7 @@ public class GrIntroduceFieldDialog extends DialogWrapper implements GrIntroduce allOccurrencesInOneMethod(myContext.occurrences, clazz) && isAlwaysInvokedConstructor(containingMethod, clazz); hasLHSUsages = hasLhsUsages(myContext); + setTitle(IntroduceFieldHandler.REFACTORING_NAME); init(); checkErrors(); } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/parameter/GrIntroduceParameterDialog.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/parameter/GrIntroduceParameterDialog.java index cdb324a04bdc..7cebfbeb7343 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/parameter/GrIntroduceParameterDialog.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/parameter/GrIntroduceParameterDialog.java @@ -19,6 +19,7 @@ import com.intellij.psi.PsiElement; import com.intellij.psi.PsiType; import com.intellij.psi.codeStyle.CodeStyleSettingsManager; import com.intellij.refactoring.JavaRefactoringSettings; +import com.intellij.refactoring.inline.InlineParameterHandler; import com.intellij.refactoring.ui.NameSuggestionsField; import com.intellij.refactoring.ui.RefactoringDialog; import com.intellij.util.ui.GridBag; @@ -84,6 +85,8 @@ public class GrIntroduceParameterDialog extends RefactoringDialog implements GrI initReplaceFieldsWithGetters(settings); myDeclareFinalCheckBox.setSelected(hasFinalModifier()); + + setTitle(InlineParameterHandler.REFACTORING_NAME); init(); } From 7883e7a3e58254e865b0a5c8fcd7d10b0ad95c56 Mon Sep 17 00:00:00 2001 From: Maxim Medvedev Date: Fri, 11 Mar 2011 11:38:16 +0300 Subject: [PATCH 04/14] IDEA-66425 Groovy: Introduce Field Refactoring applied to closure with parameter(s) doesn't allow to initialize filed in its declaration or class constructor --- .../introduce/field/GrIntroduceFieldDialog.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/field/GrIntroduceFieldDialog.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/field/GrIntroduceFieldDialog.java index 5dafb7fc3a8f..e20ab324720a 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/field/GrIntroduceFieldDialog.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/field/GrIntroduceFieldDialog.java @@ -20,7 +20,9 @@ import com.intellij.openapi.util.Ref; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiMethod; +import com.intellij.psi.PsiParameter; import com.intellij.psi.PsiType; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.refactoring.RefactoringBundle; import com.intellij.refactoring.introduceField.IntroduceFieldHandler; import com.intellij.refactoring.ui.NameSuggestionsField; @@ -339,6 +341,7 @@ public class GrIntroduceFieldDialog extends DialogWrapper implements GrIntroduce } final Ref ref = new Ref(Boolean.TRUE); + final GrExpression finalExpression = expression; expression.accept(new GroovyRecursiveElementVisitor() { @Override public void visitReferenceExpression(GrReferenceExpression refExpr) { @@ -348,6 +351,10 @@ public class GrIntroduceFieldDialog extends DialogWrapper implements GrIntroduce if (resolved instanceof GrField && scope.getManager().areElementsEquivalent(scope, ((GrField)resolved).getContainingClass())) { return; } + if (resolved instanceof PsiParameter && + PsiTreeUtil.isAncestor(finalExpression, ((PsiParameter)resolved).getDeclarationScope(), false)) { + return; + } ref.set(Boolean.FALSE); } }); From 79a8f81a7e4a965247954e9581301f00381f4270 Mon Sep 17 00:00:00 2001 From: Maxim Medvedev Date: Fri, 11 Mar 2011 12:58:38 +0300 Subject: [PATCH 05/14] correct replace closure-argument --- .../groovy/lang/psi/impl/PsiImplUtil.java | 28 +++++++++++++++---- .../arguments/GrArgumentListImpl.java | 5 ++++ .../constant/GrIntroduceConstantHandler.java | 9 +++++- .../field/GrIntroduceFieldHandler.java | 8 +++++- .../GrIntroduceParameterProcessor.java | 12 ++++---- 5 files changed, 49 insertions(+), 13 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java index d50aacda0c94..9bf34b111bbd 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java @@ -31,6 +31,7 @@ import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.MethodSignature; import com.intellij.psi.util.MethodSignatureUtil; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.ArrayUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes; @@ -103,12 +104,27 @@ public class PsiImplUtil { if (result != null) return result; } - ASTNode oldNode = oldExpr.getNode(); - ASTNode newNode = newExpr.copy().getNode(); - assert newNode != null && parentNode != null; - parentNode.replaceChild(oldNode, newNode); - - return ((GrExpression)newNode.getPsi()); + //if replace closure argument with expression + //we should add the expression in arg list + if (oldExpr instanceof GrClosableBlock && + !(newExpr instanceof GrClosableBlock) && + oldParent instanceof GrCall && + ArrayUtil.contains(oldExpr, ((GrCall)oldParent).getClosureArguments())) { + final GrClosableBlock[] closureArguments = ((GrCall)oldParent).getClosureArguments(); + final int i = ArrayUtil.find(closureArguments, oldExpr); + GrArgumentList argList = ((GrCall)oldParent).getArgumentList(); + if (argList.getText().length() == 0) argList = (GrArgumentList)argList.replace(factory.createArgumentList()); + for (int j = 0; j < i; j++) { + argList.add(closureArguments[j]); + closureArguments[j].delete(); + } + final GrExpression result = (GrExpression)argList.add(newExpr); + oldExpr.delete(); + return result; + } + else { + return (GrExpression)oldExpr.replace(newExpr); + } } /** diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/arguments/GrArgumentListImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/arguments/GrArgumentListImpl.java index 4ca9e9e0ce91..b1b443108b32 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/arguments/GrArgumentListImpl.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/arguments/GrArgumentListImpl.java @@ -175,6 +175,11 @@ public class GrArgumentListImpl extends GroovyPsiElementImpl implements GrArgume return namedArgument; } + @Override + public PsiElement add(@NotNull PsiElement element) throws IncorrectOperationException { + return addBefore(element, null); + } + @Override public PsiElement addBefore(@NotNull PsiElement element, PsiElement anchor) throws IncorrectOperationException { if (element instanceof GrNamedArgument || element instanceof GrExpression) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/constant/GrIntroduceConstantHandler.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/constant/GrIntroduceConstantHandler.java index 3e59ee304806..575381236aeb 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/constant/GrIntroduceConstantHandler.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/constant/GrIntroduceConstantHandler.java @@ -140,7 +140,14 @@ public class GrIntroduceConstantHandler extends GrIntroduceHandlerBase Date: Fri, 11 Mar 2011 14:20:57 +0300 Subject: [PATCH 06/14] IDEA-66473 Groovy: Introduce Parameter Refactoring: PsiInvalidElementAccessException at GrBlockImpl.getControlFlow() on introducing parameter of closure type --- .../lang/psi/impl/statements/blocks/GrBlockImpl.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/blocks/GrBlockImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/blocks/GrBlockImpl.java index dfd3f5412c42..f07b0f37dfea 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/blocks/GrBlockImpl.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/blocks/GrBlockImpl.java @@ -17,6 +17,7 @@ package org.jetbrains.plugins.groovy.lang.psi.impl.statements.blocks; import com.intellij.lang.ASTNode; +import com.intellij.openapi.util.Key; import com.intellij.psi.PsiElement; import com.intellij.psi.ResolveState; import com.intellij.psi.impl.source.tree.Factory; @@ -49,7 +50,7 @@ import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil; * @author ven */ public abstract class GrBlockImpl extends LazyParseablePsiElement implements GrCodeBlock, GrControlFlowOwner { - private volatile CachedValue myControlFlow = null; + private static final Key> CONTROL_FLOW = Key.create("Control flow"); protected GrBlockImpl(@NotNull IElementType type, CharSequence buffer) { super(type, buffer); @@ -75,7 +76,7 @@ public abstract class GrBlockImpl extends LazyParseablePsiElement implements GrC public void subtreeChanged() { super.subtreeChanged(); - myControlFlow = null; + putUserData(CONTROL_FLOW, null); } @Override @@ -96,14 +97,15 @@ public abstract class GrBlockImpl extends LazyParseablePsiElement implements GrC } public Instruction[] getControlFlow() { - CachedValue controlFlow = myControlFlow; + CachedValue controlFlow = getUserData(CONTROL_FLOW); if (controlFlow == null) { - myControlFlow = controlFlow = CachedValuesManager.getManager(getProject()).createCachedValue(new CachedValueProvider() { + controlFlow = CachedValuesManager.getManager(getProject()).createCachedValue(new CachedValueProvider() { @Override public Result compute() { return Result.create(new ControlFlowBuilder(getProject()).buildControlFlow(GrBlockImpl.this), getContainingFile()); } }, false); + putUserData(CONTROL_FLOW, controlFlow); } return controlFlow.getValue(); From 33206911da2c49475ec63b72f3bc6ad7eb83b006 Mon Sep 17 00:00:00 2001 From: Maxim Medvedev Date: Fri, 11 Mar 2011 14:45:07 +0300 Subject: [PATCH 07/14] tests --- .../expressions/GroovyWithTypeCastSurrounder.java | 6 ++++-- .../introduceParameter/GrIntroduceParameterTest.java | 4 ++++ .../testdata/groovy/refactoring/extractMethod/expr1.test | 2 +- .../testdata/groovy/refactoring/extractMethod/input1.test | 2 +- .../introduceParameterGroovy/closure/ClosureAfter.groovy | 1 + .../introduceParameterGroovy/closure/ClosureBefore.groovy | 1 + .../introduceParameterGroovy/closure/ClosureMyClass.groovy | 6 ++++++ 7 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureAfter.groovy create mode 100644 plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureBefore.groovy create mode 100644 plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureMyClass.groovy diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyWithTypeCastSurrounder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyWithTypeCastSurrounder.java index 6724da5e03b6..a4f063e069e7 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyWithTypeCastSurrounder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyWithTypeCastSurrounder.java @@ -29,13 +29,15 @@ import org.jetbrains.plugins.groovy.lang.psi.api.types.GrTypeElement; public class GroovyWithTypeCastSurrounder extends GroovyExpressionSurrounder { protected TextRange surroundExpression(GrExpression expression) { GrParenthesizedExpression parenthesized = (GrParenthesizedExpression) GroovyPsiElementFactory.getInstance(expression.getProject()).createTopElementFromText("((Type)a)"); - parenthesized = (GrParenthesizedExpression) expression.replaceWithExpression(parenthesized, false); GrTypeCastExpression typeCast = (GrTypeCastExpression) parenthesized.getOperand(); replaceToOldExpression(typeCast.getOperand(), expression); GrTypeElement typeElement = typeCast.getCastTypeElement(); int endOffset = typeElement.getTextRange().getStartOffset(); + parenthesized = (GrParenthesizedExpression) expression.replaceWithExpression(parenthesized, false); - typeCast.getNode().removeChild(typeElement.getNode()); + final GrTypeCastExpression newTypeCast = (GrTypeCastExpression)parenthesized.getOperand(); + final GrTypeElement newTypeElement = newTypeCast.getCastTypeElement(); + newTypeElement.delete(); return new TextRange(endOffset, endOffset); } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/GrIntroduceParameterTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/GrIntroduceParameterTest.java index 31ca9497a12a..ecfeec9e4020 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/GrIntroduceParameterTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/GrIntroduceParameterTest.java @@ -289,4 +289,8 @@ public class GrIntroduceParameterTest extends LightCodeInsightFixtureTestCase { public void testIncorrectArgumentList() { doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_NONE, true, false, true); } + + public void testClosure() { + doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_NONE, false, false, false); + } } diff --git a/plugins/groovy/testdata/groovy/refactoring/extractMethod/expr1.test b/plugins/groovy/testdata/groovy/refactoring/extractMethod/expr1.test index 3d395d6f7528..705f2f5c5657 100644 --- a/plugins/groovy/testdata/groovy/refactoring/extractMethod/expr1.test +++ b/plugins/groovy/testdata/groovy/refactoring/extractMethod/expr1.test @@ -6,7 +6,7 @@ protected def getGeneratedFileNames(String name, int boo) { ----- protected def getGeneratedFileNames(String name, int boo) { def names - names = testMethod() + names = testMethod() names } diff --git a/plugins/groovy/testdata/groovy/refactoring/extractMethod/input1.test b/plugins/groovy/testdata/groovy/refactoring/extractMethod/input1.test index b42d78fa4e79..a1cdec21b192 100644 --- a/plugins/groovy/testdata/groovy/refactoring/extractMethod/input1.test +++ b/plugins/groovy/testdata/groovy/refactoring/extractMethod/input1.test @@ -15,7 +15,7 @@ class S { Closure sin = {x -> Math.sin(x)} - testMethod(sin) + testMethod(sin) } diff --git a/plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureAfter.groovy b/plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureAfter.groovy new file mode 100644 index 000000000000..5ec9c955f113 --- /dev/null +++ b/plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureAfter.groovy @@ -0,0 +1 @@ +new A().doSmth({ println "smth" }) \ No newline at end of file diff --git a/plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureBefore.groovy b/plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureBefore.groovy new file mode 100644 index 000000000000..df0b92127858 --- /dev/null +++ b/plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureBefore.groovy @@ -0,0 +1 @@ +new A().doSmth() \ No newline at end of file diff --git a/plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureMyClass.groovy b/plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureMyClass.groovy new file mode 100644 index 000000000000..59a1a626d833 --- /dev/null +++ b/plugins/groovy/testdata/refactoring/introduceParameterGroovy/closure/ClosureMyClass.groovy @@ -0,0 +1,6 @@ +class A { + void doSmth() { + [1, 2, 3].each { println "smth" } + } +} + From f0ee218178eb73603cfc82f472d76aac7de75f14 Mon Sep 17 00:00:00 2001 From: Denis Zhdanov Date: Thu, 3 Mar 2011 18:47:54 +0300 Subject: [PATCH 08/14] IDEA-66185 Malformed format string inspection: Instruct the inspection that int -> char conversion is correct The inspection is taught to not report int -> char conversion --- .../src/com/siyeh/ig/bugs/FormatDecode.java | 33 +++++++++++++++++-- .../MalformedFormatString.java | 1 + 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/FormatDecode.java b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/FormatDecode.java index ec750545ffe5..bfbcdb8550f2 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/FormatDecode.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/FormatDecode.java @@ -17,8 +17,12 @@ package com.siyeh.ig.bugs; import com.intellij.psi.CommonClassNames; import com.intellij.psi.PsiType; +import com.intellij.util.containers.ContainerUtil; import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -44,6 +48,27 @@ class FormatDecode{ private static final Validator FLOAT_VALIDATOR = new FloatValidator(); + /** + * Holds information about validator replacement rules, i.e. allows to answer if validator of particular type may be + * safely replaced by validator of another particular type. + *

+ * For example, validator of type {@link AllValidator#type() 'all'} may be safely replaced by validator of any other + * type, e.g. {@link DateValidator#type() Date/Time} or {@link CharValidator#type() 'char validator'} may be replaced + * by {@link IntValidator#type() 'int validator'} because {@link Formatter java formatter} knows how to + * {@link Formatter.FormatSpecifier#printCharacter(Object) print character from integer} etc. + *

+ * Generally, current collection holds set of mappings where the key is type of validator that may be safely replaced + * by validator of type that is contained at 'values' collection. + */ + private static final Map> REPLACEABLE_VALIDATOR_TYPES = new HashMap>(); + static { + REPLACEABLE_VALIDATOR_TYPES.put( + ALL_VALIDATOR.type(), + ContainerUtil.set(DATE_VALIDATOR.type(), CHAR_VALIDATOR.type(), INT_VALIDATOR.type(), FLOAT_VALIDATOR.type()) + ); + REPLACEABLE_VALIDATOR_TYPES.put(CHAR_VALIDATOR.type(), ContainerUtil.set(INT_VALIDATOR.type())); + } + public static Validator[] decode(String formatString, int argumentCount){ final ArrayList parameters = new ArrayList(); @@ -108,11 +133,13 @@ class FormatDecode{ int argumentCount){ if(pos < parameters.size()){ final Validator old = parameters.get(pos); + Set replaceableTypes = REPLACEABLE_VALIDATOR_TYPES.get(old.type()); + if (replaceableTypes != null && replaceableTypes.contains(val.type())) { + parameters.set(pos, val); + } // it's OK to overwrite ALL with something more specific // it's OK to ignore overwrite of something else with ALL or itself - if (old == ALL_VALIDATOR) { - parameters.set(pos, val); - } else if (val != ALL_VALIDATOR && val != old) { + else if (val != ALL_VALIDATOR && val != old) { throw new DuplicateFormatFlagsException( "requires both " + old.type() + " and " + val.type()); } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/malformed_format_string/MalformedFormatString.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/malformed_format_string/MalformedFormatString.java index d8b0e830f731..4835464100b7 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/malformed_format_string/MalformedFormatString.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/malformed_format_string/MalformedFormatString.java @@ -21,6 +21,7 @@ public class MalformedFormatString { String warn = String.format("%s %s", 1); // this is invalid according to the inspector (correct) String invalid = String.format("%s %s" + local, 1); // this is valid according to the inspector (INCORRECT!) String interesting = String.format("%s %s" + "hmm", 1); // this is invalid according to the inspector (correct) + String intAsChar = String.format("symbol '%1$c' (numeric value %1$d)", 60); // integer->char conversion is ok (correct) } public void outOfMemory() { From 5eb6df834863e6a7003aed6dc9b7d97668d04f4e Mon Sep 17 00:00:00 2001 From: Denis Zhdanov Date: Thu, 3 Mar 2011 19:01:02 +0300 Subject: [PATCH 09/14] IDEA-66185 Malformed format string inspection: Instruct the inspection that int -> char conversion is correct Utility collection population method signature is corrected in order to covariantly return given IS-A Collection instead of Collection --- .../src/com/intellij/util/containers/ContainerUtil.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/platform/util/src/com/intellij/util/containers/ContainerUtil.java b/platform/util/src/com/intellij/util/containers/ContainerUtil.java index ecfb00ea0c68..e684e0829570 100644 --- a/platform/util/src/com/intellij/util/containers/ContainerUtil.java +++ b/platform/util/src/com/intellij/util/containers/ContainerUtil.java @@ -488,7 +488,7 @@ public class ContainerUtil { } } - public static Collection addAll(@NotNull Collection collection, @NotNull T... elements) { + public static > C addAll(@NotNull C collection, @NotNull A... elements) { //noinspection ManualArrayToCollectionCopy for (T element : elements) { collection.add(element); @@ -842,6 +842,11 @@ public class ContainerUtil { return result.toArray(emptyArray); } + @NotNull + public static Set set(T ... items) { + return addAll(new HashSet(), items); + } + public static void addIfNotNull(final T element, @NotNull Collection result) { if (element != null) { result.add(element); From 014b64a3921bb60fe0dba7ce499733eca977de47 Mon Sep 17 00:00:00 2001 From: Denis Zhdanov Date: Thu, 10 Mar 2011 17:53:13 +0300 Subject: [PATCH 10/14] IDEA-66053 Formatter: Optimize performance of applying formatting changes 1. Introduced ability to collect text changes with automatic normalization and merging (TextChangesStorage); 2. CharArray is able to function at 'defer changes' mode; 3. DocumentImpl toggles 'defer changes' mode during 'in bulk update' state change; 4. Tests are added; 5. Repackaging; 6. Xml performance test data files are renamed in order to allow their execution under Linux (the tests lookup test data file assuming that it starts from the lower-case letter but some of them were started from upper-case letter); --- .../options/DocumentChangesCollector.java | 6 +- .../editorActions/AutoHardWrapHandler.java | 2 +- .../formatting/BulkChangesMerger.java | 72 --- .../intellij/formatting/FormatProcessor.java | 6 +- .../options/DocumentChangesCollectorTest.java | 2 +- .../formatting/BulkChangesMergerTest.java | 112 ++++- .../editor/impl/BulkChangesMerger.java | 326 ++++++++++++ .../openapi/editor/impl/CharArray.java | 167 ++++++- .../openapi/editor/impl/DocumentImpl.java | 4 +- .../impl/{softwrap => }/TextChangeImpl.java | 2 +- .../editor/impl/TextChangesStorage.java | 469 ++++++++++++++++++ .../editor/impl/softwrap/SoftWrapImpl.java | 1 + .../mapping/SoftWrapApplianceManager.java | 1 + .../openapi/editor/impl/CharArrayTest.java | 167 +++++++ .../editor/impl/TextChangesStorageTest.java | 277 +++++++++++ .../impl/softwrap/TextChangeImplTest.java | 1 + .../CachingSoftWrapDataMapperTest.java | 1 + .../com/intellij/util/text/CharArrayUtil.java | 41 +- 18 files changed, 1548 insertions(+), 109 deletions(-) delete mode 100644 platform/lang-impl/src/com/intellij/formatting/BulkChangesMerger.java create mode 100644 platform/platform-impl/src/com/intellij/openapi/editor/impl/BulkChangesMerger.java rename platform/platform-impl/src/com/intellij/openapi/editor/impl/{softwrap => }/TextChangeImpl.java (99%) create mode 100644 platform/platform-impl/src/com/intellij/openapi/editor/impl/TextChangesStorage.java create mode 100644 platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/CharArrayTest.java create mode 100644 platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/TextChangesStorageTest.java diff --git a/platform/lang-impl/src/com/intellij/application/options/DocumentChangesCollector.java b/platform/lang-impl/src/com/intellij/application/options/DocumentChangesCollector.java index d078cbff1bad..fc429ae452fa 100644 --- a/platform/lang-impl/src/com/intellij/application/options/DocumentChangesCollector.java +++ b/platform/lang-impl/src/com/intellij/application/options/DocumentChangesCollector.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2010 JetBrains s.r.o. + * 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. @@ -19,7 +19,7 @@ import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.TextChange; import com.intellij.openapi.editor.event.DocumentEvent; import com.intellij.openapi.editor.event.DocumentListener; -import com.intellij.openapi.editor.impl.softwrap.TextChangeImpl; +import com.intellij.openapi.editor.impl.TextChangeImpl; import gnu.trove.TIntArrayList; import org.jetbrains.annotations.NotNull; @@ -190,7 +190,7 @@ public class DocumentChangesCollector implements DocumentListener { } private void mergeChangesIfNecessary(DocumentEvent event) { - // There is a possible case that we had more than scattered change (e.g. (3; 5) and (8; 10)) and current document change affects + // There is a possible case that we had more than one scattered change (e.g. (3; 5) and (8; 10)) and current document change affects // both of them (e.g. remove all symbols from offset (4; 9)). We have two changes then: (3; 4) and (4; 5) and want to merge them // into a single one. if (myChanges.size() < 2) { diff --git a/platform/lang-impl/src/com/intellij/codeInsight/editorActions/AutoHardWrapHandler.java b/platform/lang-impl/src/com/intellij/codeInsight/editorActions/AutoHardWrapHandler.java index a1c2d6d2c732..b1458e6434f7 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/editorActions/AutoHardWrapHandler.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/editorActions/AutoHardWrapHandler.java @@ -25,7 +25,7 @@ import com.intellij.openapi.editor.*; import com.intellij.openapi.editor.actionSystem.EditorActionManager; import com.intellij.openapi.editor.event.DocumentEvent; import com.intellij.openapi.editor.event.DocumentListener; -import com.intellij.openapi.editor.impl.softwrap.TextChangeImpl; +import com.intellij.openapi.editor.impl.TextChangeImpl; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Key; import com.intellij.psi.codeStyle.CodeStyleSettings; diff --git a/platform/lang-impl/src/com/intellij/formatting/BulkChangesMerger.java b/platform/lang-impl/src/com/intellij/formatting/BulkChangesMerger.java deleted file mode 100644 index 5147013a4caa..000000000000 --- a/platform/lang-impl/src/com/intellij/formatting/BulkChangesMerger.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright 2000-2010 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.intellij.formatting; - -import com.intellij.openapi.editor.TextChange; -import org.jetbrains.annotations.NotNull; - -import java.util.List; - -/** - * Encapsulates logic of merging set of changes into particular text. - *

- * Thread-safe. - * - * @author Denis Zhdanov - * @since 12/22/10 12:02 PM - */ -public class BulkChangesMerger { - - /** - * Merges given changes within the given text and returns result. - * - * @param text text to apply given changes for - * @param textLength interested number of symbols from the given text to use - * @param changes changes to apply to the given text. It's assumed that there are no intersections between them and that they - * are sorted by offsets in ascending order - * @return merge result - */ - @SuppressWarnings({"MethodMayBeStatic"}) - public CharSequence merge(@NotNull char[] text, int textLength, @NotNull List changes) { - int newLength = textLength; - for (TextChange change : changes) { - newLength += change.getText().length() - (change.getEnd() - change.getStart()); - } - char[] data = new char[newLength]; - int oldEndOffset = textLength; - int newEndOffset = data.length; - for (int i = changes.size() - 1; i >= 0; i--) { - TextChange change = changes.get(i); - - // Copy all unprocessed symbols from initial text that lay after the changed offset. - int symbolsToMoveNumber = oldEndOffset - change.getEnd(); - System.arraycopy(text, change.getEnd(), data, newEndOffset - symbolsToMoveNumber, symbolsToMoveNumber); - newEndOffset -= symbolsToMoveNumber; - - // Copy all change symbols. - char[] changeSymbols = change.getChars(); - newEndOffset -= changeSymbols.length; - System.arraycopy(changeSymbols, 0, data, newEndOffset, changeSymbols.length); - oldEndOffset = change.getStart(); - } - - if (oldEndOffset > 0) { - System.arraycopy(text, 0, data, 0, oldEndOffset); - } - - return new String(data); - } -} diff --git a/platform/lang-impl/src/com/intellij/formatting/FormatProcessor.java b/platform/lang-impl/src/com/intellij/formatting/FormatProcessor.java index 6017b2db2664..4870cfef4cbb 100644 --- a/platform/lang-impl/src/com/intellij/formatting/FormatProcessor.java +++ b/platform/lang-impl/src/com/intellij/formatting/FormatProcessor.java @@ -20,7 +20,8 @@ import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.TextChange; import com.intellij.openapi.editor.ex.DocumentEx; -import com.intellij.openapi.editor.impl.softwrap.TextChangeImpl; +import com.intellij.openapi.editor.impl.BulkChangesMerger; +import com.intellij.openapi.editor.impl.TextChangeImpl; import com.intellij.openapi.fileTypes.StdFileTypes; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.TextRange; @@ -44,7 +45,6 @@ class FormatProcessor { private static final int BULK_REPLACE_OPTIMIZATION_CRITERIA = 3000; private static final Logger LOG = Logger.getInstance("#com.intellij.formatting.FormatProcessor"); - private static final BulkChangesMerger ourBulkChangesMerger = new BulkChangesMerger(); private LeafBlockWrapper myCurrentBlock; @@ -298,7 +298,7 @@ class FormatProcessor { ); changes.add(new TextChangeImpl(newWs, whiteSpace.getStartOffset(), whiteSpace.getEndOffset())); } - CharSequence mergeResult = ourBulkChangesMerger.merge(document.getChars(), document.getTextLength(), changes); + CharSequence mergeResult = BulkChangesMerger.INSTANCE.mergeToCharSequence(document.getChars(), document.getTextLength(), changes); document.replaceString(0, document.getTextLength(), mergeResult); cleanupBlocks(blocksToModify); return true; diff --git a/platform/lang-impl/testSrc/com/intellij/application/options/DocumentChangesCollectorTest.java b/platform/lang-impl/testSrc/com/intellij/application/options/DocumentChangesCollectorTest.java index 2003078cc102..de4a35cf2e95 100644 --- a/platform/lang-impl/testSrc/com/intellij/application/options/DocumentChangesCollectorTest.java +++ b/platform/lang-impl/testSrc/com/intellij/application/options/DocumentChangesCollectorTest.java @@ -2,8 +2,8 @@ package com.intellij.application.options; import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.TextChange; +import com.intellij.openapi.editor.impl.TextChangeImpl; import com.intellij.openapi.editor.impl.event.DocumentEventImpl; -import com.intellij.openapi.editor.impl.softwrap.TextChangeImpl; import org.jmock.Expectations; import org.jmock.Mockery; import org.jmock.integration.junit4.JUnit4Mockery; diff --git a/platform/lang-impl/testSrc/com/intellij/formatting/BulkChangesMergerTest.java b/platform/lang-impl/testSrc/com/intellij/formatting/BulkChangesMergerTest.java index 5e57751803d6..34cfbd154679 100644 --- a/platform/lang-impl/testSrc/com/intellij/formatting/BulkChangesMergerTest.java +++ b/platform/lang-impl/testSrc/com/intellij/formatting/BulkChangesMergerTest.java @@ -15,11 +15,15 @@ */ package com.intellij.formatting; -import com.intellij.openapi.editor.TextChange; -import com.intellij.openapi.editor.impl.softwrap.TextChangeImpl; +import com.intellij.openapi.editor.impl.BulkChangesMerger; +import com.intellij.openapi.editor.impl.TextChangeImpl; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TestWatchman; +import org.junit.runners.model.FrameworkMethod; +import java.lang.annotation.*; import java.util.Arrays; import static org.junit.Assert.assertEquals; @@ -30,6 +34,30 @@ import static org.junit.Assert.assertEquals; */ public class BulkChangesMergerTest { + @Rule + public TestWatchman configReader = new TestWatchman() { + @Override + public void starting(FrameworkMethod method) { + Config config = method.getAnnotation(Config.class); + if (config != null) { + myConfig = config; + } + else { + try { + myConfig = BulkChangesMergerTest.class.getMethod("dummy").getAnnotation(Config.class); + } + catch (NoSuchMethodException e) { + throw new RuntimeException(e); + } + } + } + }; + + @Config + public static void dummy() { + } + + private Config myConfig; private BulkChangesMerger myMerger; @Before @@ -47,25 +75,87 @@ public class BulkChangesMergerTest { doTest("abcd", "a1b2c3d45", c("1", 1), c("2", 2), c("3", 3), c("45", 4)); } + @Config(initialTextLength = 4) @Test public void interestedSymbolsNumberLessThanAvailable() { - doTest("abcdefg", 4, "a12bc3d", c("12", 1), c("3", 3)); + doTest("abcdefg", "a12bc3d", c("12", 1), c("3", 3)); } - private static TextChange c(String text, int offset) { + @Config(inplace = true) + @Test + public void inplaceZeroGroups() { + doTest("0123456789", "a2bc4defg8", c("a", 0, 2), c("bc", 3, 4), c("defg", 5, 8), c("", 9, 10)); + } + + @Config(inplace = true) + @Test + public void inplaceGrowingGroupsWithLastPositive() { + doTest("abcdefghijklmnopqrst", "acABdCjkDEFGHIJKLMpqrst", c("", 1, 2), c("AB", 3), c("C", 4, 9), c("DEFGHIJKLM", 11, 15)); + } + + @Config(inplace = true) + @Test + public void inplaceGrowingGroupsWithLastNegative() { + doTest("abcdefghijk", "acABdCjk", c("", 1, 2), c("AB", 3), c("C", 4, 9)); + } + + @Config(inplace = true) + @Test + public void onlyGrowing() { + doTest("0123456789", "0ab2c3defg67hijk9", c("ab", 1, 2), c("c", 3), c("defg", 4, 6), c("hijk", 8, 9)); + } + + @Config(inplace = true) + @Test + public void onlyShrinking() { + doTest("0123456789", "0a35b9", c("a", 1, 3), c("", 4, 5), c("b", 6, 9)); + } + + @Config(inplace = true, dataArrayLength = 4) + @Test(expected = IllegalArgumentException.class) + public void insufficientLengthForInplaceMerge() { + doTest("0123", "", c("", 1, 3), c("abc", 4)); + } + + private static TextChangeImpl c(String text, int offset) { return c(text, offset, offset); } - private static TextChange c(String text, int start, int end) { + private static TextChangeImpl c(String text, int start, int end) { return new TextChangeImpl(text, start, end); } - private void doTest(String initial, String expected, TextChange ... changes) { - doTest(initial, initial.length(), expected, changes); + private void doTest(String initial, String expected, TextChangeImpl ... changes) { + if (myConfig.inplace()) { + int diff = 0; + for (TextChangeImpl change : changes) { + diff += change.getDiff(); + } + int outputUsefulLength = initial.length() + diff; + int dataLength = myConfig.dataArrayLength(); + if (dataLength < 0) { + dataLength = Math.max(outputUsefulLength, initial.length()); + } + char[] data = new char[dataLength]; + System.arraycopy(initial.toCharArray(), 0, data, 0, initial.length()); + myMerger.mergeInPlace(data, initial.length(), Arrays.asList(changes)); + assertEquals(expected, new String(data, 0, outputUsefulLength)); + } + else { + int interestedSymbolsNumber = myConfig.initialTextLength(); + if (interestedSymbolsNumber < 0) { + interestedSymbolsNumber = initial.length(); + } + CharSequence actual = myMerger.mergeToCharSequence(initial.toCharArray(), interestedSymbolsNumber, Arrays.asList(changes)); + assertEquals(expected, actual.toString()); + } } - - private void doTest(String initial, int interestedInitialSymbolsNumber, String expected, TextChange ... changes) { - CharSequence actual = myMerger.merge(initial.toCharArray(), interestedInitialSymbolsNumber, Arrays.asList(changes)); - assertEquals(expected, actual.toString()); + + @Target(ElementType.METHOD) + @Retention(RetentionPolicy.RUNTIME) + private @interface Config { + boolean inplace() default false; + int dataArrayLength() default -1; + int initialTextLength() default -1; } } diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/BulkChangesMerger.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/BulkChangesMerger.java new file mode 100644 index 000000000000..a23ad662b501 --- /dev/null +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/BulkChangesMerger.java @@ -0,0 +1,326 @@ +/* + * 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 com.intellij.openapi.editor.impl; + +import com.intellij.openapi.editor.TextChange; +import org.jetbrains.annotations.NotNull; + +import java.util.Arrays; +import java.util.List; + +/** + * Encapsulates logic of merging set of changes into particular text. + *

+ * Thread-safe. + * + * @author Denis Zhdanov + * @since 12/22/10 12:02 PM + */ +@SuppressWarnings({"MethodMayBeStatic"}) +public class BulkChangesMerger { + + public static final BulkChangesMerger INSTANCE = new BulkChangesMerger(); + + /** + * Merges given changes within the given text and returns result as a new char sequence. + * + * @param text text to apply given changes for + * @param textLength interested number of symbols from the given text to use + * @param changes changes to apply to the given text. It's assumed that there are no intersections between them and that they + * are sorted by offsets in ascending order + * @return merge result + */ + public CharSequence mergeToCharSequence(@NotNull char[] text, int textLength, @NotNull List changes) { + return new String(mergeToCharArray(text, textLength, changes)); + } + + /** + * Merges given changes within the given text and returns result as a new char array. + * + * @param text text to apply given changes for + * @param textLength interested number of symbols from the given text to use + * @param changes changes to apply to the given text. It's assumed that there are no intersections between them and that they + * are sorted by offsets in ascending order + * @return merge result + */ + public char[] mergeToCharArray(@NotNull char[] text, int textLength, @NotNull List changes) { + int newLength = textLength; + for (TextChange change : changes) { + newLength += change.getText().length() - (change.getEnd() - change.getStart()); + } + char[] data = new char[newLength]; + int oldEndOffset = textLength; + int newEndOffset = data.length; + for (int i = changes.size() - 1; i >= 0; i--) { + TextChange change = changes.get(i); + + // Copy all unprocessed symbols from initial text that lay after the changed offset. + int symbolsToMoveNumber = oldEndOffset - change.getEnd(); + System.arraycopy(text, change.getEnd(), data, newEndOffset - symbolsToMoveNumber, symbolsToMoveNumber); + newEndOffset -= symbolsToMoveNumber; + + // Copy all change symbols. + char[] changeSymbols = change.getChars(); + newEndOffset -= changeSymbols.length; + System.arraycopy(changeSymbols, 0, data, newEndOffset, changeSymbols.length); + oldEndOffset = change.getStart(); + } + + if (oldEndOffset > 0) { + System.arraycopy(text, 0, data, 0, oldEndOffset); + } + + return data; + } + + /** + * Allows to perform 'in-place' merge of the given changes to the given array. + *

+ * I.e. it's considered that given array contains particular text at [0; length) region and given changes define + * offsets against it. It's also assumed that given array length is enough to contain resulting text after applying the changes. + *

+ * Example: consider that initial text is '12345' and given changes are 'remove text at [1; 3) interval' + * and 'replace text at [4; 5) interval with 'abcde''. Resulting text is '14abcde' then and given array + * length should be not less than 7. + * + * @param data data array + * @param length initial text length (without changes) + * @param changes change to apply to the target text + * @throws IllegalArgumentException if given array is not big enough to contain the resulting text + */ + public void mergeInPlace(@NotNull char[] data, int length, @NotNull List changes) + throws IllegalArgumentException + { + // Consider two corner cases: + // 1. Every given change increase text length, i.e. change text length is more than changed region length. We can calculate + // resulting text length and start merging the changes from the right end then; + // 2. Every given change reduces text length, start from the left end then; + // The general idea is to group all of the given changes by 'add text'/ 'remove text' criteria and process them sequentially. + // Example: let's assume we have the following changes: + // 1) replace two symbols with five (diff +3); + // 2) replace two symbols by one (diff -1); + // 3) replace two symbols by one (diff -1); + // 4) replace four symbols by one (diff -3); + // 5) replace one symbol by two (diff +2); + // 6) replace one symbol by three (diff +2); + // Algorithm: + // 1. Define the first group of change. First change diff is '+3', hence, iterate all changes until the resulting diff becomes + // equal or less to the zero. So, the first four changes conduct the first group. Initial change increased text length, hence, + // we process the changes from right to left starting at offset '4-th change start + 1'; + // 2. Current diff is '-2' (4-th change diff is '-3' and one slot was necessary for previous group completion), so, that means + // that we should process the 4-th and 5-th changes as the second group. Initial change direction is negative, hence, we + // process them from left to the right; + // 3. Process the remaining change; + if (changes.isEmpty()) { + return; + } + + int diff = 0; + for (TextChangeImpl change : changes) { + diff += change.getDiff(); + } + if (length + diff > data.length) { + throw new IllegalArgumentException(String.format( + "Can't perform in-place changes merge. Reason: data array is not big enough to hold resulting text. Current size: %d, " + + "minimum size: %d", data.length, length + diff + )); + } + + for (Context context = new Context(changes, data, length, length + diff); !context.isComplete();) { + if (!context.startGroup()) { + return; + } + context.endGroup(); + } + } + + private static void copy(@NotNull char[] data, int offset, @NotNull CharSequence text) { + for (int i = 0; i < text.length(); i++) { + data[i + offset] = text.charAt(i); + } + } + + private static class Context { + + private final List myChanges; + private final char[] myData; + private final int myInputLength; + private final int myOutputLength; + private int myDataStartOffset; + private int myDataEndOffset; + private int myChangeGroupStartIndex; + private int myChangeGroupEndIndex; + private int myDiff; + private int myFirstChangeShift; + private int myLastChangeShift; + + Context(@NotNull List changes, @NotNull char[] data, int inputLength, int outputLength) { + myChanges = changes; + myData = data; + myInputLength = inputLength; + myOutputLength = outputLength; + } + + /** + * Asks current context to update its state in order to point to the first change in a group. + * + * @return true if the first change in a group is found; false otherwise + */ + @SuppressWarnings({"ForLoopThatDoesntUseLoopVariable"}) + public boolean startGroup() { + // Define first change that increases or reduces text length. + for (boolean first = true; myDiff == 0 && myChangeGroupStartIndex < myChanges.size(); myChangeGroupStartIndex++, first = false) { + TextChangeImpl change = myChanges.get(myChangeGroupStartIndex); + myDiff = change.getDiff(); + if (first) { + myDiff += myFirstChangeShift; + } + if (myDiff == 0) { + copy(myData, change.getStart() + (first ? myFirstChangeShift : 0), change.getText()); + } + else { + myDataStartOffset = change.getStart(); + if (first) { + myDataStartOffset += myFirstChangeShift; + } + break; + } + } + return myDiff != 0; + } + + public void endGroup() { + boolean includeEndChange = false; + myLastChangeShift = 0; + for (myChangeGroupEndIndex = myChangeGroupStartIndex + 1; myChangeGroupEndIndex < myChanges.size(); myChangeGroupEndIndex++) { + assert myDiff != 0 : String.format( + "Text: '%s', length: %d, changes: %s, change group indices: %d-%d", + Arrays.toString(myData), myInputLength, myChanges, myChangeGroupStartIndex, myChangeGroupEndIndex); + TextChangeImpl change = myChanges.get(myChangeGroupEndIndex); + int newDiff = myDiff + change.getDiff(); + + // Changes group results to the zero text length shift. + if (newDiff == 0) { + myDataEndOffset = change.getEnd(); + includeEndChange = true; + break; + } + + // Changes group is not constructed yet. + if (!(myDiff > 0 ^ newDiff > 0)) { + myDiff = newDiff; + continue; + } + + // Current change finishes changes group. + myDataEndOffset = change.getStart() + myDiff; + myLastChangeShift = myDiff; + break; + } + + if (myChangeGroupEndIndex >= myChanges.size()) { + if (myDiff > 0) { + processLastPositiveGroup(); + } + else { + processLastNegativeGroup(); + } + myChangeGroupStartIndex = myChangeGroupEndIndex = myChanges.size(); + } + else if (myDiff > 0) { + processPositiveGroup(includeEndChange); + } + else { + processNegativeGroup(includeEndChange); + } + myDiff = 0; + myChangeGroupStartIndex = myChangeGroupEndIndex; + if (includeEndChange) { + myChangeGroupStartIndex++; + } + myFirstChangeShift = myLastChangeShift; + } + + /** + * Asks to process changes group identified by [{@link #myChangeGroupStartIndex}; {@link #myChangeGroupEndIndex}) where + * overall group direction is 'positive' (i.e. it starts from the change that increases text length). + * + * @param includeEndChange flag that defines if change defined by {@link #myChangeGroupEndIndex} should be processed + */ + private void processPositiveGroup(boolean includeEndChange) { + int outputOffset = myDataEndOffset; + int prevChangeStart = -1; + for (int i = myChangeGroupEndIndex; i >= myChangeGroupStartIndex; i--) { + TextChangeImpl change = myChanges.get(i); + if (prevChangeStart >= 0) { + int length = prevChangeStart - change.getEnd(); + System.arraycopy(myData, change.getEnd(), myData, outputOffset - length, length); + outputOffset -= length; + } + prevChangeStart = change.getStart(); + if (i == myChangeGroupEndIndex && !includeEndChange) { + continue; + } + int length = change.getText().length(); + if (length > 0) { + copy(myData, outputOffset - length, change.getText()); + outputOffset -= length; + } + } + } + + private void processLastPositiveGroup() { + int end = myChanges.get(myChanges.size() - 1).getEnd(); + int length = myInputLength - end; + myDataEndOffset = myOutputLength - length; + System.arraycopy(myData, end, myData, myDataEndOffset, length); + myChangeGroupEndIndex = myChanges.size() - 1; + processPositiveGroup(true); + } + + private void processNegativeGroup(boolean includeEndChange) { + int prevChangeEnd = -1; + for (int i = myChangeGroupStartIndex; i <= myChangeGroupEndIndex; i++) { + TextChangeImpl change = myChanges.get(i); + if (prevChangeEnd >= 0) { + int length = change.getStart() - prevChangeEnd; + System.arraycopy(myData, prevChangeEnd, myData, myDataStartOffset, length); + myDataStartOffset += length; + } + prevChangeEnd = change.getEnd(); + if (i == myChangeGroupEndIndex && !includeEndChange) { + return; + } + int length = change.getText().length(); + if (length > 0) { + copy(myData, myDataStartOffset, change.getText()); + myDataStartOffset += length; + } + } + } + + private void processLastNegativeGroup() { + myChangeGroupEndIndex = myChanges.size() - 1; + processNegativeGroup(true); + int end = myChanges.get(myChangeGroupEndIndex).getEnd(); + System.arraycopy(myData, end, myData, myDataStartOffset, myInputLength - end); + } + + public boolean isComplete() { + return myChangeGroupStartIndex >= myChanges.size(); + } + } +} diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/CharArray.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/CharArray.java index d518590475d1..b01523e3e6db 100644 --- a/platform/platform-impl/src/com/intellij/openapi/editor/impl/CharArray.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/CharArray.java @@ -15,23 +15,41 @@ */ package com.intellij.openapi.editor.impl; +import com.intellij.openapi.editor.TextChange; import com.intellij.openapi.editor.event.DocumentEvent; import com.intellij.util.LocalTimeCounter; import com.intellij.util.text.CharArrayCharSequence; import com.intellij.util.text.CharArrayUtil; import com.intellij.util.text.CharSequenceBackedByArray; +import org.jetbrains.annotations.NotNull; import java.lang.ref.SoftReference; +import java.util.List; /** * @author cdr */ abstract class CharArray implements CharSequenceBackedByArray { + + private static final boolean DISABLE_DEFERRED_PROCESSING = Boolean.getBoolean("idea.document.deny.deferred.changes"); + + /** + * We can't exclude possibility of situation when 'defer changes' state is {@link #setDeferredChangeMode(boolean) entered} + * but not exited, hence, we want to perform automatic flushing if necessary in order to avoid memory leaks. This constant holds + * a value that defines that 'automatic flushing' criteria, i.e. every time number of stored deferred changes exceeds this value, + * they are automatically flushed. + */ + private static final int MAX_DEFERRED_CHANGES_NUMBER = 10000; + + private final TextChangesStorage myDeferredChangesStorage = new TextChangesStorage(); + private int myCount = 0; private CharSequence myOriginalSequence; private char[] myArray = null; private SoftReference myStringRef = null; // buffers String value - for not to generate it every time private int myBufferSize; + private int myDeferredShift; + private boolean myDeferredChangeMode; // max chars to hold, bufferSize == 0 means unbounded CharArray(int bufferSize) { @@ -55,6 +73,7 @@ abstract class CharArray implements CharSequenceBackedByArray { myArray = null; myCount = chars.length(); myStringRef = null; + myDeferredChangesStorage.clear(); trimToSize(subj); } @@ -69,6 +88,11 @@ abstract class CharArray implements CharSequenceBackedByArray { private void doReplace(int startOffset, int endOffset, CharSequence newString) { prepareForModification(); + if (isDeferredChangeMode()) { + storeChange(new TextChangeImpl(newString, startOffset, endOffset)); + return; + } + int newLength = newString.length(); int oldLength = endOffset - startOffset; @@ -94,6 +118,11 @@ abstract class CharArray implements CharSequenceBackedByArray { } prepareForModification(); + if (isDeferredChangeMode()) { + storeChange(new TextChangeImpl("", startIndex, endIndex)); + return; + } + if (endIndex < myCount) { System.arraycopy(myArray, endIndex, myArray, startIndex, myCount - endIndex); } @@ -111,16 +140,35 @@ abstract class CharArray implements CharSequenceBackedByArray { private void doInsert(final CharSequence s, final int startIndex) { prepareForModification(); + if (isDeferredChangeMode()) { + storeChange(new TextChangeImpl(s, startIndex)); + return; + } + int insertLength = s.length(); myArray = relocateArray(myArray, myCount + insertLength); if (startIndex < myCount) { System.arraycopy(myArray, startIndex, myArray, startIndex + insertLength, myCount - startIndex); } - CharArrayUtil.getChars(s, myArray,startIndex); + CharArrayUtil.getChars(s, myArray, startIndex); myCount += insertLength; } + /** + * Stores given change at collection of deferred changes (merging it with others if necessary) and updates current object + * state ({@link #length() length} etc). + * + * @param change new change to store + */ + private void storeChange(@NotNull TextChangeImpl change) { + if (myDeferredChangesStorage.size() >= MAX_DEFERRED_CHANGES_NUMBER) { + flushDeferredChanged(); + } + myDeferredChangesStorage.store(change); + myDeferredShift += change.getDiff(); + } + private void prepareForModification() { if (myOriginalSequence != null) { myArray = new char[myOriginalSequence.length()]; @@ -141,31 +189,57 @@ abstract class CharArray implements CharSequenceBackedByArray { if (myOriginalSequence != null) { str = myOriginalSequence.toString(); } - else { + else if (!hasDeferredChanges()) { str = new String(myArray, 0, myCount); } + else { + StringBuilder buffer = new StringBuilder(); + int start = 0; + int count = myCount + myDeferredShift; + for (TextChange change : myDeferredChangesStorage.getChanges()) { + final int length = change.getStart() - start; + if (length > 0) { + buffer.append(myArray, start, length); + count -= length; + } + if (change.getText().length() > 0) { + buffer.append(change.getText()); + count -= change.getText().length(); + } + start = change.getEnd(); + } + buffer.append(myArray, start, count); + str = buffer.toString(); + } myStringRef = new SoftReference(str); } return str; } public final int length() { - return myCount; + return myCount + myDeferredShift; } public final char charAt(int i) { - if (i < 0 || i >= myCount) { - throw new IndexOutOfBoundsException("Wrong offset: " + i+"; count:"+myCount); + if (i < 0 || i >= length()) { + throw new IndexOutOfBoundsException("Wrong offset: " + i + "; count:" + length()); } if (myOriginalSequence != null) return myOriginalSequence.charAt(i); - return myArray[i]; + if (hasDeferredChanges()) { + return myDeferredChangesStorage.charAt(myArray, i); + } + else { + return myArray[i]; + } } public CharSequence subSequence(int start, int end) { - if (start == 0 && end == myCount) return this; + //TODO den avoid flushing changes here + if (start == 0 && end == length()) return this; if (myOriginalSequence != null) { return myOriginalSequence.subSequence(start, end); } + flushDeferredChanged(); return new CharArrayCharSequence(myArray, start, end); } @@ -175,10 +249,12 @@ abstract class CharArray implements CharSequenceBackedByArray { myArray = CharArrayUtil.fromSequence(myOriginalSequence); } } + flushDeferredChanged(); return myArray; } public void getChars(final char[] dst, final int dstOffset) { + flushDeferredChanged(); if (myOriginalSequence != null) { CharArrayUtil.getChars(myOriginalSequence,dst, dstOffset); } @@ -192,7 +268,7 @@ abstract class CharArray implements CharSequenceBackedByArray { if (myOriginalSequence != null) { return myOriginalSequence.subSequence(start, end); } - return new String(myArray, start, end - start); + return myDeferredChangesStorage.substring(myArray, start, end); } private static char[] relocateArray(char[] array, int index) { @@ -213,9 +289,82 @@ abstract class CharArray implements CharSequenceBackedByArray { } private void trimToSize(DocumentImpl subj) { - if (myBufferSize != 0 && myCount > myBufferSize) { + if (myBufferSize != 0 && length() > myBufferSize) { + flushDeferredChanged(); // make a copy remove(subj,0, myCount - myBufferSize, getCharArray().subSequence(0, myCount - myBufferSize).toString()); } } + + /** + * @return true if this object is at {@link #setDeferredChangeMode(boolean) defer changes} mode; + * false otherwise + */ + public boolean isDeferredChangeMode() { + return !DISABLE_DEFERRED_PROCESSING && myDeferredChangeMode; + } + + public boolean hasDeferredChanges() { + return !myDeferredChangesStorage.isEmpty(); + } + + /** + * There is a possible case that client of this class wants to perform great number of modifications in a short amount of time + * (e.g. end-user performs formatting of the document backed by the object of the current class). It may result in significant + * performance degradation is the changes are performed one by one (every time the change is applied tail content is shifted to + * the left or right). So, we may want to optimize that by avoiding actual array modification until information about + * all target changes is provided and perform array data moves only after that. + *

+ * This method allows to define that 'defer changes' mode usages, i.e. expected usage pattern is as follows: + * 

+   * 
    
+   *   
  1. 
+   *     Client of this class enters 'defer changes' mode (calls this method with 'true' argument).
+   *     That means that all subsequent changes will not actually modify backed array data and will be stored separately;
+   *   
    2. 
+   *   
  2. 
+   *     Number of target changes are applied to the current object via standard API
+   *     ({@link #insert(DocumentImpl, CharSequence, int) insert},
+   *     {@link #remove(DocumentImpl, int, int, CharSequence) remove} and
+   *     {@link #replace(DocumentImpl, int, int, CharSequence, CharSequence, long, boolean) replace});
+   *   
    3. 
+   *   
  3. 
+   *     Client of this class indicates that 'massive change time' is over by calling this method with 'false'
+   *     argument. That flushes all deferred changes (if any) to the backed data array and makes every subsequent change to
+   *     be immediate flushed to the backed array;
+   *   
    4. 
+   * 

+   *
+ *

+ * Note: we can't exclude possibility that 'defer changes' mode is started but inadvertently not ended + * (due to programming error, unexpected exception etc). Hence, this class is free to automatically end + * 'defer changes' mode when necessary in order to avoid memory leak with infinite deferred changes storing. + * + * @param deferredChangeMode flag that defines if 'defer changes' mode should be used by the current object + */ + public void setDeferredChangeMode(boolean deferredChangeMode) { + myDeferredChangeMode = deferredChangeMode; + if (!deferredChangeMode) { + flushDeferredChanged(); + } + } + + private void flushDeferredChanged() { + List changes = myDeferredChangesStorage.getChanges(); + if (changes.isEmpty()) { + return; + } + + BulkChangesMerger changesMerger = BulkChangesMerger.INSTANCE; + if (myArray.length < length()) { + myArray = changesMerger.mergeToCharArray(myArray, myCount, changes); + } + else { + changesMerger.mergeInPlace(myArray, myCount, changes); + } + + myCount += myDeferredShift; + myDeferredShift = 0; + myDeferredChangesStorage.clear(); + } } diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java index 838d0b5770f9..2f33d1526d83 100644 --- a/platform/platform-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java @@ -719,12 +719,12 @@ public class DocumentImpl extends UserDataHolderBase implements DocumentEx { } public final void setInBulkUpdate(boolean value) { + myDoingBulkUpdate = value; + myText.setDeferredChangeMode(value); if (value) { - myDoingBulkUpdate = true; getPublisher().updateStarted(this); } else { - myDoingBulkUpdate = false; getPublisher().updateFinished(this); normalizeRangeMarkers(); } diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/softwrap/TextChangeImpl.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/TextChangeImpl.java similarity index 99% rename from platform/platform-impl/src/com/intellij/openapi/editor/impl/softwrap/TextChangeImpl.java rename to platform/platform-impl/src/com/intellij/openapi/editor/impl/TextChangeImpl.java index edac41e22bf8..a0c998895949 100644 --- a/platform/platform-impl/src/com/intellij/openapi/editor/impl/softwrap/TextChangeImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/TextChangeImpl.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.intellij.openapi.editor.impl.softwrap; +package com.intellij.openapi.editor.impl; import com.intellij.openapi.editor.TextChange; import com.intellij.openapi.util.text.StringUtil; diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/TextChangesStorage.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/TextChangesStorage.java new file mode 100644 index 000000000000..9fb1939118d8 --- /dev/null +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/TextChangesStorage.java @@ -0,0 +1,469 @@ +/* + * 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 com.intellij.openapi.editor.impl; + +import com.intellij.openapi.editor.TextChange; +import com.intellij.util.text.CharArrayUtil; +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/** + * Allows to store and retrieve {@link TextChange} objects assuming that they are applied to the same text. + *

+ * Provides ability to automatic merging them if necessary. + *

+ * Not thread-safe. + * + * @author Denis Zhdanov + * @since 3/2/11 11:55 AM + */ +public class TextChangesStorage { + + private final List myChanges = new ArrayList(); + + /** + * @return list of changes stored previously via {@link #store(TextChange)}. Note that the changes offsets relate to initial + * text and that returned list is sorted by start offset in ascending order + * @see #store(TextChange) + */ + @NotNull + public List getChanges() { + List result = new ArrayList(); + for (ChangeEntry changeEntry : myChanges) { + result.add(changeEntry.change); + } + return result; + } + + /** + * Allows to ask the storage for the list of changes that have intersections with the target text range (identified by the given + * arguments). + * + * @param start target range start offset (inclusive) + * @param end target range end offset (exclusive) + * @return list that contains all registered changes that have intersections with the target text range + */ + @NotNull + public List getChanges(int start, int end) { + assert start <= end; + + int changeStartIndex = getChangeIndex(start); + if (changeStartIndex < 0) { + changeStartIndex = -changeStartIndex - 1; + } + if (changeStartIndex >= myChanges.size()) { + return Collections.emptyList(); + } + + int changeEndIndex = getChangeIndex(end); + boolean endInclusive = true; + if (changeEndIndex < 0) { + changeEndIndex = -changeEndIndex - 1; + endInclusive = false; + } + + List result = null; + for (int i = changeStartIndex; i <= changeEndIndex; i++) { + if (!endInclusive && i == changeEndIndex) { + break; + } + if (result == null) { + result = new ArrayList(); + } + result.add(myChanges.get(i).change); + } + return result == null ? Collections.emptyList() : result; + } + + public boolean isEmpty() { + return myChanges.isEmpty(); + } + + public void clear() { + myChanges.clear(); + } + + public int size() { + return myChanges.size(); + } + + /** + * Store given change merging it with previously stored ones if necessary. + *

+ * Note: it's assumed that given change offsets are related to the current state of the text ('client text'), + * i.e. with all stored changes applied to it. Example: + *

    + *
  1. Say, we have initial text '12345';
    2. + *
  2. + * Suppose the change 'replace text at [2; 3) range with 'ABC'' is applied to it (stored at the current object). + * End-users see the text '12ABC45' now; + *
    3. + *
  3. + * This method is called with change like 'replace text at [1; 6) range with 'XY''. Change range is assumed to + * be related to the text visible to end-user, not initial one ('12ABC45', not '12345'). + * I.e. the user will see text '1XY5' now; + *
    4. + *
+ * + * @param change change to store + */ + public void store(@NotNull TextChange change) { + if (myChanges.isEmpty()) { + myChanges.add(new ChangeEntry(new TextChangeImpl(change.getText(), change.getStart(), change.getEnd()), change.getStart())); + return; + } + + // There is a big chance that the document is processed sequentially from start to end, hence, it makes sense + // to check if given change lays beyond other registered changes and register it quickly in case of success. + ChangeEntry last = myChanges.get(myChanges.size() - 1); + if (last.clientStartOffset + last.change.getText().length() < change.getStart()) { + int clientShift = last.clientStartOffset - last.change.getStart() + last.change.getDiff(); + myChanges.add(new ChangeEntry( + new TextChangeImpl(change.getText(), change.getStart() - clientShift, change.getEnd() - clientShift), + change.getStart() + )); + return; + } + + int insertionIndex = doStore(change); + if (insertionIndex < 0) { + return; + } + mergeIfNecessary(insertionIndex); + } + + /** + * Stores given change at the current storage and returns its index at {@link #myChanges changes collection} (if any). + * + * @param change change to store + * @return non-negative value that indicates index under which given change is stored at the + * {@link #myChanges changes collection}; negative value if given change only modifies sub-range of + * already registered range + */ + @SuppressWarnings({"AssignmentToForLoopParameter"}) + private int doStore(@NotNull TextChange change) { + int insertionIndex = 0; + int newChangeStart = change.getStart(); + int newChangeEnd = change.getEnd(); + int storedChangeStart = getChangeIndex(change.getStart()); + int clientShift = 0; + + if (storedChangeStart < 0) { + storedChangeStart = -storedChangeStart - 1; + if (storedChangeStart >= myChanges.size()) { + if (storedChangeStart > 0 && storedChangeStart <= myChanges.size()) { + ChangeEntry changeEntry = myChanges.get(storedChangeStart - 1); + clientShift = changeEntry.clientStartOffset - changeEntry.change.getStart() + changeEntry.change.getDiff(); + } + } + + if (storedChangeStart >= myChanges.size()) { + myChanges.add(new ChangeEntry( + new TextChangeImpl(change.getText(), change.getStart() - clientShift, change.getEnd() - clientShift), + change.getStart() + )); + return storedChangeStart; + } + } + else { + ChangeEntry changeEntry = myChanges.get(storedChangeStart); + clientShift = changeEntry.clientStartOffset - changeEntry.change.getStart(); + } + + for (int i = storedChangeStart; i < myChanges.size(); i++) { + ChangeEntry changeEntry = myChanges.get(i); + int storedClientStart = changeEntry.change.getStart() + clientShift; + CharSequence storedText = changeEntry.change.getText(); + int storedClientEnd = storedClientStart + storedText.length(); + + // Stored change lays before the new one. + if (storedClientEnd <= newChangeStart) { + clientShift += changeEntry.change.getDiff(); + insertionIndex = i + 1; + continue; + } + + // We know that given change and stored change have intersections if control flow reaches this place. + + // Check if given change target sub-range of the stored one + if (storedClientStart <= newChangeStart && storedClientEnd >= newChangeEnd) { + StringBuilder adjustedText = new StringBuilder(); + if (storedClientStart < newChangeStart) { + adjustedText.append(storedText.subSequence(0, newChangeStart - storedClientStart)); + } + adjustedText.append(change.getText()); + if (storedClientEnd > newChangeEnd) { + adjustedText.append(storedText.subSequence(newChangeEnd - storedClientStart, storedText.length())); + } + + if (adjustedText.length() == 0 && changeEntry.change.getStart() == changeEntry.change.getEnd()) { + myChanges.remove(i); + insertionIndex = -1; + break; + } + + TextChangeImpl adjusted = new TextChangeImpl(adjustedText, changeEntry.change.getStart(), changeEntry.change.getEnd()); + myChanges.set(i, new ChangeEntry(adjusted, adjusted.getStart())); + insertionIndex = -1; + break; + } + + // Check if given change completely contains stored change range. + if (newChangeStart <= storedClientStart && newChangeEnd >= storedClientEnd) { + myChanges.remove(i); + insertionIndex = i; + newChangeEnd -= changeEntry.change.getText().length(); + i--; + continue; + } + + // Check if given change intersects stored change range from the left. + if (newChangeStart <= storedClientStart && newChangeEnd < storedClientEnd) { + int numberOfStoredChangeSymbolsToRemove = newChangeEnd - storedClientStart; + CharSequence adjustedText = storedText.subSequence(numberOfStoredChangeSymbolsToRemove, storedText.length()); + changeEntry.change = new TextChangeImpl(adjustedText, changeEntry.change.getStart(), changeEntry.change.getEnd()); + newChangeEnd -= numberOfStoredChangeSymbolsToRemove; + insertionIndex = i; + continue; + } + + // Check if given change intersects stored change range from the right. + if (newChangeStart < storedClientEnd && newChangeEnd > storedClientEnd) { + CharSequence adjustedText = storedText.subSequence(0, newChangeStart - storedClientStart); + TextChangeImpl adjusted = new TextChangeImpl(adjustedText, changeEntry.change.getStart(), changeEntry.change.getEnd()); + myChanges.set(i, new ChangeEntry(adjusted, adjusted.getStart())); + clientShift += adjusted.getDiff(); + newChangeEnd -= storedClientEnd - newChangeStart; + insertionIndex = i + 1; + } + } + + if (insertionIndex >= 0) { + myChanges.add(insertionIndex, new ChangeEntry( + new TextChangeImpl(change.getText(), newChangeStart - clientShift, newChangeEnd - clientShift), + change.getStart() + )); + } + + return insertionIndex; + } + + /** + * Merges if necessary change stored at {@link #myChanges changes collection} at the given index with adjacent changes. + * + * @param insertionIndex index of the change that can potentially be merged with adjacent changes + */ + private void mergeIfNecessary(int insertionIndex) { + // Merge with previous if necessary. + ChangeEntry toMerge = myChanges.get(insertionIndex); + if (insertionIndex > 0) { + ChangeEntry left = myChanges.get(insertionIndex - 1); + if (left.change.getEnd() == toMerge.change.getStart()) { + String text = left.change.getText().toString() + toMerge.change.getText(); + left.change = new TextChangeImpl(text, left.change.getStart(), toMerge.change.getEnd()); + myChanges.remove(insertionIndex); + insertionIndex--; + } + } + + // Merge with next if necessary. + toMerge = myChanges.get(insertionIndex); + if (insertionIndex < myChanges.size() - 1) { + ChangeEntry right = myChanges.get(insertionIndex + 1); + if (toMerge.change.getEnd() == right.change.getStart()) { + String text = toMerge.change.getText().toString() + right.change.getText(); + toMerge.change = new TextChangeImpl(text, toMerge.change.getStart(), right.change.getEnd()); + myChanges.remove(insertionIndex + 1); + } + } + } + + /** + * Allows to retrieve character for the given index assuming that it should be resolved against 'client text', i.e. the text contained + * at the given original char sequence with all {@link #myChanges registered changes} applied to it. + *

+ * Example: + * 

+   * 
    
+   *   
  • Consider that original text is '01234';
    • 
+   *   
  • 
+   *     Consider that two changes are registered: 'insert text 'a' at index 1' and
+   *     'insert text 'bc' at index 3';
+   *   
    • 
+   *   
  • 'client text' now is '0a12bc34';
    • 
+   *   
  • This method is called with index '5' - symbol 'c' is returned;
    • 
+   * 

+   *
+ * + * @param originalData original text to which {@link #myChanges registered changes} are applied + * @param index target symbol index (is assumed to be 'client text' index) + * @return 'client text' symbol at the given index + */ + public char charAt(@NotNull char[] originalData, int index) { + int changeIndex = getChangeIndex(index); + if (changeIndex >= 0) { + // Target char is contained at the stored change text + ChangeEntry changeEntry = myChanges.get(changeIndex); + if (changeEntry.change.getText().length() > index - changeEntry.clientStartOffset) { + return changeEntry.change.getText().charAt(index - changeEntry.clientStartOffset); + } + else { + int originalArrayIndex = index - (changeEntry.clientStartOffset - changeEntry.change.getStart() + changeEntry.change.getDiff()); + return originalData[originalArrayIndex]; + } + } + else { + int clientShift = 0; + changeIndex = -changeIndex - 1; + if (changeIndex > 0 && changeIndex <= myChanges.size()) { + ChangeEntry changeEntry = myChanges.get(changeIndex - 1); + clientShift = changeEntry.clientStartOffset - changeEntry.change.getStart() + changeEntry.change.getDiff(); + } + return originalData[index - clientShift]; + } + } + + /** + * Allows to build substring of the client text with its changes registered within the current storage. + * + * @param originalData original text to which {@link #myChanges registered changes} are applied + * @param start target substring start offset (against the 'client text'; inclusive) + * @param end target substring end offset (against the 'client text'; exclusive) + * @return substring for the given text range + */ + public CharSequence substring(@NotNull char[] originalData, int start, int end) { + if (myChanges.isEmpty()) { + return new String(originalData, start, end - start); + } + if (end == start) { + return ""; + } + + int startChangeIndex = getChangeIndex(start); + int endChangeIndex = getChangeIndex(end); + + boolean substringAffectedByChanges = startChangeIndex < 0 && endChangeIndex < 0 && startChangeIndex == endChangeIndex; + int clientShift = 0; + int originalStart = 0; + if (startChangeIndex < 0) { + startChangeIndex = -startChangeIndex - 1; + if (startChangeIndex > 0 && startChangeIndex <= myChanges.size()) { + ChangeEntry changeEntry = myChanges.get(startChangeIndex - 1); + clientShift = changeEntry.clientStartOffset - changeEntry.change.getStart() + changeEntry.change.getDiff(); + originalStart = changeEntry.change.getEnd(); + } + } + else { + ChangeEntry changeEntry = myChanges.get(startChangeIndex); + clientShift = changeEntry.clientStartOffset - changeEntry.change.getStart(); + } + + if (substringAffectedByChanges) { + return new String(originalData, start - clientShift, end - start); + } + + char[] data = new char[end - start]; + int outputOffset = 0; + for (int i = startChangeIndex; i < myChanges.size() && outputOffset < data.length; i++) { + ChangeEntry changeEntry = myChanges.get(i); + int clientStart = changeEntry.clientStartOffset; + if (clientStart >= end) { + if (i == startChangeIndex) { + return new String(originalData, start - clientShift, end - start); + } + System.arraycopy(originalData, originalStart, data, outputOffset, data.length - outputOffset); + break; + } + int clientEnd = clientStart + changeEntry.change.getText().length(); + if (clientEnd > start) { + if (clientStart > start) { + int length = Math.min(clientStart - start, changeEntry.change.getStart() - originalStart); + length = Math.min(length, data.length - outputOffset); + System.arraycopy(originalData, changeEntry.change.getStart() - length, data, outputOffset, length); + outputOffset += length; + if (outputOffset >= data.length) { + break; + } + } + if (end >= clientStart && clientStart < clientEnd) { + int changeTextStartOffset = start <= clientStart ? 0 : start - clientStart; + int length = Math.min(clientEnd, end) - Math.max(clientStart, start); + CharArrayUtil.getChars(changeEntry.change.getText(), data, changeTextStartOffset, outputOffset, length); + outputOffset += length; + } + } + originalStart = changeEntry.change.getEnd(); + } + return new String(data); + } + + /** + * Allows to find index of the change that contains given offset (assuming that it is used against 'client text') + * or index of the first change that lays after the given offset. + * + * @param clientOffset target offset against the 'client text' + * @return non-negative value that defines index of the stored change that contains given client offset; + * negative value that indicates index of the first change that lays beyond the given offset and + * is calculated by by '-returned_index - 1' formula + */ + private int getChangeIndex(int clientOffset) { + if (myChanges.isEmpty()) { + return -1; + } + + int start = 0; + int end = myChanges.size() - 1; + + // We inline binary search here because profiling indicates that it becomes bottleneck to use Collections.binarySearch(). + while (start <= end) { + int i = (end + start) >>> 1; + ChangeEntry changeEntry = myChanges.get(i); + if (changeEntry.clientStartOffset > clientOffset) { + end = i - 1; + continue; + } + if (changeEntry.clientStartOffset + changeEntry.change.getText().length() < clientOffset) { + start = i + 1; + continue; + } + return i; + } + + return -(start + 1); + } + + /** + * Utility class that contains target {@link TextChangeImpl document change} and auxiliary information associated with it. + */ + private static class ChangeEntry { + + /** Target change. */ + public TextChangeImpl change; + + /** + * Offset of the target change start at the 'client text'. + */ + public int clientStartOffset; + + ChangeEntry(TextChangeImpl change, int clientStartOffset) { + this.change = change; + this.clientStartOffset = clientStartOffset; + } + } +} diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/softwrap/SoftWrapImpl.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/softwrap/SoftWrapImpl.java index c422fd9c0303..d420544bde71 100644 --- a/platform/platform-impl/src/com/intellij/openapi/editor/impl/softwrap/SoftWrapImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/softwrap/SoftWrapImpl.java @@ -16,6 +16,7 @@ package com.intellij.openapi.editor.impl.softwrap; import com.intellij.openapi.editor.SoftWrap; +import com.intellij.openapi.editor.impl.TextChangeImpl; import org.jetbrains.annotations.NotNull; /** diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/softwrap/mapping/SoftWrapApplianceManager.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/softwrap/mapping/SoftWrapApplianceManager.java index 679cb221a65e..168866225cb3 100644 --- a/platform/platform-impl/src/com/intellij/openapi/editor/impl/softwrap/mapping/SoftWrapApplianceManager.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/softwrap/mapping/SoftWrapApplianceManager.java @@ -25,6 +25,7 @@ import com.intellij.openapi.editor.ex.util.EditorUtil; import com.intellij.openapi.editor.impl.EditorTextRepresentationHelper; import com.intellij.openapi.editor.impl.FontInfo; import com.intellij.openapi.editor.impl.IterationState; +import com.intellij.openapi.editor.impl.TextChangeImpl; import com.intellij.openapi.editor.impl.softwrap.*; import com.intellij.openapi.editor.markup.TextAttributes; import com.intellij.openapi.util.text.StringUtil; diff --git a/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/CharArrayTest.java b/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/CharArrayTest.java new file mode 100644 index 000000000000..a440da57b58a --- /dev/null +++ b/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/CharArrayTest.java @@ -0,0 +1,167 @@ +/* + * 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 com.intellij.openapi.editor.impl; + +import static org.junit.Assert.*; + +import com.intellij.openapi.editor.event.DocumentEvent; +import com.intellij.openapi.editor.impl.event.DocumentEventImpl; +import com.intellij.util.LocalTimeCounter; +import org.jetbrains.annotations.NotNull; +import org.jmock.Expectations; +import org.jmock.api.Invocation; +import org.jmock.lib.action.CustomAction; +import org.junit.Rule; +import org.junit.Test; +import org.junit.Before; +import org.junit.After; +import org.junit.rules.TestWatchman; +import org.jmock.integration.junit4.JUnit4Mockery; +import org.jmock.Mockery; +import org.jmock.lib.legacy.ClassImposteriser; +import org.junit.runners.model.FrameworkMethod; + +import java.lang.annotation.*; + +/** + * @author Denis Zhdanov + * @since 03/01/2011 + */ +public class CharArrayTest { + + @Rule + public TestWatchman configReader = new TestWatchman() { + @Override + public void starting(FrameworkMethod method) { + Config config = method.getAnnotation(Config.class); + if (config != null) { + myConfig = config; + } + } + }; + + private CharArray myArray; + private Config myConfig; + private Mockery myMockery; + private DocumentImpl myDocument; + + @Before + public void setUp() { + myMockery = new JUnit4Mockery() {{ + setImposteriser(ClassImposteriser.INSTANCE); + }}; + myDocument = myMockery.mock(DocumentImpl.class); + + myMockery.checking(new Expectations() {{ + allowing(myDocument).getTextLength(); will(new CustomAction("getTextLength") { + @Override + public Object invoke(Invocation invocation) throws Throwable { + return myArray.length(); + } + }); + }}); + + init(10); + if (myConfig != null) { + myArray.insert(myDocument, myConfig.text(), 0); + myArray.setDeferredChangeMode(myConfig.deferred()); + } + } + + @After + public void checkExpectations() { + myMockery.assertIsSatisfied(); + } + + @Config(text = "1234", deferred = true) + @Test + public void deferredReplace() { + replace(1, 3, "abc"); + checkText("1abc4"); + assertTrue(myArray.hasDeferredChanges()); + + replace(2, 3, "XY"); + checkText("1aXYc4"); + assertTrue(myArray.hasDeferredChanges()); + + replace(3, 6, "ABC"); + checkText("1aXABC"); + assertTrue(myArray.hasDeferredChanges()); + + myArray.setDeferredChangeMode(false); + checkText("1aXABC"); + assertFalse(myArray.hasDeferredChanges()); + } + + private void init(int size) { + myArray = new CharArray(size) { + @Override + protected DocumentEvent beforeChangedUpdate(DocumentImpl subj, int offset, CharSequence oldString, CharSequence newString, + boolean wholeTextReplaced) + { + return new DocumentEventImpl(subj, offset, oldString, newString, LocalTimeCounter.currentTime(), wholeTextReplaced); + } + + @Override + protected void afterChangedUpdate(DocumentEvent event, long newModificationStamp) { + } + }; + } + + private void checkText(@NotNull String expected) { + // Test as a whole. + assertEquals(expected, myArray.toString()); + assertEquals(expected.length(), myArray.length()); + + // Test 'charAt()'. + for (int i = 0; i < expected.length(); i++) { + if (expected.charAt(i) != myArray.charAt(i)) { + fail(String.format( + "Detected incorrect 'charAt()' processing for deferred changes. Text: '%1$s'. Expected to get symbol '%2$c' " + + "(numeric value %2$d) at index %3$d but actual symbol is '%4$c' (numeric value %4$d)", + expected, (int)expected.charAt(i), i, (int)myArray.charAt(i))); + } + assertEquals(expected.charAt(i), myArray.charAt(i)); + } + + // Test 'substring()'. + for (int start = 0; start < myArray.length() - 1; start++) { + for (int end = start; end < myArray.length(); end++) { + if (!expected.substring(start, end).equals(myArray.substring(start, end).toString())) { + fail(String.format( + "Detected incorrect 'substring()' processing for deferred changes. Text: '%s', expected to get substring '%s' for " + + "interval [%d; %d) but got '%s'", expected, expected.substring(start, end), start, end, myArray.substring(start, end) + )); + } + assertEquals(expected.substring(start, end), myArray.substring(start, end).toString()); + } + } + } + + private void replace(int startOffset, int endOffset, String newText) { + myArray.replace( + myDocument, startOffset, endOffset, myArray.substring(startOffset, endOffset), newText, LocalTimeCounter.currentTime(), + startOffset == 0 && endOffset == myArray.length() + ); + } + + @Target(ElementType.METHOD) + @Retention(RetentionPolicy.RUNTIME) + private @interface Config { + String text() default ""; + boolean deferred() default false; + } +} diff --git a/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/TextChangesStorageTest.java b/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/TextChangesStorageTest.java new file mode 100644 index 000000000000..ecf250f536da --- /dev/null +++ b/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/TextChangesStorageTest.java @@ -0,0 +1,277 @@ +/* + * 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 com.intellij.openapi.editor.impl; + +import org.jetbrains.annotations.NotNull; +import org.junit.Before; +import org.junit.Test; + +import java.util.Arrays; + +import static java.util.Arrays.asList; +import static org.junit.Assert.*; + +/** + * @author Denis Zhdanov + * @since 03/02/2011 + */ +public class TextChangesStorageTest { + + private TextChangesStorage myStorage; + + @Before + public void setUp() { + myStorage = new TextChangesStorage(); + } + + @Test + public void clear() { + assertTrue(myStorage.isEmpty()); + + insert("abc", 2); + assertFalse(myStorage.isEmpty()); + assertEquals(1, myStorage.getChanges().size()); + + myStorage.clear(); + assertTrue(myStorage.isEmpty()); + assertTrue(myStorage.getChanges().isEmpty()); + } + + @Test + public void singleInsert() { + insert("abc", 2); + checkChanges(c("abc", 2)); + } + + @Test + public void disconnectedInserts() { + insert("abc", 2); + insert("def", 6); + insert("ghi", 11); + checkChanges(c("abc", 2), c("def", 3), c("ghi", 5)); + } + + @Test + public void adjacentInserts() { + insert("abc", 2); + insert("def", 5); + insert("ghi", 8); + checkChanges(c("abcdefghi", 2)); + } + + @Test + public void nestedInserts() { + insert("abc", 2); + insert("XY", 3); + insert("1234", 4); + checkChanges(c("aX1234Ybc", 2)); + } + + @Test + public void singleDelete() { + delete(2, 3); + checkChanges(c("", 2, 3)); + } + + @Test + public void disconnectedDeletes() { + delete(2, 3); + delete(3, 4); + delete(5, 6); + checkChanges(c("", 2, 3), c("", 4, 5), c("", 7, 8)); + } + + @Test + public void adjacentDeletes() { + delete(2, 3); + delete(2, 3); + delete(2, 3); + checkChanges(c("", 2, 5)); + } + + @Test + public void singleReplace() { + replace("abc", 3, 4); + checkChanges(c("abc", 3, 4)); + } + + @Test + public void disconnectedReplaces() { + replace("abc", 3, 4); + replace("de", 7, 8); + replace("fghi", 10, 11); + checkChanges(c("abc", 3, 4), c("de", 5, 6), c("fghi", 7, 8)); + } + + @Test + public void adjacentReplaces() { + replace("abc", 3, 4); + replace("de", 6, 9); + replace("fghi", 8, 9); + checkChanges(c("abcdefghi", 3, 8)); + } + + @Test + public void intersectedReplaces() { + replace("abc", 3, 4); + replace("defg", 5, 6); + replace("hi", 8, 11); + checkChanges(c("abdefhi", 3, 6)); + } + + @Test + public void intersectedReplacesFromEndToStart() { + replace("abcd", 5, 6); + replace("ef", 4, 7); + replace("g", 1, 5); + checkChanges(c("gfcd", 1, 6)); + } + + @Test + public void nestedReplaces() { + replace("abcdef", 3, 5); + replace("gh", 4, 7); + replace("i", 5, 6); + checkChanges(c("agief", 3, 5)); + } + + @Test + public void exactMultipleReplace() { + replace("abc", 3, 4); + replace("cde", 3, 6); + replace("fg", 3, 6); + checkChanges(c("fg", 3, 4)); + } + + @Test + public void insertAndExactDelete() { + insert("abc", 3); + delete(3, 6); + checkChanges(); + } + + @Test + public void insertAndDeleteInTheMiddle() { + insert("abc", 3); + delete(4, 6); + checkChanges(c("a", 3)); + } + + @Test + public void insertAndWiderDelete() { + insert("abc", 3); + delete(2, 7); + checkChanges(c("", 2, 4)); + } + + @Test + public void insertAndDeleteFromLeft() { + insert("abc", 3); + delete(2, 5); + checkChanges(c("c", 2, 3)); + } + + @Test + public void insertAndDeleteFromRight() { + insert("abc", 3); + delete(4, 7); + checkChanges(c("a", 3, 4)); + } + + @Test + public void disconnectedInsertsAndExactLinkingDelete() { + insert("a", 1); + insert("bcd", 3); + insert("efg", 8); + delete(3, 11); + checkChanges(c("a", 1), c("", 2, 4)); + } + + @Test + public void disconnectedInsertsAndWiderLinkingDelete() { + insert("abc", 3); + insert("def", 8); + delete(2, 13); + checkChanges(c("", 2, 7)); + } + + @Test + public void disconnectedInsertsAndNarrowLinkingDelete() { + insert("abc", 3); + insert("def", 8); + delete(4, 9); + checkChanges(c("aef", 3, 5)); + } + + private void checkChanges(TextChangeImpl ... changes) { + assertEquals(asList(changes), myStorage.getChanges()); + assertEquals(changes.length > 0, !myStorage.isEmpty()); + if (changes.length <= 0) { + return; + } + int length = changes[changes.length - 1].getEnd(); + char[] input = new char[length]; + char c = 'A'; + for (int i = 0; i < input.length; i++) { + input[i] = c++; + } + char[] output = BulkChangesMerger.INSTANCE.mergeToCharArray(input, input.length, asList(changes)); + + // charAt(). + for (int i = 0; i < output.length; i++) { + if (output[i] != myStorage.charAt(input, i)) { + fail(String.format( + "Detected incorrect charAt() processing. Original text: '%s', changes: %s, index: %d, expected: %c, actual: %c", + new String(input), Arrays.asList(changes), i, output[i], myStorage.charAt(input, i) + )); + } + } + + // substring(). + for (int start = 0; start < output.length; start++) { + for( int end = start; end < output.length; end++) { + String expected = new String(output, start, end - start); + String actual = myStorage.substring(input, start, end).toString(); + if (!expected.equals(actual)) { + fail(String.format( + "Detected incorrect substring() processing. Original text: '%s', changes: %s, client text: '%s', range: %d-%d, " + + "expected: '%s', actual: '%s'", new String(input), Arrays.asList(changes), new String(output), start, end, expected, actual + )); + } + } + } + } + + private static TextChangeImpl c(@NotNull String text, int startOffset) { + return c(text, startOffset, startOffset); + } + + private static TextChangeImpl c(@NotNull String text, int startOffset, int endOffset) { + return new TextChangeImpl(text, startOffset, endOffset); + } + + private void insert(@NotNull String text, int offset) { + myStorage.store(c(text, offset)); + } + + private void delete(int start, int end) { + myStorage.store(c("", start, end)); + } + + private void replace(@NotNull String text, int start, int end) { + myStorage.store(c(text, start, end)); + } +} diff --git a/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/softwrap/TextChangeImplTest.java b/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/softwrap/TextChangeImplTest.java index ad118949ac3f..f34051e13e9b 100644 --- a/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/softwrap/TextChangeImplTest.java +++ b/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/softwrap/TextChangeImplTest.java @@ -16,6 +16,7 @@ package com.intellij.openapi.editor.impl.softwrap; import com.intellij.openapi.editor.TextChange; +import com.intellij.openapi.editor.impl.TextChangeImpl; import com.intellij.openapi.util.text.StringUtil; import org.junit.Test; diff --git a/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/softwrap/mapping/CachingSoftWrapDataMapperTest.java b/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/softwrap/mapping/CachingSoftWrapDataMapperTest.java index eb56c58fe7d2..aa663d182127 100644 --- a/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/softwrap/mapping/CachingSoftWrapDataMapperTest.java +++ b/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/softwrap/mapping/CachingSoftWrapDataMapperTest.java @@ -20,6 +20,7 @@ import com.intellij.openapi.editor.*; import com.intellij.openapi.editor.ex.EditorEx; import com.intellij.openapi.editor.ex.FoldingModelEx; import com.intellij.openapi.editor.ex.SoftWrapModelEx; +import com.intellij.openapi.editor.impl.TextChangeImpl; import com.intellij.openapi.editor.impl.softwrap.*; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.TextRange; diff --git a/platform/util/src/com/intellij/util/text/CharArrayUtil.java b/platform/util/src/com/intellij/util/text/CharArrayUtil.java index b88de667f4a0..79bc1c4c5e92 100644 --- a/platform/util/src/com/intellij/util/text/CharArrayUtil.java +++ b/platform/util/src/com/intellij/util/text/CharArrayUtil.java @@ -31,39 +31,68 @@ public class CharArrayUtil { private CharArrayUtil() { } + /** + * Copies all symbols from the given char sequence to the given array + * + * @param src source data holder + * @param dst output data buffer + * @param dstOffset start offset to use within the given output data buffer + */ public static void getChars(CharSequence src, char[] dst, int dstOffset) { getChars(src, dst, dstOffset, src.length()); } + /** + * Copies necessary number of symbols from the given char sequence start to the given array. + * + * @param src source data holder + * @param dst output data buffer + * @param dstOffset start offset to use within the given output data buffer + * @param len number of source data symbols to copy to the given buffer + */ public static void getChars(CharSequence src, char[] dst, int dstOffset, int len) { + getChars(src, dst, 0, dstOffset, len); + } + + /** + * Copies necessary number of symbols from the given char sequence to the given array. + * + * @param src source data holder + * @param dst output data buffer + * @param srcOffset source text offset + * @param dstOffset start offset to use within the given output data buffer + * @param len number of source data symbols to copy to the given buffer + */ + public static void getChars(CharSequence src, char[] dst, int srcOffset, int dstOffset, int len) { if (len >= GET_CHARS_THRESHOLD) { if (src instanceof String) { - ((String)src).getChars(0, len, dst, dstOffset); + ((String)src).getChars(srcOffset, len, dst, dstOffset); return; } else if (src instanceof CharBuffer) { final CharBuffer buffer = (CharBuffer)src; final int i = buffer.position(); + buffer.position(i + srcOffset); buffer.get(dst, dstOffset, len); buffer.position(i); return; } else if (src instanceof CharSequenceBackedByArray) { - ((CharSequenceBackedByArray)src.subSequence(0, len)).getChars(dst, dstOffset); + ((CharSequenceBackedByArray)src.subSequence(srcOffset, len)).getChars(dst, dstOffset); return; } else if (src instanceof StringBuffer) { - ((StringBuffer)src).getChars(0, len, dst, dstOffset); + ((StringBuffer)src).getChars(srcOffset, len, dst, dstOffset); return; } else if (src instanceof StringBuilder) { - ((StringBuilder)src).getChars(0, len, dst, dstOffset); + ((StringBuilder)src).getChars(srcOffset, len, dst, dstOffset); return; } } - for (int i = 0; i < len; i++) { - dst[i + dstOffset] = src.charAt(i); + for (int i = 0, j = srcOffset, max = srcOffset + len; j < max && i < dst.length; i++, j++) { + dst[i + dstOffset] = src.charAt(j); } } From 8cd5bfbc9b5788cefd87020738e0be884a78496b Mon Sep 17 00:00:00 2001 From: Denis Zhdanov Date: Fri, 11 Mar 2011 14:36:06 +0300 Subject: [PATCH 11/14] IDEA-66053 Formatter: Optimize performance of applying formatting changes 1. Corrected 'substring()' processing for deferred document changes; 2. Adjusted XmlPerformanceFormatterTest timing boundaries; --- .../openapi/editor/impl/CharArray.java | 99 +++++++++++++------ .../editor/impl/TextChangesStorage.java | 8 +- .../openapi/editor/impl/CharArrayTest.java | 38 ++++++- 3 files changed, 114 insertions(+), 31 deletions(-) diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/CharArray.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/CharArray.java index b01523e3e6db..dc8dcbe03320 100644 --- a/platform/platform-impl/src/com/intellij/openapi/editor/impl/CharArray.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/CharArray.java @@ -15,13 +15,14 @@ */ package com.intellij.openapi.editor.impl; -import com.intellij.openapi.editor.TextChange; import com.intellij.openapi.editor.event.DocumentEvent; +import com.intellij.openapi.editor.impl.event.DocumentEventImpl; import com.intellij.util.LocalTimeCounter; import com.intellij.util.text.CharArrayCharSequence; import com.intellij.util.text.CharArrayUtil; import com.intellij.util.text.CharSequenceBackedByArray; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.lang.ref.SoftReference; import java.util.List; @@ -41,7 +42,15 @@ abstract class CharArray implements CharSequenceBackedByArray { */ private static final int MAX_DEFERRED_CHANGES_NUMBER = 10000; - private final TextChangesStorage myDeferredChangesStorage = new TextChangesStorage(); + @NotNull + private TextChangesStorage myDeferredChangesStorage; + + private int myStart; + /** + * This class implements {@link #subSequence(int, int)} by creating object of the same class that partially shares the same + * data as the object on which the method is called. So, this field may define interested end offset (if it's non-negative). + */ + private int myEnd = -1; private int myCount = 0; private CharSequence myOriginalSequence; @@ -53,8 +62,23 @@ abstract class CharArray implements CharSequenceBackedByArray { // max chars to hold, bufferSize == 0 means unbounded CharArray(int bufferSize) { + this(bufferSize, new TextChangesStorage(), null, -1, -1); + } + + private CharArray(int bufferSize, @NotNull TextChangesStorage deferredChangesStorage, @Nullable char[] data, int start, int end) { myBufferSize = bufferSize; - myOriginalSequence = ""; + myDeferredChangesStorage = deferredChangesStorage; + if (data == null) { + myOriginalSequence = ""; + } + else { + myArray = data; + myCount = end - start; + } + if (start >= 0 && end >= 0) { + myStart = start; + myEnd = end; + } } public void setBufferSize(int bufferSize) { @@ -73,7 +97,14 @@ abstract class CharArray implements CharSequenceBackedByArray { myArray = null; myCount = chars.length(); myStringRef = null; - myDeferredChangesStorage.clear(); + if (isSubSequence()) { + myDeferredChangesStorage = new TextChangesStorage(); + myStart = 0; + myEnd = -1; + } + else { + myDeferredChangesStorage.clear(); + } trimToSize(subj); } @@ -81,6 +112,8 @@ abstract class CharArray implements CharSequenceBackedByArray { int startOffset, int endOffset, CharSequence toDelete, CharSequence newString, long newModificationStamp, boolean wholeTextReplaced) { final DocumentEvent event = beforeChangedUpdate(subj, startOffset, toDelete, newString, wholeTextReplaced); + startOffset += myStart; + endOffset += myStart; doReplace(startOffset, endOffset, newString); afterChangedUpdate(event, newModificationStamp); } @@ -108,6 +141,8 @@ abstract class CharArray implements CharSequenceBackedByArray { public void remove(DocumentImpl subj, int startIndex, int endIndex, CharSequence toDelete) { DocumentEvent event = beforeChangedUpdate(subj, startIndex, toDelete, null, false); + startIndex += myStart; + endIndex += myStart; doRemove(startIndex, endIndex); afterChangedUpdate(event, LocalTimeCounter.currentTime()); } @@ -131,6 +166,7 @@ abstract class CharArray implements CharSequenceBackedByArray { public void insert(DocumentImpl subj, CharSequence s, int startIndex) { DocumentEvent event = beforeChangedUpdate(subj, startIndex, null, s, false); + startIndex += myStart; doInsert(s, startIndex); afterChangedUpdate(event, LocalTimeCounter.currentTime()); @@ -190,26 +226,10 @@ abstract class CharArray implements CharSequenceBackedByArray { str = myOriginalSequence.toString(); } else if (!hasDeferredChanges()) { - str = new String(myArray, 0, myCount); + str = new String(myArray, myStart, myCount); } else { - StringBuilder buffer = new StringBuilder(); - int start = 0; - int count = myCount + myDeferredShift; - for (TextChange change : myDeferredChangesStorage.getChanges()) { - final int length = change.getStart() - start; - if (length > 0) { - buffer.append(myArray, start, length); - count -= length; - } - if (change.getText().length() > 0) { - buffer.append(change.getText()); - count -= change.getText().length(); - } - start = change.getEnd(); - } - buffer.append(myArray, start, count); - str = buffer.toString(); + str = substring(0, length()).toString(); } myStringRef = new SoftReference(str); } @@ -224,6 +244,7 @@ abstract class CharArray implements CharSequenceBackedByArray { if (i < 0 || i >= length()) { throw new IndexOutOfBoundsException("Wrong offset: " + i + "; count:" + length()); } + i += myStart; if (myOriginalSequence != null) return myOriginalSequence.charAt(i); if (hasDeferredChanges()) { return myDeferredChangesStorage.charAt(myArray, i); @@ -234,15 +255,37 @@ abstract class CharArray implements CharSequenceBackedByArray { } public CharSequence subSequence(int start, int end) { - //TODO den avoid flushing changes here if (start == 0 && end == length()) return this; if (myOriginalSequence != null) { return myOriginalSequence.subSequence(start, end); } - flushDeferredChanged(); - return new CharArrayCharSequence(myArray, start, end); + if (hasDeferredChanges()) { + return new CharArray(myBufferSize, myDeferredChangesStorage, myArray, myStart + start, myStart + end) { + @Override + protected DocumentEvent beforeChangedUpdate(DocumentImpl subj, + int offset, + CharSequence oldString, + CharSequence newString, + boolean wholeTextReplaced) { + return new DocumentEventImpl(subj, offset, oldString, newString, LocalTimeCounter.currentTime(), wholeTextReplaced); + } + + @Override + protected void afterChangedUpdate(DocumentEvent event, long newModificationStamp) { + } + }; + } + else { + // We don't use the same approach as with 'defer changes' mode because the former is the new experimental one and this one + // is rather mature, hence, we just minimizes the risks that something is wrong within the new approach. + return new CharArrayCharSequence(myArray, start, end); + } } + private boolean isSubSequence() { + return myEnd >= 0; + } + public char[] getChars() { if (myOriginalSequence != null) { if (myArray == null) { @@ -259,7 +302,7 @@ abstract class CharArray implements CharSequenceBackedByArray { CharArrayUtil.getChars(myOriginalSequence,dst, dstOffset); } else { - System.arraycopy(myArray, 0, dst, dstOffset, length()); + System.arraycopy(myArray, myStart, dst, dstOffset, length()); } } @@ -268,7 +311,7 @@ abstract class CharArray implements CharSequenceBackedByArray { if (myOriginalSequence != null) { return myOriginalSequence.subSequence(start, end); } - return myDeferredChangesStorage.substring(myArray, start, end); + return myDeferredChangesStorage.substring(myArray, start + myStart, end + myStart); } private static char[] relocateArray(char[] array, int index) { @@ -292,7 +335,7 @@ abstract class CharArray implements CharSequenceBackedByArray { if (myBufferSize != 0 && length() > myBufferSize) { flushDeferredChanged(); // make a copy - remove(subj,0, myCount - myBufferSize, getCharArray().subSequence(0, myCount - myBufferSize).toString()); + remove(subj, 0, myCount - myBufferSize, getCharArray().subSequence(0, myCount - myBufferSize).toString()); } } diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/TextChangesStorage.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/TextChangesStorage.java index 9fb1939118d8..9fb79784d6a9 100644 --- a/platform/platform-impl/src/com/intellij/openapi/editor/impl/TextChangesStorage.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/TextChangesStorage.java @@ -358,7 +358,7 @@ public class TextChangesStorage { int startChangeIndex = getChangeIndex(start); int endChangeIndex = getChangeIndex(end); - boolean substringAffectedByChanges = startChangeIndex < 0 && endChangeIndex < 0 && startChangeIndex == endChangeIndex; + boolean substringAffectedByChanges = startChangeIndex >= 0 || endChangeIndex >= 0 || startChangeIndex != endChangeIndex; int clientShift = 0; int originalStart = 0; if (startChangeIndex < 0) { @@ -374,7 +374,7 @@ public class TextChangesStorage { clientShift = changeEntry.clientStartOffset - changeEntry.change.getStart(); } - if (substringAffectedByChanges) { + if (!substringAffectedByChanges) { return new String(originalData, start - clientShift, end - start); } @@ -410,6 +410,10 @@ public class TextChangesStorage { } originalStart = changeEntry.change.getEnd(); } + + if (outputOffset < data.length) { + System.arraycopy(originalData, originalStart, data, outputOffset, data.length - outputOffset); + } return new String(data); } diff --git a/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/CharArrayTest.java b/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/CharArrayTest.java index a440da57b58a..84fe8a9eb3cc 100644 --- a/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/CharArrayTest.java +++ b/platform/platform-impl/testSrc/com/intellij/openapi/editor/impl/CharArrayTest.java @@ -19,7 +19,9 @@ import static org.junit.Assert.*; import com.intellij.openapi.editor.event.DocumentEvent; import com.intellij.openapi.editor.impl.event.DocumentEventImpl; +import com.intellij.openapi.util.Pair; import com.intellij.util.LocalTimeCounter; +import com.intellij.util.containers.Stack; import org.jetbrains.annotations.NotNull; import org.jmock.Expectations; import org.jmock.api.Invocation; @@ -146,7 +148,41 @@ public class CharArrayTest { + "interval [%d; %d) but got '%s'", expected, expected.substring(start, end), start, end, myArray.substring(start, end) )); } - assertEquals(expected.substring(start, end), myArray.substring(start, end).toString()); + } + } + + // Test subSequence(). + checkSubSequence(expected, myArray, new Stack>()); + } + + private void checkSubSequence(@NotNull String expected, @NotNull CharSequence actual, + @NotNull Stack> history) + { + assertEquals(expected.length(), actual.length()); + for (int i = 0; i < expected.length(); i++) { + char expectedChar = expected.charAt(i); + char actualChar = actual.charAt(i); + if (expectedChar != actualChar) { + fail(String.format( + "Detected incorrect charAt() processing for result of subSequence() with deferred changes. Original text: '%s', " + + "actual subSequence text: '%s', index: %d, expected symbol: '%c', actual symbol: '%c', subSequence history: %s", + myArray.toString(), expected, i, expectedChar, actualChar, history + )); + } + } + if (!expected.equals(actual.toString())) { + fail(String.format( + "Detected incorrect toString() processing for result of subSequence() with deferred changes. Original text: '%s', " + + "expected subSequence text: '%s', actual subSequence text: '%s', subSequence history: %s", + myArray.toString(), expected, actual.toString(), history + )); + } + assertEquals(expected, actual.toString()); + for (int start = 0; start < expected.length(); start++) { + for (int end = start; end < expected.length(); end++) { + history.push(new Pair(start, end)); + checkSubSequence(expected.substring(start, end), actual.subSequence(start, end), history); + history.pop(); } } } From 3952cdf81956890e29b7fb1a6d6471c169cc1fa5 Mon Sep 17 00:00:00 2001 From: Maxim Medvedev Date: Fri, 11 Mar 2011 15:37:16 +0300 Subject: [PATCH 12/14] IDEA-66471 Groovy: IDEA does't show Javadoc by Ctrl+Q for groovy fields. --- .../lang/groovydoc/psi/impl/GrDocCommentUtil.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/groovydoc/psi/impl/GrDocCommentUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/groovydoc/psi/impl/GrDocCommentUtil.java index d2c5fd1cc2cb..8eb359156192 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/groovydoc/psi/impl/GrDocCommentUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/groovydoc/psi/impl/GrDocCommentUtil.java @@ -23,6 +23,8 @@ import org.jetbrains.plugins.groovy.lang.groovydoc.psi.api.GrDocComment; import org.jetbrains.plugins.groovy.lang.groovydoc.psi.api.GrDocCommentOwner; import org.jetbrains.plugins.groovy.lang.groovydoc.psi.api.GroovyDocPsiElement; import org.jetbrains.plugins.groovy.lang.parser.GroovyElementTypes; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariableDeclaration; /** * @author Maxim.Medvedev @@ -51,7 +53,13 @@ public abstract class GrDocCommentUtil { @Nullable public static GrDocComment findDocComment(GrDocCommentOwner owner) { - PsiElement element = owner.getPrevSibling(); + PsiElement element; + if (owner instanceof GrVariable && owner.getParent() instanceof GrVariableDeclaration) { + element = owner.getParent().getPrevSibling(); + } + else { + element = owner.getPrevSibling(); + } while (true) { if (element == null) return null; final ASTNode node = element.getNode(); From e48dd327d9056299ae6e8b517722c44d326b8a88 Mon Sep 17 00:00:00 2001 From: nik Date: Fri, 11 Mar 2011 15:54:35 +0300 Subject: [PATCH 13/14] duplicated methods removed --- build/scripts/dist.gant | 6 ------ 1 file changed, 6 deletions(-) diff --git a/build/scripts/dist.gant b/build/scripts/dist.gant index b9b5f6408bb1..2795d8af5abe 100644 --- a/build/scripts/dist.gant +++ b/build/scripts/dist.gant @@ -29,12 +29,6 @@ setProperty("paths", new Paths(out)) def paths = new Paths(out) -def includeFile(String filepath) { - Script s = groovyShell.parse(new File(filepath)) - s.setBinding(binding) - s -} - target(compile: "Compile project") { loadProject() From c2065fc31626705867d82e93f01b18eedc18a378 Mon Sep 17 00:00:00 2001 From: peter Date: Fri, 11 Mar 2011 14:05:29 +0100 Subject: [PATCH 14/14] fix NPE while processing @Delegate --- .../plugins/groovy/lang/psi/util/GrClassImplUtil.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GrClassImplUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GrClassImplUtil.java index 74595e8790db..afbbc968ff33 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GrClassImplUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GrClassImplUtil.java @@ -547,7 +547,8 @@ public class GrClassImplUtil { builder.addModifier(PsiModifier.PUBLIC); final PsiParameter[] originalParameters = method.getParameterList().getParameters(); for (PsiParameter originalParameter : originalParameters) { - builder.addParameter(originalParameter.getName(), substitutor.substitute(originalParameter.getType())); + PsiType type = substitutor.substitute(originalParameter.getType()); + builder.addParameter(originalParameter.getName(), type == null ? PsiType.getJavaLangObject(clazz.getManager(), clazz.getResolveScope()) : type); } builder.setBaseIcon(GroovyIcons.METHOD); return builder;