preserve-comments (#40)

* something working !?!

* Use comment.raw property instead of value

* Simplify locStart and locEnd

* Simplify printComment

* Update snapshots

* Remove unused properties from parsed comments

* Fix linter errors

* moar eslint

* add Franco Victorio as contributor

* v1.0.0-alpha.4

* update clean

Author: mattiaerre

package.json

@@ -1,6 +1,6 @@
 {
   "name": "prettier-plugin-solidity",
-  "version": "1.0.0-alpha.3",
+  "version": "1.0.0-alpha.4",
   "description": "prettier plugin for solidity",
   "main": "src",
   "scripts": {
@@ -33,6 +33,10 @@
     {
       "email": "[email protected]",
       "name": "Jed Fox"
+    },
+    {
+      "email": "[email protected]",
+      "name": "Franco Victorio"
     }
   ],
   "license": "MIT",
@@ -51,6 +55,7 @@
     "jest-watch-typeahead": "^0.2.0"
   },
   "dependencies": {
+    "extract-comments": "^1.0.0",
     "prettier": "^1.14.2",
     "solidity-parser-antlr": "^0.3.1"
   }

src/clean.js

@@ -1,6 +1,6 @@
 // eslint-disable-next-line no-unused-vars
 const clean = (ast, newObj, parent) => {
-  ['loc', 'range'].forEach(name => {
+  ['code', 'codeStart', 'loc', 'range'].forEach(name => {
     delete newObj[name]; // eslint-disable-line no-param-reassign
   });
 };

src/index.js

@@ -15,15 +15,38 @@ const languages = [
 const parsers = {
   'solidity-parse': {
     astFormat: 'solidity-ast',
+    locEnd: node => node.range[1],
+    locStart: node => node.range[0],
     parse
   }
 };
 
+function canAttachComment(node) {
+  return (
+    node.type && node.type !== 'BlockComment' && node.type !== 'LineComment'
+  );
+}
+
+function printComment(commentPath) {
+  const comment = commentPath.getValue();
+  switch (comment.type) {
+    case 'BlockComment': {
+      return `/*${comment.raw}*/`;
+    }
+    case 'LineComment':
+      return `//${comment.raw.trimRight()}`;
+    default:
+      throw new Error(`Not a comment: ${JSON.stringify(comment)}`);
+  }
+}
+
 // https://prettier.io/docs/en/plugins.html#printers
 const printers = {
   'solidity-ast': {
+    canAttachComment,
+    massageAstNode,
     print,
-    massageAstNode
+    printComment
   }
 };
 

src/parser.js

@@ -1,6 +1,11 @@
+const extract = require('extract-comments');
 // https://prettier.io/docs/en/plugins.html#parsers
 const parser = require('solidity-parser-antlr');
 
-const parse = text => parser.parse(text, { loc: true, range: true });
+const parse = text => {
+  const parsed = parser.parse(text, { loc: true, range: true });
+  parsed.comments = extract(text);
+  return parsed;
+};
 
 module.exports = parse;

src/printer.js

@@ -427,7 +427,7 @@ function genericPrint(path, options, print) {
       // @TODO
       return '';
     default:
-      throw new Error(`unknown type: ${JSON.stringify(node.type)}`);
+      throw new Error(`Unknown type: ${JSON.stringify(node.type)}`);
   }
 }
 

tests/BasicIterator/__snapshots__/jsfmt.spec.js.snap

@@ -46,21 +46,26 @@ contract BasicIterator {
     }
 }
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+/*
+	This is a very simple demonstration of a while loops. Same as JS/c.
+*/
+
 
 contract BasicIterator {
-  address creator;
-  uint8[10] integers;
+  address creator; // reserve one "address"-type spot
+  uint8[10] integers; // reserve a chunk of storage for 10 8-bit unsigned integers in an array
 
   function BasicIterator() {
-    creator = msg.sender;
-    uint8 x = 0;
-    while (x < integers.length) {
-      integers[x] = x;
+    creator = msg.sender; // set the creator address
+    uint8 x = 0; // initialize an 8-bit, unsigned integer to zero
+    while (x < integers.length) { // the variable integers was initialized to length 10
+      integers[x] = x; // set integers to [0,1,2,3,4,5,6,7,8,9] over ten iterations
       x++;
     }
   }
 
-  function getSum() constant returns(uint) {
+  function getSum() constant returns(uint) { // "constant" just means this function returns something to the caller
+    // which is immediately followed by what type gets returned, in this case a full uint256
     uint8 sum = 0;
     uint8 x = 0;
     while (x < integers.length) {
@@ -69,10 +74,14 @@ contract BasicIterator {
     }
     return sum;
   }
+  /**********
+     Standard kill() function to recover funds 
+     **********/
+
 
   function kill() {
     if (msg.sender == creator) {
-      suicide(creator);
+      suicide(creator); // kills this contract and sends remaining funds back to creator
     }
   }
 }

tests/SimpleAuction/__snapshots__/jsfmt.spec.js.snap

@@ -128,48 +128,105 @@ contract SimpleAuction {
 pragma solidity ^0.4.22;
 
 contract SimpleAuction {
+  // Parameters of the auction. Times are either
+  // absolute unix timestamps (seconds since 1970-01-01)
+  // or time periods in seconds.
   address public beneficiary;
   uint public auctionEnd;
+  // Current state of the auction.
   address public highestBidder;
   uint public highestBid;
+  // Allowed withdrawals of previous bids
   mapping(address => uint) pendingReturns;
+  // Set to true at the end, disallows any change
   bool ended;
+  // Events that will be fired on changes.
   event HighestBidIncreased(address bidder, uint amount);
   event AuctionEnded(address winner, uint amount);
+  // The following is a so-called natspec comment,
+  // recognizable by the three slashes.
+  // It will be shown when the user is asked to
+  // confirm a transaction.
+
+  /// Create a simple auction with \`_biddingTime\`
+  /// seconds bidding time on behalf of the
+  /// beneficiary address \`_beneficiary\`.
 
   constructor(uint _biddingTime, address _beneficiary) public {
     beneficiary = _beneficiary;
     auctionEnd = now + _biddingTime;
   }
+  /// Bid on the auction with the value sent
+  /// together with this transaction.
+  /// The value will only be refunded if the
+  /// auction is not won.
 
   function bid() public payable {
+    // No arguments are necessary, all
+    // information is already part of
+    // the transaction. The keyword payable
+    // is required for the function to
+    // be able to receive Ether.
+
+    // Revert the call if the bidding
+    // period is over.
     require(now <= auctionEnd, "Auction already ended.");
+    // If the bid is not higher, send the
+    // money back.
     require(msg.value > highestBid, "There already is a higher bid.");
     if (highestBid != 0) {
+      // Sending back the money by simply using
+      // highestBidder.send(highestBid) is a security risk
+      // because it could execute an untrusted contract.
+      // It is always safer to let the recipients
+      // withdraw their money themselves.
       pendingReturns[highestBidder] += highestBid;
     }
     highestBidder = msg.sender;
     highestBid = msg.value;
     emit HighestBidIncreased(msg.sender, msg.value);
   }
+  /// Withdraw a bid that was overbid.
 
   function withdraw() public returns(bool) {
     uint amount = pendingReturns[msg.sender];
     if (amount > 0) {
+      // It is important to set this to zero because the recipient
+      // can call this function again as part of the receiving call
+      // before \`send\` returns.
       pendingReturns[msg.sender] = 0;
       if (!msg.sender.send(amount)) {
+        // No need to call throw here, just reset the amount owing
         pendingReturns[msg.sender] = amount;
         return false;
       }
     }
     return true;
   }
+  /// End the auction and send the highest bid
+  /// to the beneficiary.
 
   function auctionEnd() public {
+    // It is a good guideline to structure functions that interact
+    // with other contracts (i.e. they call functions or send Ether)
+    // into three phases:
+    // 1. checking conditions
+    // 2. performing actions (potentially changing conditions)
+    // 3. interacting with other contracts
+    // If these phases are mixed up, the other contract could call
+    // back into the current contract and modify the state or cause
+    // effects (ether payout) to be performed multiple times.
+    // If functions called internally include interaction with external
+    // contracts, they also have to be considered interaction with
+    // external contracts.
+
+    // 1. Conditions
     require(now >= auctionEnd, "Auction not yet ended.");
     require(!ended, "auctionEnd has already been called.");
+    // 2. Effects
     ended = true;
     emit AuctionEnded(highestBidder, highestBid);
+    // 3. Interaction
     beneficiary.transfer(highestBid);
   }
 }

tests/__snapshots__/jsfmt.spec.js.snap

@@ -25,6 +25,16 @@ contract Inbox {
 }
 
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// This comment spans one line
+
+/*
+This
+comment
+spans
+multiple
+lines.
+*/
+
 pragma solidity ^0.4.23;
 
 contract Inbox {
@@ -119,6 +129,11 @@ contract Ownable {
 }
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 pragma solidity ^0.4.24;
+/**
+ * @title Ownable
+ * @dev The Ownable contract has an owner address, and provides basic authorization control
+ * functions, this simplifies the implementation of "user permissions".
+ */
 
 contract Ownable {
   address private _owner;
@@ -127,31 +142,58 @@ contract Ownable {
     address indexed previousOwner,
     address indexed newOwner
   );
+  /**
+   * @dev The Ownable constructor sets the original \`owner\` of the contract to the sender
+   * account.
+   */
 
   constructor() public {
     _owner = msg.sender;
   }
+  /**
+   * @return the address of the owner.
+   */
 
   function owner() public view returns(address) {
     return _owner;
   }
+  /**
+   * @dev Throws if called by any account other than the owner.
+   */
   modifier onlyOwner() {
     require(isOwner());
     _;
   }
+  /**
+   * @return true if \`msg.sender\` is the owner of the contract.
+   */
 
   function isOwner() public view returns(bool) {
     return msg.sender == _owner;
   }
+  /**
+   * @dev Allows the current owner to relinquish control of the contract.
+   * @notice Renouncing to ownership will leave the contract without an owner.
+   * It will not be possible to call the functions with the \`onlyOwner\`
+   * modifier anymore.
+   */
 
   function renounceOwnership() public onlyOwner {
     emit OwnershipRenounced(_owner);
     _owner = address(0);
   }
+  /**
+   * @dev Allows the current owner to transfer control of the contract to a newOwner.
+   * @param newOwner The address to transfer ownership to.
+   */
 
   function transferOwnership(address newOwner) public onlyOwner {
     _transferOwnership(newOwner);
   }
+  /**
+   * @dev Transfers control of the contract to a newOwner.
+   * @param newOwner The address to transfer ownership to.
+   */
 
   function _transferOwnership(address newOwner) internal {
     require(newOwner != address(0));