FEDX-165 Upgrade to analyzer 5

.github/workflows/dart_ci.yml

@@ -16,9 +16,7 @@ jobs:
       fail-fast: false
       matrix:
         # Can't run on `dev` (Dart 3) until we're fully null-safe.
-        # We can't run on Dart >=2.19 until https://github.com/dart-lang/sdk/issues/51128
-        # is fixed, or we work around it, which requires upgrading to analyzer ^5.1.0.
-        sdk: [ 2.13.4, 2.18.7 ]
+        sdk: [ 2.18.7, 2.19.6 ]
     steps:
       - uses: actions/checkout@v2
       - uses: dart-lang/setup-dart@v0.2
@@ -88,16 +86,11 @@ jobs:
       fail-fast: false
       matrix:
         # Can't run on `dev` (Dart 3) until we're fully null-safe.
-        # We can't run on Dart >=2.19 until https://github.com/dart-lang/sdk/issues/51128
-        # is fixed, or we work around it, which requires upgrading to analyzer ^5.1.0.
-        sdk: [ 2.13.4, 2.18.7 ]
+        sdk: [ 2.18.7, 2.19.6 ]
         analyzer:
-          - ^1.0.0
-          - ^2.0.0
-        # Don't validate analyzer ^2.0.0 in 2.13.4 since it can't resolve.
-        exclude:
-          - sdk: 2.13.4
-            analyzer: ^2.0.0
+          # We only have one version currently, but we'll leave this CI step in place
+          # for the next time we need to support multiple analyzer versions.
+          - ^5.1.0
     steps:
       - uses: actions/checkout@v2
       - uses: dart-lang/setup-dart@v0.2

.gitignore

@@ -45,3 +45,6 @@ pubspec.lock
 
 # Should not track the output of code coverage for now
 coverage/
+
+tools/analyzer_plugin/test/test_fixtures/**/lib/dynamic_test_files/
+tools/analyzer_plugin/test/temporary_test_fixtures/**

Dockerfile

@@ -1,4 +1,4 @@
-FROM google/dart:2.13
+FROM dart:2.18
 
 # Expose env vars for git ssh access
 ARG GIT_SSH_KEY

app/over_react_redux/todo_client/pubspec.yaml

@@ -26,7 +26,7 @@ dev_dependencies:
   test: ^1.15.7
   test_html_builder: ^3.0.0
   time: ^1.2.0
-  w_common: '>=2.0.0 <4.0.0'
+  w_common: ^3.0.0
   workiva_analysis_options: ^1.1.0
 
 dependency_overrides:

lib/src/builder/codegen/accessors_generator.dart

@@ -54,7 +54,7 @@ abstract class TypedMapAccessorsGenerator extends BoilerplateDeclarationGenerato
 
   String get accessorsMixinName;
 
-  ClassOrMixinDeclaration get node => member.node as ClassOrMixinDeclaration;
+  ClassishDeclaration get node => member.node.asClassish();
 
   TypeParameterList get typeParameters => member.nodeHelper.typeParameters;
 
@@ -363,7 +363,9 @@ class _TypedMapMixinAccessorsGenerator extends TypedMapAccessorsGenerator {
 
   @override
   void generate() {
-    outputContentsBuffer..write(_generateAccessorsMixin())..write(_generateMetaConstImpl());
+    outputContentsBuffer
+      ..write(_generateAccessorsMixin())
+      ..write(_generateMetaConstImpl());
   }
 }
 
@@ -422,9 +424,8 @@ class _LegacyTypedMapAccessorsGenerator extends TypedMapAccessorsGenerator {
       return '';
     }
 
-    final classishNode = node.asClassish();
-    final metadata = classishNode.metadata;
-    final typeParameters = classishNode.typeParameters;
+    final metadata = node.metadata;
+    final typeParameters = node.typeParameters;
     final typeParamsOnClass = typeParameters?.toSource() ?? '';
     final typeParamsOnSuper = removeBoundsFromTypeParameters(typeParameters);
     final accessorsMixinName = names.legacyAccessorsMixinName;
@@ -462,7 +463,7 @@ class TypedMapType {
   static const TypedMapType stateMixin = TypedMapType(false, false, true);
 }
 
-String _copyClassMembers(ClassOrMixinDeclaration node) {
+String _copyClassMembers(ClassishDeclaration node) {
   bool isValidForCopying(ClassMember member) {
     // Static members should be copied over as needed by [_copyStaticMembers].
     // Otherwise, fields which are not synthetic have concrete getters/setters
@@ -493,7 +494,7 @@ String _copyClassMembers(ClassOrMixinDeclaration node) {
   return buffer.toString();
 }
 
-String _copyStaticMembers(ClassOrMixinDeclaration node) {
+String _copyStaticMembers(ClassishDeclaration node) {
   final buffer = StringBuffer();
   node.members.where(isStaticMember).forEach((member) {
     // Don't copy over anything named `meta`, since the static meta field is already going to be generated.

lib/src/builder/codegen/component_factory_generator.dart

@@ -100,6 +100,8 @@ class ComponentFactoryProxyGenerator extends BoilerplateDeclarationGenerator {
       outputContentsBuffer.writeln('    skipMethods: const [],');
     }
 
-    outputContentsBuffer..writeln(');')..writeln();
+    outputContentsBuffer
+      ..writeln(');')
+      ..writeln();
   }
 }

lib/src/builder/codegen/typed_map_impl_generator.dart

@@ -133,7 +133,7 @@ abstract class TypedMapImplGenerator extends BoilerplateDeclarationGenerator {
   String _generateConcretePropsOrStateImpl({
     String componentFactoryName,
     String propKeyNamespace,
-    List<Identifier> allPropsMixins,
+    List<String> allPropsMixins,
   }) {
     if (isProps) {
       if (componentFactoryName == null || propKeyNamespace == null) {
@@ -153,7 +153,9 @@ abstract class TypedMapImplGenerator extends BoilerplateDeclarationGenerator {
       classDeclaration.write('abstract ');
     }
 
-    classDeclaration..write(_generateImplClassHeader())..write(' {');
+    classDeclaration
+      ..write(_generateImplClassHeader())
+      ..write(' {');
 
     final propsOrState = isProps ? 'props' : 'state';
 
@@ -235,7 +237,9 @@ abstract class TypedMapImplGenerator extends BoilerplateDeclarationGenerator {
     // Component2-specific classes
     if (isComponent2) {
       // TODO need to remove this workaround once https://github.com/dart-lang/sdk/issues/36217 is fixed get nice dart2js output
-      buffer..writeln()..writeln('''
+      buffer
+        ..writeln()
+        ..writeln('''
 // Concrete $propsOrState implementation that can be backed by any [Map].
 ${internalGeneratedMemberDeprecationLine()}class ${names.plainMapImplName}$typeParamsOnClass extends ${names.implName}$typeParamsOnSuper {
   // This initializer of `_$propsOrState` to an empty map, as well as the reassignment
@@ -350,7 +354,7 @@ class _TypedMapImplGenerator extends TypedMapImplGenerator {
   @override
   final Version version;
 
-  final List<Identifier> allPropsMixins;
+  final List<String> allPropsMixins;
 
   _TypedMapImplGenerator.props(ClassComponentDeclaration declaration)
       : names = TypedMapNames(declaration.props.either.name.name),
@@ -434,8 +438,9 @@ class _TypedMapImplGenerator extends TypedMapImplGenerator {
       return 'class ${names.implName}$typeParamsOnClass'
           ' extends ${isProps ? 'UiProps' : 'UiState'}'
           ' with\n'
-          ' ${names.consumerName}$typeParamsOnSuper\n,'
-          ' ${names.generatedMixinName}$typeParamsOnSuper${generatedMixinWarningCommentLine(names, isProps: isProps)}';
+          ' ${names.consumerName}$typeParamsOnSuper,\n'
+          ' ${generatedMixinWarningCommentLine(names, isProps: isProps)}'
+          ' ${names.generatedMixinName}$typeParamsOnSuper';
     } else if (member is BoilerplatePropsOrState) {
       final header = StringBuffer()
         ..write('class ${names.implName}$typeParamsOnClass'
@@ -462,14 +467,14 @@ class _TypedMapImplGenerator extends TypedMapImplGenerator {
           final names = TypedMapNames(mixin.name.name);
           header.write('${names.consumerName}$typeArguments');
           header.write(',');
+          // Add a space at the beginning of the line so that dartfmt indents it
+          // with the following line, as opposed to "sticking" it to the beginning of the line.
+          header.write('\n ' + generatedMixinWarningCommentLine(names, isProps: isProps));
           header.write('${names.generatedMixinName}$typeArguments');
           // Don't write the comma if we're at the end of the list.
-          // Do this manually instead of using `.join` so that we can always have
-          // the warning comment be at the end of the line, regardless of whether the comma is there.
           if (i != mixins.length - 1) {
             header.write(',');
           }
-          header.write(generatedMixinWarningCommentLine(names, isProps: isProps));
         }
       }
 

lib/src/builder/codegen/util.dart

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import 'package:analyzer/dart/ast/ast.dart';
 import 'package:logging/logging.dart';
 import 'package:meta/meta.dart';
 import 'package:over_react/src/component_declaration/annotations.dart' as annotations;
@@ -70,7 +69,7 @@ String generatedMixinWarningCommentLine(TypedMapNames mixinNames, {@required boo
 
 void generatePropsMeta(
   StringBuffer buffer,
-  List<Identifier> mixins, {
+  List<String> mixins, {
   String classType = 'PropsMetaCollection',
   String fieldName = 'propsMeta',
 }) {
@@ -79,7 +78,7 @@ void generatePropsMeta(
     ..writeln('  @override')
     ..writeln('  $classType get $fieldName => const $classType({');
   for (final name in mixins) {
-    final names = TypedMapNames(name.name);
+    final names = TypedMapNames(name);
     buffer.write('    ${generatedMixinWarningCommentLine(names, isProps: true)}');
     buffer.writeln('    ${names.consumerName}: ${names.publicGeneratedMetaName},');
   }

lib/src/builder/parsing/ast_util.dart

@@ -93,8 +93,8 @@ extension SuperclassConstraint on MixinDeclaration {
   }
 }
 
-/// Utilities for determining if a [ClassOrMixinDeclaration] has an abstract getter.
-extension AbstractGetter on ClassOrMixinDeclaration {
+/// Utilities for determining if a declaration has an abstract getter.
+extension AbstractGetter on ClassishDeclaration {
   /// Returns whether this class/mixin contains an abstract getter with the provided [name]
   /// and a return type exactly matching [type]
   bool hasAbstractGetter(String type, String name) =>
@@ -106,6 +106,15 @@ extension AbstractGetter on ClassOrMixinDeclaration {
           member.returnType?.toSource() == type);
 }
 
+/// An extension that supports APIs that changed from [Identifier] to [Token],
+/// in order to cut down on diffs in the analyzer 5 upgrade (and subsequent
+/// merge conflicts with the null-safety branch.
+///
+/// TODO remove this and inline the [name] member.
+extension NameIdentifierTokenCompat on Token {
+  String get name => lexeme;
+}
+
 /// Utilities that provide for easier access to [AnnotatedNode] metadata.
 extension MetadataHelper on AnnotatedNode {
   // Annotations don't always parse as expected, so `.name` can also include the constructor

lib/src/builder/parsing/ast_util/classish.dart

@@ -19,7 +19,8 @@ extension Classish on NamedCompilationUnitMember {
   ClassishDeclaration asClassish() => ClassishDeclaration(this);
 }
 
-/// Provides a common interface for [ClassOrMixinDeclaration] and [ClassTypeAlias].
+/// Provides a common interface for [ClassDeclaration], [MixinDeclaration],
+/// and [ClassTypeAlias].
 abstract class ClassishDeclaration {
   factory ClassishDeclaration(NamedCompilationUnitMember node) {
     if (node is ClassDeclaration) {
@@ -41,17 +42,17 @@ abstract class ClassishDeclaration {
   //
   // Shared
 
-  SimpleIdentifier get name => node.name;
+  Token get name => node.name;
   NodeList<Annotation> get metadata => node.metadata;
 
   TypeParameterList get typeParameters;
   List<ClassMember> get members;
   Token get classOrMixinKeyword;
 
   /// All interfaces used by this class, including mixin superclass constraints.
-  List<TypeName> get interfaces;
+  List<NamedType> get interfaces;
 
-  List<TypeName> get allSuperTypes => [
+  List<NamedType> get allSuperTypes => [
         ...interfaces,
         ...mixins,
         if (superclass != null) superclass,
@@ -63,25 +64,12 @@ abstract class ClassishDeclaration {
   WithClause get withClause;
   Token get abstractKeyword;
   bool get hasAbstractKeyword => abstractKeyword != null;
-  TypeName get superclass;
+  NamedType get superclass;
 
-  List<TypeName> get mixins => withClause?.mixinTypes ?? const [];
+  List<NamedType> get mixins => withClause?.mixinTypes ?? const [];
 }
 
-abstract class _ClassishClassOrMixin extends ClassishDeclaration {
-  _ClassishClassOrMixin._() : super._();
-
-  @override
-  ClassOrMixinDeclaration get node;
-
-  @override
-  List<ClassMember> get members => node.members;
-
-  @override
-  TypeParameterList get typeParameters => node.typeParameters;
-}
-
-class _ClassishClass extends _ClassishClassOrMixin {
+class _ClassishClass extends ClassishDeclaration {
   @override
   final ClassDeclaration node;
 
@@ -91,21 +79,27 @@ class _ClassishClass extends _ClassishClassOrMixin {
   Token get abstractKeyword => node.abstractKeyword;
 
   @override
-  List<TypeName> get interfaces => [
+  List<NamedType> get interfaces => [
         ...?node.implementsClause?.interfaces,
       ];
 
   @override
-  TypeName get superclass => node.extendsClause?.superclass;
+  NamedType get superclass => node.extendsClause?.superclass;
 
   @override
   WithClause get withClause => node.withClause;
 
   @override
   Token get classOrMixinKeyword => node.classKeyword;
+
+  @override
+  List<ClassMember> get members => node.members;
+
+  @override
+  TypeParameterList get typeParameters => node.typeParameters;
 }
 
-class _ClasssishMixin extends _ClassishClassOrMixin {
+class _ClasssishMixin extends ClassishDeclaration {
   @override
   final MixinDeclaration node;
 
@@ -118,16 +112,22 @@ class _ClasssishMixin extends _ClassishClassOrMixin {
   Token get classOrMixinKeyword => node.mixinKeyword;
 
   @override
-  List<TypeName> get interfaces => [
+  List<NamedType> get interfaces => [
         ...?node.implementsClause?.interfaces,
         ...?node.onClause?.superclassConstraints,
       ];
 
   @override
-  TypeName get superclass => null;
+  NamedType get superclass => null;
 
   @override
   WithClause get withClause => null;
+
+  @override
+  List<ClassMember> get members => node.members;
+
+  @override
+  TypeParameterList get typeParameters => node.typeParameters;
 }
 
 class _ClassishClassTypeAlias extends ClassishDeclaration {
@@ -146,7 +146,7 @@ class _ClassishClassTypeAlias extends ClassishDeclaration {
   List<ClassMember> get members => const [];
 
   @override
-  TypeName get superclass => node.superclass;
+  NamedType get superclass => node.superclass;
 
   @override
   TypeParameterList get typeParameters => node.typeParameters;
@@ -155,7 +155,7 @@ class _ClassishClassTypeAlias extends ClassishDeclaration {
   WithClause get withClause => node.withClause;
 
   @override
-  List<TypeName> get interfaces => [
+  List<NamedType> get interfaces => [
         ...?node.implementsClause?.interfaces,
       ];
 }