Skip to content

Commit

Permalink
Split up ConvertToTypedInterface.shouldRemove
Browse files Browse the repository at this point in the history 
It was previously sorting out which declarations to remove, preserve, simplify, as well as attaching the unusable JSDoc (/** @const {*} */) for undeclared symbols and warning.
This change splits up those different tasks across separate methods without changing the behavior.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=185567955
  • Loading branch information
@blickly @brad4d
blickly authored and brad4d committed Feb 14, 2018
1 parent 923f421 commit 26f0edd
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 66 deletions.
108 changes: 43 additions & 65 deletions src/com/google/javascript/jscomp/ijs/ConvertToTypedInterface.java
View file
Expand Up @@ -459,29 +459,29 @@ void simplifyAll() {
// Simplify all names in the top-level scope.
List<String> seenNames =
SHORT_TO_LONG.immutableSortedCopy(currentFile.getDeclarations().keySet());

for (String name : seenNames) {
for (PotentialDeclaration decl : currentFile.getDeclarations().get(name)) {
processDeclaration(decl);
processDeclaration(name, decl);
}
}
}

private void processDeclaration(PotentialDeclaration decl) {
switch (shouldRemove(decl)) {
case PRESERVE_ALL:
if (decl.getRhs() != null && decl.getRhs().isFunction()) {
processFunction(decl.getRhs());
} else if (decl.getRhs() != null && isClass(decl.getRhs())) {
processClass(decl.getRhs());
}
break;
case SIMPLIFY_RHS:
decl.simplify(compiler);
break;
case REMOVE_ALL:
decl.remove(compiler);
break;
private void processDeclaration(String name, PotentialDeclaration decl) {
if (shouldRemove(name, decl)) {
decl.remove(compiler);
return;
}
if (isAliasDefinition(decl)) {
return;
}
if (decl.getRhs() != null && decl.getRhs().isFunction()) {
processFunction(decl.getRhs());
} else if (decl.getRhs() != null && isClass(decl.getRhs())) {
processClass(decl.getRhs());
}
setUndeclaredToUnusableType(decl);
decl.simplify(compiler);
}

private void processClass(Node n) {
Expand Down Expand Up @@ -512,12 +512,6 @@ private void processFunctionParameters(Node paramList) {
}
}

enum RemovalType {
PRESERVE_ALL,
SIMPLIFY_RHS,
REMOVE_ALL,
}

private static boolean isClass(Node n) {
return n.isClass() || NodeUtil.isCallTo(n, "goog.defineClass");
}
Expand All @@ -530,64 +524,48 @@ private static String rootName(String qualifiedName) {
return qualifiedName.substring(0, dotIndex);
}

private RemovalType shouldRemove(PotentialDeclaration decl) {
String fullyQualifiedName = decl.getFullyQualifiedName();
if ("$jscomp".equals(rootName(fullyQualifiedName))) {
private boolean shouldRemove(String name, PotentialDeclaration decl) {
if ("$jscomp".equals(rootName(name))) {
// These are created by goog.scope processing, but clash with each other
// and should not be depended on.
return RemovalType.REMOVE_ALL;
return true;
}
// This looks like an update rather than a declaration in this file.
return !decl.isDefiniteDeclaration()
&& !currentFile.isPrefixProvided(name)
&& !currentFile.isStrictPrefixDeclared(name);
}

private void setUndeclaredToUnusableType(PotentialDeclaration decl) {
Node nameNode = decl.getLhs();
Node rhs = decl.getRhs();
JSDocInfo jsdoc = decl.getJsDoc();
boolean isExport = isExportLhs(nameNode);
if (rhs == null) {
return RemovalType.SIMPLIFY_RHS;
}
if (PotentialDeclaration.isTypedRhs(rhs)
|| isImportRhs(rhs)
|| (isExport && (rhs.isQualifiedName() || rhs.isObjectLit()))
|| (jsdoc != null && jsdoc.isConstructor() && rhs.isQualifiedName())
|| isAliasDefinition(decl)
|| (nameNode.matchesQualifiedName("goog.global") && rhs.isThis())
|| (rhs.isObjectLit()
&& !rhs.hasChildren()
&& (jsdoc == null || !JsdocUtil.hasAnnotatedType(jsdoc)))) {
return RemovalType.PRESERVE_ALL;
}
if (NodeUtil.isNamespaceDecl(nameNode)) {
return RemovalType.SIMPLIFY_RHS;
if (decl.shouldPreserve()
|| NodeUtil.isNamespaceDecl(nameNode)
|| (jsdoc != null
&& jsdoc.containsDeclaration()
&& !isConstToBeInferred(jsdoc, nameNode))) {
return;
}
maybeWarnForConstWithoutExplicitType(compiler, jsdoc, nameNode);
Node jsdocNode = NodeUtil.getBestJSDocInfoNode(nameNode);
if (isConstToBeInferred(jsdoc, nameNode)) {
jsdocNode.setJSDocInfo(JsdocUtil.getUnusableTypeJSDoc(jsdoc));
maybeWarnForConstWithoutExplicitType(compiler, jsdoc, nameNode);
return RemovalType.SIMPLIFY_RHS;
}
if (jsdoc == null || !jsdoc.containsDeclaration()) {
if (!isDeclaration(nameNode)
&& !currentFile.isPrefixProvided(fullyQualifiedName)
&& !currentFile.isStrictPrefixDeclared(fullyQualifiedName)) {
// This looks like an update rather than a declaration in this file.
return RemovalType.REMOVE_ALL;
}
jsdocNode.setJSDocInfo(JsdocUtil.getUnusableTypeJSDoc(jsdoc));
}
return RemovalType.SIMPLIFY_RHS;
jsdocNode.setJSDocInfo(JsdocUtil.getUnusableTypeJSDoc(jsdoc));
}

private boolean isAliasDefinition(PotentialDeclaration decl) {
Node rhs = decl.getRhs();
if (isConstToBeInferred(decl.getJsDoc(), decl.getLhs())
&& decl.getRhs().isQualifiedName()) {
String aliasedName = decl.getRhs().getQualifiedName();
return currentFile.isPrefixRequired(aliasedName) || currentFile.isNameDeclared(aliasedName);
&& rhs != null && rhs.isQualifiedName()) {
String aliasedName = rhs.getQualifiedName();
return rhs.isThis()
|| currentFile.isPrefixRequired(aliasedName)
|| currentFile.isNameDeclared(aliasedName);
}
return false;
}
}

// TODO(blickly): Move to NodeUtil if it makes more sense there.
private static boolean isDeclaration(Node nameNode) {
static boolean isDeclaration(Node nameNode) {
checkArgument(nameNode.isQualifiedName());
Node parent = nameNode.getParent();
switch (parent.getToken()) {
Expand All @@ -613,12 +591,12 @@ static boolean isConstToBeInferred(
&& !NodeUtil.isNamespaceDecl(nameNode);
}

private static boolean isExportLhs(Node lhs) {
static boolean isExportLhs(Node lhs) {
return (lhs.isName() && lhs.matchesQualifiedName("exports"))
|| (lhs.isGetProp() && lhs.getFirstChild().matchesQualifiedName("exports"));
}

private static boolean isImportRhs(@Nullable Node rhs) {
static boolean isImportRhs(@Nullable Node rhs) {
if (rhs == null || !rhs.isCall()) {
return false;
}
Expand Down
33 changes: 32 additions & 1 deletion src/com/google/javascript/jscomp/ijs/PotentialDeclaration.java
View file
Expand Up @@ -172,7 +172,7 @@ void simplifyNamespace(AbstractCompiler compiler) {

@Override
void simplify(AbstractCompiler compiler) {
if (getRhs() == null) {
if (getRhs() == null || shouldPreserve()) {
return;
}
Node nameNode = getLhs();
Expand Down Expand Up @@ -226,6 +226,23 @@ private static void maybeUpdateJsdoc(Node jsdocNode) {
jsdocNode.setJSDocInfo(JsdocUtil.getUnusableTypeJSDoc(jsdoc));
}
}

@Override
boolean shouldPreserve() {
Node rhs = getRhs();
Node nameNode = getLhs();
JSDocInfo jsdoc = getJsDoc();
boolean isExport = ConvertToTypedInterface.isExportLhs(nameNode);
return super.shouldPreserve()
|| ConvertToTypedInterface.isImportRhs(rhs)
|| (isExport && rhs != null && (rhs.isQualifiedName() || rhs.isObjectLit()))
|| (jsdoc != null && jsdoc.isConstructor() && rhs != null && rhs.isQualifiedName())
|| (nameNode.matchesQualifiedName("goog.global") && rhs != null && rhs.isThis())
|| (rhs != null
&& rhs.isObjectLit()
&& !rhs.hasChildren()
&& (jsdoc == null || !JsdocUtil.hasAnnotatedType(jsdoc)));
}
}

/**
Expand All @@ -242,6 +259,9 @@ static class ThisPropDeclaration extends PotentialDeclaration {

@Override
void simplify(AbstractCompiler compiler) {
if (shouldPreserve()) {
return;
}
// Just completely remove the RHS, if present, and replace with a getprop.
Node newStatement =
NodeUtil.newQNameDeclaration(compiler, getFullyQualifiedName(), null, getJsDoc());
Expand Down Expand Up @@ -286,6 +306,17 @@ Node getRemovableNode() {
}
}

boolean isDefiniteDeclaration() {
return ConvertToTypedInterface.isExportLhs(getLhs())
|| (getJsDoc() != null && getJsDoc().containsDeclaration())
|| (rhs != null && PotentialDeclaration.isTypedRhs(rhs))
|| ConvertToTypedInterface.isDeclaration(getLhs());
}

boolean shouldPreserve() {
return getRhs() != null && isTypedRhs(getRhs());
}

static boolean isTypedRhs(Node rhs) {
return rhs.isFunction()
|| rhs.isClass()
Expand Down
4 changes: 4 additions & 0 deletions test/com/google/javascript/jscomp/ijs/ConvertToTypedInterfaceTest.java
View file
Expand Up @@ -52,6 +52,10 @@ public void testExternsDefinitionsRespected() {
test(externs("/** @type {number} */ var x;"), srcs("x = 7;"), expected(""));
}

public void testUnannotatedDeclaration() {
test("var x;", "/** @const {*} */ var x;");
}

public void testSimpleConstJsdocPropagation() {
test("/** @const */ var x = 5;", "/** @const {number} */ var x;");
test("/** @const */ var x = true;", "/** @const {boolean} */ var x;");
Expand Down

0 comments on commit 26f0edd

Please sign in to comment.