Skip to content

Commit

Permalink
Add support for 'syntactic' inference of /** @const */ types for .i.j…
Browse files Browse the repository at this point in the history 
…s files.

The idea of this is to allow .i.js generation to work better with type-checking
turned off, allowing faster builds.

This initial version only supports inferring the type of a literal RHS, but
it should be easy to extend this support to work on an RHS of a simple name
that has a declared type.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=127355199
  • Loading branch information
@blickly
blickly committed Jul 13, 2016
1 parent 2bb100f commit 07077df
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 11 deletions.
82 changes: 78 additions & 4 deletions src/com/google/javascript/jscomp/ConvertToTypedInterface.java
View file
Expand Up @@ -45,6 +45,11 @@
*/ */
class ConvertToTypedInterface implements CompilerPass { class ConvertToTypedInterface implements CompilerPass {


static final DiagnosticType CONSTANT_WITHOUT_EXPLICIT_TYPE =
DiagnosticType.error(
"JSC_CONSTANT_WITHOUT_EXPLICIT_TYPE",
"/** @const */-annotated values in library API should have types explicitly specified.");

private final AbstractCompiler compiler; private final AbstractCompiler compiler;


ConvertToTypedInterface(AbstractCompiler compiler) { ConvertToTypedInterface(AbstractCompiler compiler) {
Expand All @@ -53,9 +58,73 @@ class ConvertToTypedInterface implements CompilerPass {


@Override @Override
public void process(Node externs, Node root) { public void process(Node externs, Node root) {
NodeTraversal.traverseEs6(compiler, root, new PropagateConstJsdoc(compiler));
NodeTraversal.traverseRootsEs6(compiler, new RemoveCode(compiler), externs, root); NodeTraversal.traverseRootsEs6(compiler, new RemoveCode(compiler), externs, root);
} }


private static class PropagateConstJsdoc extends NodeTraversal.AbstractPostOrderCallback {
private final AbstractCompiler compiler;

PropagateConstJsdoc(AbstractCompiler compiler) {
this.compiler = compiler;
}

@Override
public void visit(NodeTraversal t, Node n, Node parent) {
switch (n.getType()) {
case EXPR_RESULT:
if (NodeUtil.isExprAssign(n)) {
Node expr = n.getFirstChild();
processName(expr.getFirstChild());
}
break;
case VAR:
case CONST:
case LET:
if (n.getChildCount() == 1) {
processName(n.getFirstChild());
}
break;
default:
break;
}
}

private void processName(Node nameNode) {
Node jsdocNode = NodeUtil.getBestJSDocInfoNode(nameNode);
JSDocInfo jsdoc = jsdocNode.getJSDocInfo();
if (!isInferrableConst(jsdoc, nameNode)) {
return;
}
Node rhs = NodeUtil.getRValueOfLValue(nameNode);
if (rhs == null) {
return;
}
JSDocInfo newJsdoc = getTypedJSDoc(rhs, jsdoc);
if (newJsdoc != null) {
jsdocNode.setJSDocInfo(newJsdoc);
compiler.reportCodeChange();
}
}

private static JSDocInfo getTypedJSDoc(Node rhs, JSDocInfo oldJSDoc) {
switch (NodeUtil.getKnownValueType(rhs)) {
case BOOLEAN:
return getTypeJSDoc(oldJSDoc, "boolean");
case NUMBER:
return getTypeJSDoc(oldJSDoc, "number");
case STRING:
return getTypeJSDoc(oldJSDoc, "string");
case NULL:
return getTypeJSDoc(oldJSDoc, "null");
case VOID:
return getTypeJSDoc(oldJSDoc, "void");
default:
return null;
}
}
}

private static class RemoveCode implements Callback { private static class RemoveCode implements Callback {
private final AbstractCompiler compiler; private final AbstractCompiler compiler;
private final Set<String> seenNames = new HashSet<>(); private final Set<String> seenNames = new HashSet<>();
Expand Down Expand Up @@ -202,7 +271,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
Node initializer = null; Node initializer = null;
if (type == null) { if (type == null) {
initializer = NodeUtil.getRValueOfLValue(name).detachFromParent(); initializer = NodeUtil.getRValueOfLValue(name).detachFromParent();
} else if (isInferrableConst(jsdoc)) { } else if (isInferrableConst(jsdoc, name)) {
jsdoc = maybeUpdateJSDocInfoWithType(jsdoc, name); jsdoc = maybeUpdateJSDocInfoWithType(jsdoc, name);
} }
Node newProtoAssignStmt = NodeUtil.newQNameDeclaration( Node newProtoAssignStmt = NodeUtil.newQNameDeclaration(
Expand Down Expand Up @@ -248,8 +317,9 @@ private RemovalType shouldRemove(Node nameNode) {
jsdocNode.setJSDocInfo(getAllTypeJSDoc()); jsdocNode.setJSDocInfo(getAllTypeJSDoc());
return RemovalType.REMOVE_RHS; return RemovalType.REMOVE_RHS;
} }
if (isInferrableConst(jsdoc) && !NodeUtil.isNamespaceDecl(nameNode)) { if (isInferrableConst(jsdoc, nameNode)) {
if (nameNode.getJSType() == null) { if (nameNode.getJSType() == null) {
compiler.report(JSError.make(nameNode, CONSTANT_WITHOUT_EXPLICIT_TYPE));
return RemovalType.PRESERVE_ALL; return RemovalType.PRESERVE_ALL;
} }
jsdocNode.setJSDocInfo(maybeUpdateJSDocInfoWithType(jsdoc, nameNode)); jsdocNode.setJSDocInfo(maybeUpdateJSDocInfoWithType(jsdoc, nameNode));
Expand Down Expand Up @@ -311,8 +381,12 @@ private void removeEnumValues(Node objLit) {
} }
} }


private static boolean isInferrableConst(JSDocInfo jsdoc) { private static boolean isInferrableConst(JSDocInfo jsdoc, Node nameNode) {
return jsdoc != null && jsdoc.hasConstAnnotation() && !jsdoc.hasType(); return jsdoc != null
&& jsdoc.hasConstAnnotation()
&& !jsdoc.hasType()
&& !jsdoc.isConstructorOrInterface()
&& !NodeUtil.isNamespaceDecl(nameNode);
} }


private static boolean isClassMemberFunction(Node functionNode) { private static boolean isClassMemberFunction(Node functionNode) {
Expand Down
25 changes: 18 additions & 7 deletions test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java
View file
Expand Up @@ -24,7 +24,7 @@ protected CompilerPass getProcessor(final Compiler compiler) {
return new ConvertToTypedInterface(compiler); return new ConvertToTypedInterface(compiler);
} }


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


test("/** @const */ var x = 5;", "/** @const {number} */ var x;"); test("/** @const */ var x = 5;", "/** @const {number} */ var x;");
Expand All @@ -34,6 +34,23 @@ public void testInferAnnotatedTypeFromInferredType() {
"/** @constructor */ function Foo() {} \n /** @const {number} */ Foo.prototype.x;"); "/** @constructor */ function Foo() {} \n /** @const {number} */ Foo.prototype.x;");
} }


public void testConstJsdocPropagation() {

test("/** @const */ var x = 5;", "/** @const {number} */ var x;");
test("/** @const */ var x = true;", "/** @const {boolean} */ var x;");
test("/** @const */ var x = 'str';", "/** @const {string} */ var x;");
test("/** @const */ var x = null;", "/** @const {null} */ var x;");
test("/** @const */ var x = void 0;", "/** @const {void} */ var x;");

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

testError(
"/** @const */ var x = cond ? true : 5;",
ConvertToTypedInterface.CONSTANT_WITHOUT_EXPLICIT_TYPE);
}

public void testRemoveUselessStatements() { public void testRemoveUselessStatements() {
test("34", ""); test("34", "");
test("'str'", ""); test("'str'", "");
Expand Down Expand Up @@ -123,12 +140,6 @@ public void testIIFE() {


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

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() { public void testDefines() {
Expand Down

0 comments on commit 07077df

Please sign in to comment.