Skip to content

Commit 2343fdc

Browse files
gkdncopybara-github
gkdn
authored and
copybara-github
committed
Improve position reporting.
This change centralizes the logic for finding the closest meaningful source position, allowing all passes/checks to use a consistent best effort method. This also fixes jsinterop restriction checking tests since some errors were getting effectively deduped during assertion. PiperOrigin-RevId: 887084158
1 parent bfa5fb0 commit 2343fdc

File tree

7 files changed

+41
-41
lines changed

7 files changed

+41
-41
lines changed

‎transpiler/java/com/google/j2cl/common/visitor/BUILD‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ java_library(
1212
# Do not use. Temporary visible to workaround https://github.com/bazelbuild/bazel/issues/25214.
1313
"//visibility:public",
1414
],
15-
exports = [":visitor-internal"],
15+
exports = [
16+
":visitor-internal",
17+
"//transpiler/java/com/google/j2cl/common",
18+
],
1619
)
1720

1821
java_library(

‎transpiler/java/com/google/j2cl/common/visitor/generator/ProcessorPrivateClass.vm‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package ${packageName};
22

33
import com.google.j2cl.common.visitor.Processor;
4+
import com.google.j2cl.common.SourcePosition;
5+
import com.google.j2cl.common.HasSourcePosition;
46
import java.util.ArrayDeque;
57
import java.util.Deque;
68
import java.util.function.Predicate;
@@ -46,6 +48,27 @@ abstract class ProcessorPrivate implements Processor {
4648
return getParents().filter(predicate).findFirst().orElse(null);
4749
}
4850

51+
/** Returns the closest meaningful source position from an enclosing node. */
52+
public final SourcePosition getSourcePosition() {
53+
// TODO(b/494674927): This might skip Type and return source position of
54+
// parent Type. E.g.
55+
// 'class A { class B {} }', returns source info for A instead of B.
56+
HasSourcePosition hasSourcePosition =
57+
(HasSourcePosition)
58+
getParent(
59+
p ->
60+
p instanceof HasSourcePosition hs
61+
&& hs.getSourcePosition() != SourcePosition.NONE);
62+
if (hasSourcePosition == null) {
63+
if (getCurrentContext() instanceof HasSourcePosition contextPosition) {
64+
hasSourcePosition = contextPosition;
65+
}
66+
}
67+
return hasSourcePosition != null
68+
? hasSourcePosition.getSourcePosition()
69+
: SourcePosition.NONE;
70+
};
71+
4972
final void pushParent(Object p) {
5073
stackOfParent.push(p);
5174
}

‎transpiler/java/com/google/j2cl/transpiler/passes/AbstractJ2ktNormalizationPass.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@
2121
import com.google.common.collect.ImmutableList;
2222
import com.google.common.collect.ImmutableSet;
2323
import com.google.errorprone.annotations.FormatMethod;
24-
import com.google.j2cl.common.HasSourcePosition;
2524
import com.google.j2cl.common.SourcePosition;
26-
import com.google.j2cl.transpiler.ast.AbstractRewriter;
2725
import com.google.j2cl.transpiler.ast.ArrayTypeDescriptor;
2826
import com.google.j2cl.transpiler.ast.DeclaredTypeDescriptor;
2927
import com.google.j2cl.transpiler.ast.IntersectionTypeDescriptor;
@@ -60,13 +58,6 @@ final void debug(String detailMessage, Object... args) {
6058
debug(SourcePosition.NONE, detailMessage, args);
6159
}
6260

63-
/** Returns the closest meaningful source position from an enclosing node. */
64-
final SourcePosition getSourcePosition(AbstractRewriter rewriter) {
65-
HasSourcePosition hasSourcePosition =
66-
(HasSourcePosition) rewriter.getParent(HasSourcePosition.class::isInstance);
67-
return hasSourcePosition != null ? hasSourcePosition.getSourcePosition() : SourcePosition.NONE;
68-
}
69-
7061
/**
7162
* Converts captures to wildcards if they appear as a type argument, or project them to bounds if
7263
* they appear at the top-level (non as type argument).

‎transpiler/java/com/google/j2cl/transpiler/passes/ConversionContextVisitor.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
import com.google.common.collect.ImmutableList;
2222
import com.google.common.collect.Iterables;
23-
import com.google.j2cl.common.HasSourcePosition;
2423
import com.google.j2cl.common.SourcePosition;
2524
import com.google.j2cl.transpiler.ast.AbstractRewriter;
2625
import com.google.j2cl.transpiler.ast.ArrayAccess;
@@ -103,15 +102,7 @@ protected abstract static class ContextRewriter {
103102

104103
/** Returns the closest meaningful source position from an enclosing node. */
105104
public final SourcePosition getSourcePosition() {
106-
HasSourcePosition hasSourcePosition =
107-
(HasSourcePosition)
108-
visitor.getParent(
109-
p ->
110-
p instanceof HasSourcePosition hs
111-
&& hs.getSourcePosition() != SourcePosition.NONE);
112-
return hasSourcePosition != null
113-
? hasSourcePosition.getSourcePosition()
114-
: SourcePosition.NONE;
105+
return visitor.getSourcePosition();
115106
}
116107

117108
public final Stream<Object> getParents() {

‎transpiler/java/com/google/j2cl/transpiler/passes/InsertQualifierProjectionCasts.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ private Expression projectExpression(Expression expression) {
107107

108108
DebugDescriber describer = newDebugDescriber();
109109
debug(
110-
getSourcePosition(this),
110+
getSourcePosition(),
111111
"Inserting qualifier projection cast from %s to %s",
112112
describer.getDescription(typeDescriptor),
113113
describer.getDescription(projectedTypeDescriptor));

‎transpiler/java/com/google/j2cl/transpiler/passes/JsInteropRestrictionsChecker.java‎

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -339,23 +339,14 @@ private void checkParameterization(
339339
(tv, value) -> {
340340
if (tv.toRawTypeDescriptor().isNative() != value.isNative()) {
341341
problems.error(
342-
getBestSourcePosition(),
342+
getSourcePosition(),
343343
"%s %s cannot be parameterized with native JsType '%s'. (b/290992813)",
344344
prefix,
345345
context.getReadableDescription(),
346346
value.getReadableDescription());
347347
}
348348
});
349349
}
350-
351-
private SourcePosition getBestSourcePosition() {
352-
HasSourcePosition hasSourcePosition =
353-
((HasSourcePosition) getParent(HasSourcePosition.class::isInstance));
354-
if (hasSourcePosition == null) {
355-
hasSourcePosition = getCurrentType();
356-
}
357-
return hasSourcePosition.getSourcePosition();
358-
}
359350
});
360351
}
361352

@@ -387,8 +378,7 @@ public void exitBinaryExpression(BinaryExpression binaryExpression) {
387378
? binaryExpression.getLeftOperand()
388379
: binaryExpression.getRightOperand();
389380
problems.error(
390-
((HasSourcePosition) getParent(HasSourcePosition.class::isInstance))
391-
.getSourcePosition(),
381+
getSourcePosition(),
392382
"%s cannot be compared with non-native type.",
393383
getReadableDescriptionWithPrefix(operand.getTypeDescriptor()));
394384
}
@@ -2762,9 +2752,7 @@ public void exitVariable(Variable variable) {
27622752
variableTypeDescriptor,
27632753
isTypeDisallowed,
27642754
onlyCheckTypeSpecialization,
2765-
sourcePosition == SourcePosition.NONE
2766-
? getCurrentMember().getSourcePosition()
2767-
: sourcePosition,
2755+
sourcePosition == SourcePosition.NONE ? getSourcePosition() : sourcePosition,
27682756
messageSuffix,
27692757
"Variable '%s'",
27702758
variable.getName());
@@ -2817,7 +2805,7 @@ public void exitFieldAccess(FieldAccess fieldAccess) {
28172805
declaredTypeDescriptor,
28182806
isTypeDisallowed,
28192807
onlyCheckTypeSpecialization,
2820-
getCurrentMember().getSourcePosition(),
2808+
getSourcePosition(),
28212809
messageSuffix,
28222810
"Reference to field '%s'",
28232811
fieldAccess.getTarget().getReadableDescription());
@@ -2838,7 +2826,7 @@ public void exitMethodCall(MethodCall methodCall) {
28382826
declaredTypeDescriptor,
28392827
isTypeDisallowed,
28402828
onlyCheckTypeSpecialization,
2841-
getCurrentMember().getSourcePosition(),
2829+
getSourcePosition(),
28422830
messageSuffix,
28432831
"Returned type in call to method '%s'",
28442832
methodCall.getTarget().getReadableDescription());
@@ -2852,7 +2840,7 @@ public void exitNewArray(NewArray newArray) {
28522840
newArrayTypeDescriptor,
28532841
isTypeDisallowed,
28542842
onlyCheckTypeSpecialization,
2855-
getCurrentMember().getSourcePosition(),
2843+
getSourcePosition(),
28562844
messageSuffix,
28572845
"Array creation '%s'",
28582846
// TODO(b/65465035): Emit the expression source position when it is tracked, and
@@ -2868,7 +2856,7 @@ public void exitNewInstance(NewInstance newInstance) {
28682856
instanceTypeDescriptor,
28692857
isTypeDisallowed,
28702858
onlyCheckTypeSpecialization,
2871-
getCurrentMember().getSourcePosition(),
2859+
getSourcePosition(),
28722860
messageSuffix,
28732861
"Object creation '%s'",
28742862
// TODO(b/65465035): Emit the expression source position when it is tracked, and
@@ -2903,7 +2891,7 @@ public void exitCastExpression(CastExpression castExpression) {
29032891
onlyCheckTypeSpecialization)) {
29042892
// TODO(b/65465035): Emit the expression source position when it is tracked.
29052893
problems.error(
2906-
getCurrentMember().getSourcePosition(),
2894+
getSourcePosition(),
29072895
"Cannot cast to %s '%s'.%s",
29082896
disallowedTypeDescription,
29092897
castTypeDescriptor.getReadableDescription(),

‎transpiler/javatests/com/google/j2cl/transpiler/JsInteropRestrictionsCheckerTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2940,9 +2940,13 @@ class DerivedWithJsEnumArrayField extends BaseWithTArrayField<Main.MyJsEnum> {}
29402940
"Returned type in call to method 'List<MyJsEnum[][]> Main.instanceReturnsTArrayList()'"
29412941
+ " cannot be of type 'List<MyJsEnum[][]>'.",
29422942
"Object creation 'new Main.<init>()' cannot be of type 'Main<MyJsEnum[]>'.",
2943+
"Object creation 'new Main.<init>()' cannot be of type 'Main<MyJsEnum[]>'.",
2944+
"Object creation 'new Main.<init>()' cannot be of type 'Main<MyJsEnum[]>'.",
29432945
"Reference to field 'Main<MyJsEnum[]>.t' cannot be of type 'MyJsEnum[]'.",
29442946
"Reference to field 'Main<MyJsEnum>.tArray' cannot be of type 'MyJsEnum[]'.",
29452947
"Reference to field 'Main<MyJsEnum>.tArrayArray' cannot be of type 'MyJsEnum[][]'.",
2948+
"Returned type in call to method 'MyJsEnum[] List.get(int)' cannot be of type"
2949+
+ " 'MyJsEnum[]'.",
29462950
"Returned type in call to method 'MyJsEnum[] List.get(int)' cannot be of type"
29472951
+ " 'MyJsEnum[]'.",
29482952
// Array access qualifier in `List<E>.get(a)[b]` where the array expression is `E`.

0 commit comments

Comments
 (0)