Skip to content

Commit 61d7c7b

Browse files
bbakermanbbakerman
authored
The line numbers of elements are no longer off by one (graphql-java#1513)
1 parent 9214b60 commit 61d7c7b

17 files changed

Lines changed: 268 additions & 72 deletions

‎src/main/java/graphql/language/Document.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.util.ArrayList;
1010
import java.util.List;
1111
import java.util.function.Consumer;
12+
import java.util.stream.Collectors;
1213

1314
import static graphql.language.NodeChildrenContainer.newNodeChildrenContainer;
1415

@@ -38,6 +39,21 @@ public List<Definition> getDefinitions() {
3839
return new ArrayList<>(definitions);
3940
}
4041

42+
/**
43+
* Returns a list of definitions of the specific type. It uses {@link java.lang.Class#isAssignableFrom(Class)} for the test
44+
*
45+
* @param definitionClass the definition class
46+
* @param <T> the type of definition
47+
*
48+
* @return a list of definitions of that class or empty list
49+
*/
50+
public <T extends Definition> List<T> getDefinitionsOfType(Class<T> definitionClass) {
51+
return definitions.stream()
52+
.filter(d -> definitionClass.isAssignableFrom(d.getClass()))
53+
.map(definitionClass::cast)
54+
.collect(Collectors.toList());
55+
}
56+
4157
@Override
4258
public List<Node> getChildren() {
4359
return new ArrayList<>(definitions);

‎src/main/java/graphql/language/SelectionSet.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import java.util.Collection;
1111
import java.util.List;
1212
import java.util.function.Consumer;
13+
import java.util.stream.Collectors;
1314

1415
import static graphql.language.NodeChildrenContainer.newNodeChildrenContainer;
1516

@@ -39,6 +40,21 @@ public List<Selection> getSelections() {
3940
return new ArrayList<>(selections);
4041
}
4142

43+
/**
44+
* Returns a list of selections of the specific type. It uses {@link java.lang.Class#isAssignableFrom(Class)} for the test
45+
*
46+
* @param selectionClass the selection class
47+
* @param <T> the type of selection
48+
*
49+
* @return a list of selections of that class or empty list
50+
*/
51+
public <T extends Selection> List<T> getSelectionsOfType(Class<T> selectionClass) {
52+
return selections.stream()
53+
.filter(d -> selectionClass.isAssignableFrom(d.getClass()))
54+
.map(selectionClass::cast)
55+
.collect(Collectors.toList());
56+
}
57+
4258
@Override
4359
public List<Node> getChildren() {
4460
List<Node> result = new ArrayList<>();

‎src/main/java/graphql/parser/ExtendedBailStrategy.java‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package graphql.parser;
22

33
import graphql.language.SourceLocation;
4-
import graphql.parser.MultiSourceReader.SourceAndLine;
54
import org.antlr.v4.runtime.BailErrorStrategy;
65
import org.antlr.v4.runtime.Parser;
76
import org.antlr.v4.runtime.RecognitionException;
@@ -10,6 +9,8 @@
109

1110
import java.util.List;
1211

12+
import static graphql.parser.SourceLocationHelper.mkSourceLocation;
13+
1314
public class ExtendedBailStrategy extends BailErrorStrategy {
1415
private final MultiSourceReader multiSourceReader;
1516

@@ -36,12 +37,8 @@ public Token recoverInline(Parser recognizer) throws RecognitionException {
3637
}
3738

3839
InvalidSyntaxException mkMoreTokensException(Token token) {
39-
SourceAndLine sourceAndLine = multiSourceReader.getSourceAndLineFromOverallLine(token.getLine());
40-
int column = token.getCharPositionInLine();
41-
42-
// graphql spec says line numbers start at 1
43-
SourceLocation sourceLocation = new SourceLocation(sourceAndLine.getLine()+1, column, sourceAndLine.getSourceName());
44-
String sourcePreview = mkPreview(token.getLine());
40+
SourceLocation sourceLocation = mkSourceLocation(multiSourceReader, token);
41+
String sourcePreview = mkPreview(token);
4542
return new InvalidSyntaxException(sourceLocation,
4643
"There are more tokens in the query that have not been consumed",
4744
sourcePreview, token.getText(), null);
@@ -54,19 +51,16 @@ private InvalidSyntaxException mkException(Parser recognizer, RecognitionExcepti
5451
SourceLocation sourceLocation = null;
5552
Token currentToken = recognizer.getCurrentToken();
5653
if (currentToken != null) {
57-
int tokenLine = currentToken.getLine();
58-
int column = currentToken.getCharPositionInLine();
59-
SourceAndLine sourceAndLine = multiSourceReader.getSourceAndLineFromOverallLine(tokenLine);
54+
sourceLocation = mkSourceLocation(multiSourceReader, currentToken);
6055
offendingToken = currentToken.getText();
61-
sourcePreview = mkPreview(tokenLine);
62-
// graphql spec says line numbers start at 1
63-
sourceLocation = new SourceLocation(sourceAndLine.getLine()+1, column, sourceAndLine.getSourceName());
56+
sourcePreview = mkPreview(currentToken);
6457
}
6558
return new InvalidSyntaxException(sourceLocation, null, sourcePreview, offendingToken, cause);
6659
}
6760

6861
/* grabs 3 lines before and after the syntax error */
69-
private String mkPreview(int line) {
62+
private String mkPreview(Token token) {
63+
int line = token.getLine() - 1;
7064
StringBuilder sb = new StringBuilder();
7165
int startLine = line - 3;
7266
int endLine = line + 3;

‎src/main/java/graphql/parser/GraphqlAntlrToLanguage.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -810,11 +810,7 @@ protected Description newDescription(GraphqlParser.DescriptionContext descriptio
810810
}
811811

812812
protected SourceLocation getSourceLocation(Token token) {
813-
MultiSourceReader.SourceAndLine sourceAndLine = multiSourceReader.getSourceAndLineFromOverallLine(token.getLine());
814-
int column = token.getCharPositionInLine() + 1;
815-
// graphql spec says line numbers start at 1
816-
int line = sourceAndLine.getLine() + 1;
817-
return new SourceLocation(line, column, sourceAndLine.getSourceName());
813+
return SourceLocationHelper.mkSourceLocation(multiSourceReader, token);
818814
}
819815

820816
protected SourceLocation getSourceLocation(ParserRuleContext parserRuleContext) {

‎src/main/java/graphql/parser/MultiSourceReader.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,14 @@ public String getSourceName() {
7979
public int getLine() {
8080
return line;
8181
}
82+
83+
@Override
84+
public String toString() {
85+
return "SourceAndLine{" +
86+
"sourceName='" + sourceName + '\'' +
87+
", line=" + line +
88+
'}';
89+
}
8290
}
8391

8492
/**
@@ -95,6 +103,11 @@ public SourceAndLine getSourceAndLineFromOverallLine(int overallLineNumber) {
95103
if (sourceParts.isEmpty()) {
96104
return sourceAndLine;
97105
}
106+
if (overallLineNumber == 0) {
107+
sourceAndLine.sourceName = sourceParts.get(0).sourceName;
108+
sourceAndLine.line = 0;
109+
return sourceAndLine;
110+
}
98111
SourcePart currentPart;
99112
if (currentIndex >= sourceParts.size()) {
100113
currentPart = sourceParts.get(sourceParts.size() - 1);
@@ -107,9 +120,9 @@ public SourceAndLine getSourceAndLineFromOverallLine(int overallLineNumber) {
107120
sourceAndLine.sourceName = sourcePart.sourceName;
108121
if (sourcePart == currentPart) {
109122
// we cant go any further
110-
int offset = currentPart.lineReader.getLineNumber();
123+
int partLineNumber = currentPart.lineReader.getLineNumber();
111124
previousPage = page;
112-
page += offset;
125+
page += partLineNumber;
113126
if (page > overallLineNumber) {
114127
sourceAndLine.line = overallLineNumber - previousPage;
115128
} else {
@@ -118,7 +131,8 @@ public SourceAndLine getSourceAndLineFromOverallLine(int overallLineNumber) {
118131
return sourceAndLine;
119132
} else {
120133
previousPage = page;
121-
page += sourcePart.lineReader.getLineNumber();
134+
int partLineNumber = sourcePart.lineReader.getLineNumber();
135+
page += partLineNumber;
122136
if (page > overallLineNumber) {
123137
sourceAndLine.line = overallLineNumber - previousPage;
124138
return sourceAndLine;

‎src/main/java/graphql/parser/SourceLocationHelper.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package graphql.parser;
2+
3+
import graphql.language.SourceLocation;
4+
import org.antlr.v4.runtime.Token;
5+
6+
class SourceLocationHelper {
7+
8+
static SourceLocation mkSourceLocation(MultiSourceReader multiSourceReader, Token token) {
9+
//
10+
// multi source reader lines are 0 based while Antler lines are 1's based
11+
//
12+
// Antler columns ironically are 0 based - go figure!
13+
//
14+
int tokenLine = token.getLine() - 1;
15+
MultiSourceReader.SourceAndLine sourceAndLine = multiSourceReader.getSourceAndLineFromOverallLine(tokenLine);
16+
//
17+
// graphql spec says line numbers and columns start at 1
18+
int line = sourceAndLine.getLine() + 1;
19+
int column = token.getCharPositionInLine() + 1;
20+
return new SourceLocation(line, column, sourceAndLine.getSourceName());
21+
}
22+
23+
}

‎src/test/groovy/graphql/DataFetcherWithErrorsAndDataTest.groovy‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ class DataFetcherWithErrorsAndDataTest extends Specification {
108108
result.errors.size() == 1
109109
result.errors[0].path == ["root", "parent", "child", "badField"]
110110
result.errors[0].message == "badField is bad"
111-
result.errors[0].locations == [new SourceLocation(7, 27)]
111+
result.errors[0].locations == [new SourceLocation(6, 27)]
112112
113113
result.data["root"]["parent"]["child"]["goodField"] == "good"
114114
result.data["root"]["parent"]["child"]["badField"] == null
@@ -191,10 +191,10 @@ class DataFetcherWithErrorsAndDataTest extends Specification {
191191
result.errors.size() == 2
192192
result.errors[0].path == ["root", "parent", "child", "goodField"]
193193
result.errors[0].message == "goodField is bad"
194-
result.errors[0].locations == [new SourceLocation(8, 31)]
194+
result.errors[0].locations == [new SourceLocation(7, 31)]
195195
result.errors[1].path == ["root", "parent", "child", "badField"]
196196
result.errors[1].message == "badField is bad"
197-
result.errors[1].locations == [new SourceLocation(8, 31)]
197+
result.errors[1].locations == [new SourceLocation(7, 31)]
198198
199199
result.data["root"]["parent"]["child"]["goodField"] == null
200200
result.data["root"]["parent"]["child"]["badField"] == null
@@ -268,11 +268,11 @@ class DataFetcherWithErrorsAndDataTest extends Specification {
268268
result.errors.size() == 2
269269
result.errors[0].path == ["parent", "child"]
270270
result.errors[0].message == "error 1"
271-
result.errors[0].locations == [new SourceLocation(7, 27)]
271+
result.errors[0].locations == [new SourceLocation(6, 27)]
272272
273273
result.errors[1].path == ["parent", "child"]
274274
result.errors[1].message == "error 2"
275-
result.errors[1].locations == [new SourceLocation(7, 27)]
275+
result.errors[1].locations == [new SourceLocation(6, 27)]
276276
277277
result.data["parent"]["child"] == null
278278
@@ -344,7 +344,7 @@ class DataFetcherWithErrorsAndDataTest extends Specification {
344344
result.errors.size() == 1
345345
result.errors[0].path == ["parent", "child", "badField"]
346346
result.errors[0].message == "badField is bad"
347-
result.errors[0].locations == [new SourceLocation(7, 27)]
347+
result.errors[0].locations == [new SourceLocation(6, 27)]
348348
349349
result.data["parent"]["child"]["goodField"] == "good"
350350
result.data["parent"]["child"]["badField"] == null

‎src/test/groovy/graphql/GraphQLTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ class GraphQLTest extends Specification {
139139
then:
140140
errors.size() == 1
141141
errors[0].errorType == ErrorType.InvalidSyntax
142-
errors[0].locations == [new SourceLocation(1, 8)]
142+
errors[0].locations == [new SourceLocation(1, 9)]
143143
}
144144

145145
def "query with invalid syntax 2"() {
@@ -156,7 +156,7 @@ class GraphQLTest extends Specification {
156156
then:
157157
errors.size() == 1
158158
errors[0].errorType == ErrorType.InvalidSyntax
159-
errors[0].locations == [new SourceLocation(1, 7)]
159+
errors[0].locations == [new SourceLocation(1, 8)]
160160
}
161161

162162
def "non null argument is missing"() {

‎src/test/groovy/graphql/IssueNonNullDefaultAttribute.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class IssueNonNullDefaultAttribute extends Specification {
5454
result.errors.size() == 1
5555
result.errors[0].errorType == ErrorType.ValidationError
5656
result.errors[0].message == "Validation error of type WrongType: argument 'characterNumber' with value 'NullValue{}' must not be null @ 'name'"
57-
result.errors[0].locations == [new SourceLocation(4, 26)]
57+
result.errors[0].locations == [new SourceLocation(3, 26)]
5858
result.data == null
5959

6060
}

0 commit comments

Comments
 (0)