[pigeon] Improve casting and nullability-handling in generated code#11163
[pigeon] Improve casting and nullability-handling in generated code#11163auto-submit[bot] merged 9 commits intoflutter:mainfrom
Conversation
There are a number of positions where we attempt to cast a nullable expression to a non-nullable type with something like the following `foo as List<Object?>?)!`. This successfully performs the cast and avoids a lint rule which recommends not casting a nullable expression to a non-nullable type. But it can be done more simply. Here are some examples of the improved code: ```diff - data: (result[3] as Map<Object?, Object?>?)!.cast<String, String>(), + data: (result[3]! as Map<Object?, Object?>).cast<String, String>(), - final List<Object?> args = (message as List<Object?>?)!; + final List<Object?> args = message! as List<Object?>; - stringList: (result[13] as List<Object?>?)!.cast<String>(), + stringList: (result[13]! as List<Object?>).cast<String>(), ``` Since we are expecting these values to be non-null, we first null-assert them (`!`), and then cast the expression with `as`. There was previously a helper, `_makeGenericCastCall`, which generated the String representation for a call to `cast`, like `.cast<Object?>()`, which was called in three places. This change consoldiates the surrounding code to those call sites into a new helper, `_castValue`, which instead returns the complete String of casting a value to a type, possibly null-asserting the value and possibly calling `.cast()`. Combining all of the casting code into one function also allows us to avoid unnecessary parentheses: ```diff - final String? arg_aString = (args[0] as String?); + final String? arg_aString = args[0] as String?; ``` Improving the nullability-handling allows us to remove the `_addGenericTypes` helper, and rename the `addGenericTypesNullable` to just `addGenericTypes`, since the helper should always print an appropriate nullability suffix. One prominent change made to the generated code is the removal of "assert foo is not null" statements. These are redundant with the null-asserts (`!`), although they _might_ offer more information in the assertion message, though I honestly don't think so. The exception thrown by `!` has the relevant information in the stack trace.
There was a problem hiding this comment.
Code Review
This pull request refactors the nullability handling in the generated Dart code, resulting in simpler and more idiomatic casting logic. The introduction of the _castValue helper centralizes casting operations, and the removal of redundant null checks and parentheses improves code clarity. The changes across the generated files are consistent with these improvements. I've found one minor issue in a documentation comment.
Note: Security Review is unavailable for this PR.
Man, I thought I'd gotten rid of all of these years ago... |
tarrinneal
left a comment
There was a problem hiding this comment.
That's a lot of spaghetti you've cleaned up.
flutter/packages@ee460d6...ecace66 2026-03-11 srawlins@google.com [pigeon] Produce a helpful error for bad method return type (flutter/packages#11204) 2026-03-11 srawlins@google.com [pigeon] Use hasLength and isEmpty in tests for better failure messages (flutter/packages#11205) 2026-03-11 dacoharkes@google.com [rfw] Opt out of icon tree shaking (flutter/packages#11216) 2026-03-11 srawlins@google.com [pigeon] Tidy imports and "ignore" comments (flutter/packages#11149) 2026-03-11 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump org.jetbrains.kotlin:kotlin-bom from 2.2.21 to 2.3.10 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#10984) 2026-03-11 srawlins@google.com [pigeon] Improve casting and nullability-handling in generated code (flutter/packages#11163) 2026-03-10 engine-flutter-autoroll@skia.org Roll Flutter from 2ec61af to 195ae7b (36 revisions) (flutter/packages#11222) 2026-03-10 git@reb0.org [vector_graphics] Respect BoxFit parameter with viewbox (flutter/packages#11012) 2026-03-10 stuartmorgan@google.com Add AI contribution guidelines to PR checklist (flutter/packages#11195) 2026-03-10 zezohassam@gmail.com [video_player] Optimize caption retrieval with binary search in VideoPlayerController (flutter/packages#8347) 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
…lutter#11163) There are a number of positions where we attempt to cast a nullable expression to a non-nullable type with something like the following `(foo as List<Object?>?)!`. This successfully performs the cast and avoids a lint rule which recommends not casting a nullable expression to a non-nullable type. But it can be done more simply. Here are some examples of the improved code: ```diff - data: (result[3] as Map<Object?, Object?>?)!.cast<String, String>(), + data: (result[3]! as Map<Object?, Object?>).cast<String, String>(), - final List<Object?> args = (message as List<Object?>?)!; + final List<Object?> args = message! as List<Object?>; - stringList: (result[13] as List<Object?>?)!.cast<String>(), + stringList: (result[13]! as List<Object?>).cast<String>(), ``` Since we are expecting these values to be non-null, we first null-assert them (`!`), and then cast the expression with `as`. There was previously a helper, `_makeGenericCastCall`, which generated the String representation for a call to `cast`, like `.cast<Object?>()`, which was called in three places. This change consoldiates the surrounding code to those call sites into a new helper, `_castValue`, which instead returns the complete String of casting a value to a type, possibly null-asserting the value and possibly calling `.cast()`. Combining all of the casting code into one function also allows us to avoid unnecessary parentheses: ```diff - final String? arg_aString = (args[0] as String?); + final String? arg_aString = args[0] as String?; ``` Improving the nullability-handling allows us to remove the `_addGenericTypes` helper, and rename the `addGenericTypesNullable` to just `addGenericTypes`, since the helper should always print an appropriate nullability suffix. One prominent change made to the generated code is the removal of "assert foo is not null" statements. These are redundant with the null-asserts (`!`), although they _might_ offer more information in the assertion message, though I honestly don't think so. The exception thrown by `!` has the relevant information in the stack trace. Fixes flutter/flutter#116972 ## Pre-Review Checklist
…er#183517) flutter/packages@ee460d6...ecace66 2026-03-11 srawlins@google.com [pigeon] Produce a helpful error for bad method return type (flutter/packages#11204) 2026-03-11 srawlins@google.com [pigeon] Use hasLength and isEmpty in tests for better failure messages (flutter/packages#11205) 2026-03-11 dacoharkes@google.com [rfw] Opt out of icon tree shaking (flutter/packages#11216) 2026-03-11 srawlins@google.com [pigeon] Tidy imports and "ignore" comments (flutter/packages#11149) 2026-03-11 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump org.jetbrains.kotlin:kotlin-bom from 2.2.21 to 2.3.10 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#10984) 2026-03-11 srawlins@google.com [pigeon] Improve casting and nullability-handling in generated code (flutter/packages#11163) 2026-03-10 engine-flutter-autoroll@skia.org Roll Flutter from 2ec61af to 195ae7b (36 revisions) (flutter/packages#11222) 2026-03-10 git@reb0.org [vector_graphics] Respect BoxFit parameter with viewbox (flutter/packages#11012) 2026-03-10 stuartmorgan@google.com Add AI contribution guidelines to PR checklist (flutter/packages#11195) 2026-03-10 zezohassam@gmail.com [video_player] Optimize caption retrieval with binary search in VideoPlayerController (flutter/packages#8347) 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
When I landed #11149, which changes the `ignore_for_file` to use `type=lint`, I didn't examine the list of lint rules we _used to_ violate. In particular, since #11114 and #11163 landed, our generated code no longer violates the following rules: * unnecessary_parenthesis * prefer_null_aware_operators * unnecessary_import * no_leading_underscores_for_local_identifiers I believe this does not need CHANGELOG notes because users should not be using the _hidden_ 'ignore-lints' flag. This list of individually ignored lint rules should only appear in our tests or our checked-in generated Dart files. ## Pre-Review Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [ ] I read the [Tree Hygiene] page, which explains my responsibilities. - [x] I read and followed the [relevant style guides] and ran [the auto-formatter]. - [x] I signed the [CLA]. - [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. `[shared_preferences]` - [ ] I [linked to at least one issue that this PR fixes] in the description above. - [x] I followed [the version and CHANGELOG instructions], using [semantic versioning] and the [repository CHANGELOG style], or I have commented below to indicate which documented exception this PR falls under[^1]. - [ ] I updated/added any relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under[^1]. - [x] All existing and new tests are passing. Co-authored-by: stuartmorgan-g <stuartmorgan@google.com>
…er#183517) flutter/packages@ee460d6...ecace66 2026-03-11 srawlins@google.com [pigeon] Produce a helpful error for bad method return type (flutter/packages#11204) 2026-03-11 srawlins@google.com [pigeon] Use hasLength and isEmpty in tests for better failure messages (flutter/packages#11205) 2026-03-11 dacoharkes@google.com [rfw] Opt out of icon tree shaking (flutter/packages#11216) 2026-03-11 srawlins@google.com [pigeon] Tidy imports and "ignore" comments (flutter/packages#11149) 2026-03-11 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump org.jetbrains.kotlin:kotlin-bom from 2.2.21 to 2.3.10 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#10984) 2026-03-11 srawlins@google.com [pigeon] Improve casting and nullability-handling in generated code (flutter/packages#11163) 2026-03-10 engine-flutter-autoroll@skia.org Roll Flutter from 2ec61af to 195ae7b (36 revisions) (flutter/packages#11222) 2026-03-10 git@reb0.org [vector_graphics] Respect BoxFit parameter with viewbox (flutter/packages#11012) 2026-03-10 stuartmorgan@google.com Add AI contribution guidelines to PR checklist (flutter/packages#11195) 2026-03-10 zezohassam@gmail.com [video_player] Optimize caption retrieval with binary search in VideoPlayerController (flutter/packages#8347) 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
There are a number of positions where we attempt to cast a nullable expression to a non-nullable type with something like the following
(foo as List<Object?>?)!. This successfully performs the cast and avoids a lint rule which recommends not casting a nullable expression to a non-nullable type. But it can be done more simply. Here are some examples of the improved code:Since we are expecting these values to be non-null, we first null-assert them (
!), and then cast the expression withas.There was previously a helper,
_makeGenericCastCall, which generated the String representation for a call tocast, like.cast<Object?>(), which was called in three places. This change consoldiates the surrounding code to those call sites into a new helper,_castValue, which instead returns the complete String of casting a value to a type, possibly null-asserting the value and possibly calling.cast().Combining all of the casting code into one function also allows us to avoid unnecessary parentheses:
Improving the nullability-handling allows us to remove the
_addGenericTypeshelper, and rename theaddGenericTypesNullableto justaddGenericTypes, since the helper should always print an appropriate nullability suffix.One prominent change made to the generated code is the removal of "assert foo is not null" statements. These are redundant with the null-asserts (
!), although they might offer more information in the assertion message, though I honestly don't think so. The exception thrown by!has the relevant information in the stack trace.Fixes flutter/flutter#116972
Pre-Review Checklist
[shared_preferences]///).