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

pkg/intl/lib/extract_messages.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * This is for use in extracting messages from a Dart program
  * using the Intl.message() mechanism and writing them to a file for
  * translation. This provides only the stub of a mechanism, because it
  * doesn't define how the file should be written. It provides an
  * [IntlMessage] class that holds the extracted data and [parseString]
@@ skipping 74 matching lines @@
   /**
    * We keep track of the data from the last MethodDeclaration,
    * FunctionDeclaration or FunctionExpression that we saw on the way down,
    * as that will be the nearest parent of the Intl.message invocation.
    */
   FormalParameterList parameters;
   String name;
 
   /** 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;
     return true;
   }
 
   /**
    * Returns a String describing why the node is invalid, or null if no
    * reason is found, so it's presumed valid.
    */
   String checkValidity(MethodInvocation node) {
     // The containing function cannot have named parameters.
     if (parameters.parameters.any((each) => each.kind == ParameterKind.NAMED)) {
       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.
     if (!namedArguments.every((each) => each is NamedExpression)) {
       return "Message arguments except the message must be named";
     }
     var notArgs = namedArguments.where(
         (each) => each.name.label.name != 'args');
     var values = notArgs.map((each) => each.expression).toList();
     if (!values.every((each) => each is SimpleStringLiteral)) {
@@ skipping 35 matching lines @@
    * encountered before seeing the Intl.message call.
    */
   void visitFunctionDeclaration(FunctionDeclaration node) {
     parameters = node.functionExpression.parameters;
     name = node.name.name;
     super.visitFunctionDeclaration(node);
   }
 
   /**
    * 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 = messageFromMethodInvocation(node);
+    var message;
+    if (node.methodName.name == "message") {
+      message = messageFromIntlMessageCall(node);
+    } else {
+      message = messageFromDirectPluralOrGenderCall(node);
+    }
     if (message != null) messages[message.name] = message;
+    return true;
   }
 
   /**
    * 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 messageFromMethodInvocation(MethodInvocation node) {
+  MainMessage messageFromIntlMessageCall(MethodInvocation node) {
     var message = new MainMessage();
     message.name = name;
     message.arguments = parameters.parameters.elements.map(
         (x) => x.identifier.name).toList();
     var arguments = node.argumentList.arguments.elements;
     try {
       var interpolation = new InterpolationVisitor(message);
       arguments.first.accept(interpolation);
       message.messagePieces.addAll(interpolation.pieces);
     } on IntlMessageExtractionException catch (e) {
       message = null;
       print("Error $e");
       print("Processing <$node>");
       _reportErrorLocation(node);
     }
     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();
       message[name] = string;
     }
     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;
+  }
 }
 
 /**
  * Given an interpolation, find all of its chunks, validate that they are only
  * simple variable substitutions or else Intl.plural/gender calls,
  * and keep track of the pieces of text so that other parts
  * of the program can deal with the simple string sections and the generated
  * parts separately. Note that this is a SimpleASTVisitor, so it only
  * traverses one level of children rather than automatically recursing. If we
  * find a plural or gender, which requires recursion, we do it with a separate
@@ skipping 86 matching lines @@
     // TODO(alanknight): Provide better errors for malformed expressions.
     if (!looksLikePluralOrGender(node.expression)) return;
     var reason = checkValidity(node.expression);
     if (reason != null) throw reason;
     var message = messageFromMethodInvocation(node.expression);
     foundPluralOrGender = true;
     pieces.add(message);
     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;
     SimpleIdentifier target = node.target;
     if (target.token.toString() != "Intl") return false;
     return true;
   }
 
   /**
    * Returns a String describing why the node is invalid, or null if no
    * reason is found, so it's presumed valid.
    */
   String checkValidity(MethodInvocation node) {
     // TODO(alanknight): Add reasonable validity checks.
   }
 
   /**
    * Create a MainMessage from [node] using the name and
    * 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();
     } else if (node.methodName.name == "plural") {
       message = new Plural();
     } else {
       throw new IntlMessageExtractionException("Invalid plural/gender message");
     }
     message.parent = parent;
 
@@ skipping 30 matching lines @@
    */
   final String message;
 
   /**
    * Creates a new exception with an optional error [message].
    */
   const IntlMessageExtractionException([this.message = ""]);
 
   String toString() => "IntlMessageExtractionException: $message";
 }