Skip to content

Commit

Permalink
In RemoveBodies, generate type annotations for inferred @const defini…
Browse files Browse the repository at this point in the history
…tion.

e.g.
Given
    /** @const */ var x = 5;
process this to
    /** @const {number} */ var x;

Since this relies on type information, this is currently only done when
type-checking is enabled.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=124373828
  • Loading branch information
blickly committed Jun 8, 2016
1 parent aa98732 commit 2ab3c68
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 16 deletions.
55 changes: 40 additions & 15 deletions src/com/google/javascript/jscomp/RemoveBodies.java
Expand Up @@ -25,6 +25,7 @@
import com.google.javascript.rhino.JSTypeExpression;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token;
import com.google.javascript.rhino.jstype.JSType;

import java.util.HashSet;
import java.util.Set;
Expand Down Expand Up @@ -177,10 +178,18 @@ public void visit(NodeTraversal t, Node n, Node parent) {
if (!name.isGetProp() || !name.getFirstChild().isThis()) {
return;
}
JSType type = name.getJSType();
String pname = name.getLastChild().getString();
JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(name);
// Don't use an initializer unless we need it for the non-typechecked case.
Node initializer = null;
if (type == null) {
initializer = NodeUtil.getRValueOfLValue(name).detachFromParent();
} else if (isInferrableConst(jsdoc)) {
jsdoc = maybeUpdateJSDocInfoWithType(jsdoc, name);
}
Node newProtoAssignStmt = NodeUtil.newQNameDeclaration(
compiler, className + ".prototype." + pname, null, jsdoc);
compiler, className + ".prototype." + pname, initializer, jsdoc);
newProtoAssignStmt.useSourceInfoIfMissingFromForTree(expr);
// TODO(blickly): Preserve the declaration order of the this properties.
insertionPoint.getParent().addChildAfter(newProtoAssignStmt, insertionPoint);
Expand All @@ -194,7 +203,6 @@ enum RemovalType {
PRESERVE_ALL,
REMOVE_RHS,
REMOVE_ALL,
UNDECLARED,
}

private static boolean isClassMemberFunction(Node functionNode) {
Expand Down Expand Up @@ -243,11 +251,22 @@ private RemovalType shouldRemove(Node nameNode) {
if (seenNames.contains(nameNode.getQualifiedName())) {
return RemovalType.REMOVE_ALL;
}
return RemovalType.UNDECLARED;
jsdocNode.setJSDocInfo(getAllTypeJSDoc());
return RemovalType.REMOVE_RHS;
}
if (isInferrableConst(jsdoc) && !NodeUtil.isNamespaceDecl(nameNode)) {
if (nameNode.getJSType() == null) {
return RemovalType.PRESERVE_ALL;
}
jsdocNode.setJSDocInfo(maybeUpdateJSDocInfoWithType(jsdoc, nameNode));
}
return RemovalType.REMOVE_RHS;
}

private static boolean isInferrableConst(JSDocInfo jsdoc) {
return jsdoc != null && jsdoc.hasConstAnnotation() && !jsdoc.hasType();
}

private void processName(Node nameNode, Node statement) {
Preconditions.checkState(NodeUtil.isStatement(statement), statement);
if (!nameNode.isQualifiedName()) {
Expand All @@ -262,9 +281,6 @@ private void processName(Node nameNode, Node statement) {
break;
case PRESERVE_ALL:
break;
case UNDECLARED:
jsdocNode.setJSDocInfo(getAllTypeJSDoc());
// Fall-through
case REMOVE_RHS:
maybeRemoveRhs(nameNode, statement, jsdocNode.getJSDocInfo());
break;
Expand All @@ -278,6 +294,21 @@ private static JSDocInfo getAllTypeJSDoc() {
return builder.build();
}

private static JSDocInfo maybeUpdateJSDocInfoWithType(JSDocInfo oldJSDoc, Node nameNode) {
Preconditions.checkArgument(nameNode.isQualifiedName());
JSType type = nameNode.getJSType();
if (type == null) {
return oldJSDoc;
}
return getTypeJSDoc(oldJSDoc, type.toNonNullAnnotationString());
}

private static JSDocInfo getTypeJSDoc(JSDocInfo oldJSDoc, String contents) {
JSDocInfoBuilder builder = JSDocInfoBuilder.copyFrom(oldJSDoc);
builder.recordType(new JSTypeExpression(Node.newString(contents), ""));
return builder.build();
}

private void removeNode(Node n) {
if (NodeUtil.isStatement(n)) {
n.detachFromParent();
Expand All @@ -288,15 +319,9 @@ private void removeNode(Node n) {
}

private void maybeRemoveRhs(Node nameNode, Node statement, JSDocInfo jsdoc) {
if (jsdoc != null) {
if (jsdoc.hasConstAnnotation() && !jsdoc.hasType() && !NodeUtil.isNamespaceDecl(nameNode)) {
// Inferred @const types require the RHS to be left in.
return;
}
if (jsdoc.hasEnumParameterType()) {
removeEnumValues(NodeUtil.getRValueOfLValue(nameNode));
return;
}
if (jsdoc != null && jsdoc.hasEnumParameterType()) {
removeEnumValues(NodeUtil.getRValueOfLValue(nameNode));
return;
}
Node newStatement =
NodeUtil.newQNameDeclaration(compiler, nameNode.getQualifiedName(), null, jsdoc);
Expand Down
12 changes: 11 additions & 1 deletion test/com/google/javascript/jscomp/RemoveBodiesTest.java
Expand Up @@ -24,10 +24,14 @@ protected CompilerPass getProcessor(final Compiler compiler) {
return new RemoveBodies(compiler);
}

public void disabled_testInferAnnotatedTypeFromInferredType() {
public void testInferAnnotatedTypeFromInferredType() {
enableTypeCheck();

test("/** @const */ var x = 5;", "/** @const {number} */ var x;");

test(
"/** @constructor */ function Foo() { /** @const */ this.x = 5; }",
"/** @constructor */ function Foo() {} \n /** @const {number} */ Foo.prototype.x;");
}

public void testRemoveUselessStatements() {
Expand Down Expand Up @@ -114,6 +118,12 @@ public void testConstants() {

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() {
// NOTE: This is another pattern that is only allowed in externs.
test("/** @define {number} */ var x = 5;", "/** @define {number} */ var x;");
}
Expand Down

0 comments on commit 2ab3c68

Please sign in to comment.