Merge pull request #982 from Workiva/analyzer-7

FED-3977 Allow analyzer 7

Author: btr-rmconsole-4[bot]

.github/workflows/dart_ci.yml

@@ -50,17 +50,18 @@ jobs:
       - name: Print Dart SDK version
         run: dart --version
 
-      - name: Delete Dart-2-only files when running on Dart 3
+      - id: install
+        name: Install dependencies
+        run: dart pub get
+
+      - name: Delete Dart-2-only files and update tests when running on Dart 3
         run: |
           DART_VERSION="${{ steps.setup-dart.outputs.dart-version }}"
           if [[ "$DART_VERSION" =~ ^3 ]]; then
             ./tool/delete_dart_2_only_files.sh
+            ./tool/update_tests_for_dart_3.sh
           fi
 
-      - id: install
-        name: Install dependencies
-        run: dart pub get
-
       - name: Validate dependencies
         run: dart run dependency_validator
         if: always() && steps.install.outcome == 'success'
@@ -133,10 +134,13 @@ jobs:
         analyzer:
           - ^5.13.0 # This should match the lower bound
           - ^6.0.0
+          - ^7.0.0
         exclude:
-          # Analyzer 6 only resolves in Dart 3
+          # Analyzer 6+ only resolves in Dart 3
           - sdk: 2.19.6
             analyzer: ^6.0.0
+          - sdk: 2.19.6
+            analyzer: ^7.0.0
 
     steps:
       - uses: actions/checkout@v4
@@ -148,13 +152,6 @@ jobs:
       - name: Print Dart SDK version
         run: dart --version
 
-      - name: Delete Dart-2-only files when running on Dart 3
-        run: |
-          DART_VERSION="${{ steps.setup-dart.outputs.dart-version }}"
-          if [[ "$DART_VERSION" =~ ^3 ]]; then
-            ./tool/delete_dart_2_only_files.sh
-          fi
-
       - name: Update analyzer constraint to ${{ matrix.analyzer }} and validate `dart pub get` can resolve
         id: resolve
         run: |
@@ -163,6 +160,14 @@ jobs:
           git diff pubspec.yaml
           dart pub get
 
+      - name: Delete Dart-2-only files and update tests when running on Dart 3
+        run: |
+          DART_VERSION="${{ steps.setup-dart.outputs.dart-version }}"
+          if [[ "$DART_VERSION" =~ ^3 ]]; then
+            ./tool/delete_dart_2_only_files.sh
+            ./tool/update_tests_for_dart_3.sh
+          fi
+
       - name: Analyze package source
         run: dart analyze .
 
@@ -197,13 +202,6 @@ jobs:
       - name: Print Dart SDK version
         run: dart --version
 
-      - name: Delete Dart-2-only files when running on Dart 3
-        run: |
-          DART_VERSION="${{ steps.setup-dart.outputs.dart-version }}"
-          if [[ "$DART_VERSION" =~ ^3 ]]; then
-            (cd ../.. && ./tool/delete_dart_2_only_files.sh)
-          fi
-
       - id: link
         name: Override over_react dependency with local path
         run: cd ../.. && dart pub get && dart tool/travis_link_plugin_deps.dart
@@ -213,6 +211,13 @@ jobs:
         run: dart pub get
         if: always() && steps.link.outcome == 'success'
 
+      - name: Delete Dart-2-only files and update tests when running on Dart 3
+        run: |
+          DART_VERSION="${{ steps.setup-dart.outputs.dart-version }}"
+          if [[ "$DART_VERSION" =~ ^3 ]]; then
+            (cd ../.. && ./tool/delete_dart_2_only_files.sh && ./tool/update_tests_for_dart_3.sh)
+          fi
+
       - name: Validate dependencies
         run: dart run dependency_validator
         if: always() && steps.install.outcome == 'success'

CHANGELOG.md

@@ -1,5 +1,10 @@
 # OverReact Changelog
 
+## 5.4.5
+- Update analyzer dependency to `>=5.13.0 <8.0.0` (allow v8)
+- Update dart_style dependency to `>=2.0.0 <4.0.0` (allow v3)
+- Pub package tarball: exclude documentation image files, optimizing download size from ~5.9MB to ~1.3MB
+
 ## 5.4.4
 - [#972] Generate parts even when there are analysis warnings/errors in the source file's unresolved AST 
     - For example, the new `doc_directive_unknown` warning on doc comments in Dart 3

lib/src/builder/builder.dart

@@ -18,15 +18,18 @@ import 'dart:isolate';
 
 import 'package:analyzer/dart/analysis/utilities.dart';
 import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/token.dart' show LanguageVersionToken;
 import 'package:build/build.dart';
 import 'package:dart_style/dart_style.dart';
 import 'package:path/path.dart' as p;
 import 'package:package_config/package_config.dart' as pc;
+import 'package:pub_semver/pub_semver.dart' as semver;
 import 'package:source_span/source_span.dart';
 
 import './util.dart';
 import 'codegen.dart';
 import 'codegen/language_version_util.dart';
+import 'dart_style_compat.dart';
 import 'parsing.dart';
 
 Builder overReactBuilder(BuilderOptions? options) => OverReactBuilder();
@@ -62,6 +65,7 @@ class OverReactBuilder extends Builder {
       return;
     }
 
+    pc.LanguageVersion? packageConfigLanguageVersion;
     String nullSafetyReason;
     bool nullSafety;
     {
@@ -84,7 +88,6 @@ class OverReactBuilder extends Builder {
         // Catch any errors coming from our implementation of `$packageConfig`.
         // We can remove this once we switch to build 2.4.0's `packageConfig`
         // (see `$packageConfig` doc comment for more info).
-        pc.LanguageVersion? packageConfigLanguageVersion;
         try {
           packageConfigLanguageVersion = (await buildStep.$packageConfig)
               .packages
@@ -222,11 +225,30 @@ class OverReactBuilder extends Builder {
       final nullSafetyCommentText = 'Using nullSafety: $nullSafety.${nullSafety ? ' ' : ''} $nullSafetyReason';
 
       // Generated part files must have matching language version comments, so copy them over if they exist.
-      // TODO use CompilationUnit.languageVersionToken instead of parsing this manually once we're sure we can get on analyzer version 0.39.5 or greater
-      final languageVersionCommentMatch = RegExp(r'//\s*@dart\s*=\s*.+').firstMatch(source);
+      final languageVersionComment = libraryUnit.languageVersionToken?.value();
+
+      DartFormatter? formatter;
+      try {
+        formatter = constructFormatter(
+          // Try to use the actual version of the library if possible:
+          // 1. to avoid any potential parse errors
+          // 2. to preserve existing formatting in checked-in generated files in this repo when running on Dart 2
+          languageVersion: libraryUnit.languageVersionToken?.asSemver() ??
+              packageConfigLanguageVersion?.asSemver() ??
+              // TODO use DartFormatter.latestLanguageVersion here once this package supports only Dart 3 and dart_style >=2.3.7
+              semver.Version.parse(Platform.version
+                  .split(RegExp(r'\s'))
+                  .first),
+        );
+      } catch (e, st) {
+        // Formatting is not critical, so if it we can't construct a formatter, just skip it.
+        log.warning('Error constructing Dart formatter, skipping formatting step', e, st);
+      }
+
       await _writePart(buildStep, outputId, outputs,
+          formatter: formatter,
           nullSafetyCommentText: nullSafetyCommentText,
-          languageVersionComment: languageVersionCommentMatch?.group(0));
+          languageVersionComment: languageVersionComment);
     } else {
       if (hasOutputPartDirective()) {
         log.warning('An over_react part directive was found in ${buildStep.inputId.path}, '
@@ -238,8 +260,6 @@ class OverReactBuilder extends Builder {
 
   static final _headerLine = '// '.padRight(77, '*');
 
-  static final _formatter = DartFormatter();
-
   static CompilationUnit? _tryParseCompilationUnit(String source, AssetId id) {
     final result = parseString(content: source, path: id.path, throwIfDiagnostics: false);
 
@@ -264,8 +284,14 @@ class OverReactBuilder extends Builder {
     'invalid_use_of_visible_for_overriding_member',
   };
 
-  static FutureOr<void> _writePart(BuildStep buildStep, AssetId outputId, Iterable<String> outputs,
-      {required String nullSafetyCommentText, String? languageVersionComment}) async {
+  static FutureOr<void> _writePart(
+    BuildStep buildStep,
+    AssetId outputId,
+    Iterable<String> outputs, {
+    required DartFormatter? formatter,
+    required String nullSafetyCommentText,
+    String? languageVersionComment,
+  }) async {
     final partOf = "'${p.basename(buildStep.inputId.uri.toString())}'";
 
     final buffer = StringBuffer();
@@ -298,15 +324,25 @@ class OverReactBuilder extends Builder {
 
     var output = buffer.toString();
     // Output the file even if formatting fails, so that it can be used to debug the issue.
-    try {
-      output = _formatter.format(buffer.toString());
-    } catch (e, st) {
-      log.severe('Error formatting generated code', e, st);
+    if (formatter != null) {
+      try {
+        output = formatter.format(buffer.toString());
+      } catch (e, st) {
+        log.severe('Error formatting generated code', e, st);
+      }
     }
     await buildStep.writeAsString(outputId, output);
   }
 }
 
+extension on pc.LanguageVersion {
+  semver.Version asSemver() => semver.Version(major, minor, 0); // There's no patch available on this version.
+}
+
+extension on LanguageVersionToken {
+  semver.Version asSemver() => semver.Version(major, minor, 0); // There's no patch available on this version.
+}
+
 extension on BuildStep {
   // Cache the result so we don't read and parse the package config for every file.
   //

lib/src/builder/dart_style_compat.dart

@@ -0,0 +1,33 @@
+import 'dart:mirrors';
+
+import 'package:dart_style/dart_style.dart' show DartFormatter;
+import 'package:pub_semver/pub_semver.dart' show Version;
+
+/// Uses `dart:mirrors` to construct a [DartFormatter] instance, since we can't
+/// statically author code that works in both Dart 2 and dart_style 3.0.0.
+///
+/// Returns null if, for some reason, we can't construct a formatter.
+///
+// TODO remove this code once this package supports only Dart 3 and dart_style >=2.3.7
+DartFormatter? constructFormatter({required Version languageVersion}) {
+  // In dart_style 3.0.0, the languageVersion parameter is required.
+  // However, that parameter wasn't added until dart_style 2.3.7,
+  // which doesn't support Dart 2.
+
+  try {
+    // Works with dart_style >=2.3.7 <4.0.0 (Dart 3 only)
+    return _dartFormatterMirror.newInstance(
+      Symbol.empty,
+      [],
+      {#languageVersion: languageVersion},
+    ).reflectee as DartFormatter;
+  } catch (_) {}
+
+  // Works with dart_style >=2.0.0 <3.0.0 (Dart 2, Dart 3)
+  return _dartFormatterMirror.newInstance(
+    Symbol.empty,
+    [],
+  ).reflectee as DartFormatter;
+}
+
+late final _dartFormatterMirror = reflectClass(DartFormatter);

pubspec.yaml

@@ -7,13 +7,14 @@ environment:
 
 dependencies:
   collection: ^1.15.0
-  analyzer: '>=5.13.0 <7.0.0'
+  analyzer: '>=5.13.0 <8.0.0'
   build: ^2.0.0
-  dart_style: ^2.0.0
+  dart_style: '>=2.0.0 <4.0.0'
   js: ^0.6.1+1
   logging: ^1.0.0
   meta: ^1.16.0
   package_config: ^2.1.0
+  pub_semver: ^2.0.0
   path: ^1.5.1
   react: ^7.3.0
   redux: ^5.0.0
@@ -29,7 +30,7 @@ dev_dependencies:
   benchmark_harness: ^2.2.1
   build_resolvers: ^2.0.0
   build_runner: ^2.0.0
-  build_test: ^2.0.0
+  build_test: '>=2.0.0 <4.0.0'
   build_web_compilers: '>=3.2.6 <5.0.0'
   dart_dev: ^4.0.1
   dependency_validator: ^3.0.0

test/vm_tests/builder/conditionally_null_safe_builder_test.dart

@@ -29,7 +29,10 @@ main() {
             'dart', ['pub', 'run', 'build_runner', 'build'],
             workingDirectory: nonNullSafePackagePath);
 
-        expect(build.exitCode, 0);
+        expect(build.exitCode, 0,
+            reason: 'build should have succeeded.'
+                '\nstdout: ${build.stdout}'
+                '\nstderr: ${build.stderr}');
       });
 
       test('', () {
@@ -92,7 +95,10 @@ main() {
             'dart', ['pub', 'run', 'build_runner', 'build'],
             workingDirectory: nonNullSafePackagePath);
 
-        expect(build.exitCode, 0);
+        expect(build.exitCode, 0,
+            reason: 'build should have succeeded.'
+                '\nstdout: ${build.stdout}'
+                '\nstderr: ${build.stderr}');
       });
 
       test('', () {

test/vm_tests/builder/over_react_builder_test.dart

@@ -30,17 +30,28 @@ main() {
     final builder = overReactBuilder(null);
     final logger = Logger('overReactBuilderTestLogger');
 
+    // [1] Note: for this test code to analyze/run in Dart 3 with build_test 3.x,
+    //     it needs to be temporarily modified to use new APIs.
+    //
+    //     We do this in CI, and it can also be done during local development;
+    //     see tool/update_tests_for_dart_3.sh.
+    //
+    //     This is a workaround to us not being able to be compatible with build
+    //     test 2.x and 3.x at the same time, and Dart 2 not being compatible with
+    //     build_test 3.x.
+
     late AssetReader reader;
-    late InMemoryAssetWriter writer;
+    late InMemoryAssetWriter writer; // [1]
     late AssetWriterSpy writerSpy;
     late List<LogRecord> logs;
 
     setUp(() async {
+      // [1]
       reader = await PackageAssetReader.currentIsolate(
         rootPackage: 'over_react',
       );
 
-      writer = InMemoryAssetWriter();
+      writer = InMemoryAssetWriter(); // [1]
       writerSpy = AssetWriterSpy(writer);
 
       logs = [];

tool/delete_dart_2_only_files.sh

@@ -6,6 +6,7 @@
 #
 
 set -e
+set -x
 
 rm -rf test/over_react/component_declaration/non_null_safe_builder_integration_tests
 rm test/over_react/component_declaration/flux_component_test/component2/unsound_flux_component_test.dart
@@ -23,4 +24,4 @@ rm dart_test.yaml.old
 
 rm -rf tools/analyzer_plugin/test/unit/util/non_null_safe
 rm -rf tools/analyzer_plugin/test/integration/assists/non_null_safe
-rm -rf tools/analyzer_plugin/test/integration/diagnostics/non_null_safe
+rm -rf tools/analyzer_plugin/test/integration/diagnostics/non_null_safe

tool/update_tests_for_dart_3.sh

@@ -0,0 +1,18 @@
+#!/usr/bin/env bash
+
+#
+# This script makes updates to tests to make them compatible with Dart 3.
+# This is a last-resort workaround for cases that can't analyze cleanly in both versions.
+#
+
+set -e
+set -x
+
+# Update over_react_builder_test.dart by applying a patch in build_test 3 (Dart 3 only);
+# see comment in test file for more details on the changes.
+build_test_version=$(yq '.packages.build_test.version' pubspec.lock)
+if [[ "$build_test_version" == "3."* ]]; then
+  # This patch is derived from fixing tests in build_test 3 and getting the diff with:
+  #    git diff test/vm_tests/builder/over_react_builder_test.dart > tool/update_tests_for_dart_3/build_test_3_updates.patch
+  git apply tool/update_tests_for_dart_3/build_test_3_updates.patch
+fi