[Intl] Cleanup some code

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 94 matching lines @@
    * 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) {
     const validNames = const ["message", "plural", "gender", "select"];
     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;
+    return !(node.target.token.toString() != "Intl");

[Alan Knight, 2014/01/28 01:33:46]

Can we delete both "not"s and simplify this?

[vicb, 2014/01/28 07:52:45]

I did hesitate and thought there might be tricky edge cases when I first went
over the code but obviously it's equivalent, fixed.

   }
 
   Message _expectedInstance(String type) {
     switch (type) {
       case 'message' : return new MainMessage();
       case 'plural' : return new Plural();
       case 'gender' : return new Gender();
       case 'select' : return new Select();
       default: return null;
     }
@@ skipping 40 matching lines @@
   }
 
   /**
    * 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) {
     if (!addIntlMessage(node)) {
-      return super.visitMethodInvocation(node);
+      super.visitMethodInvocation(node);
     }
   }
 
   /**
    * Check that the node looks like an Intl.message invocation, and create
    * the [IntlMessage] object from it and store it in [messages]. Return true
    * if we successfully extracted a message and should stop looking. Return
    * false if we didn't, so should continue recursing.
    */
   bool addIntlMessage(MethodInvocation node) {
     if (!looksLikeIntlMessage(node)) return false;
     var reason = checkValidity(node);
     if (reason != null) {
       if (!suppressWarnings) {
-        var err = new StringBuffer();
-        err.write("Skipping invalid Intl.message invocation\n    <$node>\n");
-        err.write("    reason: $reason\n");
-        err.write(_reportErrorLocation(node));
+        var err = new StringBuffer()
+            ..write("Skipping invalid Intl.message invocation\n    <$node>\n")
+            ..writeAll(["    reason: $reason\n", _reportErrorLocation(node)]);
         warnings.add(err.toString());
         print(err);
       }
       // We found one, but it's not valid. Stop recursing.
       return true;
     }
     var message;
     if (node.methodName.name == "message") {
       message = messageFromIntlMessageCall(node);
     } else {
@@ skipping 38 matching lines @@
    */
   MainMessage messageFromIntlMessageCall(MethodInvocation node) {
 
     void extractFromIntlCall(MainMessage message, List arguments) {
       try {
         var interpolation = new InterpolationVisitor(message);
         arguments.first.accept(interpolation);
         message.messagePieces.addAll(interpolation.pieces);
       } on IntlMessageExtractionException catch (e) {
         message = null;
-        var err = new StringBuffer();
-        err.write("Error $e\n");
-        err.write("Processing <$node>\n");
-        err.write(_reportErrorLocation(node));
+        var err = new StringBuffer()
+            ..writeAll(["Error ", e, "\nProcessing <", node, ">\n"])
+            ..write(_reportErrorLocation(node));
         print(err);
         warnings.add(err);
       }
     }
 
     void setValue(MainMessage message, String fieldName, Object fieldValue) {
       message[fieldName] = fieldValue;
     }
 
     return _messageFromNode(node, extractFromIntlCall, setValue);
@@ skipping 28 matching lines @@
  * 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
  * special-purpose visitor.
  */
 class InterpolationVisitor extends SimpleASTVisitor {
-  Message message;
+  final Message message;
 
   InterpolationVisitor(this.message);
 
   List pieces = [];
   String get extractedMessage => pieces.join();
 
   void visitAdjacentStrings(AdjacentStrings node) {
     node.visitChildren(this);
     super.visitAdjacentStrings(node);
   }
@@ skipping 48 matching lines @@
 /**
  * A visitor to extract information from Intl.plural/gender sends. Note that
  * this is a SimpleASTVisitor, so it doesn't automatically recurse. So this
  * needs to be called where we expect a plural or gender immediately below.
  */
 class PluralAndGenderVisitor extends SimpleASTVisitor {
   /**
    * A plural or gender always exists in the context of a parent message,
    * which could in turn also be a plural or gender.
    */
-  ComplexMessage parent;
+  final ComplexMessage parent;
 
   /**
    * The pieces of the message. We are given an initial version of this
    * from our parent and we add to it as we find additional information.
    */
   List pieces;
 
-  PluralAndGenderVisitor(this.pieces, this.parent) : super() {}
-
   /** This will be set to true if we find a plural or gender. */
   bool foundPluralOrGender = false;
 
+  PluralAndGenderVisitor(this.pieces, this.parent) : super();
+
   visitInterpolationExpression(InterpolationExpression node) {
     // 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);
   }
 
   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", "select"].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;
+    return !(node.target.token.toString() != "Intl");

[Alan Knight, 2014/01/28 01:33:46]

Again, I think we can just remove both "not"s

[vicb, 2014/01/28 07:52:45]

fixed

   }
 
   /**
    * 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.
   }
 
@@ skipping 14 matching lines @@
     message.parent = parent;
 
     var arguments = message.argumentsOfInterestFor(node);
     arguments.forEach((key, value) {
       try {
         var interpolation = new InterpolationVisitor(message);
         value.accept(interpolation);
         message[key] = interpolation.pieces;
       } on IntlMessageExtractionException catch (e) {
         message = null;
-        var err = new StringBuffer();
-        err.write("Error $e");
-        err.write("Processing <$node>");
-        err.write(_reportErrorLocation(node));
+        var err = new StringBuffer()
+            ..writeAll(["Error ", $e, "\nProcessing <", node, ">"])
+            ..write(_reportErrorLocation(node));
         print(err);
         warnings.add(err);
       }
     });
     var mainArg = node.argumentList.arguments.firstWhere(
         (each) => each is! NamedExpression);
     if (mainArg is SimpleStringLiteral) {
       message.mainArgument = mainArg.toString();
     } else {
       message.mainArgument = mainArg.name;
@@ skipping 10 matching lines @@
    * A message describing the error.
    */
   final String message;
 
   /**
    * Creates a new exception with an optional error [message].
    */
   const IntlMessageExtractionException([this.message = ""]);
 
   String toString() => "IntlMessageExtractionException: $message";
-}
+}