From 86db7ff743a24faa91ff1d5e015e1dc810e6e8b7 Mon Sep 17 00:00:00 2001 From: dimvar Date: Fri, 20 Oct 2017 15:11:48 -0700 Subject: [PATCH] [NTI] Handle aliasing of typedefs. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=172940208 --- .../jscomp/GlobalTypeInfoCollector.java | 36 +++++++++++++++++-- .../google/javascript/jscomp/NTIScope.java | 8 +++-- .../javascript/jscomp/NewTypeInference.java | 10 ++++-- .../google/javascript/jscomp/NodeUtil.java | 23 ++++++++++++ .../jscomp/NewTypeInferenceTest.java | 20 +++++++++++ 5 files changed, 90 insertions(+), 7 deletions(-) diff --git a/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java b/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java index f28df320617..f4f75483066 100644 --- a/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java +++ b/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java @@ -68,6 +68,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import javax.annotation.Nullable; /** * Populates GlobalTypeInfo. @@ -743,6 +744,23 @@ private static boolean getsTypeInfoFromParentMethod(PropertyDef pd) { return (jsdoc.isOverride() || jsdoc.isExport()) && !jsdoc.containsFunctionDeclaration(); } + private boolean isAliasedTypedef(Node lhsQnameNode, NTIScope s) { + return getAliasedTypedef(lhsQnameNode, s) != null; + } + + /** + * If lhs represents the lhs of a typedef-aliasing statement, return that typedef. + */ + @Nullable + private Typedef getAliasedTypedef(Node lhs, NTIScope s) { + if (!NodeUtil.isAliasedConstDefinition(lhs)) { + return null; + } + Node rhs = NodeUtil.getRValueOfLValue(lhs); + checkState(rhs != null && rhs.isQualifiedName()); + return s.getTypedef(QualifiedName.fromNode(rhs)); + } + /** * Each node in the iterable is either a function expression or a statement. * The statement can be of: a function, a var, an expr_result containing an assignment, @@ -913,6 +931,8 @@ private void visitVar(Node n) { visitTypedef(nameNode); } else if (NodeUtil.isEnumDecl(nameNode)) { visitEnum(nameNode); + } else if (isAliasedTypedef(nameNode, this.currentScope)) { + visitAliasedTypedef(nameNode); } else if (isAliasedNamespaceDefinition(nameNode)) { visitAliasedNamespace(nameNode); } else if (varName.equals(WINDOW_INSTANCE) && nameNode.isFromExterns()) { @@ -967,6 +987,8 @@ private void processQualifiedDefinition(Node qnameNode) { visitTypedef(qnameNode); } else if (NodeUtil.isEnumDecl(qnameNode)) { visitEnum(qnameNode); + } else if (isAliasedTypedef(qnameNode, this.currentScope)) { + visitAliasedTypedef(qnameNode); } else if (isAliasedNamespaceDefinition(qnameNode)) { visitAliasedNamespace(qnameNode); } else if (isAliasingGlobalThis(qnameNode)) { @@ -1392,6 +1414,15 @@ private void visitAliasedNamespace(Node lhs) { } } + private void visitAliasedTypedef(Node lhs) { + if (this.currentScope.isDefined(lhs)) { + return; + } + lhs.getParent().putBooleanProp(Node.ANALYZED_DURING_GTI, true); + Typedef td = checkNotNull(getAliasedTypedef(lhs, this.currentScope)); + this.currentScope.addTypedef(lhs, td); + } + private void maybeAddFunctionToNamespace(Node funQname) { Namespace ns = currentScope.getNamespace(QualifiedName.fromNode(funQname.getFirstChild())); String internalName = getFunInternalName(funQname.getParent().getLastChild()); @@ -1989,7 +2020,7 @@ private void visitNamespacePropertyDeclaration(Node getProp) { checkArgument(getProp.isGetProp()); mayVisitWeirdCtorDefinition(getProp); // Named types have already been crawled in CollectNamedTypes - if (isNamedType(getProp)) { + if (isNamedType(getProp) || isAliasedTypedef(getProp, this.currentScope)) { return; } Node recv = getProp.getFirstChild(); @@ -2243,8 +2274,7 @@ private JSType mayInferFromRhsIfConst(Node lvalueNode) { return null; } - // If a @const doesn't have a declared type, we use the initializer to - // infer a type. + // If a @const doesn't have a declared type, we use the initializer to infer a type. // When we cannot infer the type of the initializer, we warn. // This way, people do not need to remember the cases where the compiler // can infer the type of a constant; we tell them if we cannot infer it. diff --git a/src/com/google/javascript/jscomp/NTIScope.java b/src/com/google/javascript/jscomp/NTIScope.java index f6375665b28..90b107b95d5 100644 --- a/src/com/google/javascript/jscomp/NTIScope.java +++ b/src/com/google/javascript/jscomp/NTIScope.java @@ -483,12 +483,16 @@ RawNominalType getNominalType(QualifiedName qname) { } Typedef getTypedef(String name) { - QualifiedName qname = QualifiedName.fromQualifiedString(name); + return getTypedef(QualifiedName.fromQualifiedString(name)); + } + + Typedef getTypedef(QualifiedName qname) { Declaration decl; if (qname.isIdentifier()) { decl = getDeclaration(qname, true); } else { - decl = getNamespace(qname.getLeftmostName()).getDeclaration(qname.getAllButLeftmost()); + Namespace ns = getNamespace(qname.getLeftmostName()); + decl = ns == null ? null : ns.getDeclaration(qname.getAllButLeftmost()); } return decl == null ? null : decl.getTypedef(); } diff --git a/src/com/google/javascript/jscomp/NewTypeInference.java b/src/com/google/javascript/jscomp/NewTypeInference.java index 6e1ea5335f8..9932a97dbf9 100644 --- a/src/com/google/javascript/jscomp/NewTypeInference.java +++ b/src/com/google/javascript/jscomp/NewTypeInference.java @@ -1942,8 +1942,14 @@ private EnvTypePair analyzeAssignFwd( Node expr, TypeEnv inEnv, JSType requiredType, JSType specializedType) { if (expr.getBooleanProp(Node.ANALYZED_DURING_GTI)) { expr.removeProp(Node.ANALYZED_DURING_GTI); - markAndGetTypeOfPreanalyzedNode(expr.getFirstChild(), inEnv, true); - markAndGetTypeOfPreanalyzedNode(expr.getLastChild(), inEnv, true); + // If the assignment is an aliasing of a typedef, markAndGetTypeOfPreanalyzedNode won't + // be able to find a type and we'll get a spurious warning. + // But during NTI we don't have typedef info anymore, so we back off for all aliasing + // definitions, not just ones defining typedefs. + if (!NodeUtil.isAliasedConstDefinition(expr.getFirstChild())) { + markAndGetTypeOfPreanalyzedNode(expr.getFirstChild(), inEnv, true); + markAndGetTypeOfPreanalyzedNode(expr.getLastChild(), inEnv, true); + } return new EnvTypePair(inEnv, requiredType); } mayWarnAboutConst(expr); diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index b7b4b9ec729..59bb8f63b4a 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -954,6 +954,27 @@ static boolean isSimpleOperatorType(Token type) { } } + /** + * True for aliases defined with @const, not for aliases defined with @constructor/@interface. + */ + static boolean isAliasedConstDefinition(Node lhs) { + JSDocInfo jsdoc = getBestJSDocInfo(lhs); + if (jsdoc == null && !lhs.isFromExterns()) { + return false; + } + if (jsdoc != null && !jsdoc.hasConstAnnotation()) { + return false; + } + Node rhs = getRValueOfLValue(lhs); + if (rhs == null || !rhs.isQualifiedName()) { + return false; + } + Node parent = lhs.getParent(); + return (lhs.isName() && parent.isVar()) + || (lhs.isGetProp() && lhs.isQualifiedName() + && parent.isAssign() && parent.getParent().isExprResult()); + } + static boolean isTypedefDecl(Node n) { if (n.isVar() || (n.isName() && n.getParent().isVar()) @@ -4381,6 +4402,8 @@ static boolean isConstantVar(Node node, Scope scope) { * * @param node A NAME or STRING node * @return True if a name node represents a constant variable + * + * TODO(dimvar): this method and the next two do similar but not quite identical things. Clean up */ static boolean isConstantName(Node node) { return node.getBooleanProp(Node.IS_CONSTANT_NAME); diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java index f8d32cd49bd..ec8f7ed2c93 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java @@ -22065,4 +22065,24 @@ public void testUnresolvedTypes() { "f(123);"), NewTypeInference.INVALID_ARGUMENT_TYPE); } + + public void testHandleAliasedTypedefs() { + typeCheck(LINE_JOINER.join( + "/** @typedef {number} */", + "var A;", + "/** @const */", + "var B = A;", + "var /** B */ x = 'asdf';"), + NewTypeInference.MISTYPED_ASSIGN_RHS); + + typeCheck(LINE_JOINER.join( + "/** @const */", + "var ns = {};", + "/** @typedef {number} */", + "ns.A;", + "/** @const */", + "ns.B = ns.A;", + "var /** ns.B */ x = 'asdf';"), + NewTypeInference.MISTYPED_ASSIGN_RHS); + } }