Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework string template #109

Closed
wants to merge 2 commits into from
Closed

Rework string template #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8db54be
Issue #14566: Improve lexer performance
nrmancuso Mar 3, 2024
acd0f84
minor: rework AST building/parsing
nrmancuso Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions config/checker-framework-suppressions/checker-nullness-optional-interning-suppressions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5358,6 +5358,13 @@
</details>
</checkerFrameworkError>

<checkerFrameworkError unstable="false">
<fileName>src/main/java/com/puppycrawl/tools/checkstyle/grammar/CompositeLexerContextCache.java</fileName>
<specifier>dereference.of.nullable</specifier>
<message>dereference of possibly-null reference currentContext</message>
<lineContent>if (currentContext.getCurlyBraceDepth() == 0) {</lineContent>
</checkerFrameworkError>

<checkerFrameworkError unstable="false">
<fileName>src/main/java/com/puppycrawl/tools/checkstyle/gui/BaseCellEditor.java</fileName>
<specifier>return</specifier>
Expand Down
6 changes: 6 additions & 0 deletions config/import-control.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,12 @@
<file name="CrAwareLexerSimulator">
<allow pkg="org.antlr.v4.runtime"/>
</file>
<file name="CompositeLexerContextCache">
<allow pkg="org.antlr.v4.runtime"/>
</file>
<file name="CompositeLexerContextCache">
<allow class="com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageLexer"/>
</file>
</subpackage>

<file name="Main">
Expand Down
4 changes: 4 additions & 0 deletions config/linkcheck-suppressions.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1341,3 +1341,7 @@
<a href="com/puppycrawl/tools/checkstyle/xpath/iterators/ReverseListIterator.html#%3Cinit%3E(java.util.Collection)">com/puppycrawl/tools/checkstyle/xpath/iterators/ReverseListIterator.html#%3Cinit%3E(java.util.Collection)</a>: doesn't exist.
<a href="com/puppycrawl/tools/checkstyle/utils/UnmodifiableCollectionUtil.html#copyOfArray(T%5B%5D,int)">com/puppycrawl/tools/checkstyle/utils/UnmodifiableCollectionUtil.html#copyOfArray(T%5B%5D,int)</a>: doesn't exist.
<a href="com/puppycrawl/tools/checkstyle/site/XdocsTemplateSink.CustomPrintWriter.html#%3Cinit%3E(java.io.Writer)">com/puppycrawl/tools/checkstyle/site/XdocsTemplateSink.CustomPrintWriter.html#%3Cinit%3E(java.io.Writer)</a>: doesn't exist.
<a href="com/puppycrawl/tools/checkstyle/grammar/CompositeLexerContextCache.html#%3Cinit%3E(org.antlr.v4.runtime.Lexer)">com/puppycrawl/tools/checkstyle/grammar/CompositeLexerContextCache.html#%3Cinit%3E(org.antlr.v4.runtime.Lexer)</a>: doesn't exist.
<a href="com/puppycrawl/tools/checkstyle/grammar/CompositeLexerContextCache.TemplateContext.html#%3Cinit%3E(int,int)">com/puppycrawl/tools/checkstyle/grammar/CompositeLexerContextCache.TemplateContext.html#%3Cinit%3E(int,int)</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/grammar/CompositeLexerContextCache.html#%3Cinit%3E(org.antlr.v4.runtime.Lexer)">#%3Cinit%3E(org.antlr.v4.runtime.Lexer)</a>: doesn't exist.
<a href="apidocs/com/puppycrawl/tools/checkstyle/grammar/CompositeLexerContextCache.TemplateContext.html#%3Cinit%3E(int,int)">#%3Cinit%3E(int,int)</a>: doesn't exist.
3 changes: 3 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4362,6 +4362,7 @@
<param>com.puppycrawl.tools.checkstyle.Checker*</param>
<param>com.puppycrawl.tools.checkstyle.ThreadModeSettings*</param>
<param>com.puppycrawl.tools.checkstyle.grammar.CrAwareLexerSimulator*</param>
<param>com.puppycrawl.tools.checkstyle.grammar.CompositeLexerContextCache*</param>
<!-- interfaces -->
<param>com.puppycrawl.tools.checkstyle.AuditEventFormatter*</param>
<param>com.puppycrawl.tools.checkstyle.XdocsPropertyType*</param>
Expand Down Expand Up @@ -4393,6 +4394,8 @@
<param>com.puppycrawl.tools.checkstyle.ThreadModeSettingsTest</param>
<param>com.puppycrawl.tools.checkstyle.grammar.CrAwareLexerSimulatorTest</param>
<param>com.puppycrawl.tools.checkstyle.grammar.javadoc.JavadocParseTreeTest</param>
<!-- this test is required for CompositeLexerContextCache -->
<param>com.puppycrawl.tools.checkstyle.grammar.java21.Java21AstRegressionTest</param>
<!-- this test is required for Checker -->
<param>com.puppycrawl.tools.checkstyle.filefilters.BeforeExecutionExclusionFileFilterTest</param>
<param>com.puppycrawl.tools.checkstyle.checks.TranslationCheckTest</param>
Expand Down
204 changes: 62 additions & 142 deletions src/main/java/com/puppycrawl/tools/checkstyle/JavaAstVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1808,164 +1808,84 @@ private static DetailAstImpl buildSimpleStringTemplateArgument(

@Override
public DetailAstImpl visitStringTemplate(JavaLanguageParser.StringTemplateContext ctx) {
final DetailAstImpl begin = buildStringTemplateBeginning(ctx);

final Optional<DetailAstImpl> startExpression = Optional.ofNullable(ctx.expr())
.map(this::visit);

if (startExpression.isPresent()) {
final DetailAstImpl imaginaryExpr =
createImaginary(TokenTypes.EMBEDDED_EXPRESSION);
imaginaryExpr.addChild(startExpression.orElseThrow());
begin.addChild(imaginaryExpr);
}
final DetailAstImpl stringTemplateBegin = visit(ctx.stringTemplateBegin());

Optional.ofNullable(ctx.expr())
.map(this::visit)
.ifPresent(expression -> {
final DetailAstImpl imaginaryExpression =
createImaginary(TokenTypes.EMBEDDED_EXPRESSION);
imaginaryExpression.addChild(expression);
stringTemplateBegin.addChild(imaginaryExpression);
});

ctx.stringTemplateMiddle().stream()
.map(this::buildStringTemplateMiddle)
.collect(Collectors.toUnmodifiableList())
.forEach(begin::addChild);

final DetailAstImpl end = buildStringTemplateEnd(ctx);
begin.addChild(end);
return begin;
}
.forEach(stringTemplateBegin::addChild);

/**
* Builds the beginning of a string template AST.
*
* @param ctx the StringTemplateContext to build AST from
* @return string template AST
*/
private static DetailAstImpl buildStringTemplateBeginning(
JavaLanguageParser.StringTemplateContext ctx) {

// token looks like '"' StringFragment '\{'
final TerminalNode context = ctx.STRING_TEMPLATE_BEGIN();
final Token token = context.getSymbol();
final String tokenText = context.getText();
final int tokenStartIndex = token.getCharPositionInLine();
final int tokenLineNumber = token.getLine();
final int tokenTextLength = tokenText.length();

final DetailAstImpl stringTemplateBegin = createImaginary(
TokenTypes.STRING_TEMPLATE_BEGIN, QUOTE,
tokenLineNumber, tokenStartIndex
);

// remove delimiters '"' and '\{'
final String stringFragment = tokenText.substring(
QUOTE.length(), tokenTextLength - EMBEDDED_EXPRESSION_BEGIN.length());

final DetailAstImpl stringTemplateContent = createImaginary(
TokenTypes.STRING_TEMPLATE_CONTENT, stringFragment,
tokenLineNumber, tokenStartIndex + QUOTE.length()
);
stringTemplateBegin.addChild(stringTemplateContent);

final DetailAstImpl embeddedBegin = createImaginary(
TokenTypes.EMBEDDED_EXPRESSION_BEGIN, EMBEDDED_EXPRESSION_BEGIN,
tokenLineNumber,
tokenStartIndex + tokenTextLength - EMBEDDED_EXPRESSION_BEGIN.length()
);
stringTemplateBegin.addChild(embeddedBegin);
final DetailAstImpl stringTemplateEnd = visit(ctx.stringTemplateEnd());
stringTemplateBegin.addChild(stringTemplateEnd);
return stringTemplateBegin;
}

/**
* Builds the middle of a string template AST.
* Builds a string template middle AST.
*
* @param middleContext the StringTemplateMiddleContext to build AST from
* @param ctx the StringTemplateMiddleContext to build AST from
* @return DetailAstImpl of string template middle
*/
private DetailAstImpl buildStringTemplateMiddle(
JavaLanguageParser.StringTemplateMiddleContext middleContext) {

// token looks like '}' StringFragment '\{'
final TerminalNode context = middleContext.STRING_TEMPLATE_MID();
final Token token = context.getSymbol();
final int tokenStartIndex = token.getCharPositionInLine();
final int tokenLineNumber = token.getLine();
final String tokenText = context.getText();
final int tokenTextLength = tokenText.length();

final DetailAstImpl embeddedExpressionEnd = createImaginary(
TokenTypes.EMBEDDED_EXPRESSION_END, EMBEDDED_EXPRESSION_END,
tokenLineNumber, tokenStartIndex
);

// remove delimiters '}' and '\\' '{'
final String stringFragment = tokenText.substring(
EMBEDDED_EXPRESSION_END.length(),
tokenTextLength - EMBEDDED_EXPRESSION_BEGIN.length()
);

final DetailAstImpl content = createImaginary(
TokenTypes.STRING_TEMPLATE_CONTENT, stringFragment,
tokenLineNumber, tokenStartIndex + EMBEDDED_EXPRESSION_END.length()
);
embeddedExpressionEnd.addNextSibling(content);

final DetailAstImpl embeddedBegin = createImaginary(
TokenTypes.EMBEDDED_EXPRESSION_BEGIN, EMBEDDED_EXPRESSION_BEGIN,
tokenLineNumber,
tokenStartIndex + tokenTextLength - EMBEDDED_EXPRESSION_BEGIN.length()
);
content.addNextSibling(embeddedBegin);

final Optional<DetailAstImpl> embeddedExpression = Optional.ofNullable(middleContext.expr())
.map(this::visit);

if (embeddedExpression.isPresent()) {
final DetailAstImpl imaginaryExpr =
createImaginary(TokenTypes.EMBEDDED_EXPRESSION);
imaginaryExpr.addChild(embeddedExpression.orElseThrow());
embeddedExpressionEnd.addNextSibling(imaginaryExpr);
}
JavaLanguageParser.StringTemplateMiddleContext ctx) {
final DetailAstImpl stringTemplateMiddle =
visit(ctx.stringTemplateMid());

Optional.ofNullable(ctx.expr())
.map(this::visit)
.ifPresent(expression -> {
final DetailAstImpl imaginaryExpression =
createImaginary(TokenTypes.EMBEDDED_EXPRESSION);
imaginaryExpression.addChild(expression);
addLastSibling(stringTemplateMiddle, imaginaryExpression);
});

return stringTemplateMiddle;
}

@Override
public DetailAstImpl visitStringTemplateBegin(
JavaLanguageParser.StringTemplateBeginContext ctx) {
final DetailAstImpl stringTemplateBegin =
create(ctx.STRING_TEMPLATE_BEGIN());
final Optional<DetailAstImpl> stringTemplateContent =
Optional.ofNullable(ctx.STRING_TEMPLATE_CONTENT())
.map(this::create);
stringTemplateContent.ifPresent(stringTemplateBegin::addChild);
final DetailAstImpl embeddedExpressionBegin = create(ctx.EMBEDDED_EXPRESSION_BEGIN());
stringTemplateBegin.addChild(embeddedExpressionBegin);
return stringTemplateBegin;
}

@Override
public DetailAstImpl visitStringTemplateMid(JavaLanguageParser.StringTemplateMidContext ctx) {
final DetailAstImpl embeddedExpressionEnd = create(ctx.EMBEDDED_EXPRESSION_END());
final Optional<DetailAstImpl> stringTemplateContent =
Optional.ofNullable(ctx.STRING_TEMPLATE_CONTENT())
.map(this::create);
stringTemplateContent.ifPresent(self -> addLastSibling(embeddedExpressionEnd, self));
final DetailAstImpl embeddedExpressionBegin = create(ctx.EMBEDDED_EXPRESSION_BEGIN());
addLastSibling(embeddedExpressionEnd, embeddedExpressionBegin);
return embeddedExpressionEnd;
}

/**
* Builds the end of a string template AST.
*
* @param ctx the StringTemplateContext to build AST from
* @return DetailAstImpl of string template end
*/
private static DetailAstImpl buildStringTemplateEnd(
JavaLanguageParser.StringTemplateContext ctx) {

// token looks like '}' StringFragment '"'
final TerminalNode context = ctx.STRING_TEMPLATE_END();
final Token token = context.getSymbol();
final String tokenText = context.getText();
final int tokenStartIndex = token.getCharPositionInLine();
final int tokenLineNumber = token.getLine();
final int tokenTextLength = tokenText.length();

final DetailAstImpl embeddedExpressionEnd = createImaginary(
TokenTypes.EMBEDDED_EXPRESSION_END, EMBEDDED_EXPRESSION_END,
tokenLineNumber, tokenStartIndex
);

// remove delimiters '}' and '"'
final String stringFragment = tokenText.substring(
EMBEDDED_EXPRESSION_END.length(),
tokenTextLength - QUOTE.length()
);

final DetailAstImpl endContent = createImaginary(
TokenTypes.STRING_TEMPLATE_CONTENT, stringFragment,
tokenLineNumber,
tokenStartIndex + EMBEDDED_EXPRESSION_END.length()
);
embeddedExpressionEnd.addNextSibling(endContent);

final DetailAstImpl stringTemplateEnd = createImaginary(
TokenTypes.STRING_TEMPLATE_END, QUOTE,
tokenLineNumber,
tokenStartIndex + tokenTextLength - QUOTE.length()
);
endContent.addNextSibling(stringTemplateEnd);
@Override
public DetailAstImpl visitStringTemplateEnd(JavaLanguageParser.StringTemplateEndContext ctx) {
final DetailAstImpl embeddedExpressionEnd = create(ctx.EMBEDDED_EXPRESSION_END());
final Optional<DetailAstImpl> stringTemplateContent =
Optional.ofNullable(ctx.STRING_TEMPLATE_CONTENT())
.map(this::create);
stringTemplateContent.ifPresent(self -> addLastSibling(embeddedExpressionEnd, self));
final DetailAstImpl stringTemplateEnd = create(ctx.STRING_TEMPLATE_END());
addLastSibling(embeddedExpressionEnd, stringTemplateEnd);
return embeddedExpressionEnd;
}

Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/puppycrawl/tools/checkstyle/JavaParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import com.puppycrawl.tools.checkstyle.api.FileContents;
import com.puppycrawl.tools.checkstyle.api.FileText;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.grammar.CompositeLexerContextCache;
import com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageLexer;
import com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser;
import com.puppycrawl.tools.checkstyle.utils.ParserUtil;
Expand Down Expand Up @@ -84,7 +85,9 @@ public static DetailAST parse(FileContents contents)
final String fullText = contents.getText().getFullText().toString();
final CharStream codePointCharStream = CharStreams.fromString(fullText);
final JavaLanguageLexer lexer = new JavaLanguageLexer(codePointCharStream, true);
final CompositeLexerContextCache contextCache = new CompositeLexerContextCache(lexer);
lexer.setCommentListener(contents);
lexer.setContextCache(contextCache);

final CommonTokenStream tokenStream = new CommonTokenStream(lexer);
final JavaLanguageParser parser =
Expand Down
Loading