Skip to content

Commit

Permalink
[NTI] Handle namespace definitions on object-literal keys.
Browse files Browse the repository at this point in the history 
Also, minor unrelated cleanup in GTI (attach some types to the AST instead of using maps in symbol table).

Fixes #2702.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=175298833
  • Loading branch information
@dimvar
dimvar authored and Tyler Breisacher committed Nov 13, 2017
1 parent d5501d9 commit 5ea88cb
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 79 deletions.
21 changes: 0 additions & 21 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
View file
Expand Up @@ -70,9 +70,6 @@ public class GlobalTypeInfo implements TypeIRegistry {
private final JSTypeCreatorFromJSDoc typeParser; private final JSTypeCreatorFromJSDoc typeParser;
private final Map<Node, String> anonFunNames = new LinkedHashMap<>(); private final Map<Node, String> anonFunNames = new LinkedHashMap<>();
private final UniqueNameGenerator varNameGen; private final UniqueNameGenerator varNameGen;
// TODO(dimvar): Eventually attach these to nodes, like the current types.
private final Map<Node, JSType> castTypes = new LinkedHashMap<>();
private final Map<Node, JSType> declaredObjLitProps = new LinkedHashMap<>();


private final JSTypes commonTypes; private final JSTypes commonTypes;
private final Set<String> unknownTypeNames; private final Set<String> unknownTypeNames;
Expand Down Expand Up @@ -133,14 +130,6 @@ Set<String> getExternPropertyNames() {
return this.externPropertyNames; return this.externPropertyNames;
} }


Map<Node, JSType> getCastTypes() {
return this.castTypes;
}

Map<Node, JSType> getDeclaredObjLitProps() {
return this.declaredObjLitProps;
}

public JSTypeCreatorFromJSDoc getTypeParser() { public JSTypeCreatorFromJSDoc getTypeParser() {
return this.typeParser; return this.typeParser;
} }
Expand All @@ -161,16 +150,6 @@ List<TypeMismatch> getImplicitInterfaceUses() {
return this.implicitInterfaceUses; return this.implicitInterfaceUses;
} }


JSType getCastType(Node n) {
JSType t = castTypes.get(n);
checkNotNull(t);
return t;
}

JSType getPropDeclaredType(Node n) {
return declaredObjLitProps.get(n);
}

Collection<String> getAllPropertyNames() { Collection<String> getAllPropertyNames() {
return this.allPropertyNames; return this.allPropertyNames;
} }
Expand Down
111 changes: 60 additions & 51 deletions src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java
View file
Expand Up @@ -1122,6 +1122,12 @@ private void visitObjlitNamespace(QualifiedName qname, Node defSite) {
QualifiedName propQname = new QualifiedName(propNode.getString()); QualifiedName propQname = new QualifiedName(propNode.getString());
if (isAliasedNamespaceDefinition(propNode)) { if (isAliasedNamespaceDefinition(propNode)) {
visitAliasedNamespace(QualifiedName.join(qname, propQname), propNode); visitAliasedNamespace(QualifiedName.join(qname, propQname), propNode);
} else {
JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(propNode);
Node propVal = propNode.getLastChild();
if (jsdoc != null && jsdoc.hasConstAnnotation() && propVal.isObjectLit()) {
visitObjlitNamespace(QualifiedName.join(qname, propQname), propNode);
}
} }
} }
} }
Expand Down Expand Up @@ -1526,7 +1532,7 @@ void processLendsToNamespace(
Namespace borrowerNamespace = currentScope.getNamespace(lendsQname); Namespace borrowerNamespace = currentScope.getNamespace(lendsQname);
for (Node prop : objlit.children()) { for (Node prop : objlit.children()) {
String pname = NodeUtil.getObjectLitKeyName(prop); String pname = NodeUtil.getObjectLitKeyName(prop);
JSType propDeclType = getDeclaredObjLitProps().get(prop); JSType propDeclType = (JSType) prop.getTypeI();
if (propDeclType != null) { if (propDeclType != null) {
borrowerNamespace.addProperty(pname, prop, propDeclType, false); borrowerNamespace.addProperty(pname, prop, propDeclType, false);
} else { } else {
Expand Down Expand Up @@ -1618,11 +1624,13 @@ public void visit(NodeTraversal t, Node n, Node parent) {
break; break;
} }
case CAST: case CAST:
getCastTypes().put(n, n.setTypeI(getDeclaredTypeOfNode(n.getJSDocInfo(), this.currentScope));
getDeclaredTypeOfNode(n.getJSDocInfo(), this.currentScope));
break; break;
case OBJECTLIT: case OBJECTLIT:
visitObjectLit(n, parent); if (!NodeUtil.isObjectLitKey(parent)) {
Node lval = NodeUtil.getBestLValue(n);
visitObjectLit(n, QualifiedName.fromNode(lval));
}
break; break;
case CALL: case CALL:
visitCall(n); visitCall(n);
Expand Down Expand Up @@ -1703,38 +1711,46 @@ private void visitVar(Node nameNode, Node parent) {
} }
} }


private void visitObjectLit(Node objLitNode, Node parent) { private void visitObjectLit(Node objLitNode, QualifiedName lvalueQname) {
Node parent = objLitNode.getParent();
JSDocInfo jsdoc = objLitNode.getJSDocInfo(); JSDocInfo jsdoc = objLitNode.getJSDocInfo();
if (jsdoc != null && jsdoc.getLendsName() != null) { if (jsdoc != null && jsdoc.getLendsName() != null) {
lendsObjlits.add(objLitNode); lendsObjlits.add(objLitNode);
} }
Node maybeLvalue = parent.isAssign() ? parent.getFirstChild() : parent; Node maybeLvalue = parent.isAssign() ? parent.getFirstChild() : parent;
if (NodeUtil.isNamespaceDecl(maybeLvalue) && currentScope.isNamespace(maybeLvalue)) { if (currentScope.isNamespace(lvalueQname)) {
for (Node prop : objLitNode.children()) { for (Node prop : objLitNode.children()) {
Node keyNode = NodeUtil.getObjectLitKeyNode(prop);
if (keyNode != null) {
recordPropertyName(keyNode);
}
if (!prop.isComputedProp()) { if (!prop.isComputedProp()) {
visitNamespacePropertyDeclaration(prop, maybeLvalue, prop.getString()); visitNamespacePropertyDeclaration(prop, lvalueQname, prop.getString());
} }
} }
} else if (!NodeUtil.isEnumDecl(maybeLvalue) } else if (!NodeUtil.isEnumDecl(maybeLvalue)
&& !NodeUtil.isPrototypeAssignment(maybeLvalue)) { && !NodeUtil.isPrototypeAssignment(maybeLvalue)) {
// For object literals that are not "special" (namespaces, enums, etc),
// record their declared properties so we can typecheck them later.
for (Node prop : objLitNode.children()) { for (Node prop : objLitNode.children()) {
Node keyNode = NodeUtil.getObjectLitKeyNode(prop);
if (keyNode != null) {
recordPropertyName(keyNode);
}
JSDocInfo propJsdoc = prop.getJSDocInfo(); JSDocInfo propJsdoc = prop.getJSDocInfo();
if (propJsdoc != null) { if (propJsdoc != null) {
getDeclaredObjLitProps().put(prop, getDeclaredTypeOfNode(propJsdoc, currentScope)); JSType propType = getDeclaredTypeOfNode(propJsdoc, currentScope);
prop.setTypeI(propType);
} }
if (isAnnotatedAsConst(prop)) { if (isAnnotatedAsConst(prop)) {
warnings.add(JSError.make(prop, MISPLACED_CONST_ANNOTATION)); warnings.add(JSError.make(prop, MISPLACED_CONST_ANNOTATION));
} }
} }
} }
// Record property names and recur to nested object literals.
for (Node prop : objLitNode.children()) {
Node keyNode = NodeUtil.getObjectLitKeyNode(prop);
if (keyNode != null) {
recordPropertyName(keyNode);
}
if (prop.hasChildren() && prop.getFirstChild().isObjectLit()) {
QualifiedName nestedQname = lvalueQname == null || keyNode == null
? null : QualifiedName.join(lvalueQname, new QualifiedName(keyNode.getString()));
visitObjectLit(prop.getFirstChild(), nestedQname);
}
}
} }


private void visitCall(Node call) { private void visitCall(Node call) {
Expand Down Expand Up @@ -2063,66 +2079,67 @@ private void visitNamespacePropertyDeclaration(Node getProp) {
} }
Node recv = getProp.getFirstChild(); Node recv = getProp.getFirstChild();
String pname = getProp.getLastChild().getString(); String pname = getProp.getLastChild().getString();
visitNamespacePropertyDeclaration(getProp, recv, pname); visitNamespacePropertyDeclaration(getProp, QualifiedName.fromNode(recv), pname);
} }


private void visitNamespacePropertyDeclaration(Node declNode, Node recv, String pname) { private void visitNamespacePropertyDeclaration(
checkArgument( Node defSite, QualifiedName nsQname, String pname) {
declNode.isGetProp() || NodeUtil.isObjLitProperty(declNode), checkArgument(defSite.isGetProp() || NodeUtil.isObjLitProperty(defSite), defSite);
declNode); checkArgument(currentScope.isNamespace(nsQname));
QualifiedName recvQname = QualifiedName.fromNode(recv); if (defSite.isGetterDef()) {
checkArgument(currentScope.isNamespace(recvQname));
if (declNode.isGetterDef()) {
pname = getCommonTypes().createGetterPropName(pname); pname = getCommonTypes().createGetterPropName(pname);
} else if (declNode.isSetterDef()) { } else if (defSite.isSetterDef()) {
pname = getCommonTypes().createSetterPropName(pname); pname = getCommonTypes().createSetterPropName(pname);
} }
// Named types have already been crawled in CollectNamedTypes if (defSite.isStringKey()) {
if (declNode.isStringKey() && currentScope.isNamespace(declNode.getFirstChild())) { QualifiedName propQname = new QualifiedName(defSite.getString());
return; // Namespaces have already been crawled in CollectNamedTypes
if (currentScope.isNamespace(QualifiedName.join(nsQname, propQname))) {
return;
}
} }
EnumType et = currentScope.getEnum(QualifiedName.fromNode(recv)); EnumType et = currentScope.getEnum(nsQname);
// If there is a reassignment to one of the enum's members, don't consider // If there is a reassignment to one of the enum's members, don't consider
// that a definition of a new property. // that a definition of a new property.
if (et != null && et.enumLiteralHasKey(pname)) { if (et != null && et.enumLiteralHasKey(pname)) {
return; return;
} }


Namespace ns = currentScope.getNamespace(recvQname); Namespace ns = currentScope.getNamespace(nsQname);
JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(declNode); JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(defSite);
PropertyType pt = getPropTypeHelper(jsdoc, declNode, null); PropertyType pt = getPropTypeHelper(jsdoc, defSite, null);
JSType propDeclType = pt.declType; JSType propDeclType = pt.declType;
JSType propInferredFunType = pt.inferredFunType; JSType propInferredFunType = pt.inferredFunType;
boolean isConst = isConst(declNode); boolean isConst = isConst(defSite);
if (propDeclType != null || isConst) { if (propDeclType != null || isConst) {
JSType previousPropType = ns.getPropDeclaredType(pname); JSType previousPropType = ns.getPropDeclaredType(pname);
declNode.putBooleanProp(Node.ANALYZED_DURING_GTI, true); defSite.putBooleanProp(Node.ANALYZED_DURING_GTI, true);
if (ns.hasSubnamespace(new QualifiedName(pname)) if (ns.hasSubnamespace(new QualifiedName(pname))
|| (ns.hasStaticProp(pname) || (ns.hasStaticProp(pname)
&& previousPropType != null && previousPropType != null
&& !suppressDupPropWarning(jsdoc, propDeclType, previousPropType))) { && !suppressDupPropWarning(jsdoc, propDeclType, previousPropType))) {
warnings.add(JSError.make( warnings.add(JSError.make(
declNode, REDECLARED_PROPERTY, pname, "namespace " + ns)); defSite, REDECLARED_PROPERTY, pname, "namespace " + ns));
declNode.getParent().putBooleanProp(Node.ANALYZED_DURING_GTI, true); defSite.getParent().putBooleanProp(Node.ANALYZED_DURING_GTI, true);
return; return;
} }
if (propDeclType == null) { if (propDeclType == null) {
propDeclType = mayInferFromRhsIfConst(declNode); propDeclType = mayInferFromRhsIfConst(defSite);
} }
ns.addProperty(pname, declNode, propDeclType, isConst); ns.addProperty(pname, defSite, propDeclType, isConst);
if (declNode.isGetProp() && isConst) { if (defSite.isGetProp() && isConst) {
declNode.putBooleanProp(Node.CONSTANT_PROPERTY_DEF, true); defSite.putBooleanProp(Node.CONSTANT_PROPERTY_DEF, true);
} }
} else if (propInferredFunType != null) { } else if (propInferredFunType != null) {
ns.addUndeclaredProperty(pname, declNode, propInferredFunType, false); ns.addUndeclaredProperty(pname, defSite, propInferredFunType, false);
} else { } else {
// Try to infer the prop type, but don't say that the prop is declared. // Try to infer the prop type, but don't say that the prop is declared.
Node initializer = NodeUtil.getRValueOfLValue(declNode); Node initializer = NodeUtil.getRValueOfLValue(defSite);
JSType t = initializer == null ? null : simpleInferExpr(initializer, this.currentScope); JSType t = initializer == null ? null : simpleInferExpr(initializer, this.currentScope);
if (t == null) { if (t == null) {
t = getCommonTypes().UNKNOWN; t = getCommonTypes().UNKNOWN;
} }
ns.addUndeclaredProperty(pname, declNode, t, false); ns.addUndeclaredProperty(pname, defSite, t, false);
} }
} }


Expand Down Expand Up @@ -2864,14 +2881,6 @@ private UniqueNameGenerator getVarNameGen() {
return this.globalTypeInfo.getVarNameGen(); return this.globalTypeInfo.getVarNameGen();
} }


private Map<Node, JSType> getDeclaredObjLitProps() {
return this.globalTypeInfo.getDeclaredObjLitProps();
}

private Map<Node, JSType> getCastTypes() {
return this.globalTypeInfo.getCastTypes();
}

private void recordPropertyName(Node pnameNode) { private void recordPropertyName(Node pnameNode) {
this.globalTypeInfo.recordPropertyName(pnameNode); this.globalTypeInfo.recordPropertyName(pnameNode);
} }
Expand Down
8 changes: 4 additions & 4 deletions src/com/google/javascript/jscomp/NewTypeInference.java
View file
Expand Up @@ -2546,7 +2546,7 @@ private EnvTypePair analyzeCastFwd(Node expr, TypeEnv inEnv, JSType specializedT
Node insideCast = expr.getFirstChild(); Node insideCast = expr.getFirstChild();
EnvTypePair pair = analyzeExprFwd(insideCast, inEnv, this.commonTypes.UNKNOWN, newSpecType); EnvTypePair pair = analyzeExprFwd(insideCast, inEnv, this.commonTypes.UNKNOWN, newSpecType);
JSType fromType = pair.type; JSType fromType = pair.type;
JSType toType = symbolTable.getCastType(expr); JSType toType = (JSType) expr.getTypeI();
if (!fromType.isInterfaceInstance() if (!fromType.isInterfaceInstance()
&& !toType.isInterfaceInstance() && !toType.isInterfaceInstance()
&& !JSType.haveCommonSubtype(fromType, toType) && !JSType.haveCommonSubtype(fromType, toType)
Expand Down Expand Up @@ -3035,7 +3035,7 @@ private EnvTypePair analyzeObjLitFwd(
continue; continue;
} }
QualifiedName qname = new QualifiedName(pnameNode.getString()); QualifiedName qname = new QualifiedName(pnameNode.getString());
JSType jsdocType = symbolTable.getPropDeclaredType(prop); JSType jsdocType = (JSType) prop.getTypeI();
JSType reqPtype; JSType reqPtype;
JSType specPtype; JSType specPtype;
if (jsdocType != null) { if (jsdocType != null) {
Expand Down Expand Up @@ -3918,7 +3918,7 @@ private EnvTypePair analyzeExprBwd(
return analyzeArrayLitBwd(expr, outEnv); return analyzeArrayLitBwd(expr, outEnv);
case CAST: { case CAST: {
EnvTypePair pair = analyzeExprBwd(expr.getFirstChild(), outEnv); EnvTypePair pair = analyzeExprBwd(expr.getFirstChild(), outEnv);
pair.type = symbolTable.getCastType(expr); pair.type = (JSType) expr.getTypeI();
return pair; return pair;
} }
case TEMPLATELIT: case TEMPLATELIT:
Expand Down Expand Up @@ -4275,7 +4275,7 @@ private EnvTypePair analyzeObjLitBwd(
env = analyzeExprBwd(prop, env).env; env = analyzeExprBwd(prop, env).env;
} else { } else {
QualifiedName pname = new QualifiedName(NodeUtil.getObjectLitKeyName(prop)); QualifiedName pname = new QualifiedName(NodeUtil.getObjectLitKeyName(prop));
JSType jsdocType = symbolTable.getPropDeclaredType(prop); JSType jsdocType = (JSType) prop.getTypeI();
JSType reqPtype; JSType reqPtype;
if (jsdocType != null) { if (jsdocType != null) {
reqPtype = jsdocType; reqPtype = jsdocType;
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/SimpleInference.java
View file
Expand Up @@ -166,7 +166,7 @@ private JSType inferExprRecur(Node n, NTIScope scope) {
case REGEXP: case REGEXP:
return this.commonTypes.getRegexpType(); return this.commonTypes.getRegexpType();
case CAST: case CAST:
return this.gti.getCastTypes().get(n); return (JSType) n.getTypeI();
case ARRAYLIT: { case ARRAYLIT: {
if (!n.hasChildren()) { if (!n.hasChildren()) {
return this.commonTypes.getArrayInstance(); return this.commonTypes.getArrayInstance();
Expand Down
33 changes: 31 additions & 2 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
View file
Expand Up @@ -7289,8 +7289,7 @@ public void testUnifyObjects() {
" var y = cond ? {prop: 'str'} : {prop: 5};", " var y = cond ? {prop: 'str'} : {prop: 5};",
" f({prop: x}, y);", " f({prop: x}, y);",
"}", "}",
"g({}, true);"), "g({}, true);"));
NewTypeInference.INVALID_ARGUMENT_TYPE);


typeCheck(LINE_JOINER.join( typeCheck(LINE_JOINER.join(
"function g(x, cond) {", "function g(x, cond) {",
Expand Down Expand Up @@ -12983,6 +12982,36 @@ public void testNamespacesWithNonEmptyObjectLiteral() {
" obj.ns = { /** @const */ PROP: 5 };", " obj.ns = { /** @const */ PROP: 5 };",
"}"), "}"),
GlobalTypeInfoCollector.MISPLACED_CONST_ANNOTATION); GlobalTypeInfoCollector.MISPLACED_CONST_ANNOTATION);

typeCheck(LINE_JOINER.join(
"/** @const */",
"var ns = { /** @const */ prop: {} };",
"/** @constructor */",
"ns.prop.Foo = function() {};",
"var /** !ns.prop.Foo */ x = new ns.prop.Foo();"));

typeCheck(LINE_JOINER.join(
"/** @const */",
"var ns = { /** @const */ prop: {} };",
"/** @constructor */",
"ns.prop.Foo = function() {",
" /** @type {number} */ this.a = 123; ",
"};",
"var /** string */ s = (new ns.prop.Foo()).a;"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

typeCheck(LINE_JOINER.join(
"/** @const */",
"var ns = {};",
"/** @const */",
"ns.Constants = {",
" /** @const */ Async: {",
" FEATURE_ID:'feature_id'",
" }",
"};",
"/** @const {number} */",
"var x = ns.Constants.Async.FEATURE_ID;"),
NewTypeInference.MISTYPED_ASSIGN_RHS);
} }


public void testNamespaceRedeclaredProps() { public void testNamespaceRedeclaredProps() {
Expand Down

0 comments on commit 5ea88cb

Please sign in to comment.