Merge pull request #195 from Workiva/ActionV2-type-migration

FEA-2770 FEA-2772 Action v2 type migration

analysis_options.yaml

@@ -1,7 +1,8 @@
-# analysis_options.yaml docs: https://www.dartlang.org/guides/language/analysis-options 
+# analysis_options.yaml docs: https://www.dartlang.org/guides/language/analysis-options
 analyzer:
   exclude:
     - "example/**"
+    - "w_flux_codemod/**"
 
   language:
     strict-casts: true
@@ -712,5 +713,3 @@ linter:
     # reason: Trying to assigning a value to void is an error.
     # 0 issues
     - void_checks
-
-

dart_dependency_validator.yaml

@@ -1,5 +1,6 @@
 exclude:
   - "example/**"
+  - "w_flux_codemod/**"
 ignore:
   # Ignore the pin on the test package while we have to avoid a bad version of test 1.18.1 https://github.com/dart-lang/test/issues/1620
-  - test
+  - test

w_flux_codemod/README.md

@@ -0,0 +1,91 @@
+# w_flux_codemod
+
+> **Built with [dart_codemod][dart_codemod].**
+
+A codemod to convert existing usages of non null-safe `Action` to null-safe `ActionV2`.
+
+## Motivation
+
+`w_flux` was upgraded to dart 3 and made null safe, but we ran into an issue when migrating the `Action` class.
+
+The `Action` class has a call method with an optional `payload` paramater that now must be typed as nullable. However, this means that we cannot make `listener` payloads non-nullable, since there's no guarantee that the argument was specified.
+
+```
+class Action<T> /*...*/ {
+  Future call([T? payload]) { 
+    for (final listener in _listeners) {
+      await listener(payload);
+      //             ^^^^^^^
+      // Error: can't assign T? to T
+    }
+  }
+  
+  ActionSubscription listen(dynamic onData(T event)) {/*...*/}
+  
+  /*...*/
+}
+```
+
+To be able to support non-nullable payloads (in addition to nullable payloads), we made a new `ActionV2` class with required payloads.
+
+## Usage
+
+1. Ensure you have the codemod package installed.
+    ```bash
+    dart pub global activate -sgit git@github.com:Workiva/w_flux.git --git-path=w_flux_codemod
+    ```
+
+2. Run the codemod:
+
+    - step by step:
+    ```bash
+    dart pub global run w_flux_codemod:action_v2_migrate_step_1 --yes-to-all
+    dart pub global run w_flux_codemod:action_v2_migrate_step_2 --yes-to-all
+    ```
+
+    - all at once:
+    ```bash
+    dart pub global run w_flux_codemod:action_v2_migrate --yes-to-all
+    ```
+
+    - The optional command `--yes-to-all` will automatically accept all changes. You can exclude this command to go through every change one by one.
+
+3. Review the changes:
+
+    - It's advisable to review the changes and ensure they are correct and meet your project's requirements.
+    - This codemod is not gauranteed to catch every implementation of `Action` and convert to `ActionV2`. For example: assigning `Action` to prop in a callback will be missed by this codemod.
+    - Dart Analysis should be able to catch anything missed or errors caused by the codemod, and a passing CI should suffice for QA when making these updates.
+
+
+## Example
+
+Before codemod:
+
+```dart
+import 'package:w_flux/w_flux.dart';
+
+class C {
+    Action action;
+} 
+
+void main() {
+    C().action();
+}
+```
+
+After codemod:
+
+```dart
+import 'package:w_flux/w_flux.dart';
+
+class C {
+    ActionV2 action;
+} 
+
+void main() {
+    // A payload is required for ActionV2, so `null` is added when needed.
+    C().action(null);
+}
+```
+
+[dart_codemod]: https://github.com/Workiva/dart_codemod

w_flux_codemod/analysis_options.yaml

@@ -0,0 +1 @@
+include: package:workiva_analysis_options/v2.yaml

w_flux_codemod/bin/action_v2_migrate.dart

@@ -0,0 +1,24 @@
+import 'dart:io';
+
+import 'package:codemod/codemod.dart';
+import 'package:glob/glob.dart';
+import 'package:w_flux_codemod/src/action_v2_suggestor.dart';
+import 'package:w_flux_codemod/src/utils.dart';
+
+void main(List<String> args) async {
+  final dartPaths = filePathsFromGlob(Glob('**.dart', recursive: true));
+
+  await pubGetForAllPackageRoots(dartPaths);
+  exitCode = await runInteractiveCodemod(
+    dartPaths,
+    aggregate([
+      ActionV2ParameterMigrator(),
+      ActionV2FieldAndVariableMigrator(),
+      ActionV2ReturnTypeMigrator(),
+      ActionV2SuperTypeMigrator(),
+      ActionV2DispatchMigrator(),
+      ActionV2ImportMigrator(),
+    ]),
+    args: args,
+  );
+}

w_flux_codemod/bin/action_v2_migrate_step_1.dart

@@ -0,0 +1,17 @@
+import 'dart:io';
+
+import 'package:codemod/codemod.dart';
+import 'package:glob/glob.dart';
+import 'package:w_flux_codemod/src/action_v2_suggestor.dart';
+import 'package:w_flux_codemod/src/utils.dart';
+
+void main(List<String> args) async {
+  final dartPaths = filePathsFromGlob(Glob('**.dart', recursive: true));
+
+  await pubGetForAllPackageRoots(dartPaths);
+  exitCode = await runInteractiveCodemod(
+    dartPaths,
+    ActionV2ParameterMigrator(),
+    args: args,
+  );
+}

w_flux_codemod/bin/action_v2_migrate_step_2.dart

@@ -0,0 +1,23 @@
+import 'dart:io';
+
+import 'package:codemod/codemod.dart';
+import 'package:glob/glob.dart';
+import 'package:w_flux_codemod/src/action_v2_suggestor.dart';
+import 'package:w_flux_codemod/src/utils.dart';
+
+void main(List<String> args) async {
+  final dartPaths = filePathsFromGlob(Glob('**.dart', recursive: true));
+
+  await pubGetForAllPackageRoots(dartPaths);
+  exitCode = await runInteractiveCodemod(
+    dartPaths,
+    aggregate([
+      ActionV2FieldAndVariableMigrator(),
+      ActionV2ReturnTypeMigrator(),
+      ActionV2SuperTypeMigrator(),
+      ActionV2DispatchMigrator(),
+      ActionV2ImportMigrator(),
+    ]),
+    args: args,
+  );
+}

w_flux_codemod/lib/src/action_v2_suggestor.dart

@@ -0,0 +1,127 @@
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:codemod/codemod.dart';
+
+mixin ActionV2Migrator on AstVisitingSuggestor {
+  @override
+  bool shouldResolveAst(_) => true;
+
+  @override
+  bool shouldSkip(FileContext context) =>
+      !context.sourceText.contains('Action');
+}
+
+mixin ActionV2NamedTypeMigrator on ActionV2Migrator {
+  bool shouldMigrate(NamedType node);
+
+  @override
+  visitNamedType(NamedType node) {
+    if (shouldMigrate(node)) {
+      final typeNameToken = node.name2;
+      final typeLibraryIdentifier = node.element?.library?.identifier ?? '';
+      if (typeNameToken.lexeme == 'Action' &&
+          typeLibraryIdentifier.startsWith('package:w_flux/')) {
+        yieldPatch('ActionV2', typeNameToken.offset, typeNameToken.end);
+      }
+      return super.visitNamedType(node);
+    }
+  }
+}
+
+class ActionV2ImportMigrator extends RecursiveAstVisitor
+    with AstVisitingSuggestor, ActionV2Migrator {
+  @override
+  visitShowCombinator(ShowCombinator node) {
+    final parent = node.parent;
+    if (parent is ImportDirective) {
+      final uri = parent.uri.stringValue;
+      final shownNamesList = node.shownNames.map((id) => id.name);
+      if (uri != null &&
+          uri.startsWith('package:w_flux/') &&
+          shownNamesList.contains('Action')) {
+        final updatedNamesList =
+            shownNamesList.map((name) => name == 'Action' ? 'ActionV2' : name);
+        yieldPatch('${node.keyword} ${updatedNamesList.join(', ')}',
+            node.offset, node.end);
+      }
+    }
+    return super.visitShowCombinator(node);
+  }
+
+  @override
+  visitHideCombinator(HideCombinator node) {
+    final parent = node.parent;
+    if (parent is ImportDirective) {
+      final uri = parent.uri.stringValue;
+      final hiddenNamesList = node.hiddenNames.map((id) => id.name);
+      if (uri != null &&
+          uri.startsWith('package:w_flux/') &&
+          hiddenNamesList.contains('Action')) {
+        final updatedNamesList = hiddenNamesList
+            .map((name) => name == 'Action' ? 'ActionV2' : name)
+            .join(', ');
+        yieldPatch('${node.keyword} $updatedNamesList', node.offset, node.end);
+      }
+    }
+    return super.visitHideCombinator(node);
+  }
+}
+
+class ActionV2DispatchMigrator extends RecursiveAstVisitor
+    with AstVisitingSuggestor, ActionV2Migrator {
+  @override
+  visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
+    final typeLibraryIdentifier =
+        node.function.staticType?.element?.library?.identifier ?? '';
+    final staticTypeName = node.function.staticType?.element?.name;
+    if (typeLibraryIdentifier.startsWith('package:w_flux/') &&
+        staticTypeName == 'Action' &&
+        node.argumentList.arguments.isEmpty) {
+      yieldPatch('(null)', node.end - 2, node.end);
+    }
+    return super.visitFunctionExpressionInvocation(node);
+  }
+}
+
+class ActionV2FieldAndVariableMigrator extends RecursiveAstVisitor
+    with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator {
+  @override
+  bool shouldMigrate(node) =>
+      node.thisOrAncestorOfType<MethodDeclaration>() == null &&
+      (node.parent is DeclaredIdentifier ||
+          node.parent is DeclaredVariablePattern ||
+          node.parent is FieldFormalParameter ||
+          node.parent is VariableDeclarationList ||
+          node.parent is TypeArgumentList ||
+          node.thisOrAncestorOfType<ConstructorReference>() != null ||
+          node.thisOrAncestorOfType<InstanceCreationExpression>() != null);
+}
+
+class ActionV2ParameterMigrator extends RecursiveAstVisitor
+    with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator {
+  @override
+  bool shouldMigrate(node) =>
+      node.thisOrAncestorOfType<FormalParameter>() != null &&
+      node.thisOrAncestorOfType<MethodDeclaration>() == null &&
+      node.thisOrAncestorOfType<FieldFormalParameter>() == null;
+}
+
+class ActionV2ReturnTypeMigrator extends RecursiveAstVisitor
+    with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator {
+  @override
+  shouldMigrate(node) =>
+      node.parent is FunctionDeclaration ||
+      node.parent is FunctionTypeAlias ||
+      node.parent is GenericFunctionType ||
+      node.thisOrAncestorOfType<MethodDeclaration>() != null;
+}
+
+class ActionV2SuperTypeMigrator extends RecursiveAstVisitor
+    with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator {
+  @override
+  bool shouldMigrate(node) =>
+      node.parent is ExtendsClause ||
+      node.parent is ImplementsClause ||
+      node.parent is OnClause ||
+      node.parent is TypeParameter;
+}