Merge pull request wp-cli#101 from herregroen/improve-translator-comm…

…ent-detection

Improve detection of translator's comments

features/makepot.feature

@@ -1648,6 +1648,12 @@ Feature: Generate a POT file of a WordPress project
         <h1>{__( 'Hello JSX', 'foo-plugin' )}</h1>,
         document.getElementById('root')
       );
+
+      // translators: this is wp.i18n
+      wp.i18n.__( 'Hello wp.i18n', 'foo-plugin' );
+
+      message = Object(_wordpress_i18n__WEBPACK_IMPORTED_MODULE_7__["sprintf"])( // translators: this is Webpack
+      Object(_wordpress_i18n__WEBPACK_IMPORTED_MODULE_7__["__"])('Hello webpack.', 'foo-plugin'), mediaFile.name);
       """
 
     When I try `wp i18n make-pot foo-plugin`
@@ -1709,6 +1715,14 @@ Feature: Generate a POT file of a WordPress project
       """
       #. translators: this is JSX
       """
+    And the foo-plugin/foo-plugin.pot file should contain:
+      """
+      #. translators: this is wp.i18n
+      """
+    And the foo-plugin/foo-plugin.pot file should contain:
+      """
+      #. translators: this is Webpack
+      """
 
   Scenario: Ignores any other text domain in JavaScript file
     Given an empty foo-plugin directory

src/JsFunctionsScanner.php

@@ -131,11 +131,8 @@ public function saveGettextFunctions( Translations $translations, array $options
 
 				/** @var Node\Comment $comment */
 				foreach ( $all_comments as $comment ) {
-					if ( $node->getLocation()->getStart()->getLine() - $comment->getLocation()->getEnd()->getLine() > 1 ) {
-						continue;
-					}
-
-					if ( $node->getLocation()->getStart()->getColumn() < $comment->getLocation()->getStart()->getColumn() ) {
+					// Comments should be before the translation.
+					if ( ! $this->commentPrecedesNode( $comment, $node ) ) {
 						continue;
 					}
 
@@ -181,9 +178,14 @@ private function resolveExpressionCallee( Node\CallExpression $node ) {
 			'MemberExpression' === $callee->getType() &&
 			'Identifier' === $callee->getProperty()->getType()
 		) {
+			// Make sure to unpack wp.i18n which is a nested MemberExpression.
+			$comments = 'MemberExpression' === $callee->getObject()->getType()
+				? $callee->getObject()->getObject()->getLeadingComments()
+				: $callee->getObject()->getLeadingComments();
+
 			return [
 				'name'     => $callee->getProperty()->getName(),
-				'comments' => $callee->getProperty()->getLeadingComments()
+				'comments' => $comments,
 			];
 		}
 
@@ -202,7 +204,7 @@ private function resolveExpressionCallee( Node\CallExpression $node ) {
 			if ( 'Identifier' === $property->getType() ) {
 				return [
 					'name'     => $property->getName(),
-					'comments' => $property->getLeadingComments()
+					'comments' => $callee->getCallee()->getLeadingComments(),
 				];
 			}
 
@@ -211,12 +213,38 @@ private function resolveExpressionCallee( Node\CallExpression $node ) {
 			if ( 'Literal' === $property->getType() ) {
 				return [
 					'name'     => $property->getValue(),
-					'comments' => $property->getLeadingComments()
+					'comments' => $callee->getCallee()->getLeadingComments(),
 				];
 			}
 		}
 
 		// Unknown format.
 		return false;
 	}
+
+	/**
+	 * Returns wether or not a comment precedes a node.
+	 * The comment must be before the node and on the same line or the one before.
+	 *
+	 * @param Node\Comment $comment The comment.
+	 * @param Node\Node    $node    The node.
+	 *
+	 * @return bool Whether or not the comment precedes the node.
+	 */
+	private function commentPrecedesNode( Node\Comment $comment, Node\Node $node ) {
+		// Comments should be on the same or an earlier line than the translation.
+		if ( $node->getLocation()->getStart()->getLine() - $comment->getLocation()->getEnd()->getLine() > 1 ) {
+			return false;
+		}
+
+		// Comments on the same line should be before the translation.
+		if (
+			$node->getLocation()->getStart()->getLine() === $comment->getLocation()->getEnd()->getLine() &&
+			$node->getLocation()->getStart()->getColumn() < $comment->getLocation()->getStart()->getColumn()
+		) {
+			return false;
+		}
+
+		return true;
+	}
 }