Fix IOOB exception in Javadoc hover (#1395)

- modify the logic for <pre>{@code that handles the case where the code has } in it so that it only looks for }</pre> when the pre counter matches and also add a check to ensure that the IOOB exception cannot happen - add new test to JavadocHoverTests with an invalid <pre> which forces a }</pre> to occur without a <pre>{@code - fixes #1394
eclipse-jdt · May 8, 2024 · 437edbe · 437edbe
1 parent fe10342
commit 437edbe
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 7 deletions.
diff --git a/...tion/common/org/eclipse/jdt/core/manipulation/internal/javadoc/CoreJavadocAccessImpl.java b/...tion/common/org/eclipse/jdt/core/manipulation/internal/javadoc/CoreJavadocAccessImpl.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2023 IBM Corporation and others.
+ * Copyright (c) 2008, 2024 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -125,6 +125,8 @@ public class CoreJavadocAccessImpl implements IJavadocAccess {
 
 	protected int fPreCounter;
 
+	protected int fInPreCodeCounter= -1;
+
 	public CoreJavadocAccessImpl(IJavaElement element, Javadoc javadoc, String source, JavadocLookup lookup) {
 		Assert.isNotNull(element);
 		Assert.isTrue(element instanceof IMethod || element instanceof ILocalVariable || element instanceof ITypeParameter);
@@ -800,15 +802,23 @@ protected String handlePreCounter(TagElement tagElement, String text) {
 			++fPreCounter;
 		} else if (tagElement == null && text.equals("</pre>")) { //$NON-NLS-1$
 			--fPreCounter;
+			if (fPreCounter == fInPreCodeCounter) {
+				fInPreCodeCounter= -1;
+			}
 		} else if (tagElement == null && fPreCounter > 0 && text.matches("}\\s*</pre>")) { //$NON-NLS-1$
 			// this is a temporary workaround for https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/316
 			// as the parser for @code is treating the first } it finds as the end of the code
 			// sequence but this is not the case for a <pre>{@code sequence which goes over
 			// multiple lines and may contain }'s that are part of the code
 			--fPreCounter;
-			text= "</code></pre>"; //$NON-NLS-1$
-			int lastCodeEnd= fBuf.lastIndexOf("</code>"); //$NON-NLS-1$
-			fBuf.replace(lastCodeEnd, lastCodeEnd + 7, ""); //$NON-NLS-1$
+			if (fPreCounter == fInPreCodeCounter) {
+				text= "</code></pre>"; //$NON-NLS-1$
+				int lastCodeEnd= fBuf.lastIndexOf("</code>"); //$NON-NLS-1$
+				if (lastCodeEnd >= 0) {
+					fBuf.replace(lastCodeEnd, lastCodeEnd + 7, ""); //$NON-NLS-1$
+				}
+				fInPreCodeCounter= -1;
+			}
 		}
 		return text;
 	}
@@ -926,8 +936,12 @@ protected void handleInlineTagElement(TagElement node) {
 
 		if (isLiteral || isCode || isSummary || isIndex)
 			fLiteralContent++;
-		if (isCode || (isLink && addCodeTagOnLink()))
+		if (isCode || (isLink && addCodeTagOnLink())) {
+			if (isCode && fPreCounter > 0 && fBuf.lastIndexOf("<pre>") == fBuf.length() - 5) { //$NON-NLS-1$
+				fInPreCodeCounter= fPreCounter - 1;
+			}
 			fBuf.append("<code>"); //$NON-NLS-1$
+		}
 		if (isReturn)
 			fBuf.append(JavaDocMessages.JavadocContentAccess2_returns_pre);
 

diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/hover/JavadocHoverTests.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/hover/JavadocHoverTests.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2020, 2023 GK Software SE and others.
+ * Copyright (c) 2020, 2024 GK Software SE and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -201,7 +201,55 @@ public void testCodeTagWithPre() throws Exception {
 			int index= actualHtmlContent.indexOf("<pre><code>");
 			assertFalse(index == -1);
 			String actualSnippet= actualHtmlContent.substring(index, index + expectedCodeSequence.length());
-			assertEquals("sequence doesn't match", actualSnippet, expectedCodeSequence);
+			assertEquals("sequence doesn't match", expectedCodeSequence, actualSnippet);
+		}
+	}
+
+	@Test
+	public void testCodeTagWithPre_2() throws Exception {
+		String source=
+				"package p;\n" +
+				"public class TestClass {\n" +
+			    "/**\n" +
+				" * <pre>{@see <a href=\"https://www.eclipse.org\">\n" +
+				" * www.eclipse.org\n" +
+				" * </a>}}</pre>\n" +
+				" * @see \"Hello\n" +
+				" * World\"}</pre>\n" +
+				" */\n" +
+				"int check (String value, String[] strings) {\n" +
+				"	for (String s : strings) {\n" +
+				"		if (s.equals(value)) {\n" +
+				"			return 0;\n" +
+				"		}\n" +
+				"		if (s.startsWith(value)) {\n" +
+				"			return 1;\n" +
+				"		}\n" +
+				"	}\n" +
+				"	return -1;\n" +
+				"}\n";
+		ICompilationUnit cu= getWorkingCopy("/TestSetupProject/src/p/TestClass.java", source, null);
+		assertNotNull("TestClass.java", cu);
+
+		IType type= cu.getType("TestClass");
+		// check javadoc on each member:
+		for (IJavaElement member : type.getChildren()) {
+			IJavaElement[] elements= { member };
+			ISourceRange range= ((ISourceReference) member).getNameRange();
+			JavadocBrowserInformationControlInput hoverInfo= JavadocHover.getHoverInfo(elements, cu, new Region(range.getOffset(), range.getLength()), null);
+			String actualHtmlContent= hoverInfo.getHtml();
+
+			String expectedCodeSequence=
+					"<pre>{@see <a href=\"https://www.eclipse.org\">\n"
+					+ " www.eclipse.org\n"
+					+ " </a>}}</pre><dl><dt>Parameters:</dt><dd><b>value</b> </dd><dd><b>strings</b> </dd><dt>See Also:</dt><dd> \"Hello\n"
+					+ " World\"}</pre>";
+
+			// value should be expanded:
+			int index= actualHtmlContent.indexOf("<pre>{");
+			assertFalse(index == -1);
+			String actualSnippet= actualHtmlContent.substring(index, index + expectedCodeSequence.length());
+			assertEquals("sequence doesn't match", expectedCodeSequence, actualSnippet);
 		}
 	}