Skip to content

Commit e5bff9c

Browse files
lauraharkercopybara-github
authored andcommitted
Fix JSType equality bug leading to bad type errors with native module checks.
This is a partial fix for b/140763807. The underlying issue is that while the typed universe is still being created, before a type is ultimately marked "resolved", nominal types are compared solely based on name. The Closure type system allows unique types in different scopes to share a name, though. This fixes that bug for the specific case of one or both names being in a different goog.module. It's not a general fix, but should be enough to prevent this bug from blocking native module typechecking for goog.module. PiperOrigin-RevId: 705118303
1 parent b9dd7d8 commit e5bff9c

File tree

2 files changed

+55
-1
lines changed

2 files changed

+55
-1
lines changed

src/com/google/javascript/rhino/jstype/EqualityChecker.java

+14-1
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,10 @@ private boolean areEqualInternal(JSType left, JSType right) {
235235
// TODO(b/140763807): this is not valid across scopes pre-resolution.
236236
String nameOfleft = checkNotNull(leftUnwrapped.getReferenceName());
237237
String nameOfright = checkNotNull(rightUnwrapped.getReferenceName());
238-
return Objects.equals(nameOfleft, nameOfright);
238+
String googModuleIdOfLeft = getGoogModuleId(leftUnwrapped);
239+
String googModuleIdOfRight = getGoogModuleId(rightUnwrapped);
240+
return Objects.equals(nameOfleft, nameOfright)
241+
&& Objects.equals(googModuleIdOfLeft, googModuleIdOfRight);
239242
}
240243
}
241244

@@ -286,6 +289,16 @@ private boolean areUnionEqual(UnionType left, UnionType right) {
286289
return true;
287290
}
288291

292+
private static @Nullable String getGoogModuleId(ObjectType type) {
293+
if (type instanceof FunctionType) {
294+
return ((FunctionType) type).getGoogModuleId();
295+
}
296+
if (type.getConstructor() != null) {
297+
return type.getConstructor().getGoogModuleId();
298+
}
299+
return null;
300+
}
301+
289302
/**
290303
* Two function types are equal if their signatures match. Since they don't have signatures, two
291304
* interfaces are equal if their names match.

test/com/google/javascript/rhino/jstype/JSTypeTest.java

+41
Original file line numberDiff line numberDiff line change
@@ -6437,6 +6437,47 @@ public void testEqualityOfClassTypes_withSameReferenceName_preResolution() {
64376437
}
64386438
}
64396439

6440+
@Test
6441+
public void testEqualityOfClassTypes_withSameReferenceName_differentGoogModuleId_preResolution() {
6442+
try (JSTypeResolver.Closer closer = registry.getResolver().openForDefinition()) {
6443+
FunctionType classACtor =
6444+
FunctionType.builder(registry)
6445+
.forConstructor()
6446+
.withName("Foo")
6447+
.setGoogModuleId("module1")
6448+
.build();
6449+
6450+
FunctionType classBCtor =
6451+
FunctionType.builder(registry).forConstructor().withName("Foo").build();
6452+
6453+
// the different goog module id indicate the types are not equal.
6454+
assertType(classACtor.getInstanceType()).isNotEqualTo(classBCtor.getInstanceType());
6455+
}
6456+
}
6457+
6458+
@Test
6459+
public void testEqualityOfClassTypes_withSameReferenceName_sameGoogModuleId_preResolution() {
6460+
try (JSTypeResolver.Closer closer = registry.getResolver().openForDefinition()) {
6461+
FunctionType classACtor =
6462+
FunctionType.builder(registry)
6463+
.forConstructor()
6464+
.withName("Foo")
6465+
.setGoogModuleId("module1")
6466+
.build();
6467+
6468+
FunctionType classBCtor =
6469+
FunctionType.builder(registry)
6470+
.forConstructor()
6471+
.withName("Foo")
6472+
.setGoogModuleId("module1")
6473+
.build();
6474+
6475+
// Currently, to handle NamedTypes, we treat unresolved type equality as purely based on
6476+
// reference name & goog module id.
6477+
assertType(classACtor.getInstanceType()).isEqualTo(classBCtor.getInstanceType());
6478+
}
6479+
}
6480+
64406481
@Test
64416482
public void testEqualityOfClassTypes_withSameReferenceName_postResolution() {
64426483
FunctionType classACtor =

0 commit comments

Comments
 (0)