diff --git a/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java b/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java index 6cf1a63f0dc..9d41b31f036 100644 --- a/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java +++ b/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java @@ -248,25 +248,6 @@ private void checkForLateOrMissingProvide(UnrecognizedRequire r) { compiler.report(JSError.make(r.requireNode, requiresLevel, error, r.namespace)); } - private Node getAnyValueOfType(JSDocInfo jsdoc) { - checkArgument(jsdoc.hasType()); - Node typeAst = jsdoc.getType().getRoot(); - if (typeAst.getToken() == Token.BANG) { - typeAst = typeAst.getLastChild(); - } - checkState(typeAst.isString(), typeAst); - switch (typeAst.getString()) { - case "boolean": - return IR.falseNode(); - case "string": - return IR.string(""); - case "number": - return IR.number(0); - default: - throw new RuntimeException(typeAst.getString()); - } - } - /** * @param n */ @@ -274,8 +255,7 @@ private void replaceGoogDefines(Node n) { Node parent = n.getParent(); String name = n.getSecondChild().getString(); JSDocInfo jsdoc = n.getJSDocInfo(); - Node value = - n.isFromExterns() ? getAnyValueOfType(jsdoc).srcref(n) : n.getChildAtIndex(2).detach(); + Node value = n.getChildAtIndex(2).detach(); switch (parent.getToken()) { case EXPR_RESULT: @@ -1173,9 +1153,8 @@ private boolean verifyDefine(NodeTraversal t, Node parent, Node methodName, Node // It is an error for goog.define to show up anywhere except on its own or immediately after =. if (parent.isAssign() && parent.getParent().isExprResult()) { parent = parent.getParent(); - } - if (parent.isName() && NodeUtil.isNameDeclaration(parent.getParent())) { - parent = parent.getGrandparent(); + } else if (parent.isName() && NodeUtil.isNameDeclaration(parent.getParent())) { + parent = parent.getParent(); } else if (!parent.isExprResult()) { compiler.report(t.makeError(methodName.getParent(), INVALID_CLOSURE_CALL_SCOPE_ERROR)); return false; @@ -1201,7 +1180,7 @@ private boolean verifyDefine(NodeTraversal t, Node parent, Node methodName, Node return false; } - JSDocInfo info = parent.getFirstChild().getJSDocInfo(); + JSDocInfo info = (parent.isExprResult() ? parent.getFirstChild() : parent).getJSDocInfo(); if (info == null || !info.isDefine()) { compiler.report(t.makeError(parent, MISSING_DEFINE_ANNOTATION)); return false; diff --git a/src/com/google/javascript/jscomp/ijs/PotentialDeclaration.java b/src/com/google/javascript/jscomp/ijs/PotentialDeclaration.java index a3d233338bd..0c3d9559c46 100644 --- a/src/com/google/javascript/jscomp/ijs/PotentialDeclaration.java +++ b/src/com/google/javascript/jscomp/ijs/PotentialDeclaration.java @@ -22,7 +22,9 @@ import com.google.javascript.jscomp.NodeUtil; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.JSDocInfo; +import com.google.javascript.rhino.JSTypeExpression; import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.Token; import javax.annotation.Nullable; /** @@ -75,7 +77,7 @@ static PotentialDeclaration fromStringKey(Node stringKeyNode) { static PotentialDeclaration fromDefine(Node callNode) { checkArgument(NodeUtil.isCallTo(callNode, "goog.define")); - return new DefineDeclaration(callNode); + return DefineDeclaration.from(callNode); } String getFullyQualifiedName() { @@ -261,19 +263,65 @@ void simplify(AbstractCompiler compiler) { } } - /** * A declaration declared by a call to `goog.define`. Note that a let, const, or var declaration - * annotated with @define in its JSDoc would be a NameDeclaration instead. + * annotated with @define in its JSDoc and no 'goog.define' would be a NameDeclaration instead. */ private static class DefineDeclaration extends PotentialDeclaration { - DefineDeclaration(Node callNode) { - super(callNode.getSecondChild().getString(), callNode, callNode.getLastChild()); + DefineDeclaration(String qualifiedName, Node lhs, Node rhs) { + super(qualifiedName, lhs, rhs); } @Override void simplify(AbstractCompiler compiler) { - NodeUtil.deleteNode(getLhs().getLastChild(), compiler); + JSDocInfo info = getJsDoc(); + if (info != null && info.getType() != null) { + Node newRhs = makeEmptyValueNode(info.getType()); + if (newRhs != null) { + getRhs().replaceWith(newRhs); + compiler.reportChangeToEnclosingScope(newRhs); + return; + } + } + NodeUtil.deleteNode(getRemovableNode(), compiler); + } + + static DefineDeclaration from(Node callNode) { + // Match a few different forms, depending on the call node's parent: + // 1. EXPR_RESULT: goog.define('foo', 1); + // 2. ASSIGN: a.b = goog.define('c', 2); + // 3. NAME: var x = goog.define('d', 3); + switch (callNode.getParent().getToken()) { + case EXPR_RESULT: + return new DefineDeclaration( + callNode.getSecondChild().getString(), callNode, callNode.getLastChild()); + case ASSIGN: + Node previous = callNode.getPrevious(); + return new DefineDeclaration( + previous.getQualifiedName(), previous, callNode.getLastChild()); + case NAME: + Node parent = callNode.getParent(); + return new DefineDeclaration(parent.getString(), parent, callNode.getLastChild()); + default: + throw new IllegalStateException("Unexpected parent: " + callNode.getParent().getToken()); + } + } + + static Node makeEmptyValueNode(JSTypeExpression type) { + Node n = type.getRoot(); + while (n != null && n.getToken() != Token.STRING && n.getToken() != Token.NAME) { + n = n.getFirstChild(); + } + switch (n != null ? n.getString() : "") { + case "boolean": + return new Node(Token.FALSE); + case "number": + return Node.newNumber(0); + case "string": + return Node.newString(""); + default: + return null; + } } } diff --git a/src/com/google/javascript/jscomp/ijs/ProcessConstJsdocCallback.java b/src/com/google/javascript/jscomp/ijs/ProcessConstJsdocCallback.java index dfed8b06552..13b77835fb0 100644 --- a/src/com/google/javascript/jscomp/ijs/ProcessConstJsdocCallback.java +++ b/src/com/google/javascript/jscomp/ijs/ProcessConstJsdocCallback.java @@ -21,6 +21,7 @@ import com.google.javascript.jscomp.NodeTraversal; import com.google.javascript.jscomp.NodeUtil; import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.QualifiedName; /** * A callback that calls the abstract method on every "inferrable const". This is a constant @@ -58,19 +59,23 @@ public void visit(NodeTraversal t, Node n, Node parent) { switch (expr.getToken()) { case CALL: Node callee = expr.getFirstChild(); - if (callee.matchesQualifiedName("goog.provide")) { + if (GOOG_PROVIDE.matches(callee)) { currentFile.markProvided(expr.getLastChild().getString()); - } else if (callee.matchesQualifiedName("goog.require") - || callee.matchesQualifiedName("require")) { + } else if (GOOG_REQUIRE.matches(callee) || CJS_REQUIRE.matches(callee)) { currentFile.recordImport(expr.getLastChild().getString()); - } else if (callee.matchesQualifiedName("goog.define")) { + } else if (GOOG_DEFINE.matches(callee)) { currentFile.recordDefine(expr); } break; case ASSIGN: Node lhs = expr.getFirstChild(); - currentFile.recordNameDeclaration(lhs); - processDeclarationWithRhs(t, lhs); + Node rhs = expr.getLastChild(); + if (rhs.isCall() && GOOG_DEFINE.matches(rhs.getFirstChild()) && lhs.isQualifiedName()) { + currentFile.recordDefine(rhs); + } else { + currentFile.recordNameDeclaration(lhs); + processDeclarationWithRhs(t, lhs, expr.getLastChild()); + } break; case GETPROP: currentFile.recordNameDeclaration(expr); @@ -83,14 +88,20 @@ public void visit(NodeTraversal t, Node n, Node parent) { case CONST: case LET: checkState(n.hasOneChild(), n); - recordNameDeclaration(n); - if (n.getFirstChild().isName() && n.getFirstChild().hasChildren()) { - processDeclarationWithRhs(t, n.getFirstChild()); + Node name = n.getFirstChild(); + Node rhs = name.getFirstChild(); + if (rhs != null && rhs.isCall() && GOOG_DEFINE.matches(name.getFirstFirstChild())) { + currentFile.recordDefine(rhs); + } else { + recordNameDeclaration(n); + if (name.isName() && name.hasChildren()) { + processDeclarationWithRhs(t, name, rhs); + } } break; case STRING_KEY: if (parent.isObjectLit() && n.hasOneChild()) { - processDeclarationWithRhs(t, n); + processDeclarationWithRhs(t, n, n.getFirstChild()); currentFile.recordStringKeyDeclaration(n); } break; @@ -99,6 +110,11 @@ public void visit(NodeTraversal t, Node n, Node parent) { } } + private static final QualifiedName GOOG_DEFINE = QualifiedName.of("goog.define"); + private static final QualifiedName GOOG_PROVIDE = QualifiedName.of("goog.provide"); + private static final QualifiedName GOOG_REQUIRE = QualifiedName.of("goog.require"); + private static final QualifiedName CJS_REQUIRE = QualifiedName.of("require"); + private void recordNameDeclaration(Node decl) { checkArgument(NodeUtil.isNameDeclaration(decl)); Node rhs = decl.getFirstChild().getLastChild(); @@ -112,15 +128,14 @@ private void recordNameDeclaration(Node decl) { } } - private void processDeclarationWithRhs(NodeTraversal t, Node lhs) { + private void processDeclarationWithRhs(NodeTraversal t, Node lhs, Node rhs) { checkArgument( lhs.isQualifiedName() || lhs.isStringKey() || lhs.isDestructuringLhs(), lhs); checkState(NodeUtil.getRValueOfLValue(lhs) != null, lhs); - if (!PotentialDeclaration.isConstToBeInferred(lhs)) { - return; + if (PotentialDeclaration.isConstToBeInferred(lhs)) { + processConstWithRhs(t, lhs); } - processConstWithRhs(t, lhs); } protected abstract void processConstWithRhs(NodeTraversal t, Node lhs); diff --git a/test/com/google/javascript/jscomp/ProcessClosurePrimitivesTest.java b/test/com/google/javascript/jscomp/ProcessClosurePrimitivesTest.java index 8368469bf3d..318b570929c 100644 --- a/test/com/google/javascript/jscomp/ProcessClosurePrimitivesTest.java +++ b/test/com/google/javascript/jscomp/ProcessClosurePrimitivesTest.java @@ -1545,20 +1545,6 @@ public void testDefineErrorCases() { testError(jsdoc + "goog.define(`${template}Name`, 1);", INVALID_ARGUMENT_ERROR); } - @Test - public void testDefineInExterns() { - String jsdoc = "/** @define {number} */\n"; - allowExternsChanges(); - testErrorExterns(jsdoc + "goog.define('value');"); - - testErrorExterns("goog.define('name');", MISSING_DEFINE_ANNOTATION); - testErrorExterns(jsdoc + "goog.define('name.2');", INVALID_DEFINE_NAME_ERROR); - testErrorExterns(jsdoc + "goog.define();", NULL_ARGUMENT_ERROR); - testErrorExterns(jsdoc + "goog.define(5);", INVALID_ARGUMENT_ERROR); - - testErrorExterns("/** @define {!number} */ goog.define('name');"); - } - @Test public void testInvalidDefine() { testError( diff --git a/test/com/google/javascript/jscomp/ijs/ConvertToTypedInterfaceTest.java b/test/com/google/javascript/jscomp/ijs/ConvertToTypedInterfaceTest.java index b9a8a8bb15a..49a30ac0db1 100644 --- a/test/com/google/javascript/jscomp/ijs/ConvertToTypedInterfaceTest.java +++ b/test/com/google/javascript/jscomp/ijs/ConvertToTypedInterfaceTest.java @@ -1171,10 +1171,25 @@ public void testConstants() { @Test public void testDefines() { // NOTE: This is another pattern that is only allowed in externs. - test("/** @define {number} */ var x = 5;", "/** @define {number} */ var x;"); + test( + "/** @define {number} */ var x = 5;", // + "/** @define {number} */ var x;"); + + test( + "/** @define {number} */ goog.define('goog.BLAH', 5);", + "/** @define {number} */ goog.define('goog.BLAH', 0);"); + + test( + "/** @define {string} */ const BLAH = goog.define('goog.BLAH', 'blah');", + "/** @define {string} */ const BLAH = goog.define('goog.BLAH', '');"); + + test( + "/** @define {boolean} */ goog.BLECH = goog.define('goog.BLAH', true);", + "/** @define {boolean} */ goog.BLECH = goog.define('goog.BLAH', false);"); - test("/** @define {number} */ goog.define('goog.BLAH', 5);", - "/** @define {number} */ goog.define('goog.BLAH');"); + test( + "/** @define {number|boolean} */ const X = goog.define('goog.XYZ', true);", + "/** @define {number|boolean} */ const X = goog.define('goog.XYZ', 0);"); } @Test