Skip to content

Commit

Permalink
[23] markdown javadoc should detect code regions
Browse files Browse the repository at this point in the history
+ finetune parsing start of line by new companion IMarkdownCommentHelper
  + detect excess slashes after '///'
  + determine minimum amount of whitespace per markdown comment
  + preserve additional whitespace at beginning of line

Also:
+ avoid showing closing ']' of links
+ resolve one warning (redundant null check)
+ fix and extend compliance settings in RunCompletionParserTests
+ fix & extend test source in ../Converter_23/src/markdown/testBug228648
  • Loading branch information
stephan-herrmann committed Aug 14, 2024
1 parent 16e2f4d commit 9f5f88c
Show file tree
Hide file tree
Showing 7 changed files with 440 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ public abstract class AbstractCommentParser implements JavadocTagConstants {
private int nonRegionTagCount, inlineTagCount;
final static String SINGLE_LINE_COMMENT = "//"; //$NON-NLS-1$

protected IMarkdownCommentHelper markdownHelper;

// Line pointers
private int linePtr, lastLinePtr;

Expand Down Expand Up @@ -194,6 +196,7 @@ protected boolean commentParse() {
int previousPosition = this.index;
char nextCharacter = 0;
this.markdown = this.source[this.javadocStart + 1] == '/';
this.markdownHelper = IMarkdownCommentHelper.create(this);
if (realStart == this.javadocStart) {
nextCharacter = readChar(); // second '*' or '/'
if (!this.markdown) {
Expand Down Expand Up @@ -248,7 +251,7 @@ protected boolean commentParse() {
case '@' :
// Start tag parsing only if we are on line beginning or at inline tag beginning
// https://bugs.eclipse.org/bugs/show_bug.cgi?id=206345: ignore all tags when inside @literal or @code tags
if (considerTagAsPlainText) {
if (considerTagAsPlainText || this.markdownHelper.isInCodeBlock()) {
// new tag found
if (!this.lineStarted) {
// we may want to report invalid syntax when no closing brace found,
Expand Down Expand Up @@ -287,7 +290,7 @@ protected boolean commentParse() {
invalidInlineTagLineEnd = this.lineEnd;
} else if (this.textStart != -1 && this.textStart < invalidTagLineEnd) {
if(!lookForTagsInSnippets())
pushText(this.textStart, invalidTagLineEnd);
pushText(this.textStart, invalidTagLineEnd);
}
this.scanner.resetTo(this.index, this.javadocEnd);
this.currentTokenType = -1; // flush token cache at line begin
Expand Down Expand Up @@ -338,6 +341,7 @@ protected boolean commentParse() {
lineHasStar = false;
// Fix bug 51650
this.textStart = -1;
this.markdownHelper.resetAtLineEnd();
break;
case '}' :
if (verifText && this.tagValue == TAG_RETURN_VALUE && this.returnStatement != null) {
Expand Down Expand Up @@ -417,6 +421,9 @@ protected boolean commentParse() {
}
} else if (this.lineStarted && isDomParser) {
textEndPosition = this.index;
} else if (this.markdownHelper.recordSignificantLeadingSpace()) {
if (this.textStart == -1)
this.textStart = this.index; // first relevant whitespace is start of text
}
break;
case '*' :
Expand All @@ -443,23 +450,27 @@ protected boolean commentParse() {
//$FALL-THROUGH$
case '/':
if (this.markdown) {
if (nextCharacter != '*') // fall-through
break;
this.markdownHelper.recordSlash(this.index);
break;
} else if (previousChar == '*') {
// End of javadoc
break;
}
// $FALL-THROUGH$ - fall through default case
default :
if (this.markdown && nextCharacter == '[') {
if (this.textStart != -1) {
if (this.textStart < textEndPosition) {
pushText(this.textStart, textEndPosition);
if (this.markdown) {
if (nextCharacter == '[') {
if (this.textStart != -1) {
if (this.textStart < textEndPosition) {
pushText(this.textStart, textEndPosition);
}
}
}
if (parseMarkdownLinks(previousPosition)) {
this.textStart = this.index;
break;
if (parseMarkdownLinks(previousPosition)) {
this.textStart = this.index;
break;
}
} else if (nextCharacter == '`') {
this.markdownHelper.recordBackTick(this.lineStarted);
}
}
if (isFormatterParser && nextCharacter == '<') {
Expand Down Expand Up @@ -1460,7 +1471,7 @@ else if (this.tagValue == TAG_VALUE_VALUE) {
}

// Verify that we got a reference
if (reference == null) reference = typeRef;
reference = typeRef;
if (reference == null) {
this.index = this.tokenPreviousPosition;
this.scanner.currentPosition = this.tokenPreviousPosition;
Expand Down Expand Up @@ -3234,6 +3245,75 @@ private boolean considerNextChar() {
return consider;
}

/** compute the amount of indentation common to all non-blank lines of this markdown comment. */
int peekMarkdownCommonIndent(int start) {
final int START = 0;
final int TEXT = 1;
final int NEWLINE = 2;
int slashesSeen = 3;
int min = Integer.MAX_VALUE;
int textLineStart = start;
int state = START;
int idxSave = this.index;
this.index = start;
char nlChar = '\0';
try {
while (true) {
if (this.index >= this.scanner.eofPosition)
return min;
char c = readChar();
switch (state) {
case START -> {
if (c != ' ') {
if (c == '\r' || c == '\n') {
// "blank" line, i.e., no text after "///"
state = NEWLINE;
nlChar = c;
slashesSeen = 0;
} else {
min = Math.min(min, this.index - 1 - textLineStart);
state = TEXT;
}
}
}
case TEXT -> {
if (c == '\r' || c == '\n') {
state = NEWLINE;
nlChar = c;
slashesSeen = 0;
}
}
case NEWLINE -> {
switch (c) {
case '\n' -> {
if (nlChar == '\r') {
// saw "\r\n" -> no change
} else {
return min; // blank line seen
}
}
case ' ', '\t' -> {
nlChar = '\0';
}
case '/' -> {
if (++slashesSeen == 3) {
textLineStart = this.index;
state = START;
slashesSeen = 0;
}
}
default -> {
return min; // not a markdown line
}
}
}
}
}
} finally {
this.index = idxSave;
}
}

/*
* Read token only if previous was consumed
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
/*******************************************************************************
* Copyright (c) 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
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* This is an implementation of an early-draft specification developed under the Java
* Community Process (JCP) and is made available for testing and evaluation purposes
* only. The code is not compatible with any specification of the JCP.
*
* Contributors:
* Stephan Herrmann - Initial API and implementation
*******************************************************************************/
package org.eclipse.jdt.internal.compiler.parser;

/**
* Companion class for AbstractCommentParser to decide significance of whitespace
* within a markdown comment,
* and to detect code blocks (either by indentation or fenced with {@code ```}).
*/
public interface IMarkdownCommentHelper {

void recordSlash(int nextIndex);

void recordBackTick(boolean lineStarted);

/**
* When at the beginning of a comment line, record that a whitespace was seen.
* @return {@code true} if this whitespace is significant,
* i.e., beyond the common indent of all lines of this commonmark comment.
*/
boolean recordSignificantLeadingSpace();

boolean isInCodeBlock();

/** Retrieve the start of the current text, possibly including significant leading whitespace. */
int getTextStart(int textStart);

/** Call me when we are past the first element of a line. */
void resetLineStart();

void resetAtLineEnd();


static IMarkdownCommentHelper create(AbstractCommentParser parser) {
boolean markdown = parser.source[parser.javadocStart + 1] == '/';
if (markdown) {
int lineStart = parser.javadocStart + 3;
int commonIndent = parser.peekMarkdownCommonIndent(lineStart);
return new MarkdownCommentHelper(lineStart, commonIndent);
} else {
return new NullMarkdownHelper();
}
}

}

class NullMarkdownHelper implements IMarkdownCommentHelper {
public NullMarkdownHelper() {
}
@Override
public void recordSlash(int nextIndex) {
// nop
}
@Override
public void recordBackTick(boolean lineStarted) {
// nop
}
@Override
public boolean recordSignificantLeadingSpace() {
return false;
}
@Override
public boolean isInCodeBlock() {
return false;
}
@Override
public int getTextStart(int textStart) {
return textStart;
}
@Override
public void resetLineStart() {
// nop
}
@Override
public void resetAtLineEnd() {
// nop
}
}
class MarkdownCommentHelper implements IMarkdownCommentHelper {

int commonIndent;
int slashCount = 0;
int leadingSpaces = 0;
int markdownLineStart = -1;
int backTickCount = 0;
boolean insideFence = false;

public MarkdownCommentHelper(int lineStart, int commonIndent) {
this.markdownLineStart = lineStart;
this.commonIndent = commonIndent;
}

@Override
public void recordSlash(int nextIndex) {
if (this.slashCount < 3) {
if (++this.slashCount == 3)
this.markdownLineStart = nextIndex;
}
}

@Override
public void recordBackTick(boolean lineStarted) {
if (this.backTickCount < 3) {
if (this.backTickCount == 0 && lineStarted) {
return;
}
if (++this.backTickCount == 3) {
this.insideFence^=true;
}
}
}

@Override
public boolean recordSignificantLeadingSpace() {
if (this.markdownLineStart != -1) {
if (++this.leadingSpaces > this.commonIndent)
return true;
}
return false;
}

@Override
public boolean isInCodeBlock() {
return this.insideFence || this.leadingSpaces > this.commonIndent;
}

@Override
public int getTextStart(int textStart) {
if (this.markdownLineStart > -1) {
return this.markdownLineStart + this.commonIndent;
}
return textStart;
}

@Override
public void resetLineStart() {
this.markdownLineStart = -1;
}

@Override
public void resetAtLineEnd() {
this.slashCount = 0;
this.leadingSpaces = 0;
this.markdownLineStart = -1;
this.backTickCount = 0;
// do not reset `insideFence`
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,39 @@ public static Test suite() {
TestCase.RUN_ONLY_ID = null;
all.addTest(AbstractCompilerTest.buildComplianceTestSuite(ClassFileConstants.JDK10, tests_10));
}
if ((possibleComplianceLevels & AbstractCompilerTest.F_11) != 0) {
ArrayList tests_11 = (ArrayList)testClasses.clone();
tests_11.addAll(TEST_CLASSES_1_5);
// Reset forgotten subsets tests
TestCase.TESTS_PREFIX = null;
TestCase.TESTS_NAMES = null;
TestCase.TESTS_NUMBERS= null;
TestCase.TESTS_RANGE = null;
TestCase.RUN_ONLY_ID = null;
all.addTest(AbstractCompilerTest.buildComplianceTestSuite(ClassFileConstants.JDK11, tests_11));
}
if ((possibleComplianceLevels & AbstractCompilerTest.F_17) != 0) {
ArrayList tests_17 = (ArrayList)testClasses.clone();
tests_17.addAll(TEST_CLASSES_1_5);
// Reset forgotten subsets tests
TestCase.TESTS_PREFIX = null;
TestCase.TESTS_NAMES = null;
TestCase.TESTS_NUMBERS= null;
TestCase.TESTS_RANGE = null;
TestCase.RUN_ONLY_ID = null;
all.addTest(AbstractCompilerTest.buildComplianceTestSuite(ClassFileConstants.JDK17, tests_17));
}
if ((possibleComplianceLevels & AbstractCompilerTest.F_21) != 0) {
ArrayList tests_21 = (ArrayList)testClasses.clone();
tests_21.addAll(TEST_CLASSES_1_5);
// Reset forgotten subsets tests
TestCase.TESTS_PREFIX = null;
TestCase.TESTS_NAMES = null;
TestCase.TESTS_NUMBERS= null;
TestCase.TESTS_RANGE = null;
TestCase.RUN_ONLY_ID = null;
all.addTest(AbstractCompilerTest.buildComplianceTestSuite(ClassFileConstants.JDK21, tests_21));
}
if ((possibleComplianceLevels & AbstractCompilerTest.F_23) != 0) {
ArrayList tests_23 = (ArrayList)testClasses.clone();
tests_23.addAll(TEST_CLASSES_23);
Expand All @@ -106,7 +139,7 @@ public static Test suite() {
TestCase.TESTS_NUMBERS= null;
TestCase.TESTS_RANGE = null;
TestCase.RUN_ONLY_ID = null;
all.addTest(AbstractCompilerTest.buildComplianceTestSuite(ClassFileConstants.JDK10, tests_23));
all.addTest(AbstractCompilerTest.buildComplianceTestSuite(ClassFileConstants.JDK23, tests_23));
}
return all;
}
Expand Down
Loading

0 comments on commit 9f5f88c

Please sign in to comment.