From ec93bde17026cfa94a04758a1ed809d28ec830f6 Mon Sep 17 00:00:00 2001 From: nik Date: Thu, 11 Oct 2018 11:35:45 +0300 Subject: [PATCH] refactor slicer: avoid accessing package-private members of platform modules from java modules (IDEA-200277) Add SliceRootNode::setChildren metod to avoid external writings to myCachedChildren field. --- .../slicer/SliceNullnessAnalyzerBase.java | 36 ++++++++++--------- .../intellij/slicer/SliceLeafAnalyzer.java | 14 ++++---- .../slicer/SliceLeafValueRootNode.java | 8 ++--- .../src/com/intellij/slicer/SliceNode.java | 2 +- .../com/intellij/slicer/SliceRootNode.java | 10 ++++-- .../com/intellij/slicer/SliceTreeBuilder.java | 2 +- 6 files changed, 41 insertions(+), 31 deletions(-) diff --git a/java/java-impl/src/com/intellij/slicer/SliceNullnessAnalyzerBase.java b/java/java-impl/src/com/intellij/slicer/SliceNullnessAnalyzerBase.java index 1ab5c75b5f5d..8b2b4a7a3458 100644 --- a/java/java-impl/src/com/intellij/slicer/SliceNullnessAnalyzerBase.java +++ b/java/java-impl/src/com/intellij/slicer/SliceNullnessAnalyzerBase.java @@ -34,6 +34,8 @@ import org.jetbrains.annotations.NotNull; import java.util.*; +import static com.intellij.util.containers.ContainerUtil.addIfNotNull; + public abstract class SliceNullnessAnalyzerBase { @NotNull private final SliceLeafEquality myLeafEquality; @@ -50,39 +52,38 @@ public abstract class SliceNullnessAnalyzerBase { private void groupByNullness(NullAnalysisResult result, SliceRootNode oldRoot, final Map map) { SliceRootNode root = createNewTree(result, oldRoot, map); - SliceUsage rootUsage = oldRoot.myCachedChildren.get(0).getValue(); + SliceUsage rootUsage = oldRoot.getCachedChildren().get(0).getValue(); SliceManager.getInstance(root.getProject()).createToolWindow(true, root, true, SliceManager.getElementDescription(null, rootUsage.getElement(), " Grouped by Nullness") ); } @NotNull public SliceRootNode createNewTree(NullAnalysisResult result, SliceRootNode oldRoot, final Map map) { SliceRootNode root = oldRoot.copy(); - assert oldRoot.myCachedChildren.size() == 1; - SliceNode oldRootStart = oldRoot.myCachedChildren.get(0); + assert oldRoot.getCachedChildren().size() == 1; + SliceNode oldRootStart = oldRoot.getCachedChildren().get(0); root.setChanged(); root.targetEqualUsages.clear(); - root.myCachedChildren = new ArrayList<>(); - - createValueRootNode(result, oldRoot, map, root, oldRootStart, "Null Values", NullAnalysisResult.NULLS); - createValueRootNode(result, oldRoot, map, root, oldRootStart, "NotNull Values", NullAnalysisResult.NOT_NULLS); - createValueRootNode(result, oldRoot, map, root, oldRootStart, "Other Values", NullAnalysisResult.UNKNOWNS); + List children = new ArrayList<>(); + addIfNotNull(children, createValueRootNode(result, oldRoot, map, root, oldRootStart, "Null Values", NullAnalysisResult.NULLS)); + addIfNotNull(children, createValueRootNode(result, oldRoot, map, root, oldRootStart, "NotNull Values", NullAnalysisResult.NOT_NULLS)); + addIfNotNull(children, createValueRootNode(result, oldRoot, map, root, oldRootStart, "Other Values", NullAnalysisResult.UNKNOWNS)); + root.setChildren(children); return root; } - private void createValueRootNode(NullAnalysisResult result, - SliceRootNode oldRoot, - final Map map, - SliceRootNode root, - SliceNode oldRootStart, - String nodeName, - final int group) { + private SliceLeafValueClassNode createValueRootNode(NullAnalysisResult result, + SliceRootNode oldRoot, + final Map map, + SliceRootNode root, + SliceNode oldRootStart, + String nodeName, + final int group) { Collection groupedByValue = result.groupedByValue[group]; if (groupedByValue.isEmpty()) { - return; + return null; } SliceLeafValueClassNode valueRoot = new SliceLeafValueClassNode(root.getProject(), root, nodeName); - root.myCachedChildren.add(valueRoot); Set uniqueValues = new THashSet<>(groupedByValue, myLeafEquality); for (final PsiElement expression : uniqueValues) { @@ -110,6 +111,7 @@ public abstract class SliceNullnessAnalyzerBase { Collections.singletonList(newRoot)) ); } + return valueRoot; } public void startAnalyzeNullness(@NotNull AbstractTreeStructure treeStructure, @NotNull Runnable finish) { diff --git a/platform/lang-impl/src/com/intellij/slicer/SliceLeafAnalyzer.java b/platform/lang-impl/src/com/intellij/slicer/SliceLeafAnalyzer.java index 171a296dcbbe..537467a87d88 100644 --- a/platform/lang-impl/src/com/intellij/slicer/SliceLeafAnalyzer.java +++ b/platform/lang-impl/src/com/intellij/slicer/SliceLeafAnalyzer.java @@ -54,9 +54,9 @@ public class SliceLeafAnalyzer { myProvider = provider; } - static SliceNode filterTree(SliceNode oldRoot, - NullableFunction filter, - PairProcessor> postProcessor) { + public static SliceNode filterTree(SliceNode oldRoot, + NullableFunction filter, + PairProcessor> postProcessor) { SliceNode filtered = filter.fun(oldRoot); if (filtered == null) return null; @@ -95,7 +95,7 @@ public class SliceLeafAnalyzer { SliceRootNode root = oldRoot.copy(); root.setChanged(); root.targetEqualUsages.clear(); - root.myCachedChildren = new ArrayList<>(leaves.size()); + List leafValueRoots = new ArrayList<>(leaves.size()); for (final PsiElement leafExpression : leaves) { SliceNode newNode = filterTree(oldRootStart, oldNode -> { @@ -114,8 +114,10 @@ public class SliceLeafAnalyzer { root, myProvider.createRootUsage(leafExpression, oldRoot.getValue().params), Collections.singletonList(newNode)); - root.myCachedChildren.add(lvNode); + leafValueRoots.add(lvNode); } + root.setChildren(leafValueRoots); + return root; } @@ -162,7 +164,7 @@ public class SliceLeafAnalyzer { () -> ConcurrentCollectionFactory.createMap(ContainerUtil.identityStrategy())); } - static class SliceNodeGuide implements WalkingState.TreeGuide { + public static class SliceNodeGuide implements WalkingState.TreeGuide { private final AbstractTreeStructure myTreeStructure; // use tree structure because it's setting 'parent' fields in the process diff --git a/platform/lang-impl/src/com/intellij/slicer/SliceLeafValueRootNode.java b/platform/lang-impl/src/com/intellij/slicer/SliceLeafValueRootNode.java index 01972a265fe0..3057251c6577 100644 --- a/platform/lang-impl/src/com/intellij/slicer/SliceLeafValueRootNode.java +++ b/platform/lang-impl/src/com/intellij/slicer/SliceLeafValueRootNode.java @@ -23,10 +23,10 @@ import java.util.List; public class SliceLeafValueRootNode extends SliceNode implements MyColoredTreeCellRenderer { public final List myCachedChildren; - SliceLeafValueRootNode(@NotNull Project project, - @NotNull SliceNode root, - @NotNull SliceUsage sliceUsage, - @NotNull List children) { + public SliceLeafValueRootNode(@NotNull Project project, + @NotNull SliceNode root, + @NotNull SliceUsage sliceUsage, + @NotNull List children) { super(project, sliceUsage, root.targetEqualUsages); myCachedChildren = children; } diff --git a/platform/lang-impl/src/com/intellij/slicer/SliceNode.java b/platform/lang-impl/src/com/intellij/slicer/SliceNode.java index cf10b0c4eed0..d5ca5fbbe522 100644 --- a/platform/lang-impl/src/com/intellij/slicer/SliceNode.java +++ b/platform/lang-impl/src/com/intellij/slicer/SliceNode.java @@ -56,7 +56,7 @@ public class SliceNode extends AbstractTreeNode implements Duplicate } @NotNull - SliceNode copy() { + public SliceNode copy() { SliceUsage newUsage = getValue().copy(); SliceNode newNode = new SliceNode(getProject(), newUsage, targetEqualUsages); newNode.dupNodeCalculated = dupNodeCalculated; diff --git a/platform/lang-impl/src/com/intellij/slicer/SliceRootNode.java b/platform/lang-impl/src/com/intellij/slicer/SliceRootNode.java index 6db09a1639b1..274dbef8e88e 100644 --- a/platform/lang-impl/src/com/intellij/slicer/SliceRootNode.java +++ b/platform/lang-impl/src/com/intellij/slicer/SliceRootNode.java @@ -19,8 +19,10 @@ import com.intellij.openapi.project.Project; import org.jetbrains.annotations.NotNull; import javax.swing.*; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.List; /** * @author cdr @@ -43,7 +45,7 @@ public class SliceRootNode extends SliceNode { @NotNull @Override - SliceRootNode copy() { + public SliceRootNode copy() { SliceUsage newUsage = getValue().copy(); SliceRootNode newNode = new SliceRootNode(getProject(), new DuplicateMap(), newUsage); newNode.dupNodeCalculated = dupNodeCalculated; @@ -73,7 +75,11 @@ public class SliceRootNode extends SliceNode { } @NotNull - SliceUsage getRootUsage() { + public SliceUsage getRootUsage() { return myRootUsage; } + + public void setChildren(List children) { + myCachedChildren = new ArrayList<>(children); + } } diff --git a/platform/lang-impl/src/com/intellij/slicer/SliceTreeBuilder.java b/platform/lang-impl/src/com/intellij/slicer/SliceTreeBuilder.java index ac1d5a06c153..54588dfe26f1 100644 --- a/platform/lang-impl/src/com/intellij/slicer/SliceTreeBuilder.java +++ b/platform/lang-impl/src/com/intellij/slicer/SliceTreeBuilder.java @@ -94,7 +94,7 @@ public class SliceTreeBuilder extends AbstractTreeBuilder { } - void switchToLeafNulls() { + public void switchToLeafNulls() { SliceLanguageSupportProvider provider = getRootSliceNode().getProvider(); if(provider == null){ return;