Remove an unnecessary assert #129796
Merged
Remove an unnecessary assert #129796
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The type variable `T`, when used as an expression, will always be a `Type`. The type test `T is! List` is always true (as is `T is Type`). This expression will become a warning in the analyzer in some upcoming release of the Dart SDK. This `assert` was added in a PR which reapplied an earlier PR, however the earlier PR did not include this assert. I do not see any discussion indicating the intent of this assert. The impact of testing this `T` in any way is low - this is a private class so we can see all the uses and know that the only type bound to `T` is `Uri`. Avoid the upcoming diagnostic and remove the assert entirely. This maintains existing behavior but ignores the potential intent for the check.
Contributor
Author
|
If I had to guess at the intent, it could have been to prevent usage where the If we use diff --git a/packages/flutter_tools/lib/src/protocol_discovery.dart b/packages/flutter_tools/lib/src/protocol_discovery.dart
index 578052bfb7..84f983ea5f 100644
--- a/packages/flutter_tools/lib/src/protocol_discovery.dart
+++ b/packages/flutter_tools/lib/src/protocol_discovery.dart
@@ -167,13 +167,10 @@ class _BufferedStreamController<T> {
final StreamController<T> streamControllerInstance = StreamController<T>.broadcast();
streamControllerInstance.onListen = () {
for (final dynamic event in _events) {
- if (event is T) {
- streamControllerInstance.add(event);
+ if (event is AsyncError) {
+ streamControllerInstance.addError(event.error, event.stackTrace);
} else {
- streamControllerInstance.addError(
- (event as Iterable<dynamic>).first as Object,
- event.last as StackTrace,
- );
+ streamControllerInstance.add(event as T);
}
}
_events.clear();
@@ -198,7 +195,7 @@ class _BufferedStreamController<T> {
if (_streamController.hasListener) {
_streamController.addError(error, stackTrace);
} else {
- _events.add(<dynamic>[error, stackTrace]);
+ _events.add(AsyncError(error, stackTrace));
}
} |
Contributor
|
auto label is removed for flutter/flutter, pr: 129796, Mergeability of pull request flutter/flutter/129796 could not be determined at time of merge.. |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 1, 2023
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Jul 1, 2023
flutter/flutter@ff838bc...aa5f4a2 2023-07-01 goderbauer@google.com Enable no_wildcard_variable_uses lint (flutter/flutter#129858) 2023-06-30 christopherfujino@gmail.com mark packages-autoroller bringup again (flutter/flutter#129859) 2023-06-30 36861262+QuncCccccc@users.noreply.github.com Update `SwitchTheme` tests for M2/M3 (flutter/flutter#129811) 2023-06-30 christopherfujino@gmail.com Change pub roller bot to push to flutter-pub-roller-bot/flutter.git (flutter/flutter#129844) 2023-06-30 greg@zulip.com Fix NetworkImage causing spurious warning in tests (flutter/flutter#129537) 2023-06-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from 54b573e9c4e5 to e6b8292705a8 (4 revisions) (flutter/flutter#129852) 2023-06-30 34871572+gmackall@users.noreply.github.com Upgrade integration tests to use AGP 7.3/Gradle 7.4 (flutter/flutter#129642) 2023-06-30 engine-flutter-autoroll@skia.org Roll Packages from d4752c4 to 53ed5a0 (5 revisions) (flutter/flutter#129837) 2023-06-30 32242716+ricardoamador@users.noreply.github.com Updated correct tasks for test ownership fix (flutter/flutter#129812) 2023-06-30 hans.muller@gmail.com Updated some golden image tests for M2/M3 (flutter/flutter#129794) 2023-06-30 nbosch@google.com Remove an unnecessary assert (flutter/flutter#129796) 2023-06-30 36861262+QuncCccccc@users.noreply.github.com Update `Radio` tests for M2/M3 (flutter/flutter#129814) 2023-06-30 36861262+QuncCccccc@users.noreply.github.com Update `Switch` tests for M2/M3 (flutter/flutter#129810) 2023-06-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from 099a70ebbc60 to 54b573e9c4e5 (1 revision) (flutter/flutter#129821) 2023-06-30 36861262+QuncCccccc@users.noreply.github.com Update `SwitchListTile` tests for M2/M3 (flutter/flutter#129809) 2023-06-30 tessertaha@gmail.com Fix `NavigationDrawer` selected item has wrong icon color (flutter/flutter#129625) 2023-06-30 leroux_bruno@yahoo.fr Update basic_test.dart for M3 compliance (flutter/flutter#129714) 2023-06-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from d33343430f18 to 099a70ebbc60 (7 revisions) (flutter/flutter#129818) 2023-06-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from 68cc1a7971d5 to d33343430f18 (2 revisions) (flutter/flutter#129801) 2023-06-30 godofredoc@google.com Revert no-response to fork. (flutter/flutter#129775) 2023-06-30 36861262+QuncCccccc@users.noreply.github.com Make `DropdownMenu` be able to scroll to the highlighted item when searching. (flutter/flutter#129740) 2023-06-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from cd9ce66db14a to 68cc1a7971d5 (10 revisions) (flutter/flutter#129799) 2023-06-29 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 4.1.0 to 4.2.0 (flutter/flutter#129797) 2023-06-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from eabb22900b44 to cd9ce66db14a (1 revision) (flutter/flutter#129756) 2023-06-29 5236035+fzyzcjy@users.noreply.github.com Remove `@NonNull` to avoid warning (flutter/flutter#129472) 2023-06-29 32242716+ricardoamador@users.noreply.github.com Remove use of any (flutter/flutter#129793) 2023-06-29 kustermann@google.com Prepare for utf8.encode() to return more precise Uint8List type (flutter/flutter#129769) 2023-06-29 lsaudon@gmail.com Deletes files that should be ignored (flutter/flutter#127984) 2023-06-29 christopherfujino@gmail.com Fix flutter_plugins by rolling revision (flutter/flutter#129781) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC dit@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 16, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 17, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 17, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 17, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The type variable
T, when used as an expression, will always be aType. The type testT is! Listis always true (as isT is Type).This expression will become a warning in the analyzer in some upcoming
release of the Dart SDK.
This
assertwas added in a PR which reapplied an earlier PR, howeverthe earlier PR did not include this assert. I do not see any discussion
indicating the intent of this assert.
The impact of testing this
Tin any way is low - this is a privateclass so we can see all the uses and know that the only type bound to
TisUri. Avoid the upcoming diagnostic and remove the assertentirely. This maintains existing behavior but ignores the potential
intent for the check.