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.

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

Add ParserOption to ignore single-line comments #2788

Merged
merged 1 commit into from Apr 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/main/java/graphql/language/Comment.java
Expand Up @@ -4,6 +4,9 @@

import java.io.Serializable;

/**
* A single-line comment. These are comments that start with a {@code #} in source documents.
*/
@PublicApi
public class Comment implements Serializable {
public final String content;
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/graphql/parser/GraphqlAntlrToLanguage.java
Expand Up @@ -82,6 +82,7 @@
@Internal
public class GraphqlAntlrToLanguage {

private static final List<Comment> NO_COMMENTS = ImmutableKit.emptyList();
private static final int CHANNEL_COMMENTS = 2;
private static final int CHANNEL_IGNORED_CHARS = 3;
private final CommonTokenStream tokens;
Expand Down Expand Up @@ -866,6 +867,10 @@ protected SourceLocation getSourceLocation(Token token) {
}

protected List<Comment> getComments(ParserRuleContext ctx) {
if (!parserOptions.isCaptureLineComments()) {
return NO_COMMENTS;
}

Token start = ctx.getStart();
if (start != null) {
int tokPos = start.getTokenIndex();
Expand All @@ -874,7 +879,7 @@ protected List<Comment> getComments(ParserRuleContext ctx) {
return getCommentOnChannel(refChannel);
}
}
return ImmutableKit.emptyList();
return NO_COMMENTS;
}


Expand Down
28 changes: 26 additions & 2 deletions src/main/java/graphql/parser/ParserOptions.java
Expand Up @@ -26,6 +26,7 @@ public class ParserOptions {
private static ParserOptions defaultJvmParserOptions = newParserOptions()
.captureIgnoredChars(false)
.captureSourceLocation(true)
.captureLineComments(true)
.maxTokens(MAX_QUERY_TOKENS) // to prevent a billion laughs style attacks, we set a default for graphql-java

.build();
Expand Down Expand Up @@ -66,12 +67,14 @@ public static void setDefaultParserOptions(ParserOptions options) {

private final boolean captureIgnoredChars;
private final boolean captureSourceLocation;
private final boolean captureLineComments;
private final int maxTokens;
private final ParsingListener parsingListener;

private ParserOptions(Builder builder) {
this.captureIgnoredChars = builder.captureIgnoredChars;
this.captureSourceLocation = builder.captureSourceLocation;
this.captureLineComments = builder.captureLineComments;
this.maxTokens = builder.maxTokens;
this.parsingListener = builder.parsingListener;
}
Expand All @@ -80,7 +83,7 @@ private ParserOptions(Builder builder) {
* Significant memory savings can be made if we do NOT capture ignored characters,
* especially in SDL parsing. So we have set this to false by default.
*
* @return true if ignored chars are captured in AST nodes
* @return true if ignored chars should be captured as AST nodes
*/
public boolean isCaptureIgnoredChars() {
return captureIgnoredChars;
Expand All @@ -91,14 +94,28 @@ public boolean isCaptureIgnoredChars() {
* Memory savings can be made if we do NOT set {@link graphql.language.SourceLocation}s
* on AST nodes, especially in SDL parsing.
*
* @return true if {@link graphql.language.SourceLocation}s are captured in AST nodes
* @return true if {@link graphql.language.SourceLocation}s should be captured as AST nodes
*
* @see graphql.language.SourceLocation
*/
public boolean isCaptureSourceLocation() {
return captureSourceLocation;
}

/**
* Single-line {@link graphql.language.Comment}s do not have any semantic meaning in
* GraphQL source documents, as such you may wish to ignore them.
* <p>
* This option does not ignore documentation {@link graphql.language.Description}s.
*
* @return true if {@link graphql.language.Comment}s should be captured as AST nodes
*
* @see graphql.language.SourceLocation
*/
public boolean isCaptureLineComments() {
return captureLineComments;
}

/**
* A graphql hacking vector is to send nonsensical queries that burn lots of parsing CPU time and burn
* memory representing a document that won't ever execute. To prevent this you can set a maximum number of parse
Expand Down Expand Up @@ -128,6 +145,7 @@ public static class Builder {

private boolean captureIgnoredChars = false;
private boolean captureSourceLocation = true;
private boolean captureLineComments = true;
private int maxTokens = MAX_QUERY_TOKENS;
private ParsingListener parsingListener = ParsingListener.NOOP;

Expand All @@ -137,6 +155,7 @@ public static class Builder {
Builder(ParserOptions parserOptions) {
this.captureIgnoredChars = parserOptions.captureIgnoredChars;
this.captureSourceLocation = parserOptions.captureSourceLocation;
this.captureLineComments = parserOptions.captureLineComments;
this.maxTokens = parserOptions.maxTokens;
this.parsingListener = parserOptions.parsingListener;
}
Expand All @@ -151,6 +170,11 @@ public Builder captureSourceLocation(boolean captureSourceLocation) {
return this;
}

public Builder captureLineComments(boolean captureLineComments) {
this.captureLineComments = captureLineComments;
return this;
}

public Builder maxTokens(int maxTokens) {
this.maxTokens = maxTokens;
return this;
Expand Down
27 changes: 25 additions & 2 deletions src/test/groovy/graphql/parser/ParserTest.groovy
Expand Up @@ -42,6 +42,7 @@ import graphql.language.VariableDefinition
import graphql.language.VariableReference
import org.antlr.v4.runtime.CommonTokenStream
import org.antlr.v4.runtime.ParserRuleContext
import spock.lang.Issue
import spock.lang.Specification
import spock.lang.Unroll

Expand Down Expand Up @@ -373,6 +374,28 @@ class ParserTest extends Specification {
helloField.comments.collect { c -> c.content } == [" this is some comment, which should be captured"]
}

@Issue("https://github.com/graphql-java/graphql-java/issues/2767")
def "parser does not transform comments to AST nodes when ParserOptions.captureLineComments(false)"() {
given:
def input = """
{ # this is some comment, which should be captured
hello(arg: "hello, world" ) # test
}
"""
def parserOptionsWithoutCaptureLineComments = ParserOptions.newParserOptions()
.captureLineComments(false)
.build()

when:
def document = new Parser().parseDocument(input, parserOptionsWithoutCaptureLineComments)
Field helloField = (document.definitions[0] as OperationDefinition).selectionSet.selections[0] as Field

then:
isEqual(helloField, new Field("hello", [new Argument("arg", new StringValue("hello, world"))]))
assert helloField.comments.isEmpty() // No single-line comments on lone fields
assert document.comments.isEmpty() // No single-line comments in entire document
}

@Unroll
def "parse floatValue #floatString"() {
given:
Expand Down Expand Up @@ -1118,7 +1141,7 @@ triple3 : """edge cases \\""" "" " \\"" \\" edge cases"""
then:
doc != null
count == 9
tokens == ["query" , "{", "f" , "(", "arg", ":", "1", ")", "}"]
tokens == ["query", "{", "f", "(", "arg", ":", "1", ")", "}"]

when: "integration test to prove it be supplied via EI"

Expand All @@ -1138,7 +1161,7 @@ triple3 : """edge cases \\""" "" " \\"" \\" edge cases"""
then:
er.errors.size() == 0
count == 9
tokens == ["query" , "{", "f" , "(", "arg", ":", "1", ")", "}"]
tokens == ["query", "{", "f", "(", "arg", ":", "1", ")", "}"]

}
}