Skip to content

Commit

Permalink
Refactor ijs generation to consider STRING_KEY as a PotentialDeclaration
Browse files Browse the repository at this point in the history 
This allows simplification of several places that previously couldn't use
PotentialDeclaration, because they also had to run on STRING_KEY nodes, such
as namespce simplification.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=189738924
  • Loading branch information
@blickly @brad4d
blickly authored and brad4d committed Mar 20, 2018
1 parent e97a14b commit f1e34e5
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 58 deletions.
19 changes: 12 additions & 7 deletions src/com/google/javascript/jscomp/ijs/ConvertToTypedInterface.java
View file
Expand Up @@ -85,11 +85,12 @@ public ConvertToTypedInterface(AbstractCompiler compiler) {
this.compiler = compiler;
}

static void maybeWarnForConstWithoutExplicitType(
AbstractCompiler compiler, JSDocInfo jsdoc, Node nameNode) {
if (PotentialDeclaration.isConstToBeInferred(jsdoc, nameNode)
&& !nameNode.isFromExterns()
&& !JsdocUtil.isPrivate(jsdoc)) {
private static void maybeWarnForConstWithoutExplicitType(
AbstractCompiler compiler, PotentialDeclaration decl) {
if (decl.isConstToBeInferred()
&& !decl.getLhs().isFromExterns()
&& !JsdocUtil.isPrivate(decl.getJsDoc())) {
Node nameNode = decl.getLhs();
if (nameNode.getJSType() == null) {
compiler.report(JSError.make(nameNode, CONSTANT_WITHOUT_EXPLICIT_TYPE));
} else {
Expand Down Expand Up @@ -181,6 +182,9 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
NodeUtil.deleteNode(n, t.getCompiler());
return false;
}
case COMPUTED_PROP:
NodeUtil.deleteNode(n, t.getCompiler());
return false;
case THROW:
case RETURN:
case BREAK:
Expand Down Expand Up @@ -479,7 +483,8 @@ private boolean shouldRemove(String name, PotentialDeclaration decl) {
return true;
}
// This looks like an update rather than a declaration in this file.
return !decl.isDefiniteDeclaration()
return !name.startsWith("this.")
&& !decl.isDefiniteDeclaration()
&& !currentFile.isPrefixProvided(name)
&& !currentFile.isStrictPrefixDeclared(name);
}
Expand All @@ -493,7 +498,7 @@ private void setUndeclaredToUnusableType(PotentialDeclaration decl) {
|| (jsdoc != null && jsdoc.containsDeclaration() && !decl.isConstToBeInferred())) {
return;
}
maybeWarnForConstWithoutExplicitType(compiler, jsdoc, nameNode);
maybeWarnForConstWithoutExplicitType(compiler, decl);
Node jsdocNode = NodeUtil.getBestJSDocInfoNode(nameNode);
jsdocNode.setJSDocInfo(JsdocUtil.getUnusableTypeJSDoc(jsdoc));
}
Expand Down
4 changes: 4 additions & 0 deletions src/com/google/javascript/jscomp/ijs/FileInfo.java
View file
Expand Up @@ -40,6 +40,10 @@ void recordMethod(Node functionNode) {
recordDeclaration(PotentialDeclaration.fromMethod(functionNode));
}

void recordStringKeyDeclaration(Node stringKeyNode) {
recordDeclaration(PotentialDeclaration.fromStringKey(stringKeyNode));
}

void recordDefine(Node callNode) {
recordDeclaration(PotentialDeclaration.fromDefine(callNode));
}
Expand Down
116 changes: 68 additions & 48 deletions src/com/google/javascript/jscomp/ijs/PotentialDeclaration.java
View file
Expand Up @@ -66,6 +66,13 @@ static PotentialDeclaration fromMethod(Node functionNode) {
return new MethodDeclaration(name, functionNode);
}

static PotentialDeclaration fromStringKey(Node stringKeyNode) {
checkArgument(stringKeyNode.isStringKey());
checkArgument(stringKeyNode.getParent().isObjectLit());
String name = "this." + stringKeyNode.getString();
return new StringKeyDeclaration(name, stringKeyNode);
}

static PotentialDeclaration fromDefine(Node callNode) {
checkArgument(NodeUtil.isCallTo(callNode, "goog.define"));
return new DefineDeclaration(callNode);
Expand Down Expand Up @@ -129,44 +136,18 @@ final void remove(AbstractCompiler compiler) {
* and goog.module exports
* This is the most common type of potential declaration.
*/
static class NameDeclaration extends PotentialDeclaration {
private static class NameDeclaration extends PotentialDeclaration {

NameDeclaration(String fullyQualifiedName, Node lhs, Node rhs) {
super(fullyQualifiedName, lhs, rhs);
}

private void simplifyNamespace(AbstractCompiler compiler) {
Node objLit = getRhs();
if (getRhs().isOr()) {
objLit = getRhs().getLastChild().detach();
Node objLit = getRhs().getLastChild().detach();
getRhs().replaceWith(objLit);
compiler.reportChangeToEnclosingScope(getLhs());
}
if (objLit.hasChildren()) {
for (Node key : objLit.children()) {
switch (key.getToken()) {
case STRING_KEY:
if (!isTypedRhs(key.getLastChild())) {
ConvertToTypedInterface
.maybeWarnForConstWithoutExplicitType(compiler, key.getJSDocInfo(), key);
removeStringKeyValue(key);
maybeUpdateJsdoc(key);
compiler.reportChangeToEnclosingScope(key);
}
break;
case GETTER_DEF:
case MEMBER_FUNCTION_DEF:
case SETTER_DEF:
// Nothing to simplify here...
break;
case COMPUTED_PROP:
NodeUtil.deleteNode(key, compiler);
break;
default:
throw new IllegalStateException("Unexpected object literal body: " + key);
}
}
}
}

private void simplifySymbol(AbstractCompiler compiler) {
Expand Down Expand Up @@ -229,22 +210,6 @@ private static void replaceRhsWithUnknown(Node rhs) {
rhs.replaceWith(IR.cast(IR.number(0), JsdocUtil.getQmarkTypeJSDoc()).srcrefTree(rhs));
}

private static void removeStringKeyValue(Node stringKey) {
Node value = stringKey.getOnlyChild();
Node replacementValue = IR.number(0).srcrefTree(value);
stringKey.replaceChild(value, replacementValue);
}

private static void maybeUpdateJsdoc(Node jsdocNode) {
checkArgument(jsdocNode.isStringKey(), jsdocNode);
JSDocInfo jsdoc = jsdocNode.getJSDocInfo();
if (jsdoc == null
|| !jsdoc.containsDeclaration()
|| isConstToBeInferred(jsdoc, jsdocNode)) {
jsdocNode.setJSDocInfo(JsdocUtil.getUnusableTypeJSDoc(jsdoc));
}
}

@Override
boolean shouldPreserve() {
Node rhs = getRhs();
Expand All @@ -265,7 +230,7 @@ boolean shouldPreserve() {
/**
* A declaration of a property on `this` inside a constructor.
*/
static class ThisPropDeclaration extends PotentialDeclaration {
private static class ThisPropDeclaration extends PotentialDeclaration {
private final Node insertionPoint;

ThisPropDeclaration(String fullyQualifiedName, Node lhs, Node rhs) {
Expand Down Expand Up @@ -323,6 +288,54 @@ Node getRemovableNode() {
}
}

private static class StringKeyDeclaration extends PotentialDeclaration {
StringKeyDeclaration(String name, Node stringKeyNode) {
super(name, stringKeyNode, stringKeyNode.getLastChild());
}

@Override
void simplify(AbstractCompiler compiler) {
if (shouldPreserve()) {
return;
}
if (!isTypedRhs(getRhs())) {
Node key = getLhs();
removeStringKeyValue(key);
JSDocInfo jsdoc = getJsDoc();
if (jsdoc == null
|| !jsdoc.containsDeclaration()
|| isConstToBeInferred()) {
key.setJSDocInfo(JsdocUtil.getUnusableTypeJSDoc(jsdoc));
}
compiler.reportChangeToEnclosingScope(key);
}
}

@Override
boolean shouldPreserve() {
return super.isDetached() || super.shouldPreserve() || !isInNamespace();
}

private boolean isInNamespace() {
Node stringKey = getLhs();
Node objLit = stringKey.getParent();
Node lvalue = NodeUtil.getBestLValue(objLit);
if (lvalue == null) {
return false;
}
JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(lvalue);
return !isExportLhs(lvalue)
&& !JsdocUtil.hasAnnotatedType(jsdoc)
&& NodeUtil.isNamespaceDecl(lvalue);
}

@Override
Node getRemovableNode() {
return getLhs();
}

}

boolean isDefiniteDeclaration() {
Node parent = getLhs().getParent();
switch (parent.getToken()) {
Expand All @@ -344,11 +357,11 @@ boolean shouldPreserve() {
}

boolean isConstToBeInferred() {
return isConstToBeInferred(getJsDoc(), getLhs());
return isConstToBeInferred(getLhs());
}

static boolean isConstToBeInferred(
JSDocInfo jsdoc, Node nameNode) {
static boolean isConstToBeInferred(Node nameNode) {
JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(nameNode);
boolean isConst =
nameNode.getParent().isConst()
|| isExportLhs(nameNode)
Expand Down Expand Up @@ -379,4 +392,11 @@ static boolean isImportRhs(@Nullable Node rhs) {
return callee.matchesQualifiedName("goog.require")
|| callee.matchesQualifiedName("goog.forwardDeclare");
}

private static void removeStringKeyValue(Node stringKey) {
Node value = stringKey.getOnlyChild();
Node replacementValue = IR.number(0).srcrefTree(value);
stringKey.replaceChild(value, replacementValue);
}

}
5 changes: 2 additions & 3 deletions src/com/google/javascript/jscomp/ijs/ProcessConstJsdocCallback.java
View file
Expand Up @@ -20,7 +20,6 @@

import com.google.javascript.jscomp.NodeTraversal;
import com.google.javascript.jscomp.NodeUtil;
import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.Node;

/**
Expand Down Expand Up @@ -91,6 +90,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
case STRING_KEY:
if (parent.isObjectLit() && n.hasOneChild()) {
processDeclarationWithRhs(t, n);
currentFile.recordStringKeyDeclaration(n);
}
break;
default:
Expand All @@ -116,8 +116,7 @@ private void processDeclarationWithRhs(NodeTraversal t, Node lhs) {
lhs.isQualifiedName() || lhs.isStringKey() || lhs.isDestructuringLhs(),
lhs);
checkState(NodeUtil.getRValueOfLValue(lhs) != null, lhs);
JSDocInfo originalJsdoc = NodeUtil.getBestJSDocInfo(lhs);
if (!PotentialDeclaration.isConstToBeInferred(originalJsdoc, lhs)) {
if (!PotentialDeclaration.isConstToBeInferred(lhs)) {
return;
}
processConstWithRhs(t, lhs);
Expand Down

0 comments on commit f1e34e5

Please sign in to comment.