Allow Intl.plural/gender as the top-level, omitting the Intl.message wrapper and the first layer of…

pkg/intl/lib/extract_messages.dart

Index: pkg/intl/lib/extract_messages.dart
diff --git a/pkg/intl/lib/extract_messages.dart b/pkg/intl/lib/extract_messages.dart
index 541998208ed37a53b8a28b1d8e0cb38867234db8..6e4839ae6a9a30192ef15e4810536776fd95fa65 100644
--- a/pkg/intl/lib/extract_messages.dart
+++ b/pkg/intl/lib/extract_messages.dart
@@ -92,7 +92,8 @@ class MessageFindingVisitor extends GeneralizingASTVisitor {
 
   /** Return true if [node] matches the pattern we expect for Intl.message() */
   bool looksLikeIntlMessage(MethodInvocation node) {
-    if (node.methodName.name != "message") return false;
+    const validNames = const ["message", "plural", "gender"];
+    if (!validNames.contains(node.methodName.name)) return false;
     if (!(node.target is SimpleIdentifier)) return false;
     SimpleIdentifier target = node.target;
     if (target.token.toString() != "Intl") return false;
@@ -109,9 +110,13 @@ class MessageFindingVisitor extends GeneralizingASTVisitor {
       return "Named parameters on message functions are not supported.";
     }
     var arguments = node.argumentList.arguments;
-    if (!(arguments.first is StringLiteral)) {
-      return "Intl.message messages must be string literals";
+
+    if (node.methodName.name == 'message') {
+      if (!(arguments.first is StringLiteral)) {
+        return "Intl.message messages must be string literals";
+      }
     }
+
     var namedArguments = arguments.skip(1);
     // This seems unlikely to happen, but make sure all are NamedExpression
     // before doing the tests below.
@@ -167,27 +172,39 @@ class MessageFindingVisitor extends GeneralizingASTVisitor {
 
   /**
    * Examine method invocations to see if they look like calls to Intl.message.
+   * If we've found one, stop recursing. This is important because we can have
+   * Intl.message(...Intl.plural...) and we don't want to treat the inner
+   * plural as if it was an outermost message.
    */
   void visitMethodInvocation(MethodInvocation node) {
-    addIntlMessage(node);
-    return super.visitNode(node);
+    if (!addIntlMessage(node)) {
+      return super.visitMethodInvocation(node);

[Alan Knight, 2013/07/23 23:24:29]

I changed the super call here because it just seemed to be wrong. I think
visitNode is a no-op, so it didn't really make a difference previously.

+    }
   }
 
   /**
    * Check that the node looks like an Intl.message invocation, and create
    * the [IntlMessage] object from it and store it in [messages].

[Emily Fortuna, 2013/07/24 18:35:51]

can you update the comment  here and explain what it means if it returns true vs
false?

[Alan Knight, 2013/07/24 19:50:10]

Done. Which also pointed out that I was returning the wrong value below.

    */
-  void addIntlMessage(MethodInvocation node) {
-    if (!looksLikeIntlMessage(node)) return;
+  bool addIntlMessage(MethodInvocation node) {
+    if (!looksLikeIntlMessage(node)) return false;
     var reason = checkValidity(node);
-    if (reason != null && !suppressWarnings) {
-      print("Skipping invalid Intl.message invocation\n    <$node>");
-      print("    reason: $reason");
-      _reportErrorLocation(node);
-      return;
+    if (reason != null) {

[Alan Knight, 2013/07/23 23:24:29]

If suppress warnings was on, this was including the invalid message in the
output.

+      if (!suppressWarnings) {
+        print("Skipping invalid Intl.message invocation\n    <$node>");
+        print("    reason: $reason");
+        _reportErrorLocation(node);
+      }
+      return false;
+    }
+    var message;
+    if (node.methodName.name == "message") {
+      message = messageFromIntlMessageCall(node);
+    } else {
+      message = messageFromDirectPluralOrGenderCall(node);
     }
-    var message = messageFromMethodInvocation(node);
     if (message != null) messages[message.name] = message;
+    return true;
   }
 
   /**
@@ -195,7 +212,7 @@ class MessageFindingVisitor extends GeneralizingASTVisitor {
    * parameters of the last function/method declaration we encountered
    * and the parameters to the Intl.message call.
    */
-  MainMessage messageFromMethodInvocation(MethodInvocation node) {
+  MainMessage messageFromIntlMessageCall(MethodInvocation node) {
     var message = new MainMessage();
     message.name = name;
     message.arguments = parameters.parameters.elements.map(
@@ -219,6 +236,34 @@ class MessageFindingVisitor extends GeneralizingASTVisitor {
     }
     return message;
   }
+
+  /**
+   * Create an IntlMessage from [node] using the name and
+   * parameters of the last function/method declaration we encountered
+   * and the parameters to the Intl.message call.
+   */
+  MainMessage messageFromDirectPluralOrGenderCall(MethodInvocation node) {
+    // TODO(alanknight): Refactor this to reduce code duplication.

[Emily Fortuna, 2013/07/24 18:35:51]

why not refactor now? :-)

[Alan Knight, 2013/07/24 19:50:10]

Because I looked a bit and it didn't look easy. But done now. Function
parameterized by two other functions, but it's all deep in internals and it does
cut down on the duplication.

+    var message = new MainMessage();
+    message.name = name;
+    message.arguments = parameters.parameters.elements.map(
+        (x) => x.identifier.name).toList();
+    var arguments = node.argumentList.arguments.elements;
+    var visitor = new PluralAndGenderVisitor(message.messagePieces, message);
+    node.accept(visitor);
+    var pluralOrGender = message.messagePieces.last;
+    for (NamedExpression namedArgument in arguments.skip(1)) {
+      var name = namedArgument.name.label.name;
+      var exp = namedArgument.expression;
+      var string = exp is SimpleStringLiteral ? exp.value : exp.toString();
+      if (["name", "desc", "examples", "args"].contains(name)) {
+        message[name] = string;
+      } else {
+        pluralOrGender[name] = string;
+      }
+    }
+    return message;
+  }
 }
 
 /**
@@ -325,7 +370,12 @@ class PluralAndGenderVisitor extends SimpleASTVisitor {
     super.visitInterpolationExpression(node);
   }
 
-  /** Return true if [node] matches the pattern we expect for Intl.message() */
+  visitMethodInvocation(MethodInvocation node) {
+    pieces.add(messageFromMethodInvocation(node));
+    super.visitMethodInvocation(node);
+  }
+
+  /** Return true if [node] matches the pattern for plural or gender message.*/
   bool looksLikePluralOrGender(MethodInvocation node) {
     if (!["plural", "gender"].contains(node.methodName.name)) return false;
     if (!(node.target is SimpleIdentifier)) return false;
@@ -347,7 +397,7 @@ class PluralAndGenderVisitor extends SimpleASTVisitor {
    * parameters of the last function/method declaration we encountered
    * and the parameters to the Intl.message call.
    */
-  messageFromMethodInvocation(MethodInvocation node) {
+  Message messageFromMethodInvocation(MethodInvocation node) {
     var message;
     if (node.methodName.name == "gender") {
       message = new Gender();