From 410530c9e66f3a5459b5f6b9fcf2698a1354320e Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Tue, 28 Jul 2026 12:43:40 +0200 Subject: [PATCH] Add test coverage for the Unified Diff display Extract the offset and line geometry helpers behind the unified diff code minings into a new internal UnifiedDiffText class so they can be tested, and cover them together with the diff modes in org.eclipse.team.tests.core. Also fixes the line counting in UnifiedDiffLineHeaderCodeMining#draw, which used String#split("\n") and therefore placed the detailed diff highlight one line too high when a diff started at a line start. --- .../org.eclipse.compare/META-INF/MANIFEST.MF | 3 +- .../UnifiedDiffCodeMiningProvider.java | 110 +------ .../unifieddiff/internal/UnifiedDiffText.java | 157 ++++++++++ .../team/tests/core/AllTeamUITests.java | 2 + .../team/tests/ui/UnifiedDiffManagerTest.java | 248 +++++++++++++++- .../team/tests/ui/UnifiedDiffTextTest.java | 281 ++++++++++++++++++ 6 files changed, 696 insertions(+), 105 deletions(-) create mode 100644 team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffText.java create mode 100644 team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffTextTest.java diff --git a/team/bundles/org.eclipse.compare/META-INF/MANIFEST.MF b/team/bundles/org.eclipse.compare/META-INF/MANIFEST.MF index cbecaa29b77..c4677182284 100644 --- a/team/bundles/org.eclipse.compare/META-INF/MANIFEST.MF +++ b/team/bundles/org.eclipse.compare/META-INF/MANIFEST.MF @@ -65,7 +65,8 @@ Export-Package: org.eclipse.compare; org.eclipse.ui, org.eclipse.ui.services, org.eclipse.ui.texteditor", - org.eclipse.compare.unifieddiff;x-internal:=true + org.eclipse.compare.unifieddiff;x-internal:=true, + org.eclipse.compare.unifieddiff.internal;x-friends:="org.eclipse.team.tests.core" Require-Bundle: org.eclipse.ui;bundle-version="[3.206.0,4.0.0)", org.eclipse.core.resources;bundle-version="[3.4.0,4.0.0)", org.eclipse.jface.text;bundle-version="[3.8.0,4.0.0)", diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java index 7fb75205292..0209928b23c 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java @@ -15,6 +15,12 @@ import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffManager.error; import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffManager.isOverlay; +import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffText.countLines; +import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffText.mapOffsetToTabExpanded; +import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffText.mergeStyleRanges; +import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffText.removeLeadingNewLines; +import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffText.removeTrailingNewLines; +import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffText.replaceTabWithSpaces; import java.lang.reflect.Method; import java.util.ArrayList; @@ -377,28 +383,6 @@ public UnifiedDiffLineHeaderCodeMining(Position position, ICodeMiningProvider pr ((MouseClickConsumer) getAction()).setCodeMining(this); } - private static String removeTrailingNewLines(String str) { - if (str == null) { - return str; - } - int end = str.length(); - while (end > 0 && (str.charAt(end - 1) == '\n' || str.charAt(end - 1) == '\r')) { - end--; - } - return end == str.length() ? str : str.substring(0, end); - } - - private static String removeLeadingNewLines(String str) { - if (str == null) { - return str; - } - int start = 0; - while (start < str.length() && (str.charAt(start) == '\n' || str.charAt(start) == '\r')) { - start++; - } - return start == 0 ? str : str.substring(start); - } - private static final class ForegroundInfo { final int x; @@ -470,58 +454,6 @@ public void keyPressed(KeyEvent e) { setTextEditorActionsActivated(false); } - private List mergeStyleRanges(List backgrounds, List foregrounds) { - if (backgrounds.isEmpty()) { - return foregrounds; - } - if (foregrounds.isEmpty()) { - return backgrounds; - } - List result = new ArrayList<>(); - int bgIdx = 0; - for (StyleRange fg : foregrounds) { - int fgStart = fg.start; - int fgEnd = fg.start + fg.length; - int pos = fgStart; - while (pos < fgEnd && bgIdx < backgrounds.size()) { - StyleRange bg = backgrounds.get(bgIdx); - int bgStart = bg.start; - int bgEnd = bg.start + bg.length; - if (bgEnd <= pos) { - bgIdx++; - continue; - } - if (bgStart >= fgEnd) { - break; - } - if (bgStart > pos) { - result.add(createRange(fg, pos, bgStart - pos, null)); - pos = bgStart; - } - int overlapEnd = Math.min(bgEnd, fgEnd); - result.add(createRange(fg, pos, overlapEnd - pos, bg.background)); - pos = overlapEnd; - if (bgEnd <= fgEnd) { - bgIdx++; - } - } - if (pos < fgEnd) { - result.add(createRange(fg, pos, fgEnd - pos, null)); - } - } - return result; - } - - private StyleRange createRange(StyleRange base, int start, int length, Color background) { - StyleRange r = (StyleRange) base.clone(); - r.start = start; - r.length = length; - if (background != null) { - r.background = background; - } - return r; - } - private List createDetailedDiffBackgroundRanges(UnifiedDiffLineHeaderCodeMining miningParam, String txt) { List ranges = new ArrayList<>(); @@ -566,17 +498,6 @@ private List createDetailedDiffBackgroundRanges(UnifiedDiffLineHeade return ranges; } - private static int mapOffsetToTabExpanded(String originalStr, int offset, int tabWidth) { - int tabCount = 0; - int limit = Math.min(offset, originalStr.length()); - for (int i = 0; i < limit; i++) { - if (originalStr.charAt(i) == '\t') { - tabCount++; - } - } - return offset + tabCount * (tabWidth - 1); - } - private void setTextEditorActionsActivated(boolean state) { IEditorPart part = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage() .getActiveEditor(); @@ -729,9 +650,10 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { } try { var rangeInfo = new RangeInfo(-1, -1, null); - String l = diffStr.substring(0, detailedDiffStart); - int fromLine = l.split("\n").length; //$NON-NLS-1$ - int toLine = diffStr.substring(0, detailedDiffStart + detailedDiffLength).split("\n").length; //$NON-NLS-1$ + // String#split drops trailing empty strings, so it must not be used to + // count lines: a prefix ending with \n starts the next line + int fromLine = countLines(diffStr.substring(0, detailedDiffStart)); + int toLine = countLines(diffStr.substring(0, detailedDiffStart + detailedDiffLength)); if (fromLine == toLine) { int starty = getYForLine(fromLine - 1, y, gc, textWidget); Point start = getPositionForOffset(textWidget, gc, detailedDiffStart, diffStr, ranges, @@ -1067,16 +989,4 @@ static RGB interpolate(RGB fg, RGB bg, double scale) { } return new RGB(128, 128, 128); // a gray } - - private static String replaceTabWithSpaces(String leftStr, int tabWidth) { - if (leftStr == null) { - return null; - } - StringBuilder sb = new StringBuilder(); - for (int s = 0; s < tabWidth; s++) { - sb.append(' '); - } - String result = leftStr.replace("\t", sb); //$NON-NLS-1$ - return result; - } } diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffText.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffText.java new file mode 100644 index 00000000000..c46e4279524 --- /dev/null +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffText.java @@ -0,0 +1,157 @@ +/******************************************************************************* + * Copyright (c) 2026 SAP + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * SAP - initial implementation + *******************************************************************************/ +package org.eclipse.compare.unifieddiff.internal; + +import java.util.ArrayList; +import java.util.List; + +import org.eclipse.swt.custom.StyleRange; +import org.eclipse.swt.graphics.Color; + +/** + * Side-effect free text and style-range helpers used to lay out the unified + * diff code minings. + */ +public final class UnifiedDiffText { + + private UnifiedDiffText() { + } + + /** + * Returns the number of lines of the given string, counting a trailing line + * delimiter as starting a new (empty) line. An empty string is one line. + */ + public static int countLines(String str) { + if (str == null) { + return 0; + } + int result = 1; + for (int i = 0; i < str.length(); i++) { + if (str.charAt(i) == '\n') { + result++; + } + } + return result; + } + + public static String removeTrailingNewLines(String str) { + if (str == null) { + return str; + } + int end = str.length(); + while (end > 0 && (str.charAt(end - 1) == '\n' || str.charAt(end - 1) == '\r')) { + end--; + } + return end == str.length() ? str : str.substring(0, end); + } + + public static String removeLeadingNewLines(String str) { + if (str == null) { + return str; + } + int start = 0; + while (start < str.length() && (str.charAt(start) == '\n' || str.charAt(start) == '\r')) { + start++; + } + return start == 0 ? str : str.substring(start); + } + + /** + * Expands every tab into {@code tabWidth} spaces. {@code GC#stringExtent} does + * not account for tabs, so widths are always measured on the expanded string. + */ + public static String replaceTabWithSpaces(String str, int tabWidth) { + if (str == null) { + return null; + } + StringBuilder sb = new StringBuilder(); + for (int s = 0; s < tabWidth; s++) { + sb.append(' '); + } + return str.replace("\t", sb); //$NON-NLS-1$ + } + + /** + * Maps an offset in {@code originalStr} to the corresponding offset in + * {@link #replaceTabWithSpaces(String, int)} of the same string. + */ + public static int mapOffsetToTabExpanded(String originalStr, int offset, int tabWidth) { + int tabCount = 0; + int limit = Math.min(offset, originalStr.length()); + for (int i = 0; i < limit; i++) { + if (originalStr.charAt(i) == '\t') { + tabCount++; + } + } + return offset + tabCount * (tabWidth - 1); + } + + /** + * Overlays the detailed-diff background ranges onto the syntax coloring ranges. + * Both lists must be sorted by offset and must not overlap within themselves. + * The returned ranges keep the foreground styling and cover exactly the extent + * of {@code foregrounds}. + */ + public static List mergeStyleRanges(List backgrounds, List foregrounds) { + if (backgrounds.isEmpty()) { + return foregrounds; + } + if (foregrounds.isEmpty()) { + return backgrounds; + } + List result = new ArrayList<>(); + int bgIdx = 0; + for (StyleRange fg : foregrounds) { + int fgStart = fg.start; + int fgEnd = fg.start + fg.length; + int pos = fgStart; + while (pos < fgEnd && bgIdx < backgrounds.size()) { + StyleRange bg = backgrounds.get(bgIdx); + int bgStart = bg.start; + int bgEnd = bg.start + bg.length; + if (bgEnd <= pos) { + bgIdx++; + continue; + } + if (bgStart >= fgEnd) { + break; + } + if (bgStart > pos) { + result.add(createRange(fg, pos, bgStart - pos, null)); + pos = bgStart; + } + int overlapEnd = Math.min(bgEnd, fgEnd); + result.add(createRange(fg, pos, overlapEnd - pos, bg.background)); + pos = overlapEnd; + if (bgEnd <= fgEnd) { + bgIdx++; + } + } + if (pos < fgEnd) { + result.add(createRange(fg, pos, fgEnd - pos, null)); + } + } + return result; + } + + private static StyleRange createRange(StyleRange base, int start, int length, Color background) { + StyleRange r = (StyleRange) base.clone(); + r.start = start; + r.length = length; + if (background != null) { + r.background = background; + } + return r; + } +} diff --git a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/core/AllTeamUITests.java b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/core/AllTeamUITests.java index d1e437400cc..ea1f2a5beee 100644 --- a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/core/AllTeamUITests.java +++ b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/core/AllTeamUITests.java @@ -16,6 +16,7 @@ import org.eclipse.team.tests.core.mapping.AllTeamMappingTests; import org.eclipse.team.tests.ui.SaveableCompareEditorInputTest; import org.eclipse.team.tests.ui.UnifiedDiffManagerTest; +import org.eclipse.team.tests.ui.UnifiedDiffTextTest; import org.eclipse.team.tests.ui.synchronize.AllTeamSynchronizeTests; import org.junit.platform.suite.api.SelectClasses; import org.junit.platform.suite.api.Suite; @@ -26,6 +27,7 @@ AllTeamSynchronizeTests.class, // SaveableCompareEditorInputTest.class, // UnifiedDiffManagerTest.class, // + UnifiedDiffTextTest.class, // }) public class AllTeamUITests { } diff --git a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffManagerTest.java b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffManagerTest.java index 6a4dc79ef2b..9c5ff3e96f5 100644 --- a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffManagerTest.java +++ b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffManagerTest.java @@ -18,7 +18,9 @@ import static org.eclipse.core.resources.ResourcesPlugin.getWorkspace; import static org.eclipse.core.tests.resources.ResourceTestUtil.createInWorkspace; import static org.eclipse.core.tests.resources.ResourceTestUtil.createInputStream; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.ArrayList; @@ -27,13 +29,17 @@ import org.eclipse.compare.unifieddiff.UnifiedDiff; import org.eclipse.compare.unifieddiff.UnifiedDiffMode; +import org.eclipse.compare.unifieddiff.internal.UnifiedDiffManager; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IProject; import org.eclipse.core.runtime.ILogListener; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Platform; import org.eclipse.core.tests.resources.util.WorkspaceResetExtension; +import org.eclipse.jface.text.BadLocationException; +import org.eclipse.jface.text.IDocument; import org.eclipse.jface.text.ITextViewer; +import org.eclipse.jface.text.Position; import org.eclipse.jface.text.source.Annotation; import org.eclipse.jface.text.source.IAnnotationModel; import org.eclipse.swt.custom.StyledText; @@ -158,16 +164,250 @@ public void testRevertModeProducesAnnotationsAndPaints() { forcePaintCycle(viewer.getTextWidget()); } - private static int countAnnotations(IAnnotationModel model, String type) { - int count = 0; + // ------------------------------------------------------------ REPLACE_MODE + + // REPLACE_MODE rewrites the editor document diff by diff and shifts every + // following diff by the length delta of the applied ones. The resulting + // document must be character-identical to the compared source, otherwise the + // user silently ends up with a mixture of both versions. + + @Test + public void testReplaceModeAppliesAnInsertionAtTheEndOfTheDocument() { + assertReplaceModeYields(""" + line one + line two + """, """ + line one + line two + line three + """); + } + + @Test + public void testReplaceModeAppliesADeletion() { + assertReplaceModeYields(""" + line one + line two + line three + """, """ + line one + line three + """); + } + + /** + * Several diffs of differing lengths in one document: each applied diff shifts + * the offsets of all following ones, so a wrong delta only shows up from the + * second diff onwards. + */ + @Test + public void testReplaceModeAppliesSeveralDiffsOfDifferentLength() { + String left = """ + one + two + three + four + five + six + """; + String right = """ + one + a much longer second line + three + 4 + five + six and a bit more + """; + assertReplaceModeYields(left, right); + assertTrue(UnifiedDiffManager.get(viewer()).size() >= 3, + "the three changed regions must be reported as separate diffs"); + } + + @Test + public void testReplaceModeWithWindowsLineDelimiters() { + assertReplaceModeYields("line one\r\nline two\r\nline three\r\n", + "line one\r\nline TWO modified\r\nline three\r\n"); + } + + @Test + public void testReplaceModeAnnotatesTheAppliedText() throws BadLocationException { + setEditorContent(LEFT); + + IStatus status = UnifiedDiff.create(editor, RIGHT, UnifiedDiffMode.REPLACE_MODE).open(); + assertTrue(status.isOK(), "open() should return OK status: " + status); + + List additions = annotations(annotationModel(), ADDITION_ANNO_TYPE); + assertEquals(1, additions.size(), "one addition annotation for the single diff"); + Position position = annotationModel().getPosition(additions.get(0)); + assertEquals("line TWO modified\n", document().get(position.offset, position.length), + "the annotation must cover the text that was inserted into the document"); + } + + // ---------------------------------------------------- non modifying modes + + @Test + public void testOverlayModeLeavesTheDocumentUnchanged() { + setEditorContent(LEFT); + + assertTrue(UnifiedDiff.create(editor, RIGHT, UnifiedDiffMode.OVERLAY_MODE).open().isOK()); + + assertEquals(LEFT, document().get(), "OVERLAY_MODE must not touch the editor document"); + } + + @Test + public void testRevertModeLeavesTheDocumentUnchanged() { + setEditorContent(LEFT); + + assertTrue(UnifiedDiff.create(editor, RIGHT, UnifiedDiffMode.REVERT_MODE).open().isOK()); + + assertEquals(LEFT, document().get(), "REVERT_MODE must not touch the editor document"); + } + + /** + * The detailed (token level) annotations are positioned relative to the + * enclosing diff, so their offsets must stay inside it. + */ + @Test + public void testDetailedAnnotationsStayInsideTheirDiff() throws BadLocationException { + setEditorContent(LEFT); + + assertTrue(UnifiedDiff.create(editor, RIGHT, UnifiedDiffMode.OVERLAY_READ_ONLY_MODE).open().isOK()); + + IAnnotationModel model = annotationModel(); + Position parent = model.getPosition(annotations(model, DELETION_ANNO_TYPE).get(0)); + List detailed = annotations(model, DETAILED_DELETION_ANNO_TYPE); + assertTrue(detailed.size() >= 1, "expected at least one detailed annotation"); + for (Annotation annotation : detailed) { + Position position = model.getPosition(annotation); + assertTrue(position.offset >= parent.offset && position.offset + position.length <= parent.offset + + parent.length, "detailed annotation " + position.offset + "+" + position.length + + " is outside its diff " + parent.offset + "+" + parent.length); + String text = document().get(position.offset, position.length); + assertTrue(!text.isBlank(), "a detailed annotation must not cover whitespace only, was '" + text + "'"); + } + } + + // ------------------------------------------------------------- edge cases + + @Test + public void testIdenticalContentProducesNoDiffs() { + setEditorContent(LEFT); + + IStatus status = UnifiedDiff.create(editor, LEFT, UnifiedDiffMode.OVERLAY_MODE).open(); + + assertTrue(status.isOK(), "open() should return OK status: " + status); + assertTrue(UnifiedDiffManager.get(viewer()).isEmpty(), "identical content must not produce diffs"); + assertEquals(0, countAnnotations(annotationModel(), DELETION_ANNO_TYPE)); + assertEquals(LEFT, document().get()); + } + + @Test + public void testWhitespaceOnlyChangeIsIgnoredByDefault() { + setEditorContent("line one\nline two\n"); + + assertTrue(UnifiedDiff.create(editor, "line one\nline two\n", UnifiedDiffMode.OVERLAY_MODE).open().isOK()); + + assertTrue(UnifiedDiffManager.get(viewer()).isEmpty(), + "whitespace only changes are ignored unless requested otherwise"); + } + + @Test + public void testWhitespaceOnlyChangeIsReportedWhenNotIgnored() { + setEditorContent("line one\nline two\n"); + + assertTrue(UnifiedDiff.create(editor, "line one\nline two\n", UnifiedDiffMode.OVERLAY_MODE) + .ignoreWhiteSpace(false).open().isOK()); + + assertEquals(1, UnifiedDiffManager.get(viewer()).size(), + "with ignoreWhiteSpace(false) the changed indentation is a diff"); + } + + /** + * Opening a second diff on the same editor must replace the first one instead + * of stacking annotations and diffs on top of each other. + */ + @Test + public void testReopeningReplacesThePreviousDiffs() { + setEditorContent(LEFT); + + assertTrue(UnifiedDiff.create(editor, RIGHT, UnifiedDiffMode.OVERLAY_MODE).open().isOK()); + int diffsAfterFirstOpen = UnifiedDiffManager.get(viewer()).size(); + int annotationsAfterFirstOpen = countAnnotations(annotationModel(), DELETION_ANNO_TYPE) + + countAnnotations(annotationModel(), DETAILED_DELETION_ANNO_TYPE); + + assertTrue(UnifiedDiff.create(editor, RIGHT, UnifiedDiffMode.OVERLAY_MODE).open().isOK()); + + assertEquals(diffsAfterFirstOpen, UnifiedDiffManager.get(viewer()).size(), "diffs must not accumulate"); + assertEquals(annotationsAfterFirstOpen, countAnnotations(annotationModel(), DELETION_ANNO_TYPE) + + countAnnotations(annotationModel(), DETAILED_DELETION_ANNO_TYPE), + "annotations of the previous diff must be removed"); + } + + /** + * The manager keeps its state in a static map keyed by the text viewer. If it + * is not cleaned up on dispose, every opened editor leaks its diffs and its + * documents for the rest of the session. + */ + @Test + public void testClosingTheEditorReleasesTheManagerState() { + setEditorContent(LEFT); + assertTrue(UnifiedDiff.create(editor, RIGHT, UnifiedDiffMode.OVERLAY_MODE).open().isOK()); + ITextViewer viewer = viewer(); + assertNotNull(UnifiedDiffManager.get(viewer)); + + PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage().closeEditor(editor, false); + editor = null; + processEvents(); + + assertNull(UnifiedDiffManager.get(viewer), "the manager must forget the viewer when it is disposed"); + } + + // ------------------------------------------------------------------ helpers + + private void assertReplaceModeYields(String left, String right) { + setEditorContent(left); + + IStatus status = UnifiedDiff.create(editor, right, UnifiedDiffMode.REPLACE_MODE).open(); + + assertTrue(status.isOK(), "open() should return OK status: " + status); + assertEquals(right, document().get(), "REPLACE_MODE must transform the document into the compared source"); + } + + private ITextViewer viewer() { + ITextViewer viewer = editor.getAdapter(ITextViewer.class); + assertNotNull(viewer, "editor must adapt to ITextViewer"); + return viewer; + } + + private IDocument document() { + return editor.getDocumentProvider().getDocument(editor.getEditorInput()); + } + + private IAnnotationModel annotationModel() { + IAnnotationModel model = editor.getDocumentProvider().getAnnotationModel(editor.getEditorInput()); + assertNotNull(model, "annotation model must not be null"); + return model; + } + + private void setEditorContent(String content) { + document().set(content); + processEvents(); + } + + private static List annotations(IAnnotationModel model, String type) { + List result = new ArrayList<>(); Iterator it = model.getAnnotationIterator(); while (it.hasNext()) { Annotation anno = it.next(); if (type.equals(anno.getType())) { - count++; + result.add(anno); } } - return count; + return result; + } + + private static int countAnnotations(IAnnotationModel model, String type) { + return annotations(model, type).size(); } private static void forcePaintCycle(StyledText tw) { diff --git a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffTextTest.java b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffTextTest.java new file mode 100644 index 00000000000..9b97d70bc44 --- /dev/null +++ b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffTextTest.java @@ -0,0 +1,281 @@ +/******************************************************************************* + * Copyright (c) 2026 SAP + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * SAP - initial implementation + *******************************************************************************/ +package org.eclipse.team.tests.ui; + +import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffText.countLines; +import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffText.mapOffsetToTabExpanded; +import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffText.mergeStyleRanges; +import static org.eclipse.compare.unifieddiff.internal.UnifiedDiffText.replaceTabWithSpaces; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; + +import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.StyleRange; +import org.eclipse.swt.graphics.Color; +import org.eclipse.swt.widgets.Display; +import org.junit.jupiter.api.Test; + +/** + * Tests for the geometry and text helpers behind the unified diff code minings. + * These functions decide where the detailed-diff highlight rectangles are + * painted, so an off-by-one here shows up as a misplaced highlight that is easy + * to miss in a manual test. + */ +public class UnifiedDiffTextTest { + + private static final Color BACKGROUND_1 = systemColor(SWT.COLOR_RED); + private static final Color BACKGROUND_2 = systemColor(SWT.COLOR_GREEN); + private static final Color FOREGROUND_1 = systemColor(SWT.COLOR_BLUE); + private static final Color FOREGROUND_2 = systemColor(SWT.COLOR_YELLOW); + + // ---------------------------------------------------------------- countLines + + /** + * The line a detailed diff is painted on. A diff that starts right after a line + * delimiter belongs to the following line, not to the one that ended, which is + * what {@code String#split("\n")} gets wrong: it drops trailing empty strings + * and must therefore never be used to count lines. + */ + @Test + public void testCountLinesForDetailedDiffStartOffsets() { + String diff = "first\nsecond\nthird"; + assertAll( // + () -> assertEquals(1, countLines(diff.substring(0, 0)), "offset 0 is on line 1"), // + () -> assertEquals(1, countLines(diff.substring(0, 3)), "offset inside line 1"), // + () -> assertEquals(1, countLines(diff.substring(0, 5)), "offset at end of line 1"), // + () -> assertEquals(2, countLines(diff.substring(0, 6)), "offset at start of line 2"), // + () -> assertEquals(3, countLines(diff.substring(0, 13)), "offset at start of line 3"), // + () -> assertEquals(2, countLines("first\r\nsecond"), "CRLF is one delimiter")); + } + + // ------------------------------------------------------------ tab expansion + + @Test + public void testMapOffsetToTabExpanded() { + String source = "\ta\tb"; + assertAll( // + () -> assertEquals(0, mapOffsetToTabExpanded(source, 0, 4), "offset 0 never shifts"), // + () -> assertEquals(4, mapOffsetToTabExpanded(source, 1, 4), "after the first tab"), // + () -> assertEquals(5, mapOffsetToTabExpanded(source, 2, 4)), // + () -> assertEquals(9, mapOffsetToTabExpanded(source, 3, 4), "after the second tab"), // + () -> assertEquals(3, mapOffsetToTabExpanded(source, 3, 1), "a tab width of 1 is the identity"), // + () -> assertEquals(2, mapOffsetToTabExpanded("ab\tc", 2, 4), "a tab behind the offset is ignored")); + } + + /** + * The highlight offsets are computed on the raw diff text while the label is + * painted from the tab expanded text, so both must agree for every offset. + */ + @Test + public void testMapOffsetToTabExpandedMatchesReplaceTabWithSpaces() { + for (String source : List.of("\ta\tb", "no tabs at all", "\t\t\t", "a\tb\nc\td", "")) { + for (int tabWidth : new int[] { 1, 2, 4, 8 }) { + for (int offset = 0; offset <= source.length(); offset++) { + String prefix = source.substring(0, offset); + int expected = replaceTabWithSpaces(prefix, tabWidth).length(); + int actual = mapOffsetToTabExpanded(source, offset, tabWidth); + assertEquals(expected, actual, "offset " + offset + " of '" + source.replace("\t", "\\t") + + "' with tab width " + tabWidth); + } + } + } + } + + @Test + public void testMapOffsetToTabExpandedClampsOffsetsBehindTheText() { + // callers pass detailedDiffStart + detailedDiffLength, which may run past the + // text once trailing new lines have been stripped + assertEquals(10, mapOffsetToTabExpanded("\tab", 7, 4), "tabs are only counted within the text"); + } + + // -------------------------------------------------------- mergeStyleRanges + + @Test + public void testMergeStyleRangesWithoutBackgroundsReturnsForegrounds() { + List foregrounds = List.of(foreground(0, 5, FOREGROUND_1)); + assertSame(foregrounds, mergeStyleRanges(List.of(), foregrounds)); + } + + @Test + public void testMergeStyleRangesWithoutForegroundsReturnsBackgrounds() { + List backgrounds = List.of(background(0, 5, BACKGROUND_1)); + assertSame(backgrounds, mergeStyleRanges(backgrounds, List.of())); + } + + @Test + public void testMergeStyleRangesSplitsForegroundAroundBackground() { + List result = mergeStyleRanges(List.of(background(3, 3, BACKGROUND_1)), + List.of(foreground(0, 10, FOREGROUND_1))); + + assertRanges(result, // + "0+3 fg=BLUE bg=null", // + "3+3 fg=BLUE bg=RED", // + "6+4 fg=BLUE bg=null"); + assertTilesForegrounds(result, List.of(foreground(0, 10, FOREGROUND_1))); + } + + @Test + public void testMergeStyleRangesKeepsForegroundStylingOfEverySegment() { + StyleRange bold = foreground(0, 10, FOREGROUND_1); + bold.fontStyle = SWT.BOLD; + + List result = mergeStyleRanges(List.of(background(3, 3, BACKGROUND_1)), List.of(bold)); + + for (StyleRange range : result) { + assertEquals(SWT.BOLD, range.fontStyle, "font style must survive the split"); + assertSame(FOREGROUND_1, range.foreground, "foreground must survive the split"); + } + } + + @Test + public void testMergeStyleRangesSpansSeveralForegrounds() { + List foregrounds = List.of(foreground(0, 5, FOREGROUND_1), foreground(5, 5, FOREGROUND_2)); + + List result = mergeStyleRanges(List.of(background(3, 4, BACKGROUND_1)), foregrounds); + + assertRanges(result, // + "0+3 fg=BLUE bg=null", // + "3+2 fg=BLUE bg=RED", // + "5+2 fg=YELLOW bg=RED", // + "7+3 fg=YELLOW bg=null"); + assertTilesForegrounds(result, foregrounds); + } + + @Test + public void testMergeStyleRangesWithSeveralBackgroundsInOneForeground() { + List foregrounds = List.of(foreground(0, 20, FOREGROUND_1)); + + List result = mergeStyleRanges( + List.of(background(2, 3, BACKGROUND_1), background(10, 2, BACKGROUND_2)), foregrounds); + + assertRanges(result, // + "0+2 fg=BLUE bg=null", // + "2+3 fg=BLUE bg=RED", // + "5+5 fg=BLUE bg=null", // + "10+2 fg=BLUE bg=GREEN", // + "12+8 fg=BLUE bg=null"); + assertTilesForegrounds(result, foregrounds); + } + + @Test + public void testMergeStyleRangesWithCongruentRanges() { + List foregrounds = List.of(foreground(0, 5, FOREGROUND_1)); + + List result = mergeStyleRanges(List.of(background(0, 5, BACKGROUND_1)), foregrounds); + + assertRanges(result, "0+5 fg=BLUE bg=RED"); + } + + @Test + public void testMergeStyleRangesIgnoresBackgroundsOutsideTheForegrounds() { + List foregrounds = List.of(foreground(10, 5, FOREGROUND_1)); + + assertRanges(mergeStyleRanges(List.of(background(0, 5, BACKGROUND_1)), foregrounds), // + "10+5 fg=BLUE bg=null"); + assertRanges(mergeStyleRanges(List.of(background(30, 5, BACKGROUND_1)), foregrounds), // + "10+5 fg=BLUE bg=null"); + } + + @Test + public void testMergeStyleRangesWithGapsBetweenForegrounds() { + // a background reaching into the gap between two foregrounds must not produce + // a range covering unstyled text + List foregrounds = List.of(foreground(0, 4, FOREGROUND_1), foreground(8, 4, FOREGROUND_2)); + + List result = mergeStyleRanges(List.of(background(2, 8, BACKGROUND_1)), foregrounds); + + assertRanges(result, // + "0+2 fg=BLUE bg=null", // + "2+2 fg=BLUE bg=RED", // + "8+2 fg=YELLOW bg=RED", // + "10+2 fg=YELLOW bg=null"); + assertTilesForegrounds(result, foregrounds); + } + + // ------------------------------------------------------------------ helpers + + /** + * The merged ranges are handed to {@code StyledText#setStyleRanges}, which + * rejects overlapping ranges. They must therefore tile the foregrounds exactly: + * same extent, ascending, no gaps, no overlaps, no empty ranges. + */ + private static void assertTilesForegrounds(List result, List foregrounds) { + int resultIndex = 0; + for (StyleRange fg : foregrounds) { + int expected = fg.start; + while (resultIndex < result.size() && result.get(resultIndex).start < fg.start + fg.length) { + StyleRange range = result.get(resultIndex); + assertEquals(expected, range.start, "ranges must be contiguous and ascending: " + describe(result)); + assertTrue(range.length > 0, "empty ranges are not allowed: " + describe(result)); + expected += range.length; + resultIndex++; + } + assertEquals(fg.start + fg.length, expected, + "the foreground extent must be covered completely: " + describe(result)); + } + assertEquals(result.size(), resultIndex, "no ranges outside the foregrounds: " + describe(result)); + } + + private static void assertRanges(List actual, String... expected) { + assertEquals(List.of(expected), describe(actual)); + } + + private static List describe(List ranges) { + return ranges.stream() + .map(r -> r.start + "+" + r.length + " fg=" + name(r.foreground) + " bg=" + name(r.background)).toList(); + } + + private static String name(Color color) { + if (color == null) { + return "null"; + } + if (color.equals(BACKGROUND_1)) { + return "RED"; + } + if (color.equals(BACKGROUND_2)) { + return "GREEN"; + } + if (color.equals(FOREGROUND_1)) { + return "BLUE"; + } + if (color.equals(FOREGROUND_2)) { + return "YELLOW"; + } + return color.toString(); + } + + private static StyleRange foreground(int start, int length, Color color) { + StyleRange range = new StyleRange(); + range.start = start; + range.length = length; + range.foreground = color; + return range; + } + + private static StyleRange background(int start, int length, Color color) { + StyleRange range = new StyleRange(); + range.start = start; + range.length = length; + range.background = color; + return range; + } + + private static Color systemColor(int id) { + return Display.getDefault().getSystemColor(id); + } +}