Make create-element-to-jsx properly transfer comments (#61)

Some non-obvious aspects of this change:

* For the main test, I put a comment between nearly every token. The generated
  output is really messy, but the main important thing is that every comment in
  the input appears somewhere in the output.
* For closing tags, I had to make a copy of the JSXIdentifier without any
  comments. Otherwise, `React.createElement('div' /*foo*/, null, ' ')` would
  become `<div /*foo*/> </div /*foo*/>`.
* I had to change the signature of `convertExpressionToJSXAttributes` to also
  return an `extraComments` value for any comments that didn't have an obvious
  attribute to attach to. This is also necessary since there might be no
  attributes, but still some comments.
* JSXText elements seem to ignore any comments you attach to them, so I had to
  just use the default behavior of a JSXExpressionContainer for any child
  strings that have comments.
* Attaching comments to JSXElement nodes that are children of other JSXElement
  nodes causes the comment to be incorrectly printed as a child string, e.g.
  `<div><span />/*a comment*/</div>`. To avoid this, whenever the script
  generates a child JSXElement, it wraps it in a JSXExpressionContainer if there
  are any comments.
* It looks like some comments can be neither leading nor trailing (e.g.
  `{/*foo*/}`, and that makes them not print when they're transferred to other
  nodes. So in some cases I had to explicitly set the `leading` and `trailing`
  booleans when transferring comments. This also makes the comment positioning
  look nicer in some situations.

Author: alangpierce

README.md

@@ -38,10 +38,6 @@ Converts calls to `React.createElement` into JSX elements.
 jscodeshift -t react-codemod/transforms/create-element-to-jsx.js <path>
 ```
 
-Note that this script will **destroy comments** within `createElement` usages,
-so you should manually check for any lost comments after running the
-transform.
-
 #### `findDOMNode`
 
 Updates `this.getDOMNode()` or `this.refs.foo.getDOMNode()` calls inside of

transforms/__testfixtures__/create-element-to-jsx-preserve-comments.input.js

@@ -0,0 +1,38 @@
+var React = require('react');
+
+const render = () => {
+  return /*1*/React/*2*/./*3*/createElement/*4*/(
+    /*5*/'div'/*6*/,/*7*/
+    {
+      /*8*/className/*9*/: /*10*/'foo'/*11*/,/*12*/
+      /*13*/onClick/*14*/:/*15*/ this.handleClick/*16*/, //17
+    }/*18*/,
+    /*19*/React.createElement(/*20*/TodoList/*21*/./*22*/Item/*23*/)/*24*/, //25
+    React.createElement(
+      'span',
+      /*26*/getProps()/*27*/
+    ),
+    React.createElement('input', /*28*/null/*29*/)
+  );
+};
+
+const render2 = () => {
+  return React.createElement(
+    'div', {
+      className: 'foo',  // Prop comment.
+    },
+    'hello' // Child string comment.
+  );
+};
+
+const render3 = () => {
+  return React.createElement(
+    'div',
+    null,
+    React.createElement('span') // Child element comment.
+  );
+};
+
+const render4 = () => {
+  return React.createElement(Foo, {/* No props to see here! */});
+};

transforms/__testfixtures__/create-element-to-jsx-preserve-comments.output.js

@@ -0,0 +1,26 @@
+var React = require('react');
+
+const render = () => {
+  return /*1*//*4*//*2*//*3*/</*5*/div/*6*//*7*//*18*/
+    /*8*/className/*9*/=/*10*/"foo"/*11*/
+    /*12*/
+    /*13*///17
+    onClick/*14*/={/*15*/ this.handleClick}/*16*/>{/*19*///25
+    </*20*/TodoList/*21*/./*22*/Item/*23*/ />/*24*/}<span {.../*26*/getProps()/*27*/} /><input /*28*//*29*/ /></div>;
+};
+
+const render2 = () => {
+  return <div
+    // Prop comment.
+    className="foo">{// Child string comment.
+    'hello'}</div>;
+};
+
+const render3 = () => {
+  return <div>{// Child element comment.
+    <span />}</div>;
+};
+
+const render4 = () => {
+  return <Foo/* No props to see here! */ />;
+};

transforms/__tests__/create-element-to-jsx-test.js

@@ -132,6 +132,12 @@ describe('create-element-to-jsx', () => {
     null,
     'create-element-to-jsx-no-props-arg'
   );
+  defineTest(
+      __dirname,
+      'create-element-to-jsx',
+      null,
+      'create-element-to-jsx-preserve-comments'
+  );
 
   it('throws when it does not recognize a property type', () => {
     const jscodeshift = require('jscodeshift');

transforms/create-element-to-jsx.js

@@ -20,6 +20,13 @@ module.exports = function(file, api, options) {
       .replace(/>/g, '>');
 
   const convertExpressionToJSXAttributes = (expression) => {
+    if (!expression) {
+      return {
+        attributes: [],
+        extraComments: [],
+      };
+    }
+
     const isReactSpread = expression.type === 'CallExpression' &&
       expression.callee.type === 'MemberExpression' &&
       expression.callee.object.name === 'React' &&
@@ -37,81 +44,129 @@ module.exports = function(file, api, options) {
     ];
 
     if (isReactSpread || isObjectAssign) {
-      const jsxAttributes = [];
-
-      expression.arguments.forEach((expression) =>
-        jsxAttributes.push(...convertExpressionToJSXAttributes(expression))
-      );
+      const resultAttributes = [];
+      const resultExtraComments = expression.comments || [];
+      const {callee} = expression;
+      for (const node of [callee, callee.object, callee.property]) {
+        resultExtraComments.push(...(node.comments || []));
+      }
+      expression.arguments.forEach((expression) => {
+        const {attributes, extraComments} = convertExpressionToJSXAttributes(expression);
+        resultAttributes.push(...attributes);
+        resultExtraComments.push(...extraComments);
+      });
 
-      return jsxAttributes;
+      return {
+        attributes: resultAttributes,
+        extraComments: resultExtraComments,
+      };
     } else if (validSpreadTypes.indexOf(expression.type) != -1) {
-      return [j.jsxSpreadAttribute(expression)];
+      return {
+        attributes: [j.jsxSpreadAttribute(expression)],
+        extraComments: [],
+      };
     } else if (expression.type === 'ObjectExpression') {
       const attributes = expression.properties.map((property) => {
         if (property.type === 'SpreadProperty') {
-          return j.jsxSpreadAttribute(property.argument);
-        }  else if (property.type === 'Property') {
+          const spreadAttribute = j.jsxSpreadAttribute(property.argument);
+          spreadAttribute.comments = property.comments;
+          return spreadAttribute;
+        } else if (property.type === 'Property') {
           const propertyValueType = property.value.type;
 
           let value;
           if (propertyValueType === 'Literal' && typeof property.value.value === 'string') {
             value = j.literal(property.value.value);
+            value.comments = property.value.comments;
           } else {
             value = j.jsxExpressionContainer(property.value);
           }
 
-          let propertyKeyName;
+          let jsxIdentifier;
           if (property.key.type === 'Literal') {
-            propertyKeyName = property.key.value;
+            jsxIdentifier = j.jsxIdentifier(property.key.value);
           } else {
-            propertyKeyName = property.key.name;
+            jsxIdentifier = j.jsxIdentifier(property.key.name);
           }
+          jsxIdentifier.comments = property.key.comments;
 
-          return j.jsxAttribute(
-            j.jsxIdentifier(propertyKeyName),
+          const jsxAttribute = j.jsxAttribute(
+            jsxIdentifier,
             value
           );
+          jsxAttribute.comments = property.comments;
+          return jsxAttribute;
         }
         return null;
       });
 
-      return attributes;
+      return {
+        attributes,
+        extraComments: expression.comments || [],
+      };
     } else if (expression.type === 'Literal' && expression.value === null) {
-      return [];
+      return {
+        attributes: [],
+        extraComments: expression.comments || [],
+      }
     } else {
       throw new Error(`Unexpected attribute of type "${expression.type}"`);
     }
   };
 
   const jsxIdentifierFor = node => {
+    let identifier;
     if (node.type === 'Literal') {
-      return j.jsxIdentifier(node.value);
+      identifier = j.jsxIdentifier(node.value);
     } else if (node.type === 'MemberExpression') {
-      return j.jsxMemberExpression(
+      identifier = j.jsxMemberExpression(
         jsxIdentifierFor(node.object),
         jsxIdentifierFor(node.property)
       );
     } else {
-      return j.jsxIdentifier(node.name);
+      identifier = j.jsxIdentifier(node.name);
     }
+    identifier.comments = node.comments;
+    return identifier;
   };
 
   const convertNodeToJSX = (node) => {
+    const comments = node.value.comments;
+    const {callee} = node.value;
+    for (const calleeNode of [callee, callee.object, callee.property]) {
+      for (const comment of calleeNode.comments || []) {
+        comment.leading = true;
+        comment.trailing = false;
+        comments.push(comment);
+      }
+    }
+
     const args = node.value.arguments;
 
     const jsxIdentifier = jsxIdentifierFor(args[0]);
     const props = args[1];
 
-    const attributes = props ? convertExpressionToJSXAttributes(props) : [];
+    const {attributes, extraComments} = convertExpressionToJSXAttributes(props);
+    jsxIdentifier.comments = jsxIdentifier.comments || [];
+    for (const comment of extraComments) {
+      comment.leading = false;
+      comment.trailing = true;
+      jsxIdentifier.comments.push(comment);
+    }
 
-    const children = node.value.arguments.slice(2).map((child, index) => {
-      if (child.type === 'Literal' && typeof child.value === 'string') {
+    const children = args.slice(2).map((child, index) => {
+      if (child.type === 'Literal' && typeof child.value === 'string' && !child.comments) {
         return j.jsxText(encodeJSXTextValue(child.value));
       } else if (child.type === 'CallExpression' &&
         child.callee.object &&
         child.callee.object.name === 'React' &&
         child.callee.property.name === 'createElement') {
-        return convertNodeToJSX(node.get('arguments', index + 2));
+        const jsxChild = convertNodeToJSX(node.get('arguments', index + 2));
+        if ((jsxChild.comments || []).length > 0) {
+          return j.jsxExpressionContainer(jsxChild);
+        } else {
+          return jsxChild;
+        }
       } else {
         return j.jsxExpressionContainer(child);
       }
@@ -120,14 +175,19 @@ module.exports = function(file, api, options) {
     const openingElement = j.jsxOpeningElement(jsxIdentifier, attributes);
 
     if (children.length) {
-      return j.jsxElement(
+      const endIdentifier = Object.assign({}, jsxIdentifier, {comments: []});
+      const element = j.jsxElement(
         openingElement,
-        j.jsxClosingElement(jsxIdentifier),
+        j.jsxClosingElement(endIdentifier),
         children
       );
+      element.comments = comments;
+      return element;
     } else {
       openingElement.selfClosing = true;
-      return j.jsxElement(openingElement);
+      const element = j.jsxElement(openingElement);
+      element.comments = comments;
+      return element;
     }
   };