diff --git a/src/com/google/javascript/jscomp/ConvertToTypedInterface.java b/src/com/google/javascript/jscomp/ConvertToTypedInterface.java index 5ab90d0a6fa..1017acba252 100644 --- a/src/com/google/javascript/jscomp/ConvertToTypedInterface.java +++ b/src/com/google/javascript/jscomp/ConvertToTypedInterface.java @@ -30,7 +30,6 @@ import java.util.HashSet; import java.util.List; import java.util.Set; - import javax.annotation.Nullable; /** @@ -182,7 +181,8 @@ private static JSDocInfo getJSDocForName(Var decl, JSDocInfo oldJSDoc) { * This is cleared after each file to make sure that the analysis is working on a per-file basis. */ private static class FileInfo { - private final Set prefixNames = new HashSet<>(); + private final Set providedNamespaces = new HashSet<>(); + private final Set requiredLocalNames = new HashSet<>(); private final Set seenNames = new HashSet<>(); private final List constructorsToProcess = new ArrayList<>(); @@ -191,7 +191,7 @@ boolean isNameProcessed(String fullyQualifiedName) { } boolean isPrefixProvided(String fullyQualifiedName) { - for (String prefix : Iterables.concat(seenNames, prefixNames)) { + for (String prefix : Iterables.concat(seenNames, providedNamespaces)) { if (fullyQualifiedName.startsWith(prefix)) { return true; } @@ -199,6 +199,10 @@ boolean isPrefixProvided(String fullyQualifiedName) { return false; } + boolean isRequiredName(String fullyQualifiedName) { + return requiredLocalNames.contains(fullyQualifiedName); + } + void markConstructorToProcess(Node ctorNode) { Preconditions.checkArgument(ctorNode.isFunction()); constructorsToProcess.add(ctorNode); @@ -208,12 +212,16 @@ void markNameProcessed(String fullyQualifiedName) { seenNames.add(fullyQualifiedName); } - void markPrefixDefined(String namespacePrefix) { - prefixNames.add(namespacePrefix); + void markProvided(String providedName) { + providedNamespaces.add(providedName); + } + + void markImportedName(String requiredLocalName) { + requiredLocalNames.add(requiredLocalName); } void clear() { - prefixNames.clear(); + providedNamespaces.clear(); seenNames.clear(); constructorsToProcess.clear(); } @@ -284,7 +292,7 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { t.report(n, UNSUPPORTED_GOOG_SCOPE); return false; } else if (callee.matchesQualifiedName("goog.provide")) { - currentFile.markPrefixDefined(expr.getLastChild().getString()); + currentFile.markProvided(expr.getLastChild().getString()); Node childBefore; while (null != (childBefore = n.getPrevious()) && childBefore.getBooleanProp(Node.IS_NAMESPACE)) { @@ -294,8 +302,9 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { } else if (callee.matchesQualifiedName("goog.define")) { expr.getLastChild().detach(); compiler.reportCodeChange(); - } else if (!callee.matchesQualifiedName("goog.require") - && !callee.matchesQualifiedName("goog.module")) { + } else if (callee.matchesQualifiedName("goog.require")) { + processRequire(expr); + } else if (!callee.matchesQualifiedName("goog.module")) { n.detach(); compiler.reportCodeChange(); } @@ -317,8 +326,14 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { case VAR: case CONST: case LET: - if (n.hasOneChild() && NodeUtil.isStatement(n) && !isModuleImport(n)) { - processName(n.getFirstChild(), n); + if (n.hasOneChild() && NodeUtil.isStatement(n)) { + Node lhs = n.getFirstChild(); + Node rhs = lhs.getLastChild(); + if (rhs != null && isImportRhs(rhs)) { + processRequire(rhs); + } else { + processName(lhs, n); + } } break; case THROW: @@ -383,6 +398,19 @@ public void visit(NodeTraversal t, Node n, Node parent) { } } + private void processRequire(Node requireNode) { + Preconditions.checkArgument(requireNode.isCall()); + Preconditions.checkArgument(requireNode.getLastChild().isString()); + Node parent = requireNode.getParent(); + if (parent.isExprResult()) { + currentFile.markImportedName(requireNode.getLastChild().getString()); + } else { + for (Node importedName : NodeUtil.getLhsNodesOfDeclaration(parent.getParent())) { + currentFile.markImportedName(importedName.getString()); + } + } + } + private void processFunctionParameters(Node paramList) { Preconditions.checkArgument(paramList.isParamList()); for (Node arg = paramList.getFirstChild(); arg != null; arg = arg.getNext()) { @@ -473,6 +501,7 @@ private RemovalType shouldRemove(Node nameNode) { || (isExport && (rhs.isQualifiedName() || rhs.isObjectLit())) || (rhs.isQualifiedName() && rhs.matchesQualifiedName("goog.abstractMethod")) || (rhs.isQualifiedName() && rhs.matchesQualifiedName("goog.nullFunction")) + || (jsdoc != null && jsdoc.isConstructor() && rhs.isQualifiedName()) || (rhs.isObjectLit() && !rhs.hasChildren() && (jsdoc == null || !hasAnnotatedType(jsdoc)))) { @@ -491,6 +520,9 @@ private RemovalType shouldRemove(Node nameNode) { return RemovalType.REMOVE_ALL; } if (isInferrableConst(jsdoc, nameNode, isExport)) { + if (rhs.isQualifiedName() && currentFile.isRequiredName(rhs.getQualifiedName())) { + return RemovalType.PRESERVE_ALL; + } jsdocNode.setJSDocInfo(pullJsdocTypeFromAst(compiler, jsdoc, nameNode)); } return RemovalType.REMOVE_RHS; @@ -598,18 +630,6 @@ private static boolean hasAnnotatedType(JSDocInfo jsdoc) { || jsdoc.hasEnumParameterType(); } - private static boolean isModuleImport(Node importNode) { - Preconditions.checkArgument(NodeUtil.isNameDeclaration(importNode)); - Preconditions.checkArgument(importNode.hasOneChild()); - Node declarationLhs = importNode.getFirstChild(); - if (declarationLhs.hasChildren()) { - Node importRhs = declarationLhs.getLastChild(); - return NodeUtil.isCallTo(importRhs, "goog.require") - || NodeUtil.isCallTo(importRhs, "goog.forwardDeclare"); - } - return false; - } - private static boolean isClassMemberFunction(Node functionNode) { Preconditions.checkArgument(functionNode.isFunction()); Node parent = functionNode.getParent(); diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index 211bc6a7096..f4f8f95864f 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -4384,7 +4384,7 @@ private static boolean isToStringMethodCall(Node call) { /** Return declared JSDoc type for the given name declaration, or null if none present. */ @Nullable static JSTypeExpression getDeclaredTypeExpression(Node declaration) { - Preconditions.checkArgument(declaration.isName()); + Preconditions.checkArgument(declaration.isName() || declaration.isStringKey()); JSDocInfo nameJsdoc = getBestJSDocInfo(declaration); if (nameJsdoc != null) { return nameJsdoc.getType(); diff --git a/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java b/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java index 75a12bca0ed..67ee7bddf7a 100644 --- a/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java +++ b/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java @@ -766,6 +766,84 @@ public void testDontPreserveUnknownTypeDeclarations() { testSame("goog.module('x.y.z'); var C = goog.forwardDeclare('a.b.C'); /** @type {C} */ var c;"); } + public void testAliasOfRequirePreserved() { + testSame( + LINE_JOINER.join( + "goog.provide('a.b.c');", + "", + "goog.require('ns.Foo');", + "", + "/** @const */", + "a.b.c.FooAlias = ns.Foo;")); + + testSame( + LINE_JOINER.join( + "goog.provide('a.b.c');", + "", + "goog.require('ns.Foo');", + "", + "/** @constructor */", + "a.b.c.FooAlias = ns.Foo;")); + + testSameEs6( + LINE_JOINER.join( + "goog.module('mymod');", + "", + "const {Foo} = goog.require('ns.Foo');", + "", + "/** @const */", + "var FooAlias = Foo;", + "", + "/** @param {!FooAlias} f */", + "exports = function (f) {};")); + + + testSame( + LINE_JOINER.join( + "goog.module('mymod');", + "", + "var Foo = goog.require('ns.Foo');", + "", + "/** @constructor */", + "var FooAlias = Foo;", + "", + "/** @param {!FooAlias} f */", + "exports = function (f) {};")); + } + + public void testAliasOfNonRequiredName() { + testWarning( + LINE_JOINER.join( + "goog.provide('a.b.c');", + "", + "/** @const */", + "a.b.c.FooAlias = ns.Foo;"), + ConvertToTypedInterface.CONSTANT_WITHOUT_EXPLICIT_TYPE); + + testWarning( + LINE_JOINER.join( + "goog.provide('a.b.c');", + "", + "/** @constructor */", + "a.b.c.Bar = function() {", + " /** @const */", + " this.FooAlias = ns.Foo;", + "};"), + ConvertToTypedInterface.CONSTANT_WITHOUT_EXPLICIT_TYPE); + + testWarningEs6( + LINE_JOINER.join( + "goog.module('a.b.c');", + "", + "class FooAlias {", + " constructor() {", + " /** @const */", + " this.FooAlias = window.Foo;", + " }", + "};"), + ConvertToTypedInterface.CONSTANT_WITHOUT_EXPLICIT_TYPE); + } + public void testGoogScopeNotSupported() { testSameWarning( new String[] {