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

INTL-1090 Migrate calls to addContextMenuItem #209

Merged
merged 4 commits into from
Mar 11, 2023
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Clean up, add tests
alanknight-wk committed Mar 9, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 1dea6921fd668a540777cd86c5b2c913ff5a6464
21 changes: 14 additions & 7 deletions lib/src/executables/intl_message_migration.dart
Original file line number Diff line number Diff line change
@@ -41,6 +41,7 @@ const _failOnChangesFlag = 'fail-on-changes';
const _stderrAssumeTtyFlag = 'stderr-assume-tty';
const _migrateConstants = 'migrate-constants';
const _migrateComponents = 'migrate-components';
const _migrateContextMenus = 'migrate-context-menus';
const _pruneUnused = 'prune-unused';
const _allCodemodFlags = {
_verboseFlag,
@@ -95,6 +96,12 @@ final parser = ArgParser()
help:
'Should the codemod try to migrate constant Strings that look user-visible',
)
..addFlag(
_migrateContextMenus,
negatable: true,
defaultsTo: true,
help: 'Should the codemod try to migrate calls to addContextMenuItem',
)
..addFlag(
_migrateComponents,
negatable: true,
@@ -261,17 +268,17 @@ Future<void> migratePackage(
final importMigrator = (FileContext context) =>
intlImporter(context, packageName, messages.className);
final usedMethodsChecker = UsedMethodsChecker(messages.className, messages);
final otherThingy = ContextMenuMigrator(messages.className, messages);
final contextMenuMigrator = ContextMenuMigrator(messages.className, messages);

exitCode = await runCodemodSequences(
packageDartPaths,
[
// if (parsedArgs[_migrateComponents]) [intlPropMigrator],
// if (parsedArgs[_migrateConstants]) [constantStringMigrator],
// [displayNameMigrator],
// [importMigrator],
// if (parsedArgs[_pruneUnused]) [usedMethodsChecker],
[otherThingy],
if (parsedArgs[_migrateComponents]) [intlPropMigrator],
if (parsedArgs[_migrateConstants]) [constantStringMigrator],
[displayNameMigrator],
[importMigrator],
if (parsedArgs[_pruneUnused]) [usedMethodsChecker],
if (parsedArgs[_migrateContextMenus]) [contextMenuMigrator],
],
codemodArgs);

25 changes: 15 additions & 10 deletions lib/src/intl_suggestors/intl_migrator.dart
Original file line number Diff line number Diff line change
@@ -6,38 +6,43 @@ import 'package:over_react_codemod/src/intl_suggestors/utils.dart';
import 'package:over_react_codemod/src/util/component_usage.dart';
import 'package:over_react_codemod/src/util/component_usage_migrator.dart';

/// Migrate calls to addContextMenuItem, with either literals or interpolations.
class ContextMenuMigrator extends GeneralizingAstVisitor
with AstVisitingSuggestor {
final IntlMessages _messages;
final String _className;

ContextMenuMigrator(this._className, this._messages);

@override
visitMethodInvocation(MethodInvocation node) {
migrateMenu(node);

return super.visitMethodInvocation(node);
}

void migrateMenu(MethodInvocation node) {
// We only care about calls to addContextMenuItem, with at least one
// argument, we will migrate the first.
var args = node.argumentList.arguments;
if (args.isEmpty) return;
if (node.methodName.name != 'addContextMenuItem') return;
var firstArg = args.first;
if (isValidStringLiteralNode(firstArg)) {
var literal = firstArg as StringLiteral;

if (isValidStringLiteralNode(args.first)) {
var literal = args.first as StringLiteral;
final functionCall = _messages.syntax.getterCall(literal, _className);
final functionDef =
_messages.syntax.getterDefinition(literal, _className);
yieldPatch(functionCall, literal.offset, literal.end);
addMethodToClass(_messages, functionDef);
}
if (isValidStringInterpolationNode(firstArg)) {
var interpolation = firstArg as StringInterpolation;
final functionCall = _messages.syntax
.functionCall(interpolation, _className, 'no longer used');
final functionDef = _messages.syntax
.functionDefinition(interpolation, _className, 'no longer used');
if (isValidStringInterpolationNode(args.first)) {
var interpolation = args.first as StringInterpolation;
final functionCall =
_messages.syntax.functionCall(interpolation, _className, '');
final functionDef =
_messages.syntax.functionDefinition(interpolation, _className, '');
// A lot of context menu calls are of the form 'Delete $type', which would be better done
// as a fixed number of 'Delete Audit', 'Delete Form', etc. Add a comment to point that out.
final callWithFixMe = '''
// FIXME - INTL Untranslated interpolated value. Is this one of a known set of possibilities?
$functionCall
96 changes: 96 additions & 0 deletions test/intl_suggestors/context_menu_migrator_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import 'package:file/file.dart';
import 'package:file/memory.dart';
import 'package:over_react_codemod/src/intl_suggestors/intl_messages.dart';
import 'package:over_react_codemod/src/intl_suggestors/intl_migrator.dart';
import 'package:test/test.dart';

import '../resolved_file_context.dart';
import '../util.dart';

void main() {
final resolvedContext = SharedAnalysisContext.overReact;

// Warm up analysis in a setUpAll so that if getting the resolved AST times out
// (which is more common for the WSD context), it fails here instead of failing the first test.
setUpAll(resolvedContext.warmUpAnalysis);

group('ContextMenuMigrator', () {
final FileSystem fs = MemoryFileSystem();
late IntlMessages messages;
late SuggestorTester basicSuggestor;

// Idempotency isn't a worry for this suggestor, and testing it throws off
// using a global counter for numbering messages whose name isn't otherwise
// unique, so skip that check.
Future<void> testSuggestor(
{required String input, required String expectedOutput}) =>
basicSuggestor(
input: input,
expectedOutput: expectedOutput,
testIdempotency: false);

setUp(() async {
final Directory tmp = await fs.systemTempDirectory.createTemp();
messages = IntlMessages('TestClass', directory: tmp);
// TODO: It's awkward that this test assumes the file exists, but that it doesn't have the prologue written.
messages.outputFile.createSync(recursive: true);
basicSuggestor = getSuggestorTester(
ContextMenuMigrator('TestClassIntl', messages),
resolvedContext: resolvedContext,
);
});

tearDown(() {
messages.delete();
});

group('Context menu', () {
test('Simple literal', () async {
await testSuggestor(
// To avoid defining a method by that name, use a dynamic variable so
// the analysis will pass.
input: '''
dynamic placeholder;
someFunction() {
placeholder.addContextMenuItem('abc', () => 'hello', iconGlyph: null);
}
''',
expectedOutput: '''
dynamic placeholder;
someFunction() {
placeholder.addContextMenuItem(TestClassIntl.abc, () => 'hello', iconGlyph: null);
}
''',
);
final expectedFileContent =
'\n static String get abc => Intl.message(\'abc\', name: \'TestClassIntl_abc\');';
expect(messages.messageContents(), expectedFileContent);
});

test('Interpolation', () async {
await testSuggestor(
input: '''
dynamic placeholder;
someFunction(String thing) {
placeholder.addContextMenuItem('abc \$thing', () => 'hello', iconGlyph: null);
}
''',
expectedOutput: '''
dynamic placeholder;
someFunction(String thing) {
placeholder.addContextMenuItem(
// FIXME - INTL Untranslated interpolated value. Is this one of a known set of possibilities?
TestClassIntl.abc(thing),
() => 'hello',
iconGlyph: null);
}
''',
);

final expected =
"\n static String abc(String thing) => Intl.message('abc \${thing}', args: [thing], name: 'TestClassIntl_abc');";
expect(messages.messageContents(), expected);
});
});
});
}