AF-3749: Loosen validation on Factory and meta initializers

lib/src/transformer/declaration_parsing.dart

@@ -19,6 +19,7 @@ import 'dart:mirrors';
 import 'package:analyzer/analyzer.dart';
 import 'package:barback/barback.dart' show TransformLogger;
 import 'package:over_react/src/component_declaration/annotations.dart' as annotations;
+import 'package:over_react/src/transformer/util.dart' show getMetaField;
 import 'package:source_span/source_span.dart';
 import 'package:transformer_utils/src/transformed_source_file.dart' show getSpan;
 import 'package:transformer_utils/transformer_utils.dart';
@@ -211,15 +212,7 @@ class ParsedDeclarations {
       });
     } else {
       void validateMetaField(ClassDeclaration cd, String expectedType) {
-        bool isPropsOrStateMeta(ClassMember member) {
-          if (member is! FieldDeclaration) return false;
-          final FieldDeclaration fd = member;
-          if (!fd.isStatic) return false;
-          if (fd.fields.variables.length > 1) return false;
-          if (fd.fields.variables.single.name.name != 'meta') return false;
-          return true;
-        }
-        final FieldDeclaration metaField = cd.members.firstWhere(isPropsOrStateMeta, orElse: () => null);
+        final metaField = getMetaField(cd);
         if (metaField == null) return;
 
         if (metaField.fields.type?.toSource() != expectedType) {
@@ -229,15 +222,17 @@ class ParsedDeclarations {
           );
         }
         final isClassPrivate = cd.name.name.startsWith('_');
-        final expectedInitializer = isClassPrivate
-            ? '_\$metaFor${cd.name.name.substring(1)}'
-            : '\$metaFor${cd.name.name}';
+        var expectedInitializers = ['\$metaFor${cd.name.name}', '_\$metaFor${cd.name.name}'];
+        if (isClassPrivate) {
+          expectedInitializers.add('_\$metaFor${cd.name.name.substring(1)}');
+        }
 
         final initializer = metaField.fields.variables.single.initializer?.toSource();
-        if (initializer != expectedInitializer) {
+        if (!expectedInitializers.contains(initializer)) {
           error(
-              'Static $expectedType field in accessor class must be initialized to '
-              '`$expectedInitializer`',
+              'Static $expectedType field in accessor class must be initialized to:'
+              // The second in the list of expected initializers is the one it will need to be once on Dart 2
+              '`${expectedInitializers[1]}`',
               getSpan(sourceFile, metaField),
           );
         }
@@ -280,6 +275,36 @@ class ParsedDeclarations {
       }
     }
 
+    // validate that the factory is initialized correctly
+    final factory = declarationMap[key_factory].length <= 1 ? singleOrNull(declarationMap[key_factory]) : null;
+    if (factory != null && factory is TopLevelVariableDeclaration) {
+      final String factoryName = factory.variables.variables.first.name.name;
+
+      if (factory.variables.variables.length != 1) {
+        error('Factory declarations must be a single variable.',
+            getSpan(sourceFile, factory.variables));
+      }
+
+      final variable = factory.variables.variables.first;
+      final isPrivate = factoryName.startsWith('_');
+      var expectedInitializers = ['\$$factoryName', '_\$$factoryName'];
+
+      if (isPrivate) {
+        expectedInitializers.add('_\$${factoryName.substring(1)}');
+      }
+
+      if (variable.initializer != null &&
+          !expectedInitializers.contains(variable.initializer.toString())) {
+        error(
+            'Factory variables are stubs for the generated factories, and should not have initializers '
+                'unless initialized with a valid variable name for Dart 2 builder compatibility. '
+                // The second in the list of expected initializers is the one it will need to be once on Dart 2
+                'Should be:\n    ${expectedInitializers[1]}',
+            getSpan(sourceFile, variable.initializer)
+        );
+      }
+    }
+
     if (hasErrors) {
       for (final key in declarationMap.keys) {
         declarationMap[key] = [];

lib/src/transformer/impl_generation.dart

@@ -18,7 +18,7 @@ import 'package:analyzer/analyzer.dart';
 import 'package:barback/barback.dart';
 import 'package:over_react/src/component_declaration/annotations.dart' as annotations;
 import 'package:over_react/src/transformer/declaration_parsing.dart';
-import 'package:over_react/src/transformer/text_util.dart';
+import 'package:over_react/src/transformer/util.dart';
 import 'package:source_span/source_span.dart';
 import 'package:transformer_utils/src/text_util.dart' show stringLiteral;
 import 'package:transformer_utils/src/transformed_source_file.dart' show getSpan;
@@ -105,26 +105,6 @@ class ImplGenerator {
       //   Factory implementation
       // ----------------------------------------------------------------------
 
-      if (declarations.factory.node.variables.variables.length != 1) {
-        logger.error('Factory declarations must a single variable.',
-            span: getSpan(sourceFile, declarations.factory.node.variables));
-      }
-
-      declarations.factory.node.variables.variables.forEach((variable) {
-        final isPrivate = factoryName.startsWith(privatePrefix);
-        final validInitializer = isPrivate
-            ? '$generatedPrefix${factoryName.substring(privatePrefix.length)}'
-            : '$publicGeneratedPrefix$factoryName';
-        if (variable.initializer != null && variable.initializer.toString() != validInitializer) {
-          logger.error(
-              'Factory variables are stubs for the generated factories, and should not have initializers '
-              'unless initialized with \$$factoryName for Dart 2 builder compatibility. '
-              'Should be:\n    $validInitializer',
-              span: getSpan(sourceFile, variable.initializer)
-          );
-        }
-      });
-
       transformedFile.replace(
           sourceFile.span(
               declarations.factory.node.variables.variables.first.name.end,
@@ -682,27 +662,30 @@ class ImplGenerator {
           staticGettersCompanionImpl
       );
     }
-
-    final name = (companionNode ?? typedMap.node).name.name;
-    final isPrivate = name.startsWith(privatePrefix);
-    final publicName = isPrivate ? name.substring(privatePrefix.length) : name;
-    final metaClassName = '$generatedPrefix${name}Meta';
-    final metaInstanceName = isPrivate
-        ? '${generatedPrefix}metaFor$publicName'
-        : '${publicGeneratedPrefix}metaFor$publicName';
-    final metaStructName = type == AccessorType.props
-        ? 'PropsMeta'
-        : 'StateMeta';
+
+    final classDeclaration = (companionNode ?? typedMap.node);
+    final metaField = getMetaField(classDeclaration);
     final output = new StringBuffer();
-    output.writeln('/// A class that allows us to reuse generated code from the accessors class.');
-    output.writeln('/// This is only used by other generated code, and can be simplified if needed.');
-    output.writeln('class $metaClassName {');
-    output.writeln(staticVariablesImpl);
-    output.writeln('}');
-    output.writeln('const $metaStructName $metaInstanceName = const $metaStructName(');
-    output.writeln('  fields: $metaClassName.$constantListName,');
-    output.writeln('  keys: $metaClassName.$keyListName,');
-    output.writeln(');');
+    // if metaField is null, we are on Dart 1 code which has not transitioned
+    // to the transitional Dart 2 compatible boilerplate, and thus the $metaFor
+    // constant is not needed
+    if (metaField != null) {
+      final name = classDeclaration.name.name;
+      final metaClassName = '$generatedPrefix${name}Meta';
+      final metaInstanceName = metaField.fields.variables.single.initializer.toSource();
+      final metaStructName = type == AccessorType.props
+          ? 'PropsMeta'
+          : 'StateMeta';
+      output.writeln('/// A class that allows us to reuse generated code from the accessors class.');
+      output.writeln('/// This is only used by other generated code, and can be simplified if needed.');
+      output.writeln('class $metaClassName {');
+      output.writeln(staticVariablesImpl);
+      output.writeln('}');
+      output.writeln('const $metaStructName $metaInstanceName = const $metaStructName(');
+      output.writeln('  fields: $metaClassName.$constantListName,');
+      output.writeln('  keys: $metaClassName.$keyListName,');
+      output.writeln(');');
+    }
     return new AccessorOutput(output.toString());
   }
 

lib/src/transformer/text_util.dart → lib/src/transformer/util.dart

@@ -12,7 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-library over_react.transformer.text_util;
+library over_react.transformer.util;
+
+import 'package:analyzer/analyzer.dart';
 
 String commentBanner(String bannerText, {
     int bannerWidth: 80, int textIndent: 2, bool topBorder: true, bool bottomBorder: true
@@ -30,3 +32,17 @@ String commentBanner(String bannerText, {
       (bottomBorder ? bannerBorder : '') +
       '\n';
 }
+
+/// Returns a [FieldDeclaration] for the meta field on a [ClassDeclaration] if
+/// it exists, otherwise returns null.
+FieldDeclaration getMetaField(ClassDeclaration cd) {
+  bool isPropsOrStateMeta(ClassMember member) {
+    if (member is! FieldDeclaration) return false;
+    final FieldDeclaration fd = member;
+    if (!fd.isStatic) return false;
+    if (fd.fields.variables.length > 1) return false;
+    if (fd.fields.variables.single.name.name != 'meta') return false;
+    return true;
+  }
+  return cd.members.firstWhere(isPropsOrStateMeta, orElse: () => null);
+}