Skip to content

Commit

Permalink
[23] markdown javadoc should detect code regions (#2821)
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
+ detect code block from 4+ leading spaces
+ ensuree DocCommentParser and JavadocParser are in sync

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
+ fix 1-off-bug in ASTConverterMarkdownTest.verifyPositions()
  + locally improve code structure (constant part outside the loop)

fixes #2808
  • Loading branch information
stephan-herrmann authored Aug 14, 2024
1 parent 677afd9 commit 3747271
Show file tree
Hide file tree
Showing 8 changed files with 448 additions and 28 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.lineStarted && 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,165 @@
/*******************************************************************************
* 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;
this.leadingSpaces = 0;
}
}
}

@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 >= 4);
}

@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 @@ -359,6 +359,7 @@ protected Object createReturnStatement() {
@Override
protected void createTag() {
this.tagValue = TAG_OTHERS_VALUE;
this.markdownHelper.resetLineStart();
}

@Override
Expand Down Expand Up @@ -616,10 +617,12 @@ protected boolean parseMarkdownLinks(int previousPosition) throws InvalidInputEx
}
currentChar = readChar();
}
this.markdownHelper.resetLineStart();
return valid;
}
@Override
protected boolean parseTag(int previousPosition) throws InvalidInputException {
this.markdownHelper.resetLineStart();

// Complain when tag is missing a description
// Note that if the parse of an inline tag has already started, consider it
Expand Down
Loading

0 comments on commit 3747271

Please sign in to comment.