- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.5k
[go_router_builder] Ignore experimental features #10275
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
Conversation
| @stereotype441 can you confirm this resolves the issue? We cannot know for sure since it is upstream. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request aims to address the use of experimental features from an upstream analyzer package by adding ignore comments. While this is a good step, the changes are incomplete as one of the experimental API usages is not being ignored. I've provided a suggestion to not only fix this omission but also to improve the code's readability by extracting the experimental properties into local variables, which centralizes the suppression comments.
| // ignore: experimental_member_use | ||
| if (functionType.formalParameters.length != 1 || | ||
| functionType.returnType.getDisplayString() != | ||
| type.element.typeParameters.first.displayName || | ||
| // ignore: experimental_member_use | ||
| functionType.formalParameters[0].type.getDisplayString() != 'Object?') { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've added ignore comments for formalParameters, but functionType.returnType on line 800 is also an experimental member and is missing an ignore comment. This could lead to analyzer warnings.
To make this cleaner and ensure all experimental usages are covered, consider extracting the properties that use experimental APIs into local variables. This localizes the suppression comments and improves the readability of the if condition.
| // ignore: experimental_member_use | |
| if (functionType.formalParameters.length != 1 || | |
| functionType.returnType.getDisplayString() != | |
| type.element.typeParameters.first.displayName || | |
| // ignore: experimental_member_use | |
| functionType.formalParameters[0].type.getDisplayString() != 'Object?') { | |
| // ignore: experimental_member_use | |
| final List<ParameterElement> formalParameters = functionType.formalParameters; | |
| // ignore: experimental_member_use | |
| final DartType returnType = functionType.returnType; | |
| if (formalParameters.length != 1 || | |
| returnType.getDisplayString() != | |
| type.element.typeParameters.first.displayName || | |
| formalParameters[0].type.getDisplayString() != 'Object?') { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've added
ignorecomments forformalParameters, butfunctionType.returnTypeon line 800 is also an experimental member and is missing anignorecomment. This could lead to analyzer warnings.
Hopefully everyone takes AI code reviews with a grain of salt, but just in case someone is thinking of applying this suggestion: I see no evidence to back up this claim. AFAICT, FunctionType.returnType is not an experimental member, and I don't believe it ever has been.
| If I understood correctly, the problem is with Dart from main and analyzer 8.1.0. With that setup and  so I think you need more ignores, like this Edit: I'm not sure why the warnings for 'element' that I saw are not in the original trybot failure, or at least not in the output I saw in chat, but it's harmless to ignore them. | 
| 
 It looks from https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket/8700308398682324417/+/u/analyze_Flutter_repositories/stdout like the  But I agree that it would be harmless to go ahead and ignore them as well. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. I'm fine either with @davidmorgan's additional suggested ignores or without.
| From a separate chat: "I don't believe the additional ignores suggested are necessary (his analysis was based on a different analyzer version than the version the bot is using). " I am going to land this as is for now to resolve the original blocker raised. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
flutter/packages@d113bbc...9ec29b6 2025-10-22 omar_hanafy@icloud.com [go_router] Support for top level `onEnter` callback. (flutter/packages#8339) 2025-10-22 katelovett@google.com [go_router_builder] Ignore experimental features (flutter/packages#10275) 2025-10-22 engine-flutter-autoroll@skia.org Roll Flutter from 2d34167 to 75004a6 (39 revisions) (flutter/packages#10279) 2025-10-22 47866232+chunhtai@users.noreply.github.com [ci]Adds mechanism for packages to opt in to batched release (flutter/packages#10237) 2025-10-22 1063596+reidbaker@users.noreply.github.com [tool] Change gradle-check logic to enforce alignment of java versions and a minimum (17) (flutter/packages#10206) 2025-10-22 eternalkaif@gmail.com [various] Migrate example Radio groups to new RadioGroup API (flutter/packages#10155) 2025-10-22 davidmartos96@gmail.com [mustache_template] Emoji support (flutter/packages#10110) 2025-10-22 38378650+hgraceb@users.noreply.github.com [image_picker] Fix typos in error messages for android (flutter/packages#10188) 2025-10-22 38378650+hgraceb@users.noreply.github.com [image_picker] Fix typos in error messages for platform interface (flutter/packages#10211) 2025-10-21 robert.odrowaz@leancode.pl [camera_avfoundation] Wrappers swift migration - part 1 (flutter/packages#10119) 2025-10-21 NearTox@outlook.com [go_router_builder] expand supported versions of analyzer, build and source_gen (flutter/packages#10078) 2025-10-20 engine-flutter-autoroll@skia.org Roll Flutter from 891d7d5 to 2d34167 (18 revisions) (flutter/packages#10268) 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-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/packages@d113bbc...9ec29b6 2025-10-22 omar_hanafy@icloud.com [go_router] Support for top level `onEnter` callback. (flutter/packages#8339) 2025-10-22 katelovett@google.com [go_router_builder] Ignore experimental features (flutter/packages#10275) 2025-10-22 engine-flutter-autoroll@skia.org Roll Flutter from 2d34167 to 75004a6 (39 revisions) (flutter/packages#10279) 2025-10-22 47866232+chunhtai@users.noreply.github.com [ci]Adds mechanism for packages to opt in to batched release (flutter/packages#10237) 2025-10-22 1063596+reidbaker@users.noreply.github.com [tool] Change gradle-check logic to enforce alignment of java versions and a minimum (17) (flutter/packages#10206) 2025-10-22 eternalkaif@gmail.com [various] Migrate example Radio groups to new RadioGroup API (flutter/packages#10155) 2025-10-22 davidmartos96@gmail.com [mustache_template] Emoji support (flutter/packages#10110) 2025-10-22 38378650+hgraceb@users.noreply.github.com [image_picker] Fix typos in error messages for android (flutter/packages#10188) 2025-10-22 38378650+hgraceb@users.noreply.github.com [image_picker] Fix typos in error messages for platform interface (flutter/packages#10211) 2025-10-21 robert.odrowaz@leancode.pl [camera_avfoundation] Wrappers swift migration - part 1 (flutter/packages#10119) 2025-10-21 NearTox@outlook.com [go_router_builder] expand supported versions of analyzer, build and source_gen (flutter/packages#10078) 2025-10-20 engine-flutter-autoroll@skia.org Roll Flutter from 891d7d5 to 2d34167 (18 revisions) (flutter/packages#10268) 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-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Unblock upstream analyzer change.
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3