Change in dart/sdk[master]: analyzer_plugin: Remove dynamic calls
2 views
Skip to first unread message
Samuel Rawlins (Gerrit)
Mar 22, 2021, 1:18:59 AM3/22/21
to Konstantin Shcheglov, Brian Wilkerson, rev...@dartlang.org
Attention is currently required from: Konstantin Shcheglov, Samuel Rawlins, Brian Wilkerson.
Samuel Rawlins would like Konstantin Shcheglov and Brian Wilkerson to review this change.
analyzer_plugin: Remove dynamic calls
* Apply new formatting from dartfmt 2.0.0
* Take advantage of type promotion in `is` checks.
* Enforce signature of onError callback.
Change-Id: I75ee4050808bb03a2156926fcf534c093de826ec
---
M pkg/analyzer_plugin/lib/utilities/assist/assist_contributor_mixin.dart
M pkg/analyzer_plugin/lib/utilities/fixes/fix_contributor_mixin.dart
M pkg/analyzer_plugin/lib/utilities/pair.dart
M pkg/analyzer_plugin/test/integration/support/integration_tests.dart
M pkg/analyzer_plugin/test/plugin/assist_mixin_test.dart
M pkg/analyzer_plugin/test/plugin/fix_mixin_test.dart
M pkg/analyzer_plugin/test/plugin/mocks.dart
7 files changed, 101 insertions(+), 89 deletions(-)
diff --git a/pkg/analyzer_plugin/lib/utilities/assist/assist_contributor_mixin.dart b/pkg/analyzer_plugin/lib/utilities/assist/assist_contributor_mixin.dart
index 1546f14..0d15366 100644
--- a/pkg/analyzer_plugin/lib/utilities/assist/assist_contributor_mixin.dart
+++ b/pkg/analyzer_plugin/lib/utilities/assist/assist_contributor_mixin.dart
@@ -27,7 +27,6 @@
}
change.id = kind.id;
change.message = formatList(kind.message, args);
- collector.addAssist(
- PrioritizedSourceChange(kind.priority, change));
+ collector.addAssist(PrioritizedSourceChange(kind.priority, change));
}
}
diff --git a/pkg/analyzer_plugin/lib/utilities/fixes/fix_contributor_mixin.dart b/pkg/analyzer_plugin/lib/utilities/fixes/fix_contributor_mixin.dart
index b4be5b0..29139dc 100644
--- a/pkg/analyzer_plugin/lib/utilities/fixes/fix_contributor_mixin.dart
+++ b/pkg/analyzer_plugin/lib/utilities/fixes/fix_contributor_mixin.dart
@@ -32,12 +32,12 @@
}
change.id = kind.id;
change.message = formatList(kind.message, args);
- collector.addFix(
- error, PrioritizedSourceChange(kind.priority, change));
+ collector.addFix(error, PrioritizedSourceChange(kind.priority, change));
}
@override
- Future<void> computeFixes(DartFixesRequest request, FixCollector collector) async {
+ Future<void> computeFixes(
+ DartFixesRequest request, FixCollector collector) async {
this.request = request;
this.collector = collector;
try {
diff --git a/pkg/analyzer_plugin/lib/utilities/pair.dart b/pkg/analyzer_plugin/lib/utilities/pair.dart
index 0108164..e578520 100644
--- a/pkg/analyzer_plugin/lib/utilities/pair.dart
+++ b/pkg/analyzer_plugin/lib/utilities/pair.dart
@@ -14,10 +14,10 @@
@override
bool operator ==(other) {
- if (other is! Pair) {
- return false;
+ if (other is Pair) {
+ return other.first == first && other.last == last;
}
- return other.first == first && other.last == last;
+ return false;
}
@override
diff --git a/pkg/analyzer_plugin/test/integration/support/integration_tests.dart b/pkg/analyzer_plugin/test/integration/support/integration_tests.dart
index 3d3391c..058776e 100644
--- a/pkg/analyzer_plugin/test/integration/support/integration_tests.dart
+++ b/pkg/analyzer_plugin/test/integration/support/integration_tests.dart
@@ -306,37 +306,39 @@
description.add(this.description);
@override
- void populateMismatches(item, List<MismatchDescriber> mismatches) {
- if (item is! Map) {
+ void populateMismatches(Object item, List<MismatchDescriber> mismatches) {
+ if (item is Map) {
+ if (requiredFields != null) {
+ requiredFields.forEach((String key, Matcher valueMatcher) {
+ if (!item.containsKey(key)) {
+ mismatches.add((Description mismatchDescription) =>
+ mismatchDescription
+ .add('is missing field ')
+ .addDescriptionOf(key)
+ .add(' (')
+ .addDescriptionOf(valueMatcher)
+ .add(')'));
+ } else {
+ _checkField(key, item[key], valueMatcher, mismatches);
+ }
+ });
+ }
+ item.forEach((key, value) {
+ if (requiredFields != null && requiredFields.containsKey(key)) {
+ // Already checked this field
+ } else if (optionalFields != null && optionalFields.containsKey(key)) {
+ _checkField(key as String, value, optionalFields[key], mismatches);
+ } else {
+ mismatches.add((Description mismatchDescription) =>
+ mismatchDescription
+ .add('has unexpected field ')
+ .addDescriptionOf(key));
+ }
+ });
+ } else {
mismatches.add(simpleDescription('is not a map'));
return;
}
- if (requiredFields != null) {
- requiredFields.forEach((String key, Matcher valueMatcher) {
- if (!(item as Map).containsKey(key)) {
- mismatches.add((Description mismatchDescription) =>
- mismatchDescription
- .add('is missing field ')
- .addDescriptionOf(key)
- .add(' (')
- .addDescriptionOf(valueMatcher)
- .add(')'));
- } else {
- _checkField(key, item[key], valueMatcher, mismatches);
- }
- });
- }
- item.forEach((key, value) {
- if (requiredFields != null && requiredFields.containsKey(key)) {
- // Already checked this field
- } else if (optionalFields != null && optionalFields.containsKey(key)) {
- _checkField(key as String, value, optionalFields[key], mismatches);
- } else {
- mismatches.add((Description mismatchDescription) => mismatchDescription
- .add('has unexpected field ')
- .addDescriptionOf(key));
- }
- });
}
/// Check the type of a field called [key], having value [value], using
@@ -445,45 +447,19 @@
return;
}
_recordStdio('RECV: $trimmedLine');
- var message;
+ Object message;
try {
message = json.decoder.convert(trimmedLine);
+ if (message is Map) {
+ _processMessage(message, notificationProcessor);
+ } else {
+ throw FormatException('Expected line to be JSON Map, but was '
+ "${message.runtimeType}: '$message'");
+ }
} catch (exception) {
_badDataFromServer('JSON decode failure: $exception');
return;
}
- outOfTestExpect(message, isMap);
- var messageAsMap = message as Map;
- if (messageAsMap.containsKey('id')) {
- outOfTestExpect(messageAsMap['id'], isString);
- var id = message['id'] as String;
- var completer = _pendingCommands[id];
- if (completer == null) {
- fail('Unexpected response from server: id=$id');
- } else {
- _pendingCommands.remove(id);
- }
- if (messageAsMap.containsKey('error')) {
- completer.completeError(ServerErrorMessage(messageAsMap));
- } else {
- completer.complete(messageAsMap['result']);
- }
- // Check that the message is well-formed. We do this after calling
- // completer.complete() or completer.completeError() so that we don't
- // stall the test in the event of an error.
- outOfTestExpect(message, isResponse);
- } else {
- // Message is a notification. It should have an event and possibly
- // params.
- outOfTestExpect(messageAsMap, contains('event'));
- outOfTestExpect(messageAsMap['event'], isString);
- notificationProcessor(
- messageAsMap['event'] as String, messageAsMap['params'] as Map);
- // Check that the message is well-formed. We do this after calling
- // notificationController.add() so that we don't stall the test in the
- // event of an error.
- outOfTestExpect(message, isNotification);
- }
});
_process.stderr
.transform((Utf8Codec()).decoder)
@@ -608,6 +584,40 @@
}));
}
+ void _processMessage(Map<Object, Object> message,
+ NotificationProcessor notificationProcessor) {
+ if (message.containsKey('id')) {
+ outOfTestExpect(message['id'], isString);
+ var id = message['id'] as String;
+ var completer = _pendingCommands[id];
+ if (completer == null) {
+ fail('Unexpected response from server: id=$id');
+ } else {
+ _pendingCommands.remove(id);
+ }
+ if (message.containsKey('error')) {
+ completer.completeError(ServerErrorMessage(message));
+ } else {
+ completer.complete(message['result']);
+ }
+ // Check that the message is well-formed. We do this after calling
+ // completer.complete() or completer.completeError() so that we don't
+ // stall the test in the event of an error.
+ outOfTestExpect(message, isResponse);
+ } else {
+ // Message is a notification. It should have an event and possibly
+ // params.
+ outOfTestExpect(message, contains('event'));
+ outOfTestExpect(message['event'], isString);
+ notificationProcessor(
+ message['event'] as String, message['params'] as Map);
+ // Check that the message is well-formed. We do this after calling
+ // notificationController.add() so that we don't stall the test in the
+ // event of an error.
+ outOfTestExpect(message, isNotification);
+ }
+ }
+
/// Record a message that was exchanged with the server, and print it out if
/// [debugStdio] has been called.
void _recordStdio(String line) {
@@ -686,25 +696,26 @@
.addDescriptionOf(valueMatcher);
@override
- void populateMismatches(item, List<MismatchDescriber> mismatches) {
- if (item is! Map) {
+ void populateMismatches(Object item, List<MismatchDescriber> mismatches) {
+ if (item is Map) {
+ item.forEach((key, value) {
+ checkSubstructure(
+ key,
+ keyMatcher,
+ mismatches,
+ (Description description) =>
+ description.add('key ').addDescriptionOf(key));
+ checkSubstructure(
+ value,
+ valueMatcher,
+ mismatches,
+ (Description description) =>
+ description.add('field ').addDescriptionOf(key));
+ });
+ } else {
mismatches.add(simpleDescription('is not a map'));
return;
}
- item.forEach((key, value) {
- checkSubstructure(
- key,
- keyMatcher,
- mismatches,
- (Description description) =>
- description.add('key ').addDescriptionOf(key));
- checkSubstructure(
- value,
- valueMatcher,
- mismatches,
- (Description description) =>
- description.add('field ').addDescriptionOf(key));
- });
}
}
diff --git a/pkg/analyzer_plugin/test/plugin/assist_mixin_test.dart b/pkg/analyzer_plugin/test/plugin/assist_mixin_test.dart
index 955290a..8e93c51 100644
--- a/pkg/analyzer_plugin/test/plugin/assist_mixin_test.dart
+++ b/pkg/analyzer_plugin/test/plugin/assist_mixin_test.dart
@@ -55,7 +55,8 @@
_TestAssistContributor(this.changes);
@override
- Future<void> computeAssists(AssistRequest request, AssistCollector collector) async {
+ Future<void> computeAssists(
+ AssistRequest request, AssistCollector collector) async {
for (var change in changes) {
collector.addAssist(change);
}
diff --git a/pkg/analyzer_plugin/test/plugin/fix_mixin_test.dart b/pkg/analyzer_plugin/test/plugin/fix_mixin_test.dart
index 06a1c7f..4bb0a18 100644
--- a/pkg/analyzer_plugin/test/plugin/fix_mixin_test.dart
+++ b/pkg/analyzer_plugin/test/plugin/fix_mixin_test.dart
@@ -61,7 +61,8 @@
_TestFixContributor(this.changes);
@override
- Future<void> computeFixes(FixesRequest request, FixCollector collector) async {
+ Future<void> computeFixes(
+ FixesRequest request, FixCollector collector) async {
for (var change in changes) {
collector.addFix(request.errorsToFix[0], change);
}
diff --git a/pkg/analyzer_plugin/test/plugin/mocks.dart b/pkg/analyzer_plugin/test/plugin/mocks.dart
index 6d466d2..18eba29 100644
--- a/pkg/analyzer_plugin/test/plugin/mocks.dart
+++ b/pkg/analyzer_plugin/test/plugin/mocks.dart
@@ -40,7 +40,7 @@
bool _closed = false;
void Function() _onDone;
- Function _onError;
+ void Function(Object, StackTrace) _onError;
void Function(Notification) _onNotification;
void Function(Request) _onRequest;
@@ -61,7 +61,7 @@
Function onError,
Function(Notification) onNotification}) {
_onDone = onDone;
- _onError = onError;
+ _onError = onError as void Function(Object, StackTrace);
_onNotification = onNotification;
_onRequest = onRequest;
}
To view, visit change 192289. To unsubscribe, or for help writing mail filters, visit settings.
Samuel Rawlins (Gerrit)
Mar 22, 2021, 1:19:00 AM3/22/21
to rev...@dartlang.org, Konstantin Shcheglov, Brian Wilkerson, Dart CI, Kevin Moore, commi...@chromium.org
Attention is currently required from: Konstantin Shcheglov, Samuel Rawlins, Brian Wilkerson.
Brian Wilkerson (Gerrit)
Mar 22, 2021, 11:03:20 AM3/22/21
to Samuel Rawlins, rev...@dartlang.org, Konstantin Shcheglov, Dart CI, Kevin Moore, commi...@chromium.org
Attention is currently required from: Konstantin Shcheglov, Samuel Rawlins.
Patch set 2:Code-Review +1
1 comment:
Patchset:
I'm fine with these changes, so I've +1'd the CL, but the timing is a bit unfortunate as we have an external contributor who's migrating this package to null safety and this will likely cause merge conflicts. I'm not asking you to hold off on landing it, just letting you know.
To view, visit change 192289. To unsubscribe, or for help writing mail filters, visit settings.
Konstantin Shcheglov (Gerrit)
Mar 22, 2021, 12:11:54 PM3/22/21
to Samuel Rawlins, rev...@dartlang.org, Brian Wilkerson, Dart CI, Kevin Moore, commi...@chromium.org
Attention is currently required from: Samuel Rawlins.
To view, visit change 192289. To unsubscribe, or for help writing mail filters, visit settings.
Samuel Rawlins (Gerrit)
Mar 22, 2021, 12:51:29 PM3/22/21
to rev...@dartlang.org, Konstantin Shcheglov, Brian Wilkerson, Dart CI, Kevin Moore, commi...@chromium.org
Attention is currently required from: Kevin Moore.
2 comments:
Patchset:
I'm fine with these changes, so I've +1'd the CL, but the timing is a bit unfortunate as we have an […]
Do we know if they have started? Since this is in no way urgent (and some code may not need to change as much with the new promotion in null safety), I'm happy to take on the merge after they land.
File pkg/analyzer_plugin/lib/utilities/pair.dart:
Patch Set #2, Line 17:
if (other is Pair) {=> other is Pair && other.first == first &&... […]
Done
To view, visit change 192289. To unsubscribe, or for help writing mail filters, visit settings.
Konstantin Shcheglov (Gerrit)
Mar 22, 2021, 1:12:37 PM3/22/21
to Samuel Rawlins, rev...@dartlang.org, Brian Wilkerson, Dart CI, Kevin Moore, commi...@chromium.org
Attention is currently required from: Kevin Moore, Samuel Rawlins.
Patch set 3:Code-Review +1
1 comment:
Patchset:
Do we know if they have started? Since this is in no way urgent (and some code may not need to chang […]
The migration CL: https://dart-review.googlesource.com/c/sdk/+/191622
To view, visit change 192289. To unsubscribe, or for help writing mail filters, visit settings.
Samuel Rawlins (Gerrit)
Mar 22, 2021, 1:17:57 PM3/22/21
to rev...@dartlang.org, Konstantin Shcheglov, Brian Wilkerson, Dart CI, Kevin Moore, commi...@chromium.org
Attention is currently required from: Kevin Moore.
1 comment:
Patchset:
The migration CL: https://dart-review.googlesource. […]
OK I'll wait then.
To view, visit change 192289. To unsubscribe, or for help writing mail filters, visit settings.
Samuel Rawlins (Gerrit)
Aug 21, 2021, 6:44:13 PM8/21/21
to rev...@dartlang.org, Konstantin Shcheglov, Brian Wilkerson, Dart CI, commi...@chromium.org
Samuel Rawlins abandoned this change.
Abandoned
refactored
To view, visit change 192289. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages