Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEA-2770 FEA-2772 Action v2 type migration #195

Merged
merged 16 commits into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -712,5 +713,3 @@ linter:
# reason: Trying to assigning a value to void is an error.
# 0 issues
- void_checks


3 changes: 2 additions & 1 deletion dart_dependency_validator.yaml
Original file line number Diff line number Diff line change
@@ -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
91 changes: 91 additions & 0 deletions w_flux_codemod/README.md
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions w_flux_codemod/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include: package:workiva_analysis_options/v2.yaml
43 changes: 43 additions & 0 deletions w_flux_codemod/bin/action_v2_migrate.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import 'dart:io';

import 'package:codemod/codemod.dart';
import 'package:logging/logging.dart';
import 'package:glob/glob.dart';
import 'package:w_flux_codemod/src/action_v2_suggestor.dart';
import 'package:w_flux_codemod/src/utils.dart';

final _log = Logger('orcm.required_flux_props');
sorenthompson-wk marked this conversation as resolved.
Show resolved Hide resolved

Future<void> pubGetForAllPackageRoots(Iterable<String> files) async {
_log.info(
'Running `pub get` if needed so that all Dart files can be resolved...');
final packageRoots = files.map(findPackageRootFor).toSet();
for (final packageRoot in packageRoots) {
await runPubGetIfNeeded(packageRoot);
}
}

void main(List<String> args) async {
final dartPaths = filePathsFromGlob(Glob('**.dart', recursive: true));

await pubGetForAllPackageRoots(dartPaths);
sorenthompson-wk marked this conversation as resolved.
Show resolved Hide resolved
exitCode = await runInteractiveCodemod(
dartPaths,
aggregate([
ActionV2ParameterMigrator(),
ActionV2FieldAndVariableMigrator(),
ActionV2ReturnTypeMigrator(),
ActionV2SuperTypeMigrator(),
ActionV2ImportMigrator(),
]),
args: args,
);
if (exitCode != 0) {
return;
}
exitCode = await runInteractiveCodemod(
dartPaths,
ActionV2DispatchMigrator(),
args: args,
);
}
13 changes: 13 additions & 0 deletions w_flux_codemod/bin/action_v2_migrate_step_1.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import 'dart:io';

import 'package:codemod/codemod.dart';
import 'package:glob/glob.dart';
import 'package:w_flux_codemod/src/action_v2_suggestor.dart';

void main(List<String> args) async {
exitCode = await runInteractiveCodemod(
filePathsFromGlob(Glob('**.dart', recursive: true)),
ActionV2ParameterMigrator(),
args: args,
);
}
26 changes: 26 additions & 0 deletions w_flux_codemod/bin/action_v2_migrate_step_2.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import 'dart:io';

import 'package:codemod/codemod.dart';
import 'package:glob/glob.dart';
import 'package:w_flux_codemod/src/action_v2_suggestor.dart';

void main(List<String> args) async {
exitCode = await runInteractiveCodemod(
filePathsFromGlob(Glob('**.dart', recursive: true)),
aggregate([
ActionV2FieldAndVariableMigrator(),
ActionV2ReturnTypeMigrator(),
ActionV2SuperTypeMigrator(),
ActionV2ImportMigrator(),
]),
args: args,
);
if (exitCode != 0) {
return;
}
exitCode = await runInteractiveCodemod(
filePathsFromGlob(Glob('**.dart', recursive: true)),
ActionV2DispatchMigrator(),
args: args,
);
}
127 changes: 127 additions & 0 deletions w_flux_codemod/lib/src/action_v2_suggestor.dart
Original file line number Diff line number Diff line change
@@ -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/') &&
// The type migration should have happened prior to this suggestor.
staticTypeName == 'ActionV2' &&
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.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 ||
node.typeArguments != null;
}

class ActionV2ParameterMigrator extends RecursiveAstVisitor
with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator {
@override
bool shouldMigrate(node) =>
node.thisOrAncestorOfType<FormalParameter>() != 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.parent is MethodDeclaration;
}

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;
}
Loading
Loading