Skip to content

Commit 3e4e2bb

Browse files
cgrubercgruber
committed
Fix an IAE that's thrown when attempting to contextualize empty nodes.
TreeContext formerly threw an IAE when requests for line and column information returned Diagnostic.NOPOS. No guarantee exists that valid nodes in a given CompilationUnitTree will be assigned position data other than Diagnostic.NOPOS. In practice, our implementation of SourcePosition returns this value for empty nodes (say, an empty modifier list). This change makes it so that IAE is only thrown when no NodePath exists between a TreeContext's compilation unit and the node provided. Additionally, it modifies the getNodeStartPostion and getNodeEndPosition methods so that they climb the TreePath when looking for position data. This is so that e.g. an empty modifier list may be contextualized to its associated class. ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=68841400
1 parent f9bedfb commit 3e4e2bb

File tree

3 files changed

+114
-32
lines changed

3 files changed

+114
-32
lines changed

‎src/main/java/com/google/testing/compile/TreeContext.java

Lines changed: 64 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.google.testing.compile;
1717

1818
import static com.google.common.base.Preconditions.checkArgument;
19+
import static javax.tools.Diagnostic.NOPOS;
1920

2021
import com.sun.source.tree.CompilationUnitTree;
2122
import com.sun.source.tree.LineMap;
@@ -24,8 +25,6 @@
2425
import com.sun.source.util.TreePath;
2526
import com.sun.source.util.Trees;
2627

27-
import javax.tools.Diagnostic;
28-
2928
/**
3029
* A class for managing and retrieving contextual information for Compilation Trees.
3130
*
@@ -74,71 +73,118 @@ TreePath getNodePath(Tree node) {
7473
}
7574

7675
/**
77-
* Returns start line of the given sub-{@code Tree} of this object's {@code CompilationUnitTree}.
76+
* Returns start line of the given sub-{@code Tree} of this object's {@code CompilationUnitTree},
77+
* climbing the associated {@code TreePath} until a value other than
78+
* {@link javax.tools.Diagnostic.NOPOS} is found.
79+
*
80+
* <p>This method will return {@link javax.tools.Diagnostic.NOPOS} if that value is returned
81+
* by a call to {@link SourcePositions#getStartPosition} for every node in the {@link TreePath}
82+
* provided.
7883
*
7984
* @throws IllegalArgumentException if the node provided is not a sub-{@code Tree} of this
8085
* object's {@code CompilationUnitTree}.
8186
*/
8287
long getNodeStartLine(Tree node) {
83-
return lineMap.getLineNumber(getNodeStartPosition(node));
88+
long startPosition = getNodeStartPosition(node);
89+
return startPosition == NOPOS ? NOPOS : lineMap.getLineNumber(startPosition);
8490
}
8591

8692
/**
8793
* Returns start column of the given sub-{@code Tree} of this object's
88-
* {@code CompilationUnitTree}.
94+
* {@code CompilationUnitTree}, climbing the associated {@code TreePath} until a value other than
95+
* {@link javax.tools.Diagnostic.NOPOS} is found.
96+
*
97+
* <p>This method will return {@link javax.tools.Diagnostic.NOPOS} if that value is returned
98+
* by a call to {@link SourcePositions#getStartPosition} for every node in the {@link TreePath}
99+
* provided.
89100
*
90101
* @throws IllegalArgumentException if the node provided is not a sub-{@code Tree} of this
91102
* object's {@code CompilationUnitTree}.
92103
*/
93104
long getNodeStartColumn(Tree node) {
94-
return lineMap.getColumnNumber(getNodeStartPosition(node));
105+
long startPosition = getNodeStartPosition(node);
106+
return startPosition == NOPOS ? NOPOS : lineMap.getColumnNumber(startPosition);
95107
}
96108

97109
/**
98110
* Returns end line of the given sub-{@code Tree} of this object's {@code CompilationUnitTree}.
111+
* climbing the associated {@code TreePath} until a value other than
112+
* {@link javax.tools.Diagnostic.NOPOS} is found.
113+
*
114+
* <p>This method will return {@link javax.tools.Diagnostic.NOPOS} if that value is returned
115+
* by a call to {@link SourcePositions#getEndPosition} for every node in the {@link TreePath}
116+
* provided.
99117
*
100118
* @throws IllegalArgumentException if the node provided is not a sub-{@code Tree} of this
101119
* object's {@code CompilationUnitTree}.
102120
*/
103121
long getNodeEndLine(Tree node) {
104-
return lineMap.getLineNumber(getNodeEndPosition(node));
122+
long endPosition = getNodeEndPosition(node);
123+
return endPosition == NOPOS ? NOPOS : lineMap.getLineNumber(endPosition);
105124
}
106125

107126
/**
108127
* Returns end column of the given sub-{@code Tree} of this object's {@code CompilationUnitTree}.
128+
* climbing the associated {@code TreePath} until a value other than
129+
* {@link javax.tools.Diagnostic.NOPOS} is found.
130+
*
131+
* <p>This method will return {@link javax.tools.Diagnostic.NOPOS} if that value is returned
132+
* by a call to {@link SourcePositions#getEndPosition} for every node in the {@link TreePath}
133+
* provided.
109134
*
110135
* @throws IllegalArgumentException if the node provided is not a sub-{@code Tree} of this
111136
* object's {@code CompilationUnitTree}.
112137
*/
113138
long getNodeEndColumn(Tree node) {
114-
return lineMap.getColumnNumber(getNodeEndPosition(node));
139+
long endPosition = getNodeEndPosition(node);
140+
return endPosition == NOPOS ? NOPOS : lineMap.getColumnNumber(endPosition);
115141
}
116142

117143
/**
118144
* Returns start position of the given sub-{@code Tree} of this object's
119-
* {@code CompilationUnitTree}.
145+
* {@code CompilationUnitTree}, climbing the associated {@code TreePath} until a value other than
146+
* {@link javax.tools.Diagnostic.NOPOS} is found.
147+
*
148+
* <p>This method will return {@link javax.tools.Diagnostic.NOPOS} if that value is returned
149+
* by a call to {@link SourcePositions#getStartPosition} for every node in the {@link TreePath}
150+
* provided.
120151
*
121152
* @throws IllegalArgumentException if the node provided is not a sub-{@code Tree} of this
122153
* object's {@code CompilationUnitTree}.
123154
*/
124155
long getNodeStartPosition(Tree node) {
125-
long startPosition = sourcePositions.getStartPosition(compilationUnit, node);
126-
checkArgument(startPosition != Diagnostic.NOPOS,
127-
"The node provided was not a subtree of this context's CompilationUnitTree: %s", node);
128-
return startPosition;
156+
TreePath currentNode = getNodePath(node);
157+
while (currentNode != null) {
158+
long startPosition = sourcePositions.getStartPosition(compilationUnit, currentNode.getLeaf());
159+
if (startPosition != NOPOS) {
160+
return startPosition;
161+
}
162+
currentNode = currentNode.getParentPath();
163+
}
164+
return NOPOS;
129165
}
130166

131167
/**
132168
* Returns end position of the given sub-{@code Tree} of this object's
133-
* {@code CompilationUnitTree}.
169+
* {@code CompilationUnitTree}, climbing the associated {@code TreePath} until a value other than
170+
* {@link javax.tools.Diagnostic.NOPOS} is found.
171+
*
172+
* <p>This method will return {@link javax.tools.Diagnostic.NOPOS} if that value is returned
173+
* by a call to {@link SourcePositions#getEndPosition} for every node in the {@link TreePath}
174+
* provided.
134175
*
135176
* @throws IllegalArgumentException if the node provided is not a sub-{@code Tree} of this
136177
* object's {@code CompilationUnitTree}.
137178
*/
138179
long getNodeEndPosition(Tree node) {
139-
long endPosition = sourcePositions.getEndPosition(compilationUnit, node);
140-
checkArgument(endPosition != Diagnostic.NOPOS,
141-
"The node provided was not a subtree of this context's CompilationUnitTree: %s", node);
142-
return endPosition;
180+
TreePath currentNode = getNodePath(node);
181+
while (node != null) {
182+
long endPosition = sourcePositions.getEndPosition(compilationUnit, currentNode.getLeaf());
183+
if (endPosition != NOPOS) {
184+
return endPosition;
185+
}
186+
currentNode = currentNode.getParentPath();
187+
}
188+
return NOPOS;
143189
}
144190
}

‎src/main/java/com/google/testing/compile/TreeDifference.java

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
*/
1616
package com.google.testing.compile;
1717

18+
import static javax.tools.Diagnostic.NOPOS;
19+
1820
import com.google.common.base.Joiner;
1921
import com.google.common.collect.ImmutableList;
2022

@@ -29,6 +31,7 @@
2931
* @author Stephen Pratt
3032
*/
3133
final class TreeDifference {
34+
private static final String NO_LINE = "[unavailable]";
3235

3336
private final ImmutableList<OneWayDiff> extraExpectedNodes;
3437
private final ImmutableList<OneWayDiff> extraActualNodes;
@@ -120,33 +123,39 @@ String getDiffReport(@Nullable TreeContext expectedContext, @Nullable TreeContex
120123
/** Creates a log entry about an extra node on the expected or actual tree. */
121124
private String createMessage(String details, TreePath nodePath, @Nullable TreeContext treeContext,
122125
boolean onExpected) {
123-
String contextStr = (treeContext == null) ? "[context unavailable]" : String.format(
124-
"Line %s %s.", treeContext.getNodeStartLine(nodePath.getLeaf()),
126+
long startLine = (treeContext == null)
127+
? NOPOS : treeContext.getNodeStartLine(nodePath.getLeaf());
128+
String contextStr = String.format("Line %s %s",
129+
(startLine == NOPOS) ? NO_LINE : startLine,
125130
Breadcrumbs.describeTreePath(nodePath));
126131
return Joiner.on('\n').join(
127132
String.format("> Extra node in %s tree.", onExpected ? "expected" : "actual"),
128-
String.format("\t %s", contextStr),
129-
String.format("\t Node contents: <%s>.", nodeContents(nodePath.getLeaf())),
130-
String.format("\t %s", details),
133+
String.format(" %s", contextStr),
134+
String.format(" Node contents: <%s>.", nodeContents(nodePath.getLeaf())),
135+
String.format(" %s", details),
131136
"");
132137
}
133138

134139
/** Creates a log entry about two differing nodes. */
135140
private String createMessage(String details, TreePath expectedNodePath,
136141
@Nullable TreeContext expectedTreeContext, TreePath actualNodePath,
137142
@Nullable TreeContext actualTreeContext) {
138-
String expectedContextStr = (expectedTreeContext == null)
139-
? "[context unavailable]" : String.format(
140-
"Line %s %s.", expectedTreeContext.getNodeStartLine(expectedNodePath.getLeaf()),
141-
Breadcrumbs.describeTreePath(expectedNodePath));
142-
String actualContextStr = (actualTreeContext == null) ? "[context unavailable]" : String.format(
143-
"Line %s %s.", actualTreeContext.getNodeStartLine(actualNodePath.getLeaf()),
143+
144+
long expectedTreeStartLine = (expectedTreeContext == null)
145+
? NOPOS : expectedTreeContext.getNodeStartLine(expectedNodePath.getLeaf());
146+
String expectedContextStr = String.format("Line %s %s",
147+
(expectedTreeStartLine == NOPOS) ? NO_LINE : expectedTreeStartLine,
148+
Breadcrumbs.describeTreePath(expectedNodePath));
149+
long actualTreeStartLine = (actualTreeContext == null)
150+
? NOPOS : actualTreeContext.getNodeStartLine(actualNodePath.getLeaf());
151+
String actualContextStr = String.format("Line %s %s",
152+
(actualTreeStartLine == NOPOS) ? NO_LINE : actualTreeStartLine,
144153
Breadcrumbs.describeTreePath(actualNodePath));
145154
return Joiner.on('\n').join(
146155
"> Difference in expected tree and actual tree.",
147-
String.format("\t Expected node: %s", expectedContextStr),
148-
String.format("\t Actual node: %s", actualContextStr),
149-
String.format("\t %s", details),
156+
String.format(" Expected node: %s", expectedContextStr),
157+
String.format(" Actual node: %s", actualContextStr),
158+
String.format(" %s", details),
150159
"");
151160
}
152161

‎src/test/java/com/google/testing/compile/TreeDifferenceTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,29 @@ public void getDiffReport_WithContext() {
132132
.contains(twoWayDiffContextStr());
133133
}
134134

135+
@Test
136+
public void getDiffReport_emptyElementContext() {
137+
CompilationUnitTree modifiersPresent =
138+
MoreTrees.parseLinesToTree("package test;",
139+
"final class TestClass {",
140+
" TestClass() {}",
141+
"}");
142+
CompilationUnitTree modifiersAbsent =
143+
MoreTrees.parseLinesToTree("package test;",
144+
"class TestClass {",
145+
" TestClass() {}",
146+
"}");
147+
TreeDifference diff =
148+
TreeDiffer.diffCompilationUnits(modifiersPresent, modifiersAbsent);
149+
ASSERT.that(
150+
diff.getDiffReport(treeContext(modifiersPresent), treeContext(modifiersAbsent))
151+
.isEmpty()).isFalse();
152+
diff = TreeDiffer.diffCompilationUnits(modifiersAbsent, modifiersPresent);
153+
ASSERT.that(
154+
diff.getDiffReport(treeContext(modifiersAbsent), treeContext(modifiersPresent))
155+
.isEmpty()).isFalse();
156+
}
157+
135158
private TreeDifference emptyDiff() {
136159
return new TreeDifference();
137160
}
@@ -200,4 +223,8 @@ private TreeDifference multiDiffs() {
200223
private TreeContext treeContext() {
201224
return new TreeContext(COMPILATION_UNIT, TREES);
202225
}
226+
227+
private TreeContext treeContext(CompilationUnitTree compilationUnit) {
228+
return new TreeContext(compilationUnit, TREES);
229+
}
203230
}

0 commit comments

Comments
 (0)