Skip to content

Commit

Permalink
Make getTypeThroughNamespace null tolerant to vars with null type
Browse files Browse the repository at this point in the history
This pattern does actually show up in real code (although rarely), as the added
unit test demonstates.

Also move it to TypedScope so it can be shared by both TypeInference
and TypedScopeCreator.

PiperOrigin-RevId: 558196464
  • Loading branch information
blickly authored and copybara-github committed Aug 18, 2023
1 parent 11f3f72 commit b817001
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 31 deletions.
15 changes: 1 addition & 14 deletions src/com/google/javascript/jscomp/TypeInference.java
Original file line number Diff line number Diff line change
Expand Up @@ -1996,26 +1996,13 @@ private FlowScope setCallNodeTypeAfterChildrenTraversed(Node n, FlowScope scopeA

// We validated that this is a valid "provided name" above so it is ok to look it up by
// properties.
return getTypeThroughNamespace(otherModuleScope, moduleId);
return otherModuleScope.getTypeThroughNamespace(moduleId);
}
}

return unknownType;
}

private JSType getTypeThroughNamespace(TypedScope globalScope, String moduleId) {
int split = moduleId.lastIndexOf('.');
if (split >= 0) {
String parentName = moduleId.substring(0, split);
String prop = moduleId.substring(split + 1);
return getTypeThroughNamespace(globalScope, parentName)
.assertObjectType()
.getPropertyType(prop);
} else {
return globalScope.getSlot(moduleId).getType();
}
}

private FlowScope tightenTypesAfterAssertions(FlowScope scope, Node callNode) {
Node left = callNode.getFirstChild();
Node firstParam = left.getNext();
Expand Down
16 changes: 16 additions & 0 deletions src/com/google/javascript/jscomp/TypedScope.java
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,22 @@ private boolean isDeclarativelyUnboundVarWithoutType(TypedVar var) {
}
}

final @Nullable JSType getTypeThroughNamespace(String moduleId) {
int split = moduleId.lastIndexOf('.');
if (split >= 0) {
String parentName = moduleId.substring(0, split);
String prop = moduleId.substring(split + 1);
JSType parentType = getTypeThroughNamespace(parentName);
if (parentType == null || parentType.toMaybeObjectType() == null) {
return null;
}
return parentType.assertObjectType().getPropertyType(prop);
} else {
TypedVar var = this.getSlot(moduleId);
return var != null ? var.getType() : null;
}
}

@Override
public @Nullable StaticScope getTopmostScopeOfEventualDeclaration(String name) {
if (getOwnSlot(name) != null || reservedNames.contains(name)) {
Expand Down
18 changes: 1 addition & 17 deletions src/com/google/javascript/jscomp/TypedScopeCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ void resolve() {
} else if (resolvedScope != null) {
// Some imports might not exist as fully qualified names in the given scope, but are still
// resolvable via properties. This is always true for "provideAlreadyProvided" names.
JSType nsType = getTypeThroughNamespace(resolvedScope, scopedImport.getName());
JSType nsType = resolvedScope.getTypeThroughNamespace(scopedImport.getName());
type = nsType != null ? nsType : unknownType;
isInferred = (nsType == null);
} else {
Expand All @@ -273,22 +273,6 @@ void resolve() {
}
}

private static @Nullable JSType getTypeThroughNamespace(TypedScope scope, String moduleId) {
int split = moduleId.lastIndexOf('.');
if (split >= 0) {
String parentName = moduleId.substring(0, split);
String prop = moduleId.substring(split + 1);
JSType parentType = getTypeThroughNamespace(scope, parentName);
if (parentType.toMaybeObjectType() != null) {
return parentType.assertObjectType().getPropertyType(prop);
}
return null;
} else {
TypedVar var = scope.getSlot(moduleId);
return var != null ? var.getType() : null;
}
}

/** Stores the type and qualified name for a destructuring rvalue. */
private static class RValueInfo {
final @Nullable JSType type;
Expand Down
9 changes: 9 additions & 0 deletions test/com/google/javascript/jscomp/TypedScopeCreatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -7892,6 +7892,15 @@ public void testRequireType_inheritanceChainWithIdenticalClassAndInterfaceName()
"exports = Foo;")));
}

@Test
public void testUnorderedGoogProvideAndModule_doesntCrash() {
testSame(
srcs(
"goog.provide('ns.boo');",
lines("goog.module('test');", "const boo = goog.require('ns.boo');", ""),
lines("ns = ns || {};", "ns.boo = ns.boo || {};")));
}

@Test
public void testLegacyGoogModule_redeclaredAsVar_reportsError() {
// This is a bad pattern but should not crash the compiler.
Expand Down

0 comments on commit b817001

Please sign in to comment.