Skip to content

Commit

Permalink
Keep typedefs around during NTI, so we can find them when evaluating …
Browse files Browse the repository at this point in the history 
…TTL ASTs.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164644707
  • Loading branch information
@dimvar @blickly
dimvar authored and blickly committed Aug 8, 2017
1 parent 3462da4 commit 7f73bc5
Show file tree
Hide file tree
Showing 10 changed files with 108 additions and 42 deletions.
5 changes: 2 additions & 3 deletions src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java
View file
Expand Up @@ -435,8 +435,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
this.funNameGen = null;

// If a scope s1 contains a scope s2, then s2 must be before s1 in scopes.
// The type inference relies on this fact to process deeper scopes
// before shallower scopes.
// The type inference relies on this fact to process deeper scopes before shallower scopes.
Collections.reverse(getScopes());

this.compiler.setExternProperties(ImmutableSet.copyOf(getExternPropertyNames()));
Expand Down Expand Up @@ -1016,7 +1015,7 @@ private void visitTypedef(Node qnameNode) {
return;
}
JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(qnameNode);
Typedef td = Typedef.make(jsdoc.getTypedefType());
Typedef td = Typedef.make(qnameNode, jsdoc.getTypedefType());
currentScope.addTypedef(qnameNode, td);
}

Expand Down
39 changes: 25 additions & 14 deletions src/com/google/javascript/jscomp/NTIScope.java
View file
Expand Up @@ -77,6 +77,8 @@ final class NTIScope implements DeclaredTypeRegistry, Serializable, TypeIEnv<JST
private final Map<String, NTIScope> localFunDefs = new LinkedHashMap<>();
private ImmutableSet<String> unknownTypeNames = ImmutableSet.of();
private Map<String, Typedef> localTypedefs = new LinkedHashMap<>();
// Typedefs defined inside this scope, but on a namespace, not as local variables
private Set<Typedef> namespaceTypedefs = new LinkedHashSet<>();
private Map<String, Namespace> localNamespaces = new LinkedHashMap<>();
// The namespace map that we preserve post-finalization, purely for use
// in GlobalTypeInfo for symbol table purposes.
Expand Down Expand Up @@ -482,8 +484,13 @@ RawNominalType getNominalType(QualifiedName qname) {
}

Typedef getTypedef(String name) {
checkState(!name.contains("."));
Declaration decl = getDeclaration(name, false);
QualifiedName qname = QualifiedName.fromQualifiedString(name);
Declaration decl;
if (qname.isIdentifier()) {
decl = getDeclaration(qname, true);
} else {
decl = getNamespace(qname.getLeftmostName()).getDeclaration(qname.getAllButLeftmost());
}
return decl == null ? null : decl.getTypedef();
}

Expand Down Expand Up @@ -545,6 +552,7 @@ void addTypedef(Node qnameNode, Typedef td) {
QualifiedName qname = QualifiedName.fromNode(qnameNode);
Namespace ns = getNamespace(qname.getLeftmostName());
ns.addTypedef(qname.getAllButLeftmost(), td);
namespaceTypedefs.add(td);
}
}

Expand Down Expand Up @@ -672,9 +680,13 @@ public JSType getInstanceType(String typeName) {
}

@Override
public JSType getNamespaceType(String typeName) {
public JSType getNamespaceOrTypedefType(String typeName) {
Namespace ns = getNamespaceAfterFreezing(typeName);
return ns == null ? null : ns.toJSType();
if (ns != null) {
return ns.toJSType();
}
Typedef td = getTypedef(typeName);
return td == null ? null : td.getType();
}

@Override
Expand All @@ -690,20 +702,20 @@ public JSDocInfo getJsdocOfTypeDeclaration(String typeName) {
}

void resolveTypedefs(JSTypeCreatorFromJSDoc typeParser) {
for (Typedef td : localTypedefs.values()) {
if (!td.isResolved()) {
typeParser.resolveTypedef(td, this);
}
for (Typedef td : this.localTypedefs.values()) {
typeParser.resolveTypedef(td, this);
}
for (Typedef td : this.namespaceTypedefs) {
typeParser.resolveTypedef(td, this);
}
this.namespaceTypedefs = null;
}

void resolveEnums(JSTypeCreatorFromJSDoc typeParser) {
for (EnumType e : localEnums) {
if (!e.isResolved()) {
typeParser.resolveEnum(e, this);
}
for (EnumType e : this.localEnums) {
typeParser.resolveEnum(e, this);
}
localEnums = null;
this.localEnums = null;
}

void freezeScope() {
Expand Down Expand Up @@ -738,7 +750,6 @@ void freezeScope() {
copyOuterVarsTransitively(this);
preservedNamespaces = localNamespaces;
localNamespaces = ImmutableMap.of();
localTypedefs = ImmutableMap.of();
escapedVars = ImmutableSet.of();
isFrozen = true;
}
Expand Down
21 changes: 12 additions & 9 deletions src/com/google/javascript/jscomp/TypeTransformation.java
View file
Expand Up @@ -132,23 +132,26 @@ private Keywords nameToKeyword(String s) {
}

private TypeI getType(String typeName) {
// Resolve the name and get the corresponding type
TypeI type = typeEnv.getNamespaceType(typeName);
JSDocInfo jsdoc = typeEnv.getJsdocOfTypeDeclaration(typeName);
TypeI type = registry.getType(typeName);
if (type != null) {
return type;
}
type = typeEnv.getNamespaceOrTypedefType(typeName);
if (type != null) {
// Case constructor, get the instance type
if (type.isConstructor() || type.isInterface()) {
return type.toMaybeFunctionType().getInstanceType().getRawType();
}
// Case enum
if (type.isEnumElement()) {
return type.getEnumeratedTypeOfEnumElement();
}
} else if (jsdoc != null && jsdoc.hasTypedefType()) {
return type;
}
JSDocInfo jsdoc = typeEnv.getJsdocOfTypeDeclaration(typeName);
if (jsdoc != null && jsdoc.hasTypedefType()) {
// This branch is only live when we are running the old type checker
return this.registry.evaluateTypeExpression(jsdoc.getTypedefType(), typeEnv);
}
// Otherwise handle native types
return registry.getType(typeName);
return null;
}

private TypeI getUnknownType() {
Expand Down Expand Up @@ -757,7 +760,7 @@ private TypeI evalMaprecord(Node ttlAst, NameResolver nameResolver) {

private TypeI evalTypeOfVar(Node ttlAst) {
String name = getCallArgument(ttlAst, 0).getString();
TypeI type = typeEnv.getNamespaceType(name);
TypeI type = typeEnv.getNamespaceOrTypedefType(name);
if (type == null) {
reportWarning(ttlAst, VAR_UNDEFINED, name);
return getUnknownType();
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/TypedScope.java
View file
Expand Up @@ -295,7 +295,7 @@ public Scope getClosestHoistScope() {

@SuppressWarnings("unchecked")
@Override
public JSType getNamespaceType(String typeName) {
public JSType getNamespaceOrTypedefType(String typeName) {
StaticTypedSlot<JSType> slot = getSlot(typeName);
return slot == null ? null : slot.getType();
}
Expand Down
5 changes: 1 addition & 4 deletions src/com/google/javascript/jscomp/newtypes/Declaration.java
View file
Expand Up @@ -47,11 +47,8 @@ public Declaration(JSType simpleType, Typedef typedef, Namespace ns,
}

private void checkValid() {
if (this.simpleType != null) {
checkState(this.typedef == null);
}
if (this.typedef != null) {
checkState(this.simpleType == null && this.ns == null && this.funScope == null);
checkState(this.ns == null && this.funScope == null);
}
if (this.ns != null) {
// Note: Non-null nominal with null function is allowed,
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java
View file
Expand Up @@ -512,7 +512,7 @@ private JSType getEnumPropType(EnumType e, DeclaredTypeRegistry registry) {

public void resolveEnum(EnumType e, DeclaredTypeRegistry registry) {
checkState(
e != null, "getEnum should only be " + "called when we know that the enum is defined");
e != null, "getEnum should only be called when we know that the enum is defined");
if (e.isResolved()) {
return;
}
Expand Down
1 change: 0 additions & 1 deletion src/com/google/javascript/jscomp/newtypes/Namespace.java
View file
Expand Up @@ -39,7 +39,6 @@
*/
public abstract class Namespace implements Serializable {
private Map<String, Namespace> namespaces = ImmutableMap.of();
// Freeze the namespace after GTI and null-out the typedefs.
private Map<String, Typedef> typedefs = ImmutableMap.of();
// "Simple type" properties (i.e. represented as JSTypes rather than something
// more specific).
Expand Down
18 changes: 13 additions & 5 deletions src/com/google/javascript/jscomp/newtypes/Typedef.java
View file
Expand Up @@ -21,6 +21,7 @@

import com.google.common.base.Preconditions;
import com.google.javascript.rhino.JSTypeExpression;
import com.google.javascript.rhino.Node;
import java.io.Serializable;

/**
Expand All @@ -36,21 +37,23 @@ private enum State {
RESOLVED
}

private final Node defSite;
private State state;
private JSTypeExpression typeExpr;
private JSType type;

private Typedef(JSTypeExpression typeExpr) {
checkNotNull(typeExpr);
private Typedef(Node defSite, JSTypeExpression typeExpr) {
checkState(defSite.isQualifiedName(), defSite);
this.defSite = defSite;
this.state = State.NOT_RESOLVED;
// Non-null iff the typedef is resolved
this.type = null;
// Non-null iff the typedef is not resolved
this.typeExpr = typeExpr;
}

public static Typedef make(JSTypeExpression typeExpr) {
return new Typedef(typeExpr);
public static Typedef make(Node defSite, JSTypeExpression typeExpr) {
return new Typedef(defSite, typeExpr);
}

public boolean isResolved() {
Expand Down Expand Up @@ -88,4 +91,9 @@ void resolveTypedef(JSType t) {
typeExpr = null;
type = t;
}
}

@Override
public String toString() {
return this.defSite.getQualifiedName();
}
}
2 changes: 1 addition & 1 deletion src/com/google/javascript/rhino/TypeIEnv.java
View file
Expand Up @@ -50,7 +50,7 @@ public interface TypeIEnv<T extends TypeI> {
* For example, if you pass Foo to this method, you get the Foo constructor.
* This is in contrast to TypeIRegistry#getType, where you would get the Foo instance.
*/
T getNamespaceType(String typeName);
T getNamespaceOrTypedefType(String typeName);

/**
* Returns the jsdoc at the definition site of the type represented by typeName.
Expand Down
55 changes: 52 additions & 3 deletions test/com/google/javascript/jscomp/NewTypeInferenceTTLTest.java
View file
Expand Up @@ -223,7 +223,6 @@ public void testGenerics() {
" */",
"function f() { return any(); }"));

// TODO(dimvar): this should have no warnings once typedefs inside TTL expressions are handled.
typeCheck(LINE_JOINER.join(
"/** @typedef {!Array<number>} */",
"var ArrayNumber;",
Expand All @@ -232,8 +231,7 @@ public void testGenerics() {
" * @return {T}",
" */",
"function f() { return []; }",
"f();"),
TypeTransformation.UNKNOWN_TYPENAME);
"f();"));

typeCheck(LINE_JOINER.join(
"/**",
Expand Down Expand Up @@ -915,4 +913,55 @@ public void testNoDynamicScopingWhenEvaluatingTTL() {
" x = f();",
"}"));
}

public void testTypedefsWithTTL() {
typeCheck(LINE_JOINER.join(
"/** @typedef {number} */",
"var MyNum;",
"/**",
" * @template T := 'MyNum' =:",
" * @return {T}",
" */",
"function f() { return 123; }",
"var /** number */ x = f();"));

typeCheck(LINE_JOINER.join(
"/** @typedef {number} */",
"var MyNum;",
"/**",
" * @template T := 'MyNum' =:",
" * @return {T}",
" */",
"function f() { return 123; }",
"var /** string */ x = f();"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

typeCheck(LINE_JOINER.join(
"/** @const */",
"var ns = {};",
"/** @typedef {number} */",
"ns.MyNum;",
"/**",
" * @template T := 'ns.MyNum' =:",
" * @return {T}",
" */",
"function f() { return 123; }",
"var /** string */ x = f();"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

typeCheck(LINE_JOINER.join(
"/** @const */",
"var ns = {};",
"function g() {",
" /** @typedef {number} */",
" ns.MyNum;",
"}",
"/**",
" * @template T := 'ns.MyNum' =:",
" * @return {T}",
" */",
"function f() { return 123; }",
"var /** string */ x = f();"),
NewTypeInference.MISTYPED_ASSIGN_RHS);
}
}

0 comments on commit 7f73bc5

Please sign in to comment.