Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Address review comments #196

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion w_flux_codemod/bin/action_v2_migrate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,15 @@ void main(List<String> args) async {
ActionV2ParameterMigrator(),
ActionV2ReturnTypeMigrator(),
ActionV2SuperTypeMigrator(),
ActionV2DispatchMigrator(),
]),
args: args,
);
if (exitCode != 0) {
return;
}
exitCode = await runInteractiveCodemod(
filePathsFromGlob(Glob('**.dart', recursive: true)),
ActionV2DispatchMigrator(),
args: args,
);
}
9 changes: 8 additions & 1 deletion w_flux_codemod/bin/action_v2_migrate_step_2.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,15 @@ void main(List<String> args) async {
ActionV2FieldAndVariableMigrator(),
ActionV2ReturnTypeMigrator(),
ActionV2SuperTypeMigrator(),
ActionV2DispatchMigrator(),
]),
args: args,
);
if (exitCode != 0) {
return;
}
exitCode = await runInteractiveCodemod(
filePathsFromGlob(Glob('**.dart', recursive: true)),
ActionV2DispatchMigrator(),
args: args,
);
}
46 changes: 18 additions & 28 deletions w_flux_codemod/lib/src/action_v2_suggestor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion w_flux_codemod/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
57 changes: 49 additions & 8 deletions w_flux_codemod/test/action_v2_suggestor_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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', () {
Expand All @@ -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(
Expand All @@ -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,
);
});

Expand Down
Loading