diff --git a/src/com/google/javascript/jscomp/ConvertToTypedInterface.java b/src/com/google/javascript/jscomp/ConvertToTypedInterface.java index 1ba6885832e..f57f9934fc6 100644 --- a/src/com/google/javascript/jscomp/ConvertToTypedInterface.java +++ b/src/com/google/javascript/jscomp/ConvertToTypedInterface.java @@ -45,6 +45,11 @@ */ class ConvertToTypedInterface implements CompilerPass { + static final DiagnosticType CONSTANT_WITHOUT_EXPLICIT_TYPE = + DiagnosticType.error( + "JSC_CONSTANT_WITHOUT_EXPLICIT_TYPE", + "/** @const */-annotated values in library API should have types explicitly specified."); + private final AbstractCompiler compiler; ConvertToTypedInterface(AbstractCompiler compiler) { @@ -53,9 +58,73 @@ class ConvertToTypedInterface implements CompilerPass { @Override public void process(Node externs, Node root) { + NodeTraversal.traverseEs6(compiler, root, new PropagateConstJsdoc(compiler)); NodeTraversal.traverseRootsEs6(compiler, new RemoveCode(compiler), externs, root); } + private static class PropagateConstJsdoc extends NodeTraversal.AbstractPostOrderCallback { + private final AbstractCompiler compiler; + + PropagateConstJsdoc(AbstractCompiler compiler) { + this.compiler = compiler; + } + + @Override + public void visit(NodeTraversal t, Node n, Node parent) { + switch (n.getType()) { + case EXPR_RESULT: + if (NodeUtil.isExprAssign(n)) { + Node expr = n.getFirstChild(); + processName(expr.getFirstChild()); + } + break; + case VAR: + case CONST: + case LET: + if (n.getChildCount() == 1) { + processName(n.getFirstChild()); + } + break; + default: + break; + } + } + + private void processName(Node nameNode) { + Node jsdocNode = NodeUtil.getBestJSDocInfoNode(nameNode); + JSDocInfo jsdoc = jsdocNode.getJSDocInfo(); + if (!isInferrableConst(jsdoc, nameNode)) { + return; + } + Node rhs = NodeUtil.getRValueOfLValue(nameNode); + if (rhs == null) { + return; + } + JSDocInfo newJsdoc = getTypedJSDoc(rhs, jsdoc); + if (newJsdoc != null) { + jsdocNode.setJSDocInfo(newJsdoc); + compiler.reportCodeChange(); + } + } + + private static JSDocInfo getTypedJSDoc(Node rhs, JSDocInfo oldJSDoc) { + switch (NodeUtil.getKnownValueType(rhs)) { + case BOOLEAN: + return getTypeJSDoc(oldJSDoc, "boolean"); + case NUMBER: + return getTypeJSDoc(oldJSDoc, "number"); + case STRING: + return getTypeJSDoc(oldJSDoc, "string"); + case NULL: + return getTypeJSDoc(oldJSDoc, "null"); + case VOID: + return getTypeJSDoc(oldJSDoc, "void"); + default: + return null; + } + } + } + private static class RemoveCode implements Callback { private final AbstractCompiler compiler; private final Set seenNames = new HashSet<>(); @@ -202,7 +271,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { Node initializer = null; if (type == null) { initializer = NodeUtil.getRValueOfLValue(name).detachFromParent(); - } else if (isInferrableConst(jsdoc)) { + } else if (isInferrableConst(jsdoc, name)) { jsdoc = maybeUpdateJSDocInfoWithType(jsdoc, name); } Node newProtoAssignStmt = NodeUtil.newQNameDeclaration( @@ -248,8 +317,9 @@ private RemovalType shouldRemove(Node nameNode) { jsdocNode.setJSDocInfo(getAllTypeJSDoc()); return RemovalType.REMOVE_RHS; } - if (isInferrableConst(jsdoc) && !NodeUtil.isNamespaceDecl(nameNode)) { + if (isInferrableConst(jsdoc, nameNode)) { if (nameNode.getJSType() == null) { + compiler.report(JSError.make(nameNode, CONSTANT_WITHOUT_EXPLICIT_TYPE)); return RemovalType.PRESERVE_ALL; } jsdocNode.setJSDocInfo(maybeUpdateJSDocInfoWithType(jsdoc, nameNode)); @@ -311,8 +381,12 @@ private void removeEnumValues(Node objLit) { } } - private static boolean isInferrableConst(JSDocInfo jsdoc) { - return jsdoc != null && jsdoc.hasConstAnnotation() && !jsdoc.hasType(); + private static boolean isInferrableConst(JSDocInfo jsdoc, Node nameNode) { + return jsdoc != null + && jsdoc.hasConstAnnotation() + && !jsdoc.hasType() + && !jsdoc.isConstructorOrInterface() + && !NodeUtil.isNamespaceDecl(nameNode); } private static boolean isClassMemberFunction(Node functionNode) { diff --git a/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java b/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java index 11dab808468..d9c4ddcf34a 100644 --- a/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java +++ b/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java @@ -24,7 +24,7 @@ protected CompilerPass getProcessor(final Compiler compiler) { return new ConvertToTypedInterface(compiler); } - public void testInferAnnotatedTypeFromInferredType() { + public void testInferAnnotatedTypeFromTypeInference() { enableTypeCheck(); test("/** @const */ var x = 5;", "/** @const {number} */ var x;"); @@ -34,6 +34,23 @@ public void testInferAnnotatedTypeFromInferredType() { "/** @constructor */ function Foo() {} \n /** @const {number} */ Foo.prototype.x;"); } + public void testConstJsdocPropagation() { + + test("/** @const */ var x = 5;", "/** @const {number} */ var x;"); + test("/** @const */ var x = true;", "/** @const {boolean} */ var x;"); + test("/** @const */ var x = 'str';", "/** @const {string} */ var x;"); + test("/** @const */ var x = null;", "/** @const {null} */ var x;"); + test("/** @const */ var x = void 0;", "/** @const {void} */ var x;"); + + test( + "/** @constructor */ function Foo() { /** @const */ this.x = 5; }", + "/** @constructor */ function Foo() {} \n /** @const {number} */ Foo.prototype.x;"); + + testError( + "/** @const */ var x = cond ? true : 5;", + ConvertToTypedInterface.CONSTANT_WITHOUT_EXPLICIT_TYPE); + } + public void testRemoveUselessStatements() { test("34", ""); test("'str'", ""); @@ -123,12 +140,6 @@ public void testIIFE() { public void testConstants() { test("/** @const {number} */ var x = 5;", "/** @const {number} */ var x;"); - - testSame("/** @const */ var x = 5;"); - - test( - "/** @constructor */ function Foo() { /** @const */ this.x = 5; }", - "/** @constructor */ function Foo() {} \n /** @const */ Foo.prototype.x = 5;"); } public void testDefines() {