Make gen-l10n error handling independent of logger state (#119644)

* init

* lint

* lint again

packages/flutter_tools/lib/src/localizations/gen_l10n.dart

@@ -579,6 +579,10 @@ class LocalizationsGenerator {
   // Whether we want to use escaping for ICU messages.
   bool useEscaping = false;
 
+  /// Whether any errors were caught. This is set after encountering any errors
+  /// from calling [_generateMethod].
+  bool hadErrors = false;
+
   /// The list of all arb path strings in [inputDirectory].
   List<String> get arbPathStrings {
     return _allBundles.bundles.map((AppResourceBundle bundle) => bundle.file.path).toList();
@@ -1093,151 +1097,152 @@ class LocalizationsGenerator {
 
   String _generateMethod(Message message, LocaleInfo locale) {
     try {
-    // Determine if we must import intl for date or number formatting.
-    if (message.placeholdersRequireFormatting) {
-      requiresIntlImport = true;
-    }
+      // Determine if we must import intl for date or number formatting.
+      if (message.placeholdersRequireFormatting) {
+        requiresIntlImport = true;
+      }
 
-    final String translationForMessage = message.messages[locale]!;
-    final Node node = message.parsedMessages[locale]!;
-    // If parse tree is only a string, then return a getter method.
-    if (node.children.every((Node child) => child.type == ST.string)) {
-      // Use the parsed translation to handle escaping with the same behavior.
-      return getterTemplate
-        .replaceAll('@(name)', message.resourceId)
-        .replaceAll('@(message)', "'${generateString(node.children.map((Node child) => child.value!).join())}'");
-    }
+      final String translationForMessage = message.messages[locale]!;
+      final Node node = message.parsedMessages[locale]!;
+      // If parse tree is only a string, then return a getter method.
+      if (node.children.every((Node child) => child.type == ST.string)) {
+        // Use the parsed translation to handle escaping with the same behavior.
+        return getterTemplate
+          .replaceAll('@(name)', message.resourceId)
+          .replaceAll('@(message)', "'${generateString(node.children.map((Node child) => child.value!).join())}'");
+      }
 
-    final List<String> tempVariables = <String>[];
-    // Get a unique temporary variable name.
-    int variableCount = 0;
-    String getTempVariableName() {
-      return '_temp${variableCount++}';
-    }
+      final List<String> tempVariables = <String>[];
+      // Get a unique temporary variable name.
+      int variableCount = 0;
+      String getTempVariableName() {
+        return '_temp${variableCount++}';
+      }
 
-    // Do a DFS post order traversal through placeholderExpr, pluralExpr, and selectExpr nodes.
-    // When traversing through a placeholderExpr node, return "$placeholderName".
-    // When traversing through a pluralExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
-    // When traversing through a selectExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
-    // When traversing through a message node, return concatenation of all of "generateVariables(child)" for each child.
-    String generateVariables(Node node, { bool isRoot = false }) {
-      switch (node.type) {
-        case ST.message:
-          final List<String> expressions = node.children.map<String>((Node node) {
-            if (node.type == ST.string) {
-              return node.value!;
-            }
-            return generateVariables(node);
-          }).toList();
-          return generateReturnExpr(expressions);
-
-        case ST.placeholderExpr:
-          assert(node.children[1].type == ST.identifier);
-          final String identifier = node.children[1].value!;
-          final Placeholder placeholder = message.placeholders[identifier]!;
-          if (placeholder.requiresFormatting) {
-            return '\$${node.children[1].value}String';
-          }
-          return '\$${node.children[1].value}';
-
-        case ST.pluralExpr:
-          requiresIntlImport = true;
-          final Map<String, String> pluralLogicArgs = <String, String>{};
-          // Recall that pluralExpr are of the form
-          // pluralExpr := "{" ID "," "plural" "," pluralParts "}"
-          assert(node.children[1].type == ST.identifier);
-          assert(node.children[5].type == ST.pluralParts);
-
-          final Node identifier = node.children[1];
-          final Node pluralParts = node.children[5];
-
-          for (final Node pluralPart in pluralParts.children.reversed) {
-            String pluralCase;
-            Node pluralMessage;
-            if (pluralPart.children[0].value == '=') {
-              assert(pluralPart.children[1].type == ST.number);
-              assert(pluralPart.children[3].type == ST.message);
-              pluralCase = pluralPart.children[1].value!;
-              pluralMessage = pluralPart.children[3];
-            } else {
-              assert(pluralPart.children[0].type == ST.identifier || pluralPart.children[0].type == ST.other);
-              assert(pluralPart.children[2].type == ST.message);
-              pluralCase = pluralPart.children[0].value!;
-              pluralMessage = pluralPart.children[2];
+      // Do a DFS post order traversal through placeholderExpr, pluralExpr, and selectExpr nodes.
+      // When traversing through a placeholderExpr node, return "$placeholderName".
+      // When traversing through a pluralExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
+      // When traversing through a selectExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
+      // When traversing through a message node, return concatenation of all of "generateVariables(child)" for each child.
+      String generateVariables(Node node, { bool isRoot = false }) {
+        switch (node.type) {
+          case ST.message:
+            final List<String> expressions = node.children.map<String>((Node node) {
+              if (node.type == ST.string) {
+                return node.value!;
+              }
+              return generateVariables(node);
+            }).toList();
+            return generateReturnExpr(expressions);
+
+          case ST.placeholderExpr:
+            assert(node.children[1].type == ST.identifier);
+            final String identifier = node.children[1].value!;
+            final Placeholder placeholder = message.placeholders[identifier]!;
+            if (placeholder.requiresFormatting) {
+              return '\$${node.children[1].value}String';
             }
-            if (!pluralLogicArgs.containsKey(pluralCases[pluralCase])) {
-              final String pluralPartExpression = generateVariables(pluralMessage);
-              final String? transformedPluralCase = pluralCases[pluralCase];
-              // A valid plural case is one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other".
-              if (transformedPluralCase == null) {
-                throw L10nParserException(
-                  '''
+            return '\$${node.children[1].value}';
+
+          case ST.pluralExpr:
+            requiresIntlImport = true;
+            final Map<String, String> pluralLogicArgs = <String, String>{};
+            // Recall that pluralExpr are of the form
+            // pluralExpr := "{" ID "," "plural" "," pluralParts "}"
+            assert(node.children[1].type == ST.identifier);
+            assert(node.children[5].type == ST.pluralParts);
+
+            final Node identifier = node.children[1];
+            final Node pluralParts = node.children[5];
+
+            for (final Node pluralPart in pluralParts.children.reversed) {
+              String pluralCase;
+              Node pluralMessage;
+              if (pluralPart.children[0].value == '=') {
+                assert(pluralPart.children[1].type == ST.number);
+                assert(pluralPart.children[3].type == ST.message);
+                pluralCase = pluralPart.children[1].value!;
+                pluralMessage = pluralPart.children[3];
+              } else {
+                assert(pluralPart.children[0].type == ST.identifier || pluralPart.children[0].type == ST.other);
+                assert(pluralPart.children[2].type == ST.message);
+                pluralCase = pluralPart.children[0].value!;
+                pluralMessage = pluralPart.children[2];
+              }
+              if (!pluralLogicArgs.containsKey(pluralCases[pluralCase])) {
+                final String pluralPartExpression = generateVariables(pluralMessage);
+                final String? transformedPluralCase = pluralCases[pluralCase];
+                // A valid plural case is one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other".
+                if (transformedPluralCase == null) {
+                  throw L10nParserException(
+                    '''
 The plural cases must be one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other.
     $pluralCase is not a valid plural case.''',
-                  _inputFileNames[locale]!,
-                  message.resourceId,
-                  translationForMessage,
-                  pluralPart.positionInMessage,
-                );
-              }
-              pluralLogicArgs[transformedPluralCase] = '      ${pluralCases[pluralCase]}: $pluralPartExpression,';
-            } else if (!suppressWarnings) {
-              logger.printWarning('''
+                    _inputFileNames[locale]!,
+                    message.resourceId,
+                    translationForMessage,
+                    pluralPart.positionInMessage,
+                  );
+                }
+                pluralLogicArgs[transformedPluralCase] = '      ${pluralCases[pluralCase]}: $pluralPartExpression,';
+              } else if (!suppressWarnings) {
+                logger.printWarning('''
 [${_inputFileNames[locale]}:${message.resourceId}] ICU Syntax Warning: The plural part specified below is overridden by a later plural part.
     $translationForMessage
     ${Parser.indentForError(pluralPart.positionInMessage)}''');
+              }
             }
-          }
-          final String tempVarName = getTempVariableName();
-          tempVariables.add(pluralVariableTemplate
-            .replaceAll('@(varName)', tempVarName)
-            .replaceAll('@(count)', identifier.value!)
-            .replaceAll('@(pluralLogicArgs)', pluralLogicArgs.values.join('\n'))
-          );
-          return '\$$tempVarName';
-
-        case ST.selectExpr:
-          requiresIntlImport = true;
-          // Recall that pluralExpr are of the form
-          // pluralExpr := "{" ID "," "plural" "," pluralParts "}"
-          assert(node.children[1].type == ST.identifier);
-          assert(node.children[5].type == ST.selectParts);
-
-          final Node identifier = node.children[1];
-          final List<String> selectLogicArgs = <String>[];
-          final Node selectParts = node.children[5];
-          for (final Node selectPart in selectParts.children) {
-            assert(selectPart.children[0].type == ST.identifier || selectPart.children[0].type == ST.other);
-            assert(selectPart.children[2].type == ST.message);
-            final String selectCase = selectPart.children[0].value!;
-            final Node selectMessage = selectPart.children[2];
-            final String selectPartExpression = generateVariables(selectMessage);
-            selectLogicArgs.add("        '$selectCase': $selectPartExpression,");
-          }
-          final String tempVarName = getTempVariableName();
-          tempVariables.add(selectVariableTemplate
-            .replaceAll('@(varName)', tempVarName)
-            .replaceAll('@(choice)', identifier.value!)
-            .replaceAll('@(selectCases)', selectLogicArgs.join('\n'))
-          );
-          return '\$$tempVarName';
-        // ignore: no_default_cases
-        default:
-          throw Exception('Cannot call "generateHelperMethod" on node type ${node.type}');
+            final String tempVarName = getTempVariableName();
+            tempVariables.add(pluralVariableTemplate
+              .replaceAll('@(varName)', tempVarName)
+              .replaceAll('@(count)', identifier.value!)
+              .replaceAll('@(pluralLogicArgs)', pluralLogicArgs.values.join('\n'))
+            );
+            return '\$$tempVarName';
+
+          case ST.selectExpr:
+            requiresIntlImport = true;
+            // Recall that pluralExpr are of the form
+            // pluralExpr := "{" ID "," "plural" "," pluralParts "}"
+            assert(node.children[1].type == ST.identifier);
+            assert(node.children[5].type == ST.selectParts);
+
+            final Node identifier = node.children[1];
+            final List<String> selectLogicArgs = <String>[];
+            final Node selectParts = node.children[5];
+            for (final Node selectPart in selectParts.children) {
+              assert(selectPart.children[0].type == ST.identifier || selectPart.children[0].type == ST.other);
+              assert(selectPart.children[2].type == ST.message);
+              final String selectCase = selectPart.children[0].value!;
+              final Node selectMessage = selectPart.children[2];
+              final String selectPartExpression = generateVariables(selectMessage);
+              selectLogicArgs.add("        '$selectCase': $selectPartExpression,");
+            }
+            final String tempVarName = getTempVariableName();
+            tempVariables.add(selectVariableTemplate
+              .replaceAll('@(varName)', tempVarName)
+              .replaceAll('@(choice)', identifier.value!)
+              .replaceAll('@(selectCases)', selectLogicArgs.join('\n'))
+            );
+            return '\$$tempVarName';
+          // ignore: no_default_cases
+          default:
+            throw Exception('Cannot call "generateHelperMethod" on node type ${node.type}');
+        }
       }
-    }
-    final String messageString = generateVariables(node, isRoot: true);
-    final String tempVarLines = tempVariables.isEmpty ? '' : '${tempVariables.join('\n')}\n';
-    return methodTemplate
-              .replaceAll('@(name)', message.resourceId)
-              .replaceAll('@(parameters)', generateMethodParameters(message).join(', '))
-              .replaceAll('@(dateFormatting)', generateDateFormattingLogic(message))
-              .replaceAll('@(numberFormatting)', generateNumberFormattingLogic(message))
-              .replaceAll('@(tempVars)', tempVarLines)
-              .replaceAll('@(message)', messageString)
-              .replaceAll('@(none)\n', '');
+      final String messageString = generateVariables(node, isRoot: true);
+      final String tempVarLines = tempVariables.isEmpty ? '' : '${tempVariables.join('\n')}\n';
+      return methodTemplate
+                .replaceAll('@(name)', message.resourceId)
+                .replaceAll('@(parameters)', generateMethodParameters(message).join(', '))
+                .replaceAll('@(dateFormatting)', generateDateFormattingLogic(message))
+                .replaceAll('@(numberFormatting)', generateNumberFormattingLogic(message))
+                .replaceAll('@(tempVars)', tempVarLines)
+                .replaceAll('@(message)', messageString)
+                .replaceAll('@(none)\n', '');
     } on L10nParserException catch (error) {
       logger.printError(error.toString());
+      hadErrors = true;
       return '';
     }
   }
@@ -1247,7 +1252,7 @@ The plural cases must be one of "=0", "=1", "=2", "zero", "one", "two", "few", "
     final String generatedLocalizationsFile = _generateCode();
 
     // If there were any syntax errors, don't write to files.
-    if (logger.hadErrorOutput) {
+    if (hadErrors) {
       throw L10nException('Found syntax errors.');
     }
 

packages/flutter_tools/test/general.shard/generate_localizations_test.dart

@@ -785,6 +785,28 @@ void main() {
   });
 
   group('generateLocalizations', () {
+    // Regression test for https://github.com/flutter/flutter/issues/119593
+    testWithoutContext('other logs from flutter_tools does not affect gen-l10n', () async {
+      _standardFlutterDirectoryL10nSetup(fs);
+
+      final Logger logger = BufferLogger.test();
+      logger.printError('An error output from a different tool in flutter_tools');
+
+      // Should run without error.
+      generateLocalizations(
+        fileSystem: fs,
+        options: LocalizationOptions(
+          arbDirectory: Uri.directory(defaultL10nPathString),
+          outputDirectory: Uri.directory(defaultL10nPathString, windows: false),
+          templateArbFile: Uri.file(defaultTemplateArbFileName, windows: false),
+          useSyntheticPackage: false,
+        ),
+        logger: logger,
+        projectDir: fs.currentDirectory,
+        dependenciesDir: fs.currentDirectory,
+      );
+    });
+
     testWithoutContext('forwards arguments correctly', () async {
       _standardFlutterDirectoryL10nSetup(fs);
       final LocalizationOptions options = LocalizationOptions(