Skip to content

Commit

Permalink
Add @constructor JSDoc when extracting classes.
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=215825326
  • Loading branch information
brad4d authored and lauraharker committed Oct 5, 2018
1 parent 3fe9981 commit f975b48
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 6 deletions.
57 changes: 54 additions & 3 deletions src/com/google/javascript/jscomp/Es6ExtractClasses.java
Expand Up @@ -189,13 +189,64 @@ private void extractClass(NodeTraversal t, Node classNode, Node parent) {
Node statement = NodeUtil.getEnclosingStatement(parent); Node statement = NodeUtil.getEnclosingStatement(parent);
JSType classType = classNode.getJSType(); JSType classType = classNode.getJSType();
checkState(!compiler.hasTypeCheckingRun() || classType != null); checkState(!compiler.hasTypeCheckingRun() || classType != null);
Node className = IR.name(name).setJSType(classType); // class name node used as LHS in newly created assignment
parent.replaceChild(classNode, className.cloneTree()); Node classNameLhs = IR.name(name).setJSType(classType);
// class name node that replaces the class literal in the original statement
Node classNameRhs = classNameLhs.cloneTree();
parent.replaceChild(classNode, classNameRhs);
Node classDeclaration = Node classDeclaration =
IR.constNode(className, classNode).useSourceInfoIfMissingFromForTree(classNode); IR.constNode(classNameLhs, classNode).useSourceInfoIfMissingFromForTree(classNode);
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.CONST_DECLARATIONS); NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.CONST_DECLARATIONS);
classDeclaration.setJSDocInfo(JSDocInfoBuilder.maybeCopyFrom(info).build()); classDeclaration.setJSDocInfo(JSDocInfoBuilder.maybeCopyFrom(info).build());
statement.getParent().addChildBefore(classDeclaration, statement); statement.getParent().addChildBefore(classDeclaration, statement);

// If the original statement was a variable declaration or qualified name assignment like
// like these:
// var ClassName = class {...
// OR
// some.qname.ClassName = class {...
//
// We will have changed the original statement to
//
// var ClassName = generatedName;
// OR
// some.qname.ClassName = generatedName;
//
// This is creating a type alias for a class, but since there's no literal class on the RHS,
// it doesn't look like one. Add at-constructor JSDoc to make it clear that this is happening.
//
// This was added to fix a specific problem where the original definition was for an abstract
// class, so its JSDoc included at-abstract.
// This caused ClosureCodeRemoval to think this rewritten assignment was a removable abstract
// method definition instead of the definition of an abstract class.
//
// TODO(b/117292942): Make ClosureCodeRemoval smarter so this hack isn't necessary to
// prevent incorrect removal of assignments.
if (NodeUtil.isNameDeclaration(statement)
&& statement.hasOneChild()
&& statement.getOnlyChild() == parent) {
// var ClassName = generatedName;
addAtConstructor(statement);
} else if (statement.isExprResult()) {
Node expr = statement.getOnlyChild();
if (expr.isAssign()
&& expr.getFirstChild().isQualifiedName()
&& expr.getSecondChild() == classNameRhs) {
// some.qname.ClassName = generatedName;
addAtConstructor(expr);
}
}
compiler.reportChangeToEnclosingScope(classDeclaration); compiler.reportChangeToEnclosingScope(classDeclaration);
} }

/**
* Add at-constructor to the JSDoc of the given node.
*
* @param node
*/
private void addAtConstructor(Node node) {
JSDocInfoBuilder builder = JSDocInfoBuilder.maybeCopyFrom(node.getJSDocInfo());
builder.recordConstructor();
node.setJSDocInfo(builder.build());
}
} }
14 changes: 11 additions & 3 deletions test/com/google/javascript/jscomp/Es6ExtractClassesTest.java
Expand Up @@ -62,6 +62,7 @@ public void testSelfReference1() {
"const testcode$classdecl$var0 = class {", "const testcode$classdecl$var0 = class {",
" constructor() { alert(testcode$classdecl$var0); }", " constructor() { alert(testcode$classdecl$var0); }",
"};", "};",
"/** @constructor */",
"var Outer=testcode$classdecl$var0")); "var Outer=testcode$classdecl$var0"));


test( test(
Expand All @@ -70,6 +71,7 @@ public void testSelfReference1() {
"const testcode$classdecl$var0 = class {", "const testcode$classdecl$var0 = class {",
" constructor() { alert(testcode$classdecl$var0); }", " constructor() { alert(testcode$classdecl$var0); }",
"};", "};",
"/** @constructor */",
"let Outer=testcode$classdecl$var0")); "let Outer=testcode$classdecl$var0"));


test( test(
Expand All @@ -78,6 +80,7 @@ public void testSelfReference1() {
"const testcode$classdecl$var0 = class {", "const testcode$classdecl$var0 = class {",
" constructor() { alert(testcode$classdecl$var0); }", " constructor() { alert(testcode$classdecl$var0); }",
"};", "};",
"/** @constructor */",
"const Outer=testcode$classdecl$var0")); "const Outer=testcode$classdecl$var0"));
} }


Expand Down Expand Up @@ -118,7 +121,11 @@ public void testSelfReference_googModule() {
"/** @const */ const testcode$classdecl$var0=class {", "/** @const */ const testcode$classdecl$var0=class {",
" constructor(){ alert(testcode$classdecl$var0); }", " constructor(){ alert(testcode$classdecl$var0); }",
"};", "};",
"/** @const */ var module$exports$example=testcode$classdecl$var0")); "/**",
" * @constructor",
" * @const",
" */ ",
"var module$exports$example=testcode$classdecl$var0"));
} }


@Test @Test
Expand All @@ -136,6 +143,7 @@ public void testSelfReference_qualifiedName() {
" alert(testcode$classdecl$var0);", " alert(testcode$classdecl$var0);",
" }", " }",
"};", "};",
"/** @constructor */",
"outer.qual.Name = testcode$classdecl$var0;")); "outer.qual.Name = testcode$classdecl$var0;"));
} }


Expand Down Expand Up @@ -167,13 +175,13 @@ public void testVarAssignment() {
} }


@Test @Test
public void testJSDoc() { public void testJSDocOnVar() {
test( test(
"/** @unrestricted */ var foo = class bar {};", "/** @unrestricted */ var foo = class bar {};",
lines( lines(
"/** @unrestricted */", "/** @unrestricted */",
"const testcode$classdecl$var0 = class {};", "const testcode$classdecl$var0 = class {};",
"/** @unrestricted */", "/** @unrestricted @constructor */",
"var foo = testcode$classdecl$var0;")); "var foo = testcode$classdecl$var0;"));
} }


Expand Down
4 changes: 4 additions & 0 deletions test/com/google/javascript/jscomp/Es6RewriteClassTest.java
Expand Up @@ -676,6 +676,7 @@ public void testNamedClassExpressionInVar() {
lines( lines(
"/** @constructor @struct */", "/** @constructor @struct */",
"const testcode$classdecl$var0 = function() {};", "const testcode$classdecl$var0 = function() {};",
"/** @constructor */",
"var C = testcode$classdecl$var0;")); "var C = testcode$classdecl$var0;"));
} }


Expand All @@ -688,6 +689,7 @@ public void testNamedClassExpressionInVarWithMethod() {
"const testcode$classdecl$var0 = function() {}", "const testcode$classdecl$var0 = function() {}",
"testcode$classdecl$var0.prototype.foo = function() {};", "testcode$classdecl$var0.prototype.foo = function() {};",
"", "",
"/** @constructor */",
"var C = testcode$classdecl$var0;")); "var C = testcode$classdecl$var0;"));
} }


Expand Down Expand Up @@ -1703,6 +1705,7 @@ public void testMultiNameClass_inVarDeclaration() {
lines( lines(
"/** @constructor @struct */", "/** @constructor @struct */",
"const testcode$classdecl$var0 = function(){};", "const testcode$classdecl$var0 = function(){};",
"/** @constructor */",
"var F = testcode$classdecl$var0;")); "var F = testcode$classdecl$var0;"));
} }


Expand All @@ -1713,6 +1716,7 @@ public void testMultiNameClass_inAssignment() {
lines( lines(
"/** @constructor @struct */", "/** @constructor @struct */",
"const testcode$classdecl$var0 = function(){};", "const testcode$classdecl$var0 = function(){};",
"/** @constructor */",
"F = testcode$classdecl$var0;")); "F = testcode$classdecl$var0;"));
} }


Expand Down
Expand Up @@ -479,6 +479,7 @@ public void testClassExpressionInVar() {
lines( lines(
"/** @constructor @struct @const */", "/** @constructor @struct @const */",
"var testcode$classdecl$var0 = function() {};", "var testcode$classdecl$var0 = function() {};",
"/** @constructor */",
"var C = testcode$classdecl$var0;")); "var C = testcode$classdecl$var0;"));


test( test(
Expand All @@ -488,6 +489,7 @@ public void testClassExpressionInVar() {
"var testcode$classdecl$var0 = function() {}", "var testcode$classdecl$var0 = function() {}",
"testcode$classdecl$var0.prototype.foo = function() {};", "testcode$classdecl$var0.prototype.foo = function() {};",
"", "",
"/** @constructor */",
"var C = testcode$classdecl$var0;")); "var C = testcode$classdecl$var0;"));
} }


Expand Down Expand Up @@ -1258,13 +1260,15 @@ public void testMultiNameClass() {
lines( lines(
"/** @constructor @struct @const */", "/** @constructor @struct @const */",
"var testcode$classdecl$var0 = function(){};", "var testcode$classdecl$var0 = function(){};",
"/** @constructor */",
"var F = testcode$classdecl$var0;")); "var F = testcode$classdecl$var0;"));


test( test(
"F = class G {}", "F = class G {}",
lines( lines(
"/** @constructor @struct @const */", "/** @constructor @struct @const */",
"var testcode$classdecl$var0 = function(){};", "var testcode$classdecl$var0 = function(){};",
"/** @constructor */",
"F = testcode$classdecl$var0;")); "F = testcode$classdecl$var0;"));
} }


Expand Down

0 comments on commit f975b48

Please sign in to comment.