From 21125b41a87744ae7f0a34cf68f547046e346b3d Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Wed, 8 Nov 2023 13:27:30 -0700 Subject: [PATCH 01/14] create codemod infrastructure --- w_flux_codemod/README.md | 1 + w_flux_codemod/analysis_options.yaml | 1 + w_flux_codemod/bin/action_v2_migrate.dart | 39 +++++++++++++++++ w_flux_codemod/pubspec.yaml | 16 +++++++ w_flux_codemod/tests/action_v2_test.dart | 53 +++++++++++++++++++++++ 5 files changed, 110 insertions(+) create mode 100644 w_flux_codemod/README.md create mode 100644 w_flux_codemod/analysis_options.yaml create mode 100644 w_flux_codemod/bin/action_v2_migrate.dart create mode 100644 w_flux_codemod/pubspec.yaml create mode 100644 w_flux_codemod/tests/action_v2_test.dart diff --git a/w_flux_codemod/README.md b/w_flux_codemod/README.md new file mode 100644 index 0000000..1a0cd3c --- /dev/null +++ b/w_flux_codemod/README.md @@ -0,0 +1 @@ +TODO: FILL OUT LATER \ No newline at end of file diff --git a/w_flux_codemod/analysis_options.yaml b/w_flux_codemod/analysis_options.yaml new file mode 100644 index 0000000..a67a6c8 --- /dev/null +++ b/w_flux_codemod/analysis_options.yaml @@ -0,0 +1 @@ +include: package:workiva_analysis_options/v2.yaml diff --git a/w_flux_codemod/bin/action_v2_migrate.dart b/w_flux_codemod/bin/action_v2_migrate.dart new file mode 100644 index 0000000..54fe5a2 --- /dev/null +++ b/w_flux_codemod/bin/action_v2_migrate.dart @@ -0,0 +1,39 @@ +import 'dart:io'; + +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:codemod/codemod.dart'; +import 'package:glob/glob.dart'; + +void main(List args) async { + exitCode = await runInteractiveCodemod( + filePathsFromGlob(Glob('**.dart', recursive: true)), + ActionV2Suggestor(), + args: args, + ); +} + +class ActionV2Suggestor extends GeneralizingAstVisitor + with AstVisitingSuggestor { + @override + bool shouldResolveAst(_) => true; + + @override + visitSimpleFormalParameter(SimpleFormalParameter node) { + final nodeType = node.type; + final typeNameToken = nodeType is NamedType ? nodeType.name2 : null; + if (typeNameToken != null) { + final typeName = + context.sourceFile.getText(typeNameToken.offset, typeNameToken.end); + final identifier = + node.declaredElement?.type.element?.library?.identifier; + print(identifier); + print(typeName); + if (typeName == 'Action' && + identifier?.startsWith('package:w_flux/') == true) { + yieldPatch('ActionV2', typeNameToken.offset, typeNameToken.end); + } + } + return super.visitSimpleFormalParameter(node); + } +} diff --git a/w_flux_codemod/pubspec.yaml b/w_flux_codemod/pubspec.yaml new file mode 100644 index 0000000..602141a --- /dev/null +++ b/w_flux_codemod/pubspec.yaml @@ -0,0 +1,16 @@ +name: w_flux_codemod +version: 1.0.0 +description: codemod for migrating to ActionV2 +homepage: https://github.com/Workiva/w_flux/w_flux_codemod + +environment: + sdk: ">=2.12.0 <3.0.0" + +dependencies: + analyzer: ^5.13.0 + codemod: ^1.1.0 + glob: ^2.1.2 + workiva_analysis_options: ^1.3.0 + +executables: + action_v2_migrate: diff --git a/w_flux_codemod/tests/action_v2_test.dart b/w_flux_codemod/tests/action_v2_test.dart new file mode 100644 index 0000000..91d3117 --- /dev/null +++ b/w_flux_codemod/tests/action_v2_test.dart @@ -0,0 +1,53 @@ +import 'package:codemod/test.dart'; +import 'package:test/test.dart'; + +import '../bin/action_v2_migrate.dart'; + +final testCases = [ + { + 'context': '''Function(Action action) {}''', + 'expectedOutput': '''Function(ActionV2 action) {}''', + }, + { + 'context': '''Function(Action action) {}''', + 'expectedOutput': '''Function(ActionV2 action) {}''', + }, + { + 'context': '''Function(Action action) {}''', + 'expectedOutput': '''Function(ActionV2 action) {}''', + }, + { + 'context': '''Function(List action) {}''', + 'expectedOutput': '''Function(List action) {}''', + }, + // { + // 'context': 'class SuperFancyClass {' + // 'Action get action;' + // '}', + // 'expectedOutput': 'Function(ActionV2 action) {}', + // }, +]; + +void main() { + group('DeprecatedRemover', () { + final filename = 'test.dart'; + for (final testCase in testCases) { + test('removes deprecated variable', () async { + final context = await fileContextForTest(filename, ''' + // Not deprecated. + var foo = 'foo'; + @deprecated + var bar = 'bar';'''); + final expectedOutput = ''' + // Not deprecated. + var foo = 'foo'; + '''; + expectSuggestorGeneratesPatches( + ActionV2Suggestor(), + context, + expectedOutput, + ); + }); + } + }); +} From 80d158a1e969c2c3deb7d9fbfc3c170c3b6c3fe0 Mon Sep 17 00:00:00 2001 From: evanweible-wf Date: Wed, 15 Nov 2023 14:07:02 -0700 Subject: [PATCH 02/14] Implement ActionV2 suggestors --- analysis_options.yaml | 9 +- dart_dependency_validator.yaml | 3 +- w_flux_codemod/bin/action_v2_migrate.dart | 35 +-- .../bin/action_v2_migrate_step_1.dart | 13 + .../bin/action_v2_migrate_step_2.dart | 17 ++ .../lib/src/action_v2_suggestor.dart | 73 ++++++ w_flux_codemod/pubspec.yaml | 21 +- .../test/action_v2_suggestor_test.dart | 239 ++++++++++++++++++ w_flux_codemod/tests/action_v2_test.dart | 53 ---- 9 files changed, 372 insertions(+), 91 deletions(-) create mode 100644 w_flux_codemod/bin/action_v2_migrate_step_1.dart create mode 100644 w_flux_codemod/bin/action_v2_migrate_step_2.dart create mode 100644 w_flux_codemod/lib/src/action_v2_suggestor.dart create mode 100644 w_flux_codemod/test/action_v2_suggestor_test.dart delete mode 100644 w_flux_codemod/tests/action_v2_test.dart diff --git a/analysis_options.yaml b/analysis_options.yaml index f9ea6d8..ef7dc9a 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -1,12 +1,13 @@ -# 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/**" # Strong mode is required. Applies to the current project. strong-mode: - # When compiling to JS, both implicit options apply to the current - # project and all dependencies. They are useful to find possible + # When compiling to JS, both implicit options apply to the current + # project and all dependencies. They are useful to find possible # Type fixes or areas for explicit typing. implicit-casts: true implicit-dynamic: true @@ -717,5 +718,3 @@ linter: # reason: Trying to assigning a value to void is an error. # 0 issues - void_checks - - diff --git a/dart_dependency_validator.yaml b/dart_dependency_validator.yaml index 91c26d3..c039c0d 100644 --- a/dart_dependency_validator.yaml +++ b/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 \ No newline at end of file + - test diff --git a/w_flux_codemod/bin/action_v2_migrate.dart b/w_flux_codemod/bin/action_v2_migrate.dart index 54fe5a2..21b7158 100644 --- a/w_flux_codemod/bin/action_v2_migrate.dart +++ b/w_flux_codemod/bin/action_v2_migrate.dart @@ -1,39 +1,18 @@ import 'dart:io'; -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/ast/visitor.dart'; import 'package:codemod/codemod.dart'; import 'package:glob/glob.dart'; +import 'package:w_flux_codemod/src/action_v2_suggestor.dart'; void main(List args) async { exitCode = await runInteractiveCodemod( filePathsFromGlob(Glob('**.dart', recursive: true)), - ActionV2Suggestor(), + aggregate([ + ActionV2FieldAndVariableMigrator(), + ActionV2ParameterMigrator(), + ActionV2ReturnTypeMigrator(), + ActionV2SuperTypeMigrator(), + ]), args: args, ); } - -class ActionV2Suggestor extends GeneralizingAstVisitor - with AstVisitingSuggestor { - @override - bool shouldResolveAst(_) => true; - - @override - visitSimpleFormalParameter(SimpleFormalParameter node) { - final nodeType = node.type; - final typeNameToken = nodeType is NamedType ? nodeType.name2 : null; - if (typeNameToken != null) { - final typeName = - context.sourceFile.getText(typeNameToken.offset, typeNameToken.end); - final identifier = - node.declaredElement?.type.element?.library?.identifier; - print(identifier); - print(typeName); - if (typeName == 'Action' && - identifier?.startsWith('package:w_flux/') == true) { - yieldPatch('ActionV2', typeNameToken.offset, typeNameToken.end); - } - } - return super.visitSimpleFormalParameter(node); - } -} diff --git a/w_flux_codemod/bin/action_v2_migrate_step_1.dart b/w_flux_codemod/bin/action_v2_migrate_step_1.dart new file mode 100644 index 0000000..cfe9c41 --- /dev/null +++ b/w_flux_codemod/bin/action_v2_migrate_step_1.dart @@ -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 args) async { + exitCode = await runInteractiveCodemod( + filePathsFromGlob(Glob('**.dart', recursive: true)), + ActionV2ParameterMigrator(), + 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 new file mode 100644 index 0000000..9706337 --- /dev/null +++ b/w_flux_codemod/bin/action_v2_migrate_step_2.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'; + +void main(List args) async { + exitCode = await runInteractiveCodemod( + filePathsFromGlob(Glob('**.dart', recursive: true)), + aggregate([ + ActionV2FieldAndVariableMigrator(), + ActionV2ReturnTypeMigrator(), + ActionV2SuperTypeMigrator(), + ]), + args: args, + ); +} diff --git a/w_flux_codemod/lib/src/action_v2_suggestor.dart b/w_flux_codemod/lib/src/action_v2_suggestor.dart new file mode 100644 index 0000000..685397a --- /dev/null +++ b/w_flux_codemod/lib/src/action_v2_suggestor.dart @@ -0,0 +1,73 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:codemod/codemod.dart'; + +mixin ActionV2Migrator on AstVisitingSuggestor { + bool shouldMigrate(NamedType node); + + @override + bool shouldResolveAst(_) => true; + + @override + bool shouldSkip(FileContext context) => + !context.sourceText.contains('Action'); + + @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); + } +} + +/// 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 {} + +class ActionV2FieldAndVariableMigrator extends RecursiveAstVisitor + with AstVisitingSuggestor, ActionV2Migrator { + @override + bool shouldMigrate(NamedType node) => + node.parent is DeclaredIdentifier || + node.parent is DeclaredVariablePattern || + node.parent is FieldFormalParameter || + node.parent is VariableDeclarationList || + node.thisOrAncestorOfType() != null || + node.thisOrAncestorOfType() != null; +} + +class ActionV2ParameterMigrator extends RecursiveAstVisitor + with AstVisitingSuggestor, ActionV2Migrator { + @override + bool shouldMigrate(NamedType node) => + node.thisOrAncestorOfType() != null && + node.thisOrAncestorOfType() == null; +} + +class ActionV2ReturnTypeMigrator extends RecursiveAstVisitor + with AstVisitingSuggestor, ActionV2Migrator { + @override + bool shouldMigrate(NamedType node) => + node.parent is FunctionDeclaration || + node.parent is FunctionTypeAlias || + node.parent is GenericFunctionType || + node.parent is MethodDeclaration; +} + +class ActionV2SuperTypeMigrator extends RecursiveAstVisitor + with AstVisitingSuggestor, ActionV2Migrator { + @override + bool shouldMigrate(NamedType node) => + node.parent is ExtendsClause || + node.parent is ImplementsClause || + node.parent is OnClause || + node.parent is TypeParameter; +} diff --git a/w_flux_codemod/pubspec.yaml b/w_flux_codemod/pubspec.yaml index 602141a..174ba21 100644 --- a/w_flux_codemod/pubspec.yaml +++ b/w_flux_codemod/pubspec.yaml @@ -1,10 +1,15 @@ name: w_flux_codemod version: 1.0.0 description: codemod for migrating to ActionV2 -homepage: https://github.com/Workiva/w_flux/w_flux_codemod +homepage: https://github.com/Workiva/w_flux/tree/master/w_flux_codemod environment: - sdk: ">=2.12.0 <3.0.0" + sdk: ">=2.19.0 <3.0.0" + +executables: + action_v2_migrate: + action_v2_migrate_step_1: + action_v2_migrate_step_2: dependencies: analyzer: ^5.13.0 @@ -12,5 +17,13 @@ dependencies: glob: ^2.1.2 workiva_analysis_options: ^1.3.0 -executables: - action_v2_migrate: +dev_dependencies: + meta: ^1.11.0 + source_span: ^1.10.0 + test: ^1.24.3 + +dependency_overrides: + codemod: + git: + url: git@github.com:Workiva/dart_codemod.git + ref: add-test-apis-for-testing-resolved-ast-suggestors diff --git a/w_flux_codemod/test/action_v2_suggestor_test.dart b/w_flux_codemod/test/action_v2_suggestor_test.dart new file mode 100644 index 0000000..813e279 --- /dev/null +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -0,0 +1,239 @@ +library; + +import 'package:codemod/codemod.dart'; +import 'package:codemod/test.dart'; +import 'package:meta/meta.dart'; +import 'package:test/test.dart'; +import 'package:w_flux_codemod/src/action_v2_suggestor.dart'; + +const pubspec = ''' +name: pkg +publish_to: none +environment: + sdk: '>=2.11.0 <4.0.0' +dependencies: + w_flux: ^2.11.0 +'''; + +const wFluxImport = "import 'package:w_flux/w_flux.dart';"; + +void main() { + group('ActionV2 suggestors', () { + late PackageContextForTest pkg; + + setUpAll(() async { + pkg = await PackageContextForTest.fromPubspec(pubspec); + }); + + @isTest + void testSuggestor( + String description, + Suggestor Function() suggestor, + String before, + String after, { + bool shouldImportWFlux = true, + bool shouldMigrateVariablesAndFields = false, + }) { + test(description, () async { + final context = await pkg.addFile(''' +${shouldImportWFlux ? wFluxImport : ''} +${before} +'''); + final expectedOutput = ''' +${shouldImportWFlux ? wFluxImport : ''} +${after} +'''; + expectSuggestorGeneratesPatches( + suggestor(), + context, + expectedOutput, + ); + }); + } + + group('ActionV2DispatchMigrator', () { + // TODO + }); + + group('FieldAndVariableMigrator', () { + Suggestor suggestor() => ActionV2FieldAndVariableMigrator(); + group('VariableDeclarationList', () { + testSuggestor( + 'with type, no intializer', + suggestor, + 'Action action;', + 'ActionV2 action;', + ); + testSuggestor( + 'with type and intializer', + suggestor, + 'Action action = Action();', + 'ActionV2 action = ActionV2();', + ); + testSuggestor( + 'no type, with intializer', + suggestor, + 'var action = Action();', + 'var action = ActionV2();', + ); + testSuggestor( + 'dynamic Actions', + suggestor, + 'Action a; Action b = Action(); var c = Action();', + 'ActionV2 a; ActionV2 b = ActionV2(); var c = ActionV2();', + ); + }); + group('FieldDeclaration', () { + testSuggestor( + 'with type, no intializer', + suggestor, + 'class C { Action action; }', + 'class C { ActionV2 action; }', + ); + testSuggestor( + 'with type and intializer', + suggestor, + 'class C { Action action = Action(); }', + 'class C { ActionV2 action = ActionV2(); }', + ); + testSuggestor( + 'no type, with intializer', + suggestor, + 'class C { var action = Action(); }', + 'class C { var action = ActionV2(); }', + ); + testSuggestor( + 'dynamic Actions', + suggestor, + 'class C { Action a; Action b = Action(); var c = Action(); }', + 'class C { ActionV2 a; ActionV2 b = ActionV2(); var c = ActionV2(); }', + ); + }); + group('InstanceCreationExpression', () { + testSuggestor( + 'variable initialization', + suggestor, + 'var action; action = Action();', + 'var action; action = ActionV2();', + ); + testSuggestor( + 'field initialization', + suggestor, + 'class C { var action; C() { action = Action(); } }', + 'class C { var action; C() { action = ActionV2(); } }', + ); + testSuggestor( + 'skips dynamic Actions', + suggestor, + 'Action a; a = Action();', + 'Action a; Action b = Action(); var c = Action();', + ); + }); + group('Action dispatch', () { + // Should update 0-arg dispatch calls to `(null)` + }); + }); + + group('ParameterMigrator', () { + Suggestor suggestor() => ActionV2ParameterMigrator(); + testSuggestor( + 'SimpleFormalParameter.type (function)', + suggestor, + 'fn(Action action) {}', + 'fn(ActionV2 action) {}', + ); + testSuggestor( + 'SimpleFormalParameter.type (method)', + suggestor, + 'class C { m(Action action) {} }', + 'class C { m(ActionV2 action) {} }', + ); + testSuggestor( + 'Parameter type with a generic', + suggestor, + 'fn(Action action) {}', + 'fn(ActionV2 action) {}', + ); + }); + + group('ReturnTypeMigrator', () { + Suggestor suggestor() => ActionV2ReturnTypeMigrator(); + testSuggestor( + 'FunctionDeclaration.returnType', + suggestor, + 'Action fn() {}', + 'ActionV2 fn() {}', + ); + testSuggestor( + 'FunctionTypeAlias.returnType', + suggestor, + 'typedef t = Action Function();', + 'typedef t = ActionV2 Function();', + ); + testSuggestor( + 'FunctionTypedFormalParameter.returnType (function)', + suggestor, + 'fn(Action Function() fn) {}', + 'fn(ActionV2 Function() fn) {}', + ); + testSuggestor( + 'FunctionTypedFormalParameter.returnType (method)', + suggestor, + 'class C { m(Action Function() fn) {} }', + 'class C { m(ActionV2 Function() fn) {} }', + ); + testSuggestor( + 'GenericFunctionType.returnType', + suggestor, + 'fn(Action Function() callback) {}', + 'fn(ActionV2 Function() callback) {}', + ); + testSuggestor( + 'MethodDeclaration.returnType', + suggestor, + 'class C { Action m() {} }', + 'class C { ActionV2 m() {} }', + ); + testSuggestor( + 'Return type with a generic', + suggestor, + 'Action fn() {}', + 'ActionV2 fn() {}', + ); + }); + + group('TypeParameterMigrator', () { + Suggestor suggestor() => ActionV2SuperTypeMigrator(); + testSuggestor( + 'ExtendsClause.superclass', + suggestor, + 'class C extends Action {}', + 'class C extends ActionV2 {}', + ); + testSuggestor( + 'ImplementsClause.interfaces', + suggestor, + 'class C implements Action {}', + 'class C implements ActionV2 {}', + ); + testSuggestor( + 'OnClause.superclassConstraints', + suggestor, + 'class C extends Action {}', + 'class C extends ActionV2 {}', + ); + testSuggestor( + 'Type parameter', + suggestor, + 'fn() {}', + 'fn() {}', + ); + testSuggestor( + 'Type parameter with a generic', + suggestor, + 'fn>() {}', + 'fn>() {}', + ); + }); + }); +} diff --git a/w_flux_codemod/tests/action_v2_test.dart b/w_flux_codemod/tests/action_v2_test.dart deleted file mode 100644 index 91d3117..0000000 --- a/w_flux_codemod/tests/action_v2_test.dart +++ /dev/null @@ -1,53 +0,0 @@ -import 'package:codemod/test.dart'; -import 'package:test/test.dart'; - -import '../bin/action_v2_migrate.dart'; - -final testCases = [ - { - 'context': '''Function(Action action) {}''', - 'expectedOutput': '''Function(ActionV2 action) {}''', - }, - { - 'context': '''Function(Action action) {}''', - 'expectedOutput': '''Function(ActionV2 action) {}''', - }, - { - 'context': '''Function(Action action) {}''', - 'expectedOutput': '''Function(ActionV2 action) {}''', - }, - { - 'context': '''Function(List action) {}''', - 'expectedOutput': '''Function(List action) {}''', - }, - // { - // 'context': 'class SuperFancyClass {' - // 'Action get action;' - // '}', - // 'expectedOutput': 'Function(ActionV2 action) {}', - // }, -]; - -void main() { - group('DeprecatedRemover', () { - final filename = 'test.dart'; - for (final testCase in testCases) { - test('removes deprecated variable', () async { - final context = await fileContextForTest(filename, ''' - // Not deprecated. - var foo = 'foo'; - @deprecated - var bar = 'bar';'''); - final expectedOutput = ''' - // Not deprecated. - var foo = 'foo'; - '''; - expectSuggestorGeneratesPatches( - ActionV2Suggestor(), - context, - expectedOutput, - ); - }); - } - }); -} From e0193b1bafb82407acf391ac6df9d7a4d36f2eac Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Mon, 11 Dec 2023 17:00:43 -0700 Subject: [PATCH 03/14] Dispath migrator WIP --- .../lib/src/action_v2_suggestor.dart | 27 ++++++++++++++++++- w_flux_codemod/pubspec.yaml | 6 ----- .../test/action_v2_suggestor_test.dart | 9 ++++++- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/w_flux_codemod/lib/src/action_v2_suggestor.dart b/w_flux_codemod/lib/src/action_v2_suggestor.dart index 685397a..4a0b41f 100644 --- a/w_flux_codemod/lib/src/action_v2_suggestor.dart +++ b/w_flux_codemod/lib/src/action_v2_suggestor.dart @@ -24,13 +24,38 @@ mixin ActionV2Migrator on AstVisitingSuggestor { } return super.visitNamedType(node); } + + @override + visitInvocationExpression(InvocationExpression node) { + if (shouldMigrate(node)) { + for (var arg in node.argumentList.arguments) { + final typeLibraryIdentifier = + arg.staticType.element?.library?.identifier ?? ''; + if (arg.staticParameterElement.name == 'Action' && + typeLibraryIdentifier.startsWith('package:w_flux/')) { + if (arg.staticParameterElement.parameters.isEmpty) { + yieldPatch('ActionV2(null)', arg.offset, arg.end); + } else { + yieldPatch('ActionV2', arg.offset, arg.end); + } + } + } + } + return super.visitInvocationExpression(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 {} + with AstVisitingSuggestor, ActionV2Migrator { + @override + bool shouldMigrate(InvocationExpression node) => + node.staticInvokeType != null && + node.argumentList.arguments.isNotEmpty && + node.function.staticParameterElement.name == 'dispatch'; +} class ActionV2FieldAndVariableMigrator extends RecursiveAstVisitor with AstVisitingSuggestor, ActionV2Migrator { diff --git a/w_flux_codemod/pubspec.yaml b/w_flux_codemod/pubspec.yaml index 174ba21..bdc5cf3 100644 --- a/w_flux_codemod/pubspec.yaml +++ b/w_flux_codemod/pubspec.yaml @@ -21,9 +21,3 @@ dev_dependencies: meta: ^1.11.0 source_span: ^1.10.0 test: ^1.24.3 - -dependency_overrides: - codemod: - git: - url: git@github.com:Workiva/dart_codemod.git - ref: add-test-apis-for-testing-resolved-ast-suggestors diff --git a/w_flux_codemod/test/action_v2_suggestor_test.dart b/w_flux_codemod/test/action_v2_suggestor_test.dart index 813e279..b619f56 100644 --- a/w_flux_codemod/test/action_v2_suggestor_test.dart +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -1,3 +1,4 @@ +@TestOn('browser') library; import 'package:codemod/codemod.dart'; @@ -52,7 +53,13 @@ ${after} } group('ActionV2DispatchMigrator', () { - // TODO + Suggestor suggestor() => ActionV2DispatchMigrator(); + testSuggestor( + 'Dispatch action with parameter', + suggestor, + 'dispatch(Action(fn() {}))', + 'dispatch(ActionV2(fn() {}))', + ); }); group('FieldAndVariableMigrator', () { From 8b134d3978bebe1d7af394bc08c9396e63bf5b71 Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Fri, 15 Dec 2023 13:35:24 -0700 Subject: [PATCH 04/14] Action dispatch migrator --- w_flux_codemod/bin/action_v2_migrate.dart | 1 + .../bin/action_v2_migrate_step_2.dart | 1 + .../lib/src/action_v2_suggestor.dart | 82 ++++++++++--------- .../test/action_v2_suggestor_test.dart | 30 +++---- 4 files changed, 62 insertions(+), 52 deletions(-) diff --git a/w_flux_codemod/bin/action_v2_migrate.dart b/w_flux_codemod/bin/action_v2_migrate.dart index 21b7158..cfcab77 100644 --- a/w_flux_codemod/bin/action_v2_migrate.dart +++ b/w_flux_codemod/bin/action_v2_migrate.dart @@ -12,6 +12,7 @@ void main(List args) async { ActionV2ParameterMigrator(), ActionV2ReturnTypeMigrator(), ActionV2SuperTypeMigrator(), + 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 9706337..277b8bf 100644 --- a/w_flux_codemod/bin/action_v2_migrate_step_2.dart +++ b/w_flux_codemod/bin/action_v2_migrate_step_2.dart @@ -11,6 +11,7 @@ void main(List args) async { ActionV2FieldAndVariableMigrator(), ActionV2ReturnTypeMigrator(), ActionV2SuperTypeMigrator(), + 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 4a0b41f..8919a3c 100644 --- a/w_flux_codemod/lib/src/action_v2_suggestor.dart +++ b/w_flux_codemod/lib/src/action_v2_suggestor.dart @@ -3,7 +3,7 @@ import 'package:analyzer/dart/ast/visitor.dart'; import 'package:codemod/codemod.dart'; mixin ActionV2Migrator on AstVisitingSuggestor { - bool shouldMigrate(NamedType node); + bool shouldMigrate(dynamic node); @override bool shouldResolveAst(_) => true; @@ -26,22 +26,16 @@ mixin ActionV2Migrator on AstVisitingSuggestor { } @override - visitInvocationExpression(InvocationExpression node) { + visitFunctionExpressionInvocation(FunctionExpressionInvocation node) { if (shouldMigrate(node)) { - for (var arg in node.argumentList.arguments) { - final typeLibraryIdentifier = - arg.staticType.element?.library?.identifier ?? ''; - if (arg.staticParameterElement.name == 'Action' && - typeLibraryIdentifier.startsWith('package:w_flux/')) { - if (arg.staticParameterElement.parameters.isEmpty) { - yieldPatch('ActionV2(null)', arg.offset, arg.end); - } else { - yieldPatch('ActionV2', arg.offset, arg.end); - } - } + 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.visitInvocationExpression(node); + return super.visitFunctionExpressionInvocation(node); } } @@ -51,48 +45,60 @@ mixin ActionV2Migrator on AstVisitingSuggestor { class ActionV2DispatchMigrator extends RecursiveAstVisitor with AstVisitingSuggestor, ActionV2Migrator { @override - bool shouldMigrate(InvocationExpression node) => - node.staticInvokeType != null && - node.argumentList.arguments.isNotEmpty && - node.function.staticParameterElement.name == 'dispatch'; + 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'; + } + return false; + } } class ActionV2FieldAndVariableMigrator extends RecursiveAstVisitor with AstVisitingSuggestor, ActionV2Migrator { @override - bool shouldMigrate(NamedType node) => - node.parent is DeclaredIdentifier || - node.parent is DeclaredVariablePattern || - node.parent is FieldFormalParameter || - node.parent is VariableDeclarationList || - node.thisOrAncestorOfType() != null || - node.thisOrAncestorOfType() != null; + bool shouldMigrate(node) { + node as NamedType; + return node.parent is DeclaredIdentifier || + node.parent is DeclaredVariablePattern || + node.parent is FieldFormalParameter || + node.parent is VariableDeclarationList || + node.thisOrAncestorOfType() != null || + node.thisOrAncestorOfType() != null; + } } class ActionV2ParameterMigrator extends RecursiveAstVisitor with AstVisitingSuggestor, ActionV2Migrator { @override - bool shouldMigrate(NamedType node) => - node.thisOrAncestorOfType() != null && - node.thisOrAncestorOfType() == null; + bool shouldMigrate(node) { + node as NamedType; + return node.thisOrAncestorOfType() != null && + node.thisOrAncestorOfType() == null; + } } class ActionV2ReturnTypeMigrator extends RecursiveAstVisitor with AstVisitingSuggestor, ActionV2Migrator { @override - bool shouldMigrate(NamedType node) => - node.parent is FunctionDeclaration || - node.parent is FunctionTypeAlias || - node.parent is GenericFunctionType || - node.parent is MethodDeclaration; + shouldMigrate(node) { + node as NamedType; + return node.parent is FunctionDeclaration || + node.parent is FunctionTypeAlias || + node.parent is GenericFunctionType || + node.parent is MethodDeclaration; + } } class ActionV2SuperTypeMigrator extends RecursiveAstVisitor with AstVisitingSuggestor, ActionV2Migrator { @override - bool shouldMigrate(NamedType node) => - node.parent is ExtendsClause || - node.parent is ImplementsClause || - node.parent is OnClause || - node.parent is TypeParameter; + bool shouldMigrate(node) { + node as NamedType; + return node.parent is ExtendsClause || + node.parent is ImplementsClause || + node.parent is OnClause || + node.parent is TypeParameter; + } } diff --git a/w_flux_codemod/test/action_v2_suggestor_test.dart b/w_flux_codemod/test/action_v2_suggestor_test.dart index b619f56..8e6fc00 100644 --- a/w_flux_codemod/test/action_v2_suggestor_test.dart +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -1,4 +1,3 @@ -@TestOn('browser') library; import 'package:codemod/codemod.dart'; @@ -13,7 +12,7 @@ publish_to: none environment: sdk: '>=2.11.0 <4.0.0' dependencies: - w_flux: ^2.11.0 + w_flux: ^3.0.0 '''; const wFluxImport = "import 'package:w_flux/w_flux.dart';"; @@ -55,10 +54,16 @@ ${after} group('ActionV2DispatchMigrator', () { Suggestor suggestor() => ActionV2DispatchMigrator(); testSuggestor( - 'Dispatch action with parameter', + 'Function Expression Invocation', suggestor, - 'dispatch(Action(fn() {}))', - 'dispatch(ActionV2(fn() {}))', + 'class C { ActionV2 action; } void main() { C().action(); }', + 'class C { ActionV2 action; } void main() { C().action(null); }', + ); + testSuggestor( + 'Function Expression Invocation type 2', + suggestor, + 'void main() { var a = ActionV2(); a(); }', + 'void main() { var a = ActionV2(); a(null); }', ); }); @@ -129,15 +134,12 @@ ${after} 'class C { var action; C() { action = Action(); } }', 'class C { var action; C() { action = ActionV2(); } }', ); - testSuggestor( - 'skips dynamic Actions', - suggestor, - 'Action a; a = Action();', - 'Action a; Action b = Action(); var c = Action();', - ); - }); - group('Action dispatch', () { - // Should update 0-arg dispatch calls to `(null)` + // testSuggestor( + // 'skips dynamic Actions', + // suggestor, + // 'Action a; a = Action();', + // 'Action a; Action b = Action(); var c = Action();', + // ); }); }); From c37159d0ea5b2ff0d99239511e774977f17b7d68 Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Fri, 15 Dec 2023 13:44:04 -0700 Subject: [PATCH 05/14] fix test? --- w_flux_codemod/test/action_v2_suggestor_test.dart | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/w_flux_codemod/test/action_v2_suggestor_test.dart b/w_flux_codemod/test/action_v2_suggestor_test.dart index 8e6fc00..864322f 100644 --- a/w_flux_codemod/test/action_v2_suggestor_test.dart +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -134,12 +134,12 @@ ${after} 'class C { var action; C() { action = Action(); } }', 'class C { var action; C() { action = ActionV2(); } }', ); - // testSuggestor( - // 'skips dynamic Actions', - // suggestor, - // 'Action a; a = Action();', - // 'Action a; Action b = Action(); var c = Action();', - // ); + testSuggestor( + 'skips dynamic Actions', + suggestor, + 'Action a; Action b = Action(); var c = Action();', + 'Action a; Action b = Action(); var c = Action();', + ); }); }); From b17a2984b2a4538b0d89a79d88a110c265e85016 Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Fri, 15 Dec 2023 14:20:18 -0700 Subject: [PATCH 06/14] fix test --- w_flux_codemod/test/action_v2_suggestor_test.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/w_flux_codemod/test/action_v2_suggestor_test.dart b/w_flux_codemod/test/action_v2_suggestor_test.dart index 864322f..df0acea 100644 --- a/w_flux_codemod/test/action_v2_suggestor_test.dart +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -1,5 +1,3 @@ -library; - import 'package:codemod/codemod.dart'; import 'package:codemod/test.dart'; import 'package:meta/meta.dart'; @@ -138,7 +136,7 @@ ${after} 'skips dynamic Actions', suggestor, 'Action a; Action b = Action(); var c = Action();', - 'Action a; Action b = Action(); var c = Action();', + 'ActionV2 a; ActionV2 b = ActionV2(); var c = ActionV2();', ); }); }); From e8a435b3b69e945ce6dee37595f9663a9b18af66 Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Mon, 18 Dec 2023 11:53:45 -0700 Subject: [PATCH 07/14] Add README --- w_flux_codemod/README.md | 90 ++++++++++++++++++++++- w_flux_codemod/bin/action_v2_migrate.dart | 2 +- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/w_flux_codemod/README.md b/w_flux_codemod/README.md index 1a0cd3c..b4cd3e8 100644 --- a/w_flux_codemod/README.md +++ b/w_flux_codemod/README.md @@ -1 +1,89 @@ -TODO: FILL OUT LATER \ No newline at end of file +# 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 /*...*/ { + 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 w_flux_codemod + ``` + +2. Run the codemod: + + - step by step: + ```bash + dart pub global run w_flux_codemod:action_v2_migrate_step_1.dart + dart pub global run w_flux_codemod:action_v2_migrate_step_2.dart + ``` + + - all at once: + ```bash + dart pub global run w_flux_codemod:action_v2_migrate.dart + ``` + +3. Review the changes: + + - It's advisable to review the changes and ensure they are correct and meet your project's requirements. + + - Dart Analysis should be able to catch any errors that may occur with 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 \ No newline at end of file diff --git a/w_flux_codemod/bin/action_v2_migrate.dart b/w_flux_codemod/bin/action_v2_migrate.dart index cfcab77..3aed4ac 100644 --- a/w_flux_codemod/bin/action_v2_migrate.dart +++ b/w_flux_codemod/bin/action_v2_migrate.dart @@ -8,8 +8,8 @@ void main(List args) async { exitCode = await runInteractiveCodemod( filePathsFromGlob(Glob('**.dart', recursive: true)), aggregate([ - ActionV2FieldAndVariableMigrator(), ActionV2ParameterMigrator(), + ActionV2FieldAndVariableMigrator(), ActionV2ReturnTypeMigrator(), ActionV2SuperTypeMigrator(), ActionV2DispatchMigrator(), From 642f364144bc7d2c0756f124607da9b9fb211ddd Mon Sep 17 00:00:00 2001 From: evanweible-wf Date: Mon, 18 Dec 2023 12:46:55 -0700 Subject: [PATCH 08/14] 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, ); }); From c09cc89b231f6e809b342d4c1125e778f025ce80 Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Mon, 18 Dec 2023 14:28:51 -0700 Subject: [PATCH 09/14] Address PR Feedback --- w_flux_codemod/README.md | 6 +-- .../lib/src/action_v2_suggestor.dart | 50 ++++++++----------- .../test/action_v2_suggestor_test.dart | 2 +- 3 files changed, 25 insertions(+), 33 deletions(-) diff --git a/w_flux_codemod/README.md b/w_flux_codemod/README.md index b4cd3e8..c4644e9 100644 --- a/w_flux_codemod/README.md +++ b/w_flux_codemod/README.md @@ -39,13 +39,13 @@ To be able to support non-nullable payloads (in addition to nullable payloads), - step by step: ```bash - dart pub global run w_flux_codemod:action_v2_migrate_step_1.dart - dart pub global run w_flux_codemod:action_v2_migrate_step_2.dart + dart pub global run w_flux_codemod:action_v2_migrate_step_1 + dart pub global run w_flux_codemod:action_v2_migrate_step_2 ``` - all at once: ```bash - dart pub global run w_flux_codemod:action_v2_migrate.dart + dart pub global run w_flux_codemod:action_v2_migrate ``` 3. Review the changes: diff --git a/w_flux_codemod/lib/src/action_v2_suggestor.dart b/w_flux_codemod/lib/src/action_v2_suggestor.dart index b30e6b2..017a2fd 100644 --- a/w_flux_codemod/lib/src/action_v2_suggestor.dart +++ b/w_flux_codemod/lib/src/action_v2_suggestor.dart @@ -12,7 +12,7 @@ mixin ActionV2Migrator on AstVisitingSuggestor { } mixin ActionV2NamedTypeMigrator on ActionV2Migrator { - bool shouldMigrate(dynamic node); + bool shouldMigrate(NamedType node); @override visitNamedType(NamedType node) { @@ -48,47 +48,39 @@ class ActionV2DispatchMigrator extends RecursiveAstVisitor class ActionV2FieldAndVariableMigrator extends RecursiveAstVisitor with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator { @override - bool shouldMigrate(node) { - node as NamedType; - return node.parent is DeclaredIdentifier || - node.parent is DeclaredVariablePattern || - node.parent is FieldFormalParameter || - node.parent is VariableDeclarationList || - node.thisOrAncestorOfType() != null || - node.thisOrAncestorOfType() != null; - } + bool shouldMigrate(node) => + node.parent is DeclaredIdentifier || + node.parent is DeclaredVariablePattern || + node.parent is FieldFormalParameter || + node.parent is VariableDeclarationList || + node.thisOrAncestorOfType() != null || + node.thisOrAncestorOfType() != null; } class ActionV2ParameterMigrator extends RecursiveAstVisitor with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator { @override - bool shouldMigrate(node) { - node as NamedType; - return node.thisOrAncestorOfType() != null && - node.thisOrAncestorOfType() == null; - } + bool shouldMigrate(node) => + node.thisOrAncestorOfType() != null && + node.thisOrAncestorOfType() == null; } class ActionV2ReturnTypeMigrator extends RecursiveAstVisitor with AstVisitingSuggestor, ActionV2Migrator, ActionV2NamedTypeMigrator { @override - shouldMigrate(node) { - node as NamedType; - return node.parent is FunctionDeclaration || - node.parent is FunctionTypeAlias || - node.parent is GenericFunctionType || - node.parent is MethodDeclaration; - } + 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 as NamedType; - return node.parent is ExtendsClause || - node.parent is ImplementsClause || - node.parent is OnClause || - node.parent is TypeParameter; - } + bool shouldMigrate(node) => + node.parent is ExtendsClause || + node.parent is ImplementsClause || + node.parent is OnClause || + node.parent is TypeParameter; } diff --git a/w_flux_codemod/test/action_v2_suggestor_test.dart b/w_flux_codemod/test/action_v2_suggestor_test.dart index 61a807a..6ea4cb5 100644 --- a/w_flux_codemod/test/action_v2_suggestor_test.dart +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -174,7 +174,7 @@ ${after} 'class C { var action; C() { action = ActionV2(); } }', ); testSuggestor( - 'skips dynamic Actions', + 'function initialization', suggestor, 'Action a; Action b = Action(); var c = Action();', 'ActionV2 a; ActionV2 b = ActionV2(); var c = ActionV2();', From b88468fdad34b991d48fe3b350371593b27a059a Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Tue, 19 Dec 2023 13:44:06 -0700 Subject: [PATCH 10/14] Add additional tests --- .../test/action_v2_suggestor_test.dart | 108 +++++++++++++++--- 1 file changed, 90 insertions(+), 18 deletions(-) diff --git a/w_flux_codemod/test/action_v2_suggestor_test.dart b/w_flux_codemod/test/action_v2_suggestor_test.dart index 6ea4cb5..7b20844 100644 --- a/w_flux_codemod/test/action_v2_suggestor_test.dart +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -109,12 +109,28 @@ ${after} group('FieldAndVariableMigrator', () { Suggestor suggestor() => ActionV2FieldAndVariableMigrator(); group('VariableDeclarationList', () { - testSuggestor( - 'with type, no intializer', - suggestor, - 'Action action;', - 'ActionV2 action;', - ); + group('with type, no intializer', () { + testSuggestor( + 'standard import', + suggestor, + 'Action action;', + 'ActionV2 action;', + ); + testSuggestor( + 'prefixed import', + suggestor, + 'w_flux.Action action;', + 'w_flux.ActionV2 action;', + importMode: WFluxImportMode.prefixed, + ); + testSuggestor( + 'ignore types not from w_flux', + suggestor, + 'Action action;', + 'Action action;', + importMode: WFluxImportMode.none, + ); + }); testSuggestor( 'with type and intializer', suggestor, @@ -184,12 +200,28 @@ ${after} group('ParameterMigrator', () { Suggestor suggestor() => ActionV2ParameterMigrator(); - testSuggestor( - 'SimpleFormalParameter.type (function)', - suggestor, - 'fn(Action action) {}', - 'fn(ActionV2 action) {}', - ); + group('SimpleFormalParameter.type (function)', () { + testSuggestor( + 'standard prefix', + suggestor, + 'fn(Action action) {}', + 'fn(ActionV2 action) {}', + ); + testSuggestor( + 'prefixed import', + suggestor, + 'fn(w_flux.Action action) {}', + 'fn(w_flux.ActionV2 action) {}', + importMode: WFluxImportMode.prefixed, + ); + testSuggestor( + 'ignore types not from w_flux', + suggestor, + 'fn(Action action) {}', + 'fn(Action action) {}', + importMode: WFluxImportMode.none, + ); + }); testSuggestor( 'SimpleFormalParameter.type (method)', suggestor, @@ -206,12 +238,29 @@ ${after} group('ReturnTypeMigrator', () { Suggestor suggestor() => ActionV2ReturnTypeMigrator(); - testSuggestor( - 'FunctionDeclaration.returnType', - suggestor, - 'Action fn() {}', - 'ActionV2 fn() {}', - ); + + group('FunctionDeclaration.returnType', () { + testSuggestor( + 'standard import', + suggestor, + 'Action fn() {}', + 'ActionV2 fn() {}', + ); + testSuggestor( + 'prefixed import', + suggestor, + 'w_flux.Action fn() {}', + 'w_flux.ActionV2 fn() {}', + importMode: WFluxImportMode.prefixed, + ); + testSuggestor( + 'ignore types not from w_flux', + suggestor, + 'Action fn() {}', + 'Action fn() {}', + importMode: WFluxImportMode.none, + ); + }); testSuggestor( 'FunctionTypeAlias.returnType', suggestor, @@ -252,6 +301,29 @@ ${after} group('TypeParameterMigrator', () { Suggestor suggestor() => ActionV2SuperTypeMigrator(); + + group('ExtendsClause.superclass', () { + testSuggestor( + 'standard import', + suggestor, + 'class C extends Action {}', + 'class C extends ActionV2 {}', + ); + testSuggestor( + 'prefixed import', + suggestor, + 'class C extends w_flux.Action {}', + 'class C extends w_flux.ActionV2 {}', + importMode: WFluxImportMode.prefixed, + ); + testSuggestor( + 'ignore types not from w_flux', + suggestor, + 'class C extends Action {}', + 'class C extends Action {}', + importMode: WFluxImportMode.none, + ); + }); testSuggestor( 'ExtendsClause.superclass', suggestor, From 916c05af06ed0e66acad692f60e3ec86bc9f6492 Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Tue, 19 Dec 2023 14:08:46 -0700 Subject: [PATCH 11/14] cleanup --- w_flux_codemod/README.md | 4 +- .../test/action_v2_suggestor_test.dart | 129 ++++++------------ 2 files changed, 44 insertions(+), 89 deletions(-) diff --git a/w_flux_codemod/README.md b/w_flux_codemod/README.md index c4644e9..ab994b6 100644 --- a/w_flux_codemod/README.md +++ b/w_flux_codemod/README.md @@ -51,8 +51,8 @@ To be able to support non-nullable payloads (in addition to nullable payloads), 3. Review the changes: - It's advisable to review the changes and ensure they are correct and meet your project's requirements. - - - Dart Analysis should be able to catch any errors that may occur with the codemod, and a passing CI should suffice for QA when making these updates. + - 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 diff --git a/w_flux_codemod/test/action_v2_suggestor_test.dart b/w_flux_codemod/test/action_v2_suggestor_test.dart index 7b20844..4b9f9ee 100644 --- a/w_flux_codemod/test/action_v2_suggestor_test.dart +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -66,12 +66,29 @@ ${after} group('ActionV2DispatchMigrator', () { Suggestor suggestor() => ActionV2DispatchMigrator(); - testSuggestor( - 'local invocation of variable', - suggestor, - 'void main() { var a = ActionV2(); a(); }', - 'void main() { var a = ActionV2(); a(null); }', - ); + // test each import type for the dispatch migrator + group('local invocation of variable', () { + testSuggestor( + 'with standard import', + suggestor, + 'void main() { var a = ActionV2(); a(); }', + 'void main() { var a = ActionV2(); a(null); }', + ); + testSuggestor( + 'with import prefix', + suggestor, + 'void main() { var a = w_flux.ActionV2(); a(); }', + 'void main() { var a = w_flux.ActionV2(); a(null); }', + importMode: WFluxImportMode.prefixed, + ); + testSuggestor( + '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, + ); + }); testSuggestor( 'local invocation of field', suggestor, @@ -84,31 +101,18 @@ ${after} 'class C { ActionV2 action; } void main() { C().action(); }', 'class C { ActionV2 action; } void main() { C().action(null); }', ); - testSuggestor( - 'with import prefix', - suggestor, - '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, - ); }); group('FieldAndVariableMigrator', () { Suggestor suggestor() => ActionV2FieldAndVariableMigrator(); group('VariableDeclarationList', () { + // test each import type on a named type migrator group('with type, no intializer', () { testSuggestor( 'standard import', @@ -200,28 +204,12 @@ ${after} group('ParameterMigrator', () { Suggestor suggestor() => ActionV2ParameterMigrator(); - group('SimpleFormalParameter.type (function)', () { - testSuggestor( - 'standard prefix', - suggestor, - 'fn(Action action) {}', - 'fn(ActionV2 action) {}', - ); - testSuggestor( - 'prefixed import', - suggestor, - 'fn(w_flux.Action action) {}', - 'fn(w_flux.ActionV2 action) {}', - importMode: WFluxImportMode.prefixed, - ); - testSuggestor( - 'ignore types not from w_flux', - suggestor, - 'fn(Action action) {}', - 'fn(Action action) {}', - importMode: WFluxImportMode.none, - ); - }); + testSuggestor( + 'SimpleFormalParameter.type (function)', + suggestor, + 'fn(Action action) {}', + 'fn(ActionV2 action) {}', + ); testSuggestor( 'SimpleFormalParameter.type (method)', suggestor, @@ -239,28 +227,12 @@ ${after} group('ReturnTypeMigrator', () { Suggestor suggestor() => ActionV2ReturnTypeMigrator(); - group('FunctionDeclaration.returnType', () { - testSuggestor( - 'standard import', - suggestor, - 'Action fn() {}', - 'ActionV2 fn() {}', - ); - testSuggestor( - 'prefixed import', - suggestor, - 'w_flux.Action fn() {}', - 'w_flux.ActionV2 fn() {}', - importMode: WFluxImportMode.prefixed, - ); - testSuggestor( - 'ignore types not from w_flux', - suggestor, - 'Action fn() {}', - 'Action fn() {}', - importMode: WFluxImportMode.none, - ); - }); + testSuggestor( + 'FunctionDeclaration.returnType', + suggestor, + 'Action fn() {}', + 'ActionV2 fn() {}', + ); testSuggestor( 'FunctionTypeAlias.returnType', suggestor, @@ -301,29 +273,12 @@ ${after} group('TypeParameterMigrator', () { Suggestor suggestor() => ActionV2SuperTypeMigrator(); - - group('ExtendsClause.superclass', () { - testSuggestor( - 'standard import', - suggestor, - 'class C extends Action {}', - 'class C extends ActionV2 {}', - ); - testSuggestor( - 'prefixed import', - suggestor, - 'class C extends w_flux.Action {}', - 'class C extends w_flux.ActionV2 {}', - importMode: WFluxImportMode.prefixed, - ); - testSuggestor( - 'ignore types not from w_flux', - suggestor, - 'class C extends Action {}', - 'class C extends Action {}', - importMode: WFluxImportMode.none, - ); - }); + testSuggestor( + 'standard import', + suggestor, + 'class C extends Action {}', + 'class C extends ActionV2 {}', + ); testSuggestor( 'ExtendsClause.superclass', suggestor, From 07e7654063f3cde2cf704dbd3255304ede662375 Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Thu, 21 Dec 2023 15:11:43 -0700 Subject: [PATCH 12/14] Edit README, add pub get check, and combinator suggestor --- w_flux_codemod/README.md | 10 +- w_flux_codemod/bin/action_v2_migrate.dart | 21 +- .../bin/action_v2_migrate_step_2.dart | 1 + .../lib/src/action_v2_suggestor.dart | 45 ++++- w_flux_codemod/lib/src/utils.dart | 121 ++++++++++++ w_flux_codemod/pubspec.yaml | 1 + .../test/action_v2_suggestor_test.dart | 181 ++++++++++++++++-- 7 files changed, 355 insertions(+), 25 deletions(-) create mode 100644 w_flux_codemod/lib/src/utils.dart diff --git a/w_flux_codemod/README.md b/w_flux_codemod/README.md index ab994b6..521a487 100644 --- a/w_flux_codemod/README.md +++ b/w_flux_codemod/README.md @@ -32,22 +32,24 @@ To be able to support non-nullable payloads (in addition to nullable payloads), 1. Ensure you have the codemod package installed. ```bash - dart pub global activate w_flux_codemod + 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 - dart pub global run w_flux_codemod:action_v2_migrate_step_2 + 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 + 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. diff --git a/w_flux_codemod/bin/action_v2_migrate.dart b/w_flux_codemod/bin/action_v2_migrate.dart index 6a7ab15..59cf4a7 100644 --- a/w_flux_codemod/bin/action_v2_migrate.dart +++ b/w_flux_codemod/bin/action_v2_migrate.dart @@ -1,17 +1,34 @@ 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'); + +Future pubGetForAllPackageRoots(Iterable 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 args) async { + final dartPaths = filePathsFromGlob(Glob('**.dart', recursive: true)); + + await pubGetForAllPackageRoots(dartPaths); exitCode = await runInteractiveCodemod( - filePathsFromGlob(Glob('**.dart', recursive: true)), + dartPaths, aggregate([ ActionV2ParameterMigrator(), ActionV2FieldAndVariableMigrator(), ActionV2ReturnTypeMigrator(), ActionV2SuperTypeMigrator(), + ActionV2ImportMigrator(), ]), args: args, ); @@ -19,7 +36,7 @@ void main(List args) async { return; } exitCode = await runInteractiveCodemod( - filePathsFromGlob(Glob('**.dart', recursive: true)), + dartPaths, 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 111a81e..6b37146 100644 --- a/w_flux_codemod/bin/action_v2_migrate_step_2.dart +++ b/w_flux_codemod/bin/action_v2_migrate_step_2.dart @@ -11,6 +11,7 @@ void main(List args) async { ActionV2FieldAndVariableMigrator(), ActionV2ReturnTypeMigrator(), ActionV2SuperTypeMigrator(), + ActionV2ImportMigrator(), ]), 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 017a2fd..fe49b34 100644 --- a/w_flux_codemod/lib/src/action_v2_suggestor.dart +++ b/w_flux_codemod/lib/src/action_v2_suggestor.dart @@ -23,8 +23,47 @@ mixin ActionV2NamedTypeMigrator on ActionV2Migrator { typeLibraryIdentifier.startsWith('package:w_flux/')) { yieldPatch('ActionV2', typeNameToken.offset, typeNameToken.end); } + return super.visitNamedType(node); } - 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); } } @@ -53,8 +92,10 @@ class ActionV2FieldAndVariableMigrator extends RecursiveAstVisitor node.parent is DeclaredVariablePattern || node.parent is FieldFormalParameter || node.parent is VariableDeclarationList || + node.parent is TypeArgumentList || node.thisOrAncestorOfType() != null || - node.thisOrAncestorOfType() != null; + node.thisOrAncestorOfType() != null || + node.typeArguments != null; } class ActionV2ParameterMigrator extends RecursiveAstVisitor diff --git a/w_flux_codemod/lib/src/utils.dart b/w_flux_codemod/lib/src/utils.dart new file mode 100644 index 0000000..7c25095 --- /dev/null +++ b/w_flux_codemod/lib/src/utils.dart @@ -0,0 +1,121 @@ +import 'dart:io'; + +import 'package:collection/collection.dart'; +import 'package:logging/logging.dart'; +import 'package:path/path.dart' as p; + +final _logger = Logger('orcm.pubspec'); + +bool _isPubGetNecessary(String packageRoot) { + final packageConfig = + File(p.join(packageRoot, '.dart_tool', 'package_config.json')); + final pubspec = File(p.join(packageRoot, 'pubspec.yaml')); + final pubspecLock = File(p.join(packageRoot, 'pubspec.lock')); + + if (!pubspec.existsSync()) { + throw ArgumentError('pubspec.yaml not found in directory: $packageRoot'); + } + + if (packageConfig.existsSync() && pubspecLock.existsSync()) { + return !pubspecLock.lastModifiedSync().isAfter(pubspec.lastModifiedSync()); + } + + return true; +} + +/// Runs `pub get` in [packageRoot] unless running `pub get` would have no effect. +Future runPubGetIfNeeded(String packageRoot) async { + if (_isPubGetNecessary(packageRoot)) { + await runPubGet(packageRoot); + } else { + _logger.info( + 'Skipping `dart pub get`, which has already been run, in `$packageRoot`'); + } +} + +/// Runs `dart pub get` in [workingDirectory], and throws if the command +/// completed with a non-zero exit code. +/// +/// For convenience, tries running with `dart pub get --offline` if `pub get` +/// fails, for a better experience when not authenticated to private pub servers. +Future runPubGet(String workingDirectory) async { + _logger.info('Running `dart pub get` in `$workingDirectory`...'); + + final process = await Process.start('dart', ['pub', 'get'], + workingDirectory: workingDirectory, + runInShell: true, + mode: ProcessStartMode.inheritStdio); + final exitCode = await process.exitCode; + + if (exitCode == 69) { + _logger.info( + 'Re-running `dart pub get` but with `--offline`, to hopefully fix the above error.'); + final process = await Process.start('dart', ['pub', 'get', '--offline'], + workingDirectory: workingDirectory, + runInShell: true, + mode: ProcessStartMode.inheritStdio); + final exitCode = await process.exitCode; + if (exitCode != 0) { + throw Exception('dart pub get failed with exit code: $exitCode'); + } + } else if (exitCode != 0) { + throw Exception('dart pub get failed with exit code: $exitCode'); + } +} + +/// Returns a path to the closest Dart package root (i.e., a directory with a +/// pubspec.yaml file) to [path], throwing if no package root can be found. +/// +/// If [path] is itself a package root, it will be returned. +/// +/// Example: +/// +/// ```dart +/// // All of these return a path to 'some_package' +/// findPackageRootFor('some_package/lib/src/file.dart'); +/// findPackageRootFor('some_package/lib/'); +/// findPackageRootFor('some_package'); +/// +/// // Returns a path to 'some_package/subpackages/some_nested_package' +/// findPackageRootFor('some_package/some_nested_package/lib/file.dart'); +/// ``` +String findPackageRootFor(String path) { + final packageRoot = [ + path, + ...ancestorsOfPath(path) + ].firstWhereOrNull((path) => File(p.join(path, 'pubspec.yaml')).existsSync()); + + if (packageRoot == null) { + throw Exception('Could not find package root for file `$path`'); + } + + return packageRoot; +} + +/// Returns canonicalized paths for all the the ancestor directories of [path], +/// starting with its parent and working upwards. +Iterable ancestorsOfPath(String path) sync* { + path = p.canonicalize(path); + + // p.dirname of the root directory is the root directory, so if they're the same, stop. + final parent = p.dirname(path); + if (p.equals(path, parent)) return; + + yield parent; + yield* ancestorsOfPath(parent); +} + +/// Returns whether [file] is within a top-level `build` directory of a package root. +bool isNotWithinTopLevelBuildOutputDir(File file) => + !isWithinTopLevelDir(file, 'build'); + +/// Returns whether [file] is within a top-level `tool` directory of a package root. +bool isNotWithinTopLevelToolDir(File file) => + !isWithinTopLevelDir(file, 'tool'); + +/// Returns whether [file] is within a top-level [topLevelDir] directory +/// (e.g., `bin`, `lib`, `web`) of a package root. +bool isWithinTopLevelDir(File file, String topLevelDir) => + ancestorsOfPath(file.path).any((ancestor) => + p.basename(ancestor) == topLevelDir && + File(p.join(p.dirname(ancestor), 'pubspec.yaml')).existsSync()); diff --git a/w_flux_codemod/pubspec.yaml b/w_flux_codemod/pubspec.yaml index e000804..017eb57 100644 --- a/w_flux_codemod/pubspec.yaml +++ b/w_flux_codemod/pubspec.yaml @@ -14,6 +14,7 @@ executables: dependencies: analyzer: ^5.13.0 codemod: ^1.2.0 + logging: ^1.0.1 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 4b9f9ee..a04a4a7 100644 --- a/w_flux_codemod/test/action_v2_suggestor_test.dart +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -13,7 +13,7 @@ dependencies: w_flux: ^3.0.0 '''; -String wFluxImport(WFluxImportMode mode) { +String wFluxInputImport(WFluxImportMode mode) { switch (mode) { case WFluxImportMode.none: return ''; @@ -21,6 +21,27 @@ String wFluxImport(WFluxImportMode mode) { return "import 'package:w_flux/w_flux.dart';"; case WFluxImportMode.prefixed: return "import 'package:w_flux/w_flux.dart' as w_flux;"; + case WFluxImportMode.shown: + return "import 'package:w_flux/w_flux.dart' show Action;"; + case WFluxImportMode.multipleShown: + return "import 'package:w_flux/w_flux.dart' show Action, FluxComponent;"; + case WFluxImportMode.hidden: + return "import 'package:w_flux/w_flux.dart' hide Action;"; + } +} + +String wFluxOutputImport(WFluxImportMode mode) { + switch (mode) { + case WFluxImportMode.none: + case WFluxImportMode.standard: + case WFluxImportMode.prefixed: + return wFluxInputImport(mode); + case WFluxImportMode.shown: + return "import 'package:w_flux/w_flux.dart' show ActionV2;"; + case WFluxImportMode.multipleShown: + return "import 'package:w_flux/w_flux.dart' show ActionV2, FluxComponent;"; + case WFluxImportMode.hidden: + return "import 'package:w_flux/w_flux.dart' hide ActionV2;"; } } @@ -28,6 +49,9 @@ enum WFluxImportMode { none, // don't import w_flux standard, // import w_flux prefixed, // import w_flux with a prefix + shown, // import but just show Action + multipleShown, + hidden, // hide from w_flux } void main() { @@ -39,21 +63,17 @@ void main() { }); @isTest - void testSuggestor( - String description, - Suggestor Function() suggestor, - String before, - String after, { - WFluxImportMode importMode = WFluxImportMode.standard, - bool shouldMigrateVariablesAndFields = false, - }) { + void testSuggestor(String description, Suggestor Function() suggestor, + String before, String after, + {WFluxImportMode importMode = WFluxImportMode.standard, + shouldImport = true}) { test(description, () async { final context = await pkg.addFile(''' -${wFluxImport(importMode)} +${shouldImport ? wFluxInputImport(importMode) : ''} ${before} '''); final expectedOutput = ''' -${wFluxImport(importMode)} +${shouldImport ? wFluxOutputImport(importMode) : ''} ${after} '''; expectSuggestorGeneratesPatches( @@ -64,25 +84,99 @@ ${after} }); } + group('ActionV2ImportMigrator', () { + Suggestor suggestor() => ActionV2ImportMigrator(); + + testSuggestor( + 'shown import', + suggestor, + '', + '', + importMode: WFluxImportMode.shown, + ); + + testSuggestor( + 'multiple shown import', + suggestor, + '', + '', + importMode: WFluxImportMode.multipleShown, + ); + + testSuggestor( + 'hidden import', + suggestor, + '', + '', + importMode: WFluxImportMode.hidden, + ); + }); + group('ActionV2DispatchMigrator', () { Suggestor suggestor() => ActionV2DispatchMigrator(); - // test each import type for the dispatch migrator - group('local invocation of variable', () { + + group( + 'test each import type for the dispatch migrator - local variable invocation', + () { testSuggestor( - 'with standard import', + 'standard import', suggestor, 'void main() { var a = ActionV2(); a(); }', 'void main() { var a = ActionV2(); a(null); }', ); testSuggestor( - 'with import prefix', + 'import prefix', suggestor, 'void main() { var a = w_flux.ActionV2(); a(); }', 'void main() { var a = w_flux.ActionV2(); a(null); }', importMode: WFluxImportMode.prefixed, ); + // the following 3 tests use the "output" import statement because those + // import statements should have been migrated by the other suggestors. testSuggestor( - 'types not from w_flux', + 'shown import', + suggestor, + ''' + ${wFluxOutputImport(WFluxImportMode.shown)} + void main() { var a = ActionV2(); a(); } + ''', + ''' + ${wFluxOutputImport(WFluxImportMode.shown)} + void main() { var a = ActionV2(); a(null); } + ''', + importMode: WFluxImportMode.shown, + shouldImport: false, + ); + testSuggestor( + 'multiple shown imports', + suggestor, + ''' + ${wFluxOutputImport(WFluxImportMode.multipleShown)} + void main() { var a = ActionV2(); a(); } + ''', + ''' + ${wFluxOutputImport(WFluxImportMode.multipleShown)} + void main() { var a = ActionV2(); a(null); } + ''', + importMode: WFluxImportMode.multipleShown, + shouldImport: false, + ); + testSuggestor( + 'ignores types when hidden from w_flux', + suggestor, + ''' + ${wFluxOutputImport(WFluxImportMode.hidden)} + void main() { var a = ActionV2(); a(); } + ''', + ''' + ${wFluxOutputImport(WFluxImportMode.hidden)} + void main() { var a = ActionV2(); a(); } + ''', + importMode: WFluxImportMode.hidden, + shouldImport: false, + ); + 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(); }', @@ -107,13 +201,31 @@ ${after} 'void main() { var a = Action(); a(); }', 'void main() { var a = Action(); a(); }', ); + testSuggestor( + 'nested dispatch', + suggestor, + ''' + class A { final ActionV2 action = ActionV2(); } + class B { final actions = A(); } + void main() { + B().actions.action(); + } + ''', + ''' + class A { final ActionV2 action = ActionV2(); } + class B { final actions = A(); } + void main() { + B().actions.action(null); + } + ''', + ); }); group('FieldAndVariableMigrator', () { Suggestor suggestor() => ActionV2FieldAndVariableMigrator(); group('VariableDeclarationList', () { // test each import type on a named type migrator - group('with type, no intializer', () { + group('each import variation for type, with no intializer', () { testSuggestor( 'standard import', suggestor, @@ -127,6 +239,13 @@ ${after} 'w_flux.ActionV2 action;', importMode: WFluxImportMode.prefixed, ); + testSuggestor( + 'ignore Action when hidden from w_flux', + suggestor, + 'Action action;', + 'Action action;', + importMode: WFluxImportMode.hidden, + ); testSuggestor( 'ignore types not from w_flux', suggestor, @@ -153,6 +272,12 @@ ${after} 'Action a; Action b = Action(); var c = Action();', 'ActionV2 a; ActionV2 b = ActionV2(); var c = ActionV2();', ); + testSuggestor( + 'nested type', + suggestor, + 'List> actions = [Action(), Action()];', + 'List> actions = [ActionV2(), ActionV2()];', + ); }); group('FieldDeclaration', () { testSuggestor( @@ -179,6 +304,28 @@ ${after} 'class C { Action a; Action b = Action(); var c = Action(); }', 'class C { ActionV2 a; ActionV2 b = ActionV2(); var c = ActionV2(); }', ); + testSuggestor( + 'nested Action type', + suggestor, + '''abstract class Actions { + final Action openAction = Action(); + final Action closeAction = Action(); + + List get actions => [ + openAction, + closeAction, + ]; + }''', + '''abstract class Actions { + final ActionV2 openAction = ActionV2(); + final ActionV2 closeAction = ActionV2(); + + List get actions => [ + openAction, + closeAction, + ]; + }''', + ); }); group('InstanceCreationExpression', () { testSuggestor( From 9865c082a612874cc59ea4291dbc60f6384e91ea Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Wed, 10 Jan 2024 11:47:24 -0700 Subject: [PATCH 13/14] combine suggestors into one interactive codemod and solve overlap issues --- w_flux_codemod/bin/action_v2_migrate.dart | 21 +--- .../bin/action_v2_migrate_step_1.dart | 6 +- .../bin/action_v2_migrate_step_2.dart | 15 ++- .../lib/src/action_v2_suggestor.dart | 22 ++-- w_flux_codemod/lib/src/utils.dart | 11 +- .../test/action_v2_suggestor_test.dart | 102 ++++++++++-------- 6 files changed, 93 insertions(+), 84 deletions(-) diff --git a/w_flux_codemod/bin/action_v2_migrate.dart b/w_flux_codemod/bin/action_v2_migrate.dart index 59cf4a7..2dd7f2e 100644 --- a/w_flux_codemod/bin/action_v2_migrate.dart +++ b/w_flux_codemod/bin/action_v2_migrate.dart @@ -1,22 +1,10 @@ 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'); - -Future pubGetForAllPackageRoots(Iterable 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 args) async { final dartPaths = filePathsFromGlob(Glob('**.dart', recursive: true)); @@ -28,16 +16,9 @@ void main(List args) async { ActionV2FieldAndVariableMigrator(), ActionV2ReturnTypeMigrator(), ActionV2SuperTypeMigrator(), + ActionV2DispatchMigrator(), ActionV2ImportMigrator(), ]), args: args, ); - if (exitCode != 0) { - return; - } - exitCode = await runInteractiveCodemod( - dartPaths, - ActionV2DispatchMigrator(), - args: args, - ); } diff --git a/w_flux_codemod/bin/action_v2_migrate_step_1.dart b/w_flux_codemod/bin/action_v2_migrate_step_1.dart index cfe9c41..90605d1 100644 --- a/w_flux_codemod/bin/action_v2_migrate_step_1.dart +++ b/w_flux_codemod/bin/action_v2_migrate_step_1.dart @@ -3,10 +3,14 @@ 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 args) async { + final dartPaths = filePathsFromGlob(Glob('**.dart', recursive: true)); + + await pubGetForAllPackageRoots(dartPaths); exitCode = await runInteractiveCodemod( - filePathsFromGlob(Glob('**.dart', recursive: true)), + dartPaths, ActionV2ParameterMigrator(), 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 6b37146..0739707 100644 --- a/w_flux_codemod/bin/action_v2_migrate_step_2.dart +++ b/w_flux_codemod/bin/action_v2_migrate_step_2.dart @@ -3,24 +3,21 @@ 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 args) async { + final dartPaths = filePathsFromGlob(Glob('**.dart', recursive: true)); + + await pubGetForAllPackageRoots(dartPaths); exitCode = await runInteractiveCodemod( - filePathsFromGlob(Glob('**.dart', recursive: true)), + dartPaths, aggregate([ ActionV2FieldAndVariableMigrator(), ActionV2ReturnTypeMigrator(), ActionV2SuperTypeMigrator(), + ActionV2DispatchMigrator(), ActionV2ImportMigrator(), ]), 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 fe49b34..874cb0c 100644 --- a/w_flux_codemod/lib/src/action_v2_suggestor.dart +++ b/w_flux_codemod/lib/src/action_v2_suggestor.dart @@ -75,8 +75,7 @@ class ActionV2DispatchMigrator extends RecursiveAstVisitor 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' && + staticTypeName == 'Action' && node.argumentList.arguments.isEmpty) { yieldPatch('(null)', node.end - 2, node.end); } @@ -88,14 +87,14 @@ 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() != null || - node.thisOrAncestorOfType() != null || - node.typeArguments != null; + node.thisOrAncestorOfType() == null && + (node.parent is DeclaredIdentifier || + node.parent is DeclaredVariablePattern || + node.parent is FieldFormalParameter || + node.parent is VariableDeclarationList || + node.parent is TypeArgumentList || + node.thisOrAncestorOfType() != null || + node.thisOrAncestorOfType() != null); } class ActionV2ParameterMigrator extends RecursiveAstVisitor @@ -103,6 +102,7 @@ class ActionV2ParameterMigrator extends RecursiveAstVisitor @override bool shouldMigrate(node) => node.thisOrAncestorOfType() != null && + node.thisOrAncestorOfType() == null && node.thisOrAncestorOfType() == null; } @@ -113,7 +113,7 @@ class ActionV2ReturnTypeMigrator extends RecursiveAstVisitor node.parent is FunctionDeclaration || node.parent is FunctionTypeAlias || node.parent is GenericFunctionType || - node.parent is MethodDeclaration; + node.thisOrAncestorOfType() != null; } class ActionV2SuperTypeMigrator extends RecursiveAstVisitor diff --git a/w_flux_codemod/lib/src/utils.dart b/w_flux_codemod/lib/src/utils.dart index 7c25095..b5e60bd 100644 --- a/w_flux_codemod/lib/src/utils.dart +++ b/w_flux_codemod/lib/src/utils.dart @@ -4,7 +4,16 @@ import 'package:collection/collection.dart'; import 'package:logging/logging.dart'; import 'package:path/path.dart' as p; -final _logger = Logger('orcm.pubspec'); +final _logger = Logger('w_flux_codemod.pubspec'); + +Future pubGetForAllPackageRoots(Iterable files) async { + _logger.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); + } +} bool _isPubGetNecessary(String packageRoot) { final packageConfig = diff --git a/w_flux_codemod/test/action_v2_suggestor_test.dart b/w_flux_codemod/test/action_v2_suggestor_test.dart index a04a4a7..f828b9f 100644 --- a/w_flux_codemod/test/action_v2_suggestor_test.dart +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -121,14 +121,14 @@ ${after} testSuggestor( 'standard import', suggestor, - 'void main() { var a = ActionV2(); a(); }', - 'void main() { var a = ActionV2(); a(null); }', + 'void main() { var a = Action(); a(); }', + 'void main() { var a = Action(); a(null); }', ); testSuggestor( 'import prefix', suggestor, - 'void main() { var a = w_flux.ActionV2(); a(); }', - 'void main() { var a = w_flux.ActionV2(); a(null); }', + 'void main() { var a = w_flux.Action(); a(); }', + 'void main() { var a = w_flux.Action(); a(null); }', importMode: WFluxImportMode.prefixed, ); // the following 3 tests use the "output" import statement because those @@ -136,50 +136,29 @@ ${after} testSuggestor( 'shown import', suggestor, - ''' - ${wFluxOutputImport(WFluxImportMode.shown)} - void main() { var a = ActionV2(); a(); } - ''', - ''' - ${wFluxOutputImport(WFluxImportMode.shown)} - void main() { var a = ActionV2(); a(null); } - ''', + 'void main() { var a = Action(); a(); }', + 'void main() { var a = Action(); a(null); }', importMode: WFluxImportMode.shown, - shouldImport: false, ); testSuggestor( 'multiple shown imports', suggestor, - ''' - ${wFluxOutputImport(WFluxImportMode.multipleShown)} - void main() { var a = ActionV2(); a(); } - ''', - ''' - ${wFluxOutputImport(WFluxImportMode.multipleShown)} - void main() { var a = ActionV2(); a(null); } - ''', + 'void main() { var a = Action(); a(); }', + 'void main() { var a = Action(); a(null); }', importMode: WFluxImportMode.multipleShown, - shouldImport: false, ); testSuggestor( 'ignores types when hidden from w_flux', suggestor, - ''' - ${wFluxOutputImport(WFluxImportMode.hidden)} - void main() { var a = ActionV2(); a(); } - ''', - ''' - ${wFluxOutputImport(WFluxImportMode.hidden)} - void main() { var a = ActionV2(); a(); } - ''', + 'void main() { var a = Action(); a(); }', + 'void main() { var a = Action(); a(); }', importMode: WFluxImportMode.hidden, - shouldImport: false, ); 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(); }', + 'class Action { call(); } void main() { var a = Action(); a(); }', + 'class Action { call(); } void main() { var a = Action(); a(); }', importMode: WFluxImportMode.none, ); }); @@ -212,7 +191,7 @@ ${after} } ''', ''' - class A { final ActionV2 action = ActionV2(); } + class A { final Action action = Action(); } class B { final actions = A(); } void main() { B().actions.action(null); @@ -242,9 +221,16 @@ ${after} testSuggestor( 'ignore Action when hidden from w_flux', suggestor, - 'Action action;', - 'Action action;', + ''' + ${wFluxOutputImport(WFluxImportMode.hidden)} + 'Action action;' + ''', + ''' + ${wFluxOutputImport(WFluxImportMode.hidden)} + 'Action action;' + ''', importMode: WFluxImportMode.hidden, + shouldImport: false, ); testSuggestor( 'ignore types not from w_flux', @@ -304,27 +290,32 @@ ${after} 'class C { Action a; Action b = Action(); var c = Action(); }', 'class C { ActionV2 a; ActionV2 b = ActionV2(); var c = ActionV2(); }', ); + // List is a return type and will not be modified in this suggestor testSuggestor( 'nested Action type', suggestor, - '''abstract class Actions { + ''' + abstract class Actions { final Action openAction = Action(); final Action closeAction = Action(); - List get actions => [ + List get actions => [ openAction, closeAction, ]; - }''', - '''abstract class Actions { + } + ''', + ''' + abstract class Actions { final ActionV2 openAction = ActionV2(); final ActionV2 closeAction = ActionV2(); - List get actions => [ + List get actions => [ openAction, closeAction, ]; - }''', + } + ''', ); }); group('InstanceCreationExpression', () { @@ -416,6 +407,33 @@ ${after} 'Action fn() {}', 'ActionV2 fn() {}', ); + // field declarations (final Action) should not be migrated in this codemod + testSuggestor( + 'nested return type', + suggestor, + ''' + abstract class Actions { + final Action openAction = Action(); + final Action closeAction = Action(); + + List get actions => [ + openAction, + closeAction, + ]; + } + ''', + ''' + abstract class Actions { + final Action openAction = Action(); + final Action closeAction = Action(); + + List get actions => [ + openAction, + closeAction, + ]; + } + ''', + ); }); group('TypeParameterMigrator', () { From b64a2711e922ec79dd1aaa4246e71d4db9da6c0d Mon Sep 17 00:00:00 2001 From: sorenthompson-wk Date: Wed, 10 Jan 2024 12:06:43 -0700 Subject: [PATCH 14/14] Fix tests --- .../test/action_v2_suggestor_test.dart | 63 ++++++++----------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/w_flux_codemod/test/action_v2_suggestor_test.dart b/w_flux_codemod/test/action_v2_suggestor_test.dart index f828b9f..28f4f51 100644 --- a/w_flux_codemod/test/action_v2_suggestor_test.dart +++ b/w_flux_codemod/test/action_v2_suggestor_test.dart @@ -13,7 +13,7 @@ dependencies: w_flux: ^3.0.0 '''; -String wFluxInputImport(WFluxImportMode mode) { +String wFluxImport(WFluxImportMode mode) { switch (mode) { case WFluxImportMode.none: return ''; @@ -30,12 +30,12 @@ String wFluxInputImport(WFluxImportMode mode) { } } -String wFluxOutputImport(WFluxImportMode mode) { +String wFluxMigratedImport(WFluxImportMode mode) { switch (mode) { case WFluxImportMode.none: case WFluxImportMode.standard: case WFluxImportMode.prefixed: - return wFluxInputImport(mode); + return wFluxImport(mode); case WFluxImportMode.shown: return "import 'package:w_flux/w_flux.dart' show ActionV2;"; case WFluxImportMode.multipleShown: @@ -66,16 +66,16 @@ void main() { void testSuggestor(String description, Suggestor Function() suggestor, String before, String after, {WFluxImportMode importMode = WFluxImportMode.standard, - shouldImport = true}) { + migrateImport = false}) { test(description, () async { final context = await pkg.addFile(''' -${shouldImport ? wFluxInputImport(importMode) : ''} -${before} -'''); + ${wFluxImport(importMode)} + ${before} + '''); final expectedOutput = ''' -${shouldImport ? wFluxOutputImport(importMode) : ''} -${after} -'''; + ${migrateImport ? wFluxMigratedImport(importMode) : wFluxImport(importMode)} + ${after} + '''; expectSuggestorGeneratesPatches( suggestor(), context, @@ -93,6 +93,7 @@ ${after} '', '', importMode: WFluxImportMode.shown, + migrateImport: true, ); testSuggestor( @@ -101,6 +102,7 @@ ${after} '', '', importMode: WFluxImportMode.multipleShown, + migrateImport: true, ); testSuggestor( @@ -109,6 +111,7 @@ ${after} '', '', importMode: WFluxImportMode.hidden, + migrateImport: true, ); }); @@ -165,26 +168,20 @@ ${after} testSuggestor( 'local invocation of field', suggestor, - 'class C { ActionV2 action; dispatch() => action(); }', - 'class C { ActionV2 action; dispatch() => action(null); }', + 'class C { Action action; dispatch() => action(); }', + 'class C { Action 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( - 'ignores Action type', - suggestor, - 'void main() { var a = Action(); a(); }', - 'void main() { var a = Action(); a(); }', + 'class C { Action action; } void main() { C().action(); }', + 'class C { Action action; } void main() { C().action(null); }', ); testSuggestor( 'nested dispatch', suggestor, ''' - class A { final ActionV2 action = ActionV2(); } + class A { final Action action = Action(); } class B { final actions = A(); } void main() { B().actions.action(); @@ -221,16 +218,9 @@ ${after} testSuggestor( 'ignore Action when hidden from w_flux', suggestor, - ''' - ${wFluxOutputImport(WFluxImportMode.hidden)} - 'Action action;' - ''', - ''' - ${wFluxOutputImport(WFluxImportMode.hidden)} - 'Action action;' - ''', + 'Action action;', + 'Action action;', importMode: WFluxImportMode.hidden, - shouldImport: false, ); testSuggestor( 'ignore types not from w_flux', @@ -348,12 +338,6 @@ ${after} 'fn(Action action) {}', 'fn(ActionV2 action) {}', ); - testSuggestor( - 'SimpleFormalParameter.type (method)', - suggestor, - 'class C { m(Action action) {} }', - 'class C { m(ActionV2 action) {} }', - ); testSuggestor( 'Parameter type with a generic', suggestor, @@ -401,6 +385,13 @@ ${after} 'class C { Action m() {} }', 'class C { ActionV2 m() {} }', ); + + testSuggestor( + 'MethodDeclaration SimpleFormalParameter.type', + suggestor, + 'class C { m(Action action) {} }', + 'class C { m(ActionV2 action) {} }', + ); testSuggestor( 'Return type with a generic', suggestor,