diff --git a/platform/analysis-impl/src/com/intellij/codeInsight/daemon/impl/HighlightInfoUpdaterImpl.java b/platform/analysis-impl/src/com/intellij/codeInsight/daemon/impl/HighlightInfoUpdaterImpl.java index b09e16f06b29..21b2f19d427d 100644 --- a/platform/analysis-impl/src/com/intellij/codeInsight/daemon/impl/HighlightInfoUpdaterImpl.java +++ b/platform/analysis-impl/src/com/intellij/codeInsight/daemon/impl/HighlightInfoUpdaterImpl.java @@ -47,11 +47,9 @@ import com.intellij.psi.util.PsiUtilCore; import com.intellij.util.ArrayUtil; import com.intellij.util.ConcurrencyUtil; import com.intellij.util.ExceptionUtil; -import com.intellij.util.ObjectUtils; import com.intellij.util.concurrency.AppExecutorUtil; import com.intellij.util.containers.CollectionFactory; import com.intellij.util.containers.ContainerUtil; -import com.intellij.util.containers.FreezableArrayList; import com.intellij.util.containers.HashingStrategy; import it.unimi.dsi.fastutil.longs.Long2ObjectMap; import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; @@ -274,12 +272,30 @@ public final class HighlightInfoUpdaterImpl extends HighlightInfoUpdater impleme return "; progress=" + (indicator == original ? "" : "wrapped:")+ (indicator == null ? "null\n" + ExceptionUtil.getThrowableText(new Throwable()) : indicator); } + private static int compareNaturalNullable(@Nullable String o1, @Nullable String o2) { + //noinspection StringEquality + return o1 == o2 ? 0 : o1 == null ? -1 : o2 == null ? 1 : o1.compareTo(o2); + } + private static final Comparator BY_OFFSETS_AND_HASH_ERRORS_FIRST = (o1, o2) -> { + if (o1 == o2) return 0; + int r = Segment.BY_START_OFFSET_THEN_END_OFFSET.compare(o1, o2); + if (r != 0) return r; + r = o2.getSeverity().compareTo(o1.getSeverity()); // NB: reversed - errors first + if (r != 0) return r; + r = compareNaturalNullable(o1.getDescription(), o2.getDescription()); + if (r != 0) return r; + // have to compare highlighters because we don't want to remove otherwise equal HighlightInfo except for its (recreated) highlighter + RangeHighlighterEx h1 = o1.getHighlighter(); + RangeHighlighterEx h2 = o2.getHighlighter(); + return Integer.compare(System.identityHashCode(h1), System.identityHashCode(h2)); + }; // remove `psis` from `data` in one batch for all infos in the list because there can be a lot of them - private static void removeFromDataAtomically(@NotNull Map data, - @NotNull @Unmodifiable Collection psis, - @NotNull HighlightingSession session) { - if (psis.isEmpty()) return; + // return true if was removed + private static boolean removeFromDataAtomically(@NotNull Map data, + @NotNull @Unmodifiable Collection psis, + @NotNull HighlightingSession session) { + if (psis.isEmpty()) return true; Map>> byPsiElement = new HashMap<>(); for (InvalidPsi invalidPsi : psis) { Object toolId = invalidPsi.info().toolId; @@ -288,29 +304,28 @@ public final class HighlightInfoUpdaterImpl extends HighlightInfoUpdater impleme .computeIfAbsent(invalidPsi.psiElement(), __ -> new ArrayList<>()); infos.add(invalidPsi.info()); } + boolean removed = true; for (Map.Entry>> entry : byPsiElement.entrySet()) { Object toolId = entry.getKey(); ToolHighlights toolHighlights = data.get(toolId); - if (toolHighlights == null) continue; + if (toolHighlights == null) { + removed = false; + continue; + } Map> byPsiMap = entry.getValue(); for (Map.Entry> byPsiEntry : byPsiMap.entrySet()) { PsiElement psiElement = byPsiEntry.getKey(); - List oldInfos = ObjectUtils.notNull(toolHighlights.elementHighlights.get(psiElement), List.of()); - List toRemove = ContainerUtil.sorted(byPsiEntry.getValue(), Segment.BY_START_OFFSET_THEN_END_OFFSET); + List oldL = toolHighlights.elementHighlights.get(psiElement); + List oldInfos = oldL == null ? List.of() : ContainerUtil.sorted(oldL, BY_OFFSETS_AND_HASH_ERRORS_FIRST); // need to-resort in case the range-highlighters invalidated and offsets are skewed + List toRemove = ContainerUtil.sorted(byPsiEntry.getValue(), BY_OFFSETS_AND_HASH_ERRORS_FIRST); List resultInfos = new ArrayList<>(); - ContainerUtil.processSortedListsInOrder(oldInfos, toRemove, (o1, o2) -> { - int r = Segment.BY_START_OFFSET_THEN_END_OFFSET.compare(o1, o2); - if (r != 0) return r; - // have to compare highlighters because we don't want to remove otherwise equal HighlightInfo except for its (recreated) highlighter - RangeHighlighterEx h1 = o1.getHighlighter(); - RangeHighlighterEx h2 = o2.getHighlighter(); - return System.identityHashCode(h1)-System.identityHashCode(h2); - }, true, (info, result) -> { + ContainerUtil.processSortedListsInOrder(oldInfos, toRemove, BY_OFFSETS_AND_HASH_ERRORS_FIRST, true, (info, result) -> { if (result == ContainerUtil.MergeResult.COPIED_FROM_LIST1) { resultInfos.add(info); } // in other cases when the info is either from toRemove or both, skip it }); + removed &= oldInfos.size() - toRemove.size() == resultInfos.size(); if (resultInfos.isEmpty()) { toolHighlights.elementHighlights.remove(psiElement); } @@ -324,6 +339,7 @@ public final class HighlightInfoUpdaterImpl extends HighlightInfoUpdater impleme } } } + return removed; } private synchronized void recycleInvalidPsiElements(@NotNull PsiFile psiFile, @@ -579,7 +595,7 @@ public final class HighlightInfoUpdaterImpl extends HighlightInfoUpdater impleme } } - private synchronized PsiElement findPsiElement(HighlightInfo info, @NotNull PsiFile psiFile) { + private synchronized PsiElement findPsiElement(@NotNull HighlightInfo info, @NotNull PsiFile psiFile) { Map data = getData(psiFile); ToolHighlights highlights = data.get(info.toolId); for (Map.Entry> entry : highlights.elementHighlights.entrySet()) { @@ -734,7 +750,7 @@ public final class HighlightInfoUpdaterImpl extends HighlightInfoUpdater impleme .filter(e -> isInspectionToolId(e.getKey())) // inspections only .flatMap(e -> e.getValue().elementHighlights.values().stream()) .flatMap(l->l.stream()) - .sorted(UpdateHighlightersUtil.BY_ACTUAL_START_OFFSET_NO_DUPS) + .sorted(BY_OFFSETS_AND_HASH_ERRORS_FIRST) // sort by errors first, so that when the warning appears, the overlapping errors are already in the 'overlappingIntervals' queue .toList(); SweepProcessor.Generator generator = processor -> ContainerUtil.process(sorted, processor); SeverityRegistrar severityRegistrar = SeverityRegistrar.getSeverityRegistrar(session.getProject()); @@ -1213,13 +1229,11 @@ public final class HighlightInfoUpdaterImpl extends HighlightInfoUpdater impleme SeverityRegistrar severityRegistrar = SeverityRegistrar.getSeverityRegistrar(session.getProject()); Long2ObjectMap range2markerCache = new Long2ObjectOpenHashMap<>(10); // infos which highlighters are recycled from `invalidElementRecycler` and which need to be removed from `data` to avoid its highlighter to be registered in two places - // this list must be sorted by Segment.BY_START_OFFSET_THEN_END_OFFSET - List recycledInvalidPsiHighlightersToBeRemovedFromData = new ArrayList<>(newInfos.size()); - List sorted = ContainerUtil.sorted(newInfos, Segment.BY_START_OFFSET_THEN_END_OFFSET); - FreezableArrayList newInfosToStore = new FreezableArrayList<>(sorted.size()); + // this list must be sorted by BY_OFFSETS_AND_HASH + List newInfosToStore = new ArrayList<>(newInfos.size()); //noinspection ForLoopReplaceableByForEach - for (int i = 0; i < sorted.size(); i++) { - HighlightInfo newInfo = sorted.get(i); + for (int i = 0; i < newInfos.size(); i++) { + HighlightInfo newInfo = newInfos.get(i); assert toolId.equals(newInfo.toolId) : "HighlightInfo generated by "+toolId + "(" + toolId.getClass() + ") must have consistent toolId, but got:"+ newInfo; boolean isFileLevel = newInfo.isFileLevelAnnotation(); long finalInfoRange = isFileLevel @@ -1239,9 +1253,6 @@ public final class HighlightInfoUpdaterImpl extends HighlightInfoUpdater impleme String from; if (recycled == null) { recycled = invalidElementRecycler.pickupHighlighterFromGarbageBin(infoStartOffset, infoEndOffset, layer, newInfo.getDescription()); - if (recycled != null) { - recycledInvalidPsiHighlightersToBeRemovedFromData.add(recycled); - } from = "invalidElementRecycler"; } else { @@ -1251,14 +1262,16 @@ public final class HighlightInfoUpdaterImpl extends HighlightInfoUpdater impleme if (LOG.isDebugEnabled()) { LOG.debug("assignRangeHighlighters: pickedup " + recycled + " from " + from+ " "+session.getProgressIndicator()); } + // remove highlighter from the data before changing its "errorStripe" to avoid having same highlighter for different Infos + removeFromDataAtomically(data, List.of(recycled), session); } - changeRangeHighlighterAttributes(session, psiFile, markup, newInfo, range2markerCache, finalInfoRange, recycled, isFileLevel, infoStartOffset, infoEndOffset, layer, - severityRegistrar); + changeRangeHighlighterAttributes(session, psiFile, markup, newInfo, range2markerCache, finalInfoRange, recycled, isFileLevel, infoStartOffset, infoEndOffset, layer, severityRegistrar); } - // this list must be sorted by Segment.BY_START_OFFSET_THEN_END_OFFSET - for (int i = 0; i < newInfosToStore.size(); i++) { - HighlightInfo info = newInfosToStore.get(i); + List sorted = ContainerUtil.sorted(newInfosToStore, BY_OFFSETS_AND_HASH_ERRORS_FIRST); + // this list must be sorted by BY_OFFSETS_AND_HASH + for (int i = 0; i < sorted.size(); i++) { + HighlightInfo info = sorted.get(i); //todo fails because of ProblemDescriptorWithReporterName //assert toolId.equals(info.toolId) : info + "; " + toolId + "(" + toolId.getClass() + ")"; assert info.getHighlighter() != null : info; @@ -1266,11 +1279,11 @@ public final class HighlightInfoUpdaterImpl extends HighlightInfoUpdater impleme HighlightInfo assignedInfo = HighlightInfo.fromRangeHighlighter(info.getHighlighter()); assert assignedInfo == info : "from RH: " + assignedInfo + "(" + System.identityHashCode(assignedInfo)+ "); but expected: " + info+ "(" + System.identityHashCode(info)+ ")"; if (i>0) { - assert Segment.BY_START_OFFSET_THEN_END_OFFSET.compare(newInfosToStore.get(i-1), newInfosToStore.get(i)) <= 0 : "assignRangeHighlighters returned unsorted list: "+newInfosToStore; + int compare = BY_OFFSETS_AND_HASH_ERRORS_FIRST.compare(sorted.get(i - 1), sorted.get(i)); + assert compare <= 0 : "assignRangeHighlighters returned unsorted list: " + sorted +"; "+compare; } } - removeFromDataAtomically(data, recycledInvalidPsiHighlightersToBeRemovedFromData, session); - return newInfosToStore.emptyOrFrozen(); + return sorted; } private static void changeRangeHighlighterAttributes(@NotNull HighlightingSession session, @@ -1356,14 +1369,4 @@ public final class HighlightInfoUpdaterImpl extends HighlightInfoUpdater impleme return highlighter; } - private void assertNoInfoInMarkup(@NotNull HighlightInfo info, @NotNull MarkupModelEx markup, @NotNull ManagedHighlighterRecycler recycler, @NotNull ManagedHighlighterRecycler invalidPsiRecycler) { - if (!isAssertInvariants()) return; - if (ContainerUtil.mapNotNull(markup.getAllHighlighters(), m-> HighlightInfo.fromRangeHighlighter(m)).contains(info) - && !ContainerUtil.mapNotNull(recycler.forAllInGarbageBin(), i->i.info()).contains(info) - && !ContainerUtil.mapNotNull(invalidPsiRecycler.forAllInGarbageBin(), i->i.info()).contains(info)) { - throw new AssertionError("Info " + info + " found in markup among "+ - "\n "+StringUtil.join(ContainerUtil.mapNotNull(markup.getAllHighlighters(), m-> HighlightInfo.fromRangeHighlighter(m)), Object::toString,"\n ")+ - " --, even though these recyclers do not contain it:\n "+recycler.forAllInGarbageBin()+";\n "+invalidPsiRecycler.forAllInGarbageBin()); - } - } }