Skip to content

Commit

Permalink
Fix JSType equality bug leading to bad type errors with native module…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
lauraharker authored and copybara-github committed Dec 11, 2024
1 parent b9dd7d8 commit e5bff9c
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
15 changes: 14 additions & 1 deletion src/com/google/javascript/rhino/jstype/EqualityChecker.java
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,10 @@ private boolean areEqualInternal(JSType left, JSType right) {
// TODO(b/140763807): this is not valid across scopes pre-resolution.
String nameOfleft = checkNotNull(leftUnwrapped.getReferenceName());
String nameOfright = checkNotNull(rightUnwrapped.getReferenceName());
return Objects.equals(nameOfleft, nameOfright);
String googModuleIdOfLeft = getGoogModuleId(leftUnwrapped);
String googModuleIdOfRight = getGoogModuleId(rightUnwrapped);
return Objects.equals(nameOfleft, nameOfright)
&& Objects.equals(googModuleIdOfLeft, googModuleIdOfRight);
}
}

Expand Down Expand Up @@ -286,6 +289,16 @@ private boolean areUnionEqual(UnionType left, UnionType right) {
return true;
}

private static @Nullable String getGoogModuleId(ObjectType type) {
if (type instanceof FunctionType) {
return ((FunctionType) type).getGoogModuleId();
}
if (type.getConstructor() != null) {
return type.getConstructor().getGoogModuleId();
}
return null;
}

/**
* Two function types are equal if their signatures match. Since they don't have signatures, two
* interfaces are equal if their names match.
Expand Down
41 changes: 41 additions & 0 deletions test/com/google/javascript/rhino/jstype/JSTypeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -6437,6 +6437,47 @@ public void testEqualityOfClassTypes_withSameReferenceName_preResolution() {
}
}

@Test
public void testEqualityOfClassTypes_withSameReferenceName_differentGoogModuleId_preResolution() {
try (JSTypeResolver.Closer closer = registry.getResolver().openForDefinition()) {
FunctionType classACtor =
FunctionType.builder(registry)
.forConstructor()
.withName("Foo")
.setGoogModuleId("module1")
.build();

FunctionType classBCtor =
FunctionType.builder(registry).forConstructor().withName("Foo").build();

// the different goog module id indicate the types are not equal.
assertType(classACtor.getInstanceType()).isNotEqualTo(classBCtor.getInstanceType());
}
}

@Test
public void testEqualityOfClassTypes_withSameReferenceName_sameGoogModuleId_preResolution() {
try (JSTypeResolver.Closer closer = registry.getResolver().openForDefinition()) {
FunctionType classACtor =
FunctionType.builder(registry)
.forConstructor()
.withName("Foo")
.setGoogModuleId("module1")
.build();

FunctionType classBCtor =
FunctionType.builder(registry)
.forConstructor()
.withName("Foo")
.setGoogModuleId("module1")
.build();

// Currently, to handle NamedTypes, we treat unresolved type equality as purely based on
// reference name & goog module id.
assertType(classACtor.getInstanceType()).isEqualTo(classBCtor.getInstanceType());
}
}

@Test
public void testEqualityOfClassTypes_withSameReferenceName_postResolution() {
FunctionType classACtor =
Expand Down

0 comments on commit e5bff9c

Please sign in to comment.