Skip to content

Commit

Permalink
Allow aliases of qualified names to be preserved in .i.js files.
Browse files Browse the repository at this point in the history 
This means that @const annotated names without an inferrable type will cease
giving the warning about requiring an explicit type.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=146810572
  • Loading branch information
@blickly
blickly committed Feb 8, 2017
1 parent 4d19a90 commit 42d1055
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 24 deletions.
66 changes: 43 additions & 23 deletions src/com/google/javascript/jscomp/ConvertToTypedInterface.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -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<String> prefixNames = new HashSet<>();
private final Set<String> providedNamespaces = new HashSet<>();
private final Set<String> requiredLocalNames = new HashSet<>();
private final Set<String> seenNames = new HashSet<>();
private final List<Node> constructorsToProcess = new ArrayList<>();

Expand All @@ -191,14 +191,18 @@ 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;
}
}
return false;
}

boolean isRequiredName(String fullyQualifiedName) {
return requiredLocalNames.contains(fullyQualifiedName);
}

void markConstructorToProcess(Node ctorNode) {
Preconditions.checkArgument(ctorNode.isFunction());
constructorsToProcess.add(ctorNode);
Expand All @@ -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();
}
Expand Down Expand Up @@ -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)) {
Expand All @@ -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();
}
Expand All @@ -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:
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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)))) {
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/NodeUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
78 changes: 78 additions & 0 deletions test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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[] {
Expand Down

0 comments on commit 42d1055

Please sign in to comment.