From 642f364144bc7d2c0756f124607da9b9fb211ddd Mon Sep 17 00:00:00 2001 From: evanweible-wf Date: Mon, 18 Dec 2023 12:46:55 -0700 Subject: [PATCH] Address review comments --- w_flux_codemod/bin/action_v2_migrate.dart | 9 ++- .../bin/action_v2_migrate_step_2.dart | 9 ++- .../lib/src/action_v2_suggestor.dart | 46 ++++++--------- w_flux_codemod/pubspec.yaml | 2 +- .../test/action_v2_suggestor_test.dart | 57 ++++++++++++++++--- 5 files changed, 84 insertions(+), 39 deletions(-) diff --git a/w_flux_codemod/bin/action_v2_migrate.dart b/w_flux_codemod/bin/action_v2_migrate.dart index cfcab77..e5a4a96 100644 --- a/w_flux_codemod/bin/action_v2_migrate.dart +++ b/w_flux_codemod/bin/action_v2_migrate.dart @@ -12,8 +12,15 @@ void main(List args) async { ActionV2ParameterMigrator(), ActionV2ReturnTypeMigrator(), ActionV2SuperTypeMigrator(), - ActionV2DispatchMigrator(), ]), args: args, ); + if (exitCode != 0) { + return; + } + exitCode = await runInteractiveCodemod( + filePathsFromGlob(Glob('**.dart', recursive: true)), + ActionV2DispatchMigrator(), + args: args, + ); } diff --git a/w_flux_codemod/bin/action_v2_migrate_step_2.dart b/w_flux_codemod/bin/action_v2_migrate_step_2.dart index 277b8bf..111a81e 100644 --- a/w_flux_codemod/bin/action_v2_migrate_step_2.dart +++ b/w_flux_codemod/bin/action_v2_migrate_step_2.dart @@ -11,8 +11,15 @@ void main(List args) async { ActionV2FieldAndVariableMigrator(), ActionV2ReturnTypeMigrator(), ActionV2SuperTypeMigrator(), - ActionV2DispatchMigrator(), ]), args: args, ); + if (exitCode != 0) { + return; + } + exitCode = await runInteractiveCodemod( + filePathsFromGlob(Glob('**.dart', recursive: true)), + ActionV2DispatchMigrator(), + args: args, + ); } diff --git a/w_flux_codemod/lib/src/action_v2_suggestor.dart b/w_flux_codemod/lib/src/action_v2_suggestor.dart index 8919a3c..b30e6b2 100644 --- a/w_flux_codemod/lib/src/action_v2_suggestor.dart +++ b/w_flux_codemod/lib/src/action_v2_suggestor.dart @@ -3,14 +3,16 @@ import 'package:analyzer/dart/ast/visitor.dart'; import 'package:codemod/codemod.dart'; mixin ActionV2Migrator on AstVisitingSuggestor { - bool shouldMigrate(dynamic node); - @override bool shouldResolveAst(_) => true; @override bool shouldSkip(FileContext context) => !context.sourceText.contains('Action'); +} + +mixin ActionV2NamedTypeMigrator on ActionV2Migrator { + bool shouldMigrate(dynamic node); @override visitNamedType(NamedType node) { @@ -24,39 +26,27 @@ mixin ActionV2Migrator on AstVisitingSuggestor { } return super.visitNamedType(node); } - - @override - visitFunctionExpressionInvocation(FunctionExpressionInvocation node) { - if (shouldMigrate(node)) { - final typeLibraryIdentifier = - node.function.staticType?.element?.library?.identifier ?? ''; - if (typeLibraryIdentifier.startsWith('package:w_flux/') && - node.argumentList.arguments.isEmpty) { - yieldPatch('(null)', node.end - 2, node.end); - } - } - return super.visitFunctionExpressionInvocation(node); - } } -/// TODO - will likely require overriding [visitInvocationExpression] -/// and checking the type of the thing being invoked to see if it's an Action -/// If it is and there are no arguments passed, need to pass in `null` class ActionV2DispatchMigrator extends RecursiveAstVisitor with AstVisitingSuggestor, ActionV2Migrator { @override - bool shouldMigrate(node) { - if (node is FunctionExpressionInvocation) { - final name = node.function.staticType?.element?.displayName; - // the type migration should have happened prior to this suggestor. - return name == 'ActionV2'; + 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 false; + return super.visitFunctionExpressionInvocation(node); } } class ActionV2FieldAndVariableMigrator extends RecursiveAstVisitor - with AstVisitingSuggestor, ActionV2Migrator { + with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator { @override bool shouldMigrate(node) { node as NamedType; @@ -70,7 +60,7 @@ class ActionV2FieldAndVariableMigrator extends RecursiveAstVisitor } class ActionV2ParameterMigrator extends RecursiveAstVisitor - with AstVisitingSuggestor, ActionV2Migrator { + with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator { @override bool shouldMigrate(node) { node as NamedType; @@ -80,7 +70,7 @@ class ActionV2ParameterMigrator extends RecursiveAstVisitor } class ActionV2ReturnTypeMigrator extends RecursiveAstVisitor - with AstVisitingSuggestor, ActionV2Migrator { + with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator { @override shouldMigrate(node) { node as NamedType; @@ -92,7 +82,7 @@ class ActionV2ReturnTypeMigrator extends RecursiveAstVisitor } class ActionV2SuperTypeMigrator extends RecursiveAstVisitor - with AstVisitingSuggestor, ActionV2Migrator { + with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator { @override bool shouldMigrate(node) { node as NamedType; diff --git a/w_flux_codemod/pubspec.yaml b/w_flux_codemod/pubspec.yaml index bdc5cf3..e000804 100644 --- a/w_flux_codemod/pubspec.yaml +++ b/w_flux_codemod/pubspec.yaml @@ -13,7 +13,7 @@ executables: dependencies: analyzer: ^5.13.0 - codemod: ^1.1.0 + codemod: ^1.2.0 glob: ^2.1.2 workiva_analysis_options: ^1.3.0 diff --git a/w_flux_codemod/test/action_v2_suggestor_test.dart b/w_flux_codemod/test/action_v2_suggestor_test.dart index df0acea..61a807a 100644 --- a/w_flux_codemod/test/action_v2_suggestor_test.dart +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -13,7 +13,22 @@ dependencies: w_flux: ^3.0.0 '''; -const wFluxImport = "import 'package:w_flux/w_flux.dart';"; +String wFluxImport(WFluxImportMode mode) { + switch (mode) { + case WFluxImportMode.none: + return ''; + case WFluxImportMode.standard: + return "import 'package:w_flux/w_flux.dart';"; + case WFluxImportMode.prefixed: + return "import 'package:w_flux/w_flux.dart' as w_flux;"; + } +} + +enum WFluxImportMode { + none, // don't import w_flux + standard, // import w_flux + prefixed, // import w_flux with a prefix +} void main() { group('ActionV2 suggestors', () { @@ -29,16 +44,16 @@ void main() { Suggestor Function() suggestor, String before, String after, { - bool shouldImportWFlux = true, + WFluxImportMode importMode = WFluxImportMode.standard, bool shouldMigrateVariablesAndFields = false, }) { test(description, () async { final context = await pkg.addFile(''' -${shouldImportWFlux ? wFluxImport : ''} +${wFluxImport(importMode)} ${before} '''); final expectedOutput = ''' -${shouldImportWFlux ? wFluxImport : ''} +${wFluxImport(importMode)} ${after} '''; expectSuggestorGeneratesPatches( @@ -52,16 +67,42 @@ ${after} group('ActionV2DispatchMigrator', () { Suggestor suggestor() => ActionV2DispatchMigrator(); testSuggestor( - 'Function Expression Invocation', + 'local invocation of variable', + suggestor, + 'void main() { var a = ActionV2(); a(); }', + 'void main() { var a = ActionV2(); a(null); }', + ); + testSuggestor( + 'local invocation of field', + suggestor, + 'class C { ActionV2 action; dispatch() => action(); }', + 'class C { ActionV2 action; dispatch() => action(null); }', + ); + testSuggestor( + 'external invocation of field', suggestor, 'class C { ActionV2 action; } void main() { C().action(); }', 'class C { ActionV2 action; } void main() { C().action(null); }', ); testSuggestor( - 'Function Expression Invocation type 2', + 'with import prefix', suggestor, - 'void main() { var a = ActionV2(); a(); }', - 'void main() { var a = ActionV2(); a(null); }', + 'void main() { var a = w_flux.ActionV2(); a(); }', + 'void main() { var a = w_flux.ActionV2(); a(null); }', + importMode: WFluxImportMode.prefixed, + ); + testSuggestor( + 'ignores Action type', + suggestor, + 'void main() { var a = Action(); a(); }', + 'void main() { var a = Action(); a(); }', + ); + testSuggestor( + 'ignores types not from w_flux', + suggestor, + 'class ActionV2 { call(); } void main() { var a = ActionV2(); a(); }', + 'class ActionV2 { call(); } void main() { var a = ActionV2(); a(); }', + importMode: WFluxImportMode.none, ); });