Use shared consts for bool.fromEnvironment expressions

protoc_plugin/CHANGELOG.md

@@ -24,6 +24,10 @@
   `.pbenum.dart` files, same as `.pb.dart` files. ([9aad6aa])
 * Fix decoding map fields when key or value (or both) fields of a map entry is
   missing. ([#719], [#745])
+* Generated files now split `ignore_for_file` comments across multiple lines
+  when necessary.
+* Generated files now uses shared consts to eliminate repeated
+  `bool.fromEnvironment()` expressions.
 
 [#679]: https://github.com/google/protobuf.dart/pull/679
 [#703]: https://github.com/google/protobuf.dart/pull/703

protoc_plugin/Makefile

@@ -103,6 +103,10 @@ protos: $(PLUGIN_PATH) $(TEST_PROTO_LIBS)
 run-tests: protos
 	dart run test
 
+update-goldens: protos
+	rm -rf test/goldens
+	dart run test
+
 clean:
 	rm -rf benchmark/lib/generated
 	rm -rf $(OUTPUT_DIR)

protoc_plugin/analysis_options.yaml

@@ -1,15 +1,20 @@
 include: package:lints/recommended.yaml
 
 analyzer:
-  strong-mode:
-    implicit-casts: false
+  language:
+    strict-casts: true
 
 linter:
   rules:
+    - always_declare_return_types
+    - avoid_bool_literals_in_conditional_expressions
     - camel_case_types
     - comment_references
     - directives_ordering
     - omit_local_variable_types
     - prefer_relative_imports
     - prefer_single_quotes
+    - sort_pub_dependencies
     - throw_in_finally
+    - type_annotate_public_apis
+    - unawaited_futures

protoc_plugin/lib/const_generator.dart

@@ -7,7 +7,7 @@ import 'string_escape.dart';
 
 /// Writes JSON data as a Dart constant expression.
 /// Accepts null, bool, num, String, and maps and lists.
-void writeJsonConst(IndentingWriter out, val) {
+void writeJsonConst(IndentingWriter out, Object? val) {
   if (val is Map) {
     if (val.values.any(_nonEmptyListOrMap)) {
       out.addBlock(

protoc_plugin/lib/indenting_writer.dart

@@ -27,6 +27,9 @@ class IndentingWriter {
   int _previousOffset = 0;
   final String? _sourceFile;
 
+  // Named text sections to write at the end of the file.
+  Map<String, String> suffixes = {};
+
   IndentingWriter({String? filename}) : _sourceFile = filename;
 
   /// Appends a string indented to the current level.
@@ -101,8 +104,23 @@ class IndentingWriter {
     }
   }
 
+  void addSuffix(String suffixName, String text) {
+    suffixes[suffixName] = text;
+  }
+
   @override
-  String toString() => _buffer.toString();
+  String toString() {
+    if (suffixes.isNotEmpty) {
+      // TODO: We may want to introduce the notion of closing the file stream.
+      println('');
+      for (var key in suffixes.keys.toList()..sort()) {
+        println(suffixes[key]!);
+      }
+      suffixes.clear();
+    }
+
+    return _buffer.toString();
+  }
 
   /// Writes part of a line of text.
   /// Adds indentation if we're at the start of a line.

protoc_plugin/lib/src/code_generator.dart

@@ -107,7 +107,7 @@ class CodeGenerator {
 
       var response = CodeGeneratorResponse();
 
-      // Parse the options in the request. Return the errors is any.
+      // Parse the options in the request. Return the errors if any.
       var options = parseGenerationOptions(request, response, optionParsers);
       if (options == null) {
         _streamOut.add(response.writeToBuffer());

protoc_plugin/lib/src/enum_generator.dart

@@ -116,8 +116,10 @@ class EnumGenerator extends ProtobufContainer {
       for (var i = 0; i < _canonicalValues.length; i++) {
         var val = _canonicalValues[i];
         final name = dartNames[val.name]!;
-        final conditionalValName = configurationDependent(
-            'protobuf.omit_enum_names', quoted(val.name));
+        final omitEnumNames = ConditionalConstDefinition('omit_enum_names');
+        out.addSuffix(
+            omitEnumNames.constFieldName, omitEnumNames.constDefinition);
+        final conditionalValName = omitEnumNames.createTernary(val.name);
         out.printlnAnnotated(
             'static const $classname $name = '
             '$classname._(${val.number}, $conditionalValName);',

protoc_plugin/lib/src/extension_generator.dart

@@ -90,12 +90,18 @@ class ExtensionGenerator {
     if (!_resolved) throw StateError('resolve not called');
 
     var name = _extensionName;
-    final conditionalName = configurationDependent(
-        'protobuf.omit_field_names', quoted(_extensionName));
     var type = _field.baseType;
     var dartType = type.getDartType(fileGen!);
-    final conditionalExtendedName = configurationDependent(
-        'protobuf.omit_message_names', quoted(_extendedFullName));
+
+    final omitFieldNames = ConditionalConstDefinition('omit_field_names');
+    out.addSuffix(
+        omitFieldNames.constFieldName, omitFieldNames.constDefinition);
+    final conditionalName = omitFieldNames.createTernary(_extensionName);
+    final omitMessageNames = ConditionalConstDefinition('omit_message_names');
+    out.addSuffix(
+        omitMessageNames.constFieldName, omitMessageNames.constDefinition);
+    final conditionalExtendedName =
+        omitMessageNames.createTernary(_extendedFullName);
 
     String invocation;
     var positionals = <String>[];

protoc_plugin/lib/src/file_generator.dart

@@ -4,9 +4,9 @@
 
 part of '../protoc.dart';
 
-final _dartIdentifier = RegExp(r'^\w+$');
-const String _convertImportPrefix = r'$convert';
+final RegExp _dartIdentifier = RegExp(r'^\w+$');
 
+const String _convertImportPrefix = r'$convert';
 const String _fixnumImportPrefix = r'$fixnum';
 const String _typedDataImportPrefix = r'$typed_data';
 const String _protobufImport =
@@ -16,19 +16,9 @@ const String _coreImport = "import 'dart:core' as $coreImportPrefix;";
 const String _typedDataImport =
     "import 'dart:typed_data' as $_typedDataImportPrefix;";
 const String _convertImport = "import 'dart:convert' as $_convertImportPrefix;";
-
 const String _grpcImport =
     "import 'package:grpc/service_api.dart' as $grpcImportPrefix;";
 
-/// Generates code that will evaluate to the empty string if
-/// `const bool.fromEnvironment(envName)` is `true` and evaluate to [value]
-/// otherwise.
-String configurationDependent(String envName, String value) {
-  return 'const $coreImportPrefix.bool.fromEnvironment(${quoted(envName)})'
-      ' ? \'\' '
-      ': $value';
-}
-
 enum ProtoSyntax {
   proto2,
   proto3,
@@ -421,12 +411,13 @@ class FileGenerator extends ProtobufContainer {
     if (!_linked) throw StateError('not linked');
 
     var out = makeWriter();
-    _writeHeading(out);
+    _writeHeading(out, extraIgnores: {
+      if (enumCount > 0) 'undefined_shown_name',
+    });
 
     if (enumCount > 0) {
       // Make sure any other symbols in dart:core don't cause name conflicts
       // with enums that have the same name.
-      out.println('// ignore_for_file: undefined_shown_name');
       out.println(_coreImport);
       out.println();
       out.println(_protobufImport);
@@ -553,8 +544,8 @@ class FileGenerator extends ProtobufContainer {
     _writeHeading(out,
         extraIgnores: {'deprecated_member_use_from_same_package'});
 
-    out.println(_coreImport);
     out.println(_convertImport);
+    out.println(_coreImport);
     out.println(_typedDataImport);
     // Import the .pbjson.dart files we depend on.
     var imports = _findJsonProtosToImport();
@@ -567,16 +558,19 @@ class FileGenerator extends ProtobufContainer {
       e.generateConstants(out);
       writeBinaryDescriptor(
           out, e.binaryDescriptorName, e._descriptor.name, e._descriptor);
+      out.println('');
     }
     for (var m in messageGenerators) {
       m.generateConstants(out);
       writeBinaryDescriptor(
           out, m.binaryDescriptorName, m._descriptor.name, m._descriptor);
+      out.println('');
     }
     for (var s in serviceGenerators) {
       s.generateConstants(out);
       writeBinaryDescriptor(
           out, s.binaryDescriptorName, s._descriptor.name, s._descriptor);
+      out.println('');
     }
 
     return out.toString();
@@ -604,11 +598,7 @@ class FileGenerator extends ProtobufContainer {
     IndentingWriter out, {
     Set<String> extraIgnores = const <String>{},
   }) {
-    final ignores = ({
-      ..._ignores,
-      ...extraIgnores,
-    }).toList()
-      ..sort();
+    final ignores = ({..._fileIgnores, ...extraIgnores}).toList()..sort();
 
     // Group the ignores into lines not longer than 80 chars.
     var ignorelines = <String>[];
@@ -662,7 +652,36 @@ class FileGenerator extends ProtobufContainer {
   }
 }
 
-const _ignores = {
+class ConditionalConstDefinition {
+  final String envName;
+  late String _fieldName;
+
+  ConditionalConstDefinition(this.envName) {
+    _fieldName = _convertToCamelCase(envName);
+  }
+
+  String get constFieldName => _fieldName;
+
+  String get constDefinition {
+    return 'const $constFieldName = '
+        "$coreImportPrefix.bool.fromEnvironment(${quoted('protobuf.$envName')});";
+  }
+
+  String createTernary(String ifFalse) {
+    return "$constFieldName ? '' : ${quoted(ifFalse)}";
+  }
+
+  // Convert foo_bar_baz to _fooBarBaz.
+  String _convertToCamelCase(String lowerUnderscoreCase) {
+    var parts = lowerUnderscoreCase.split('_');
+    var rest = parts.skip(1).map((item) {
+      return item.substring(0, 1).toUpperCase() + item.substring(1);
+    }).join();
+    return '_${parts.first}$rest';
+  }
+}
+
+const _fileIgnores = {
   'annotate_overrides',
   'camel_case_types',
   'constant_identifier_names',