[pigeon] Fix NSnumber edge cases and null value bug by tarrinneal · Pull Request #12997 · flutter/packages

tarrinneal · 2026-09-24T05:07:34Z

Fixes flutter/flutter#193266
Fixes two Swift FFI value serialization bugs in SwiftGenerator:

CFBoolean and floating-point NSNumber bridging in wrapNumber: When an Objective-C boolean (__NSCFBoolean / CFBoolean) or a whole-number floating-point NSNumber (such as 3.0 returned from UserDefaults.object(forKey:) or NSDictionary) is boxed inside Any, Swift's dynamic as? Int cast succeeds before reaching case let value as Double: or case let value as Bool:. wrapNumber now checks CFGetTypeID(number as CFTypeRef) == CFBooleanGetTypeID() (type: 3) and CFNumberIsFloatType(nsNumber) (type: 2) prior to switching on numeric types, matching FlutterStandardCodec behavior.
Dictionary value isNullish check in writeValue: Fixes a typo in writeValue where dictionary serialization checked isNullish(key) instead of isNullish(value) when serializing dictionary values.

Fixes https://github.com/flutter/flutter/issues/<ISSUE_NUMBER>

Pre-Review Checklist

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-assist bot 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.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩²

gemini-code-assist

Code Review

This pull request bumps the Pigeon version to 29.0.4 and introduces fixes for Swift FFI number wrapping and dictionary serialization. Specifically, it corrects dictionary serialization to check isNullish on the value rather than the key, and updates wrapNumber to check CFBooleanGetTypeID and CFNumberIsFloatType before numeric casts. Feedback suggests safely casting number to NSNumber before calling CFGetTypeID to avoid potential runtime crashes with Swift value types, and updating the corresponding test expectations.

gemini-code-assist · 2026-09-24T05:08:43Z

+        indent.format('''
+  if CFGetTypeID(number as CFTypeRef) == CFBooleanGetTypeID(), let value = number as? Bool {
+    return ${_classNamePrefix}NumberWrapper(number: NSNumber(value: value), type: 3)
+  }
+  if let nsNumber = number as? NSNumber, CFNumberIsFloatType(nsNumber) {
+    return ${_classNamePrefix}NumberWrapper(number: nsNumber, type: 2)
+  }
+''');


Directly casting number as CFTypeRef can lead to runtime crashes if number is a Swift value type (like a custom struct or enum) that cannot be bridged to an Objective-C class, as it gets boxed in _SwiftValue which does not support CoreFoundation runtime type queries. Safely casting number to NSNumber first ensures that CFGetTypeID is only called on valid CoreFoundation objects.

indent.format(''' if let nsNumber = number as? NSNumber { if CFGetTypeID(nsNumber as CFTypeRef) == CFBooleanGetTypeID(), let value = number as? Bool { return ${_classNamePrefix}NumberWrapper(number: NSNumber(value: value), type: 3) } if CFNumberIsFloatType(nsNumber) { return ${_classNamePrefix}NumberWrapper(number: nsNumber, type: 2) } } ''');

gemini-code-assist · 2026-09-24T05:08:43Z

+    expect(
+      code,
+      contains(
+        'if CFGetTypeID(number as CFTypeRef) == CFBooleanGetTypeID(), let value = number as? Bool {\n'
+        '    return NumberWrapper(number: NSNumber(value: value), type: 3)\n'
+        '  }\n'
+        '  if let nsNumber = number as? NSNumber, CFNumberIsFloatType(nsNumber) {\n'
+        '    return NumberWrapper(number: nsNumber, type: 2)\n'
+        '  }',
+      ),
+    );


Update the test expectation to match the safer wrapNumber implementation that checks if number is an NSNumber before calling CFGetTypeID.

Suggested change

expect(

code,

contains(

'if CFGetTypeID(number as CFTypeRef) == CFBooleanGetTypeID(), let value = number as? Bool {\n'

' return NumberWrapper(number: NSNumber(value: value), type: 3)\n'

' }\n'

' if let nsNumber = number as? NSNumber, CFNumberIsFloatType(nsNumber) {\n'

' return NumberWrapper(number: nsNumber, type: 2)\n'

' }',

),

);

expect(

code,

contains(

' if let nsNumber = number as? NSNumber {\n'

' if CFGetTypeID(nsNumber as CFTypeRef) == CFBooleanGetTypeID(), let value = number as? Bool {\n'

' return NumberWrapper(number: NSNumber(value: value), type: 3)\n'

' }\n'

' if CFNumberIsFloatType(nsNumber) {\n'

' return NumberWrapper(number: nsNumber, type: 2)\n'

' }\n'

' }',

),

);

stuartmorgan-g

LGTM with one question, but I would really recommend an integration test for this.

stuartmorgan-g · 2026-09-24T15:46:39Z

+    }
+    if CFNumberIsFloatType(nsNumber) {
+      return NativeInteropExampleNumberWrapper(number: nsNumber, type: 2)
+    }


Why don't we need int-as-NSNumber handling here too?

The as int check works for that

If the as checks work, then why do we need this code at all vs. just changing the evaluation order of the existing checks?

The issue is that ordering the switch block to handle all cases isn't possible. The bool and float checks make it so all other cases that succeed into as int work correctly.

…s it was excessive

auto-submit · 2026-09-24T20:05:53Z

autosubmit label was removed for flutter/packages/12997, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

…r#193351) flutter/packages@431ea69...e55e7ac 2026-09-25 [email protected] [material_ui] Add labelTextDirection handling in InputDecorator (flutter/packages#12607) 2026-09-24 [email protected] [pigeon] Fix NSnumber edge cases and null value bug (flutter/packages#12997) 2026-09-24 [email protected] [camera] add custom path ouput to recording (flutter/packages#11774) 2026-09-24 [email protected] [Android 17] Update packages CI test runners to SDK 37 (flutter/packages#12376) 2026-09-24 [email protected] [pigeon][video_player] Disable `video_player` and `pigeon` flakes blocking latest flutter -> packages roll (flutter/packages#13003) 2026-09-24 [email protected] [google_maps_flutter] Add onPointOfInterestTap platform implementations (flutter/packages#12880) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] 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

Fix NSnumber edge cases and null value bug

5379152

tarrinneal requested a review from stuartmorgan-g September 24, 2026 05:07

flutter-dashboard Bot added the CICD Run CI/CD label Sep 24, 2026

github-actions Bot added p: pigeon platform-ios platform-macos labels Sep 24, 2026

gemini-code-assist Bot reviewed Sep 24, 2026

View reviewed changes

tarrinneal mentioned this pull request Sep 24, 2026

[shared_preferences] switch to ffi/jni pigeon #12329

Open

/gemini

c6856ca

stuartmorgan-g approved these changes Sep 24, 2026

View reviewed changes

tarrinneal added 3 commits September 24, 2026 11:10

add handling for ints and integration tests, removed generator test a…

5b12f86

…s it was excessive

remove now unnecessary switch block

039a3cb

double tests

c3ae5e9

tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 24, 2026

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 24, 2026

generate

d883581

tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 24, 2026

auto-submit Bot merged commit 40b7e64 into flutter:main Sep 24, 2026
13 checks passed

engine-flutter-autoroll mentioned this pull request Sep 25, 2026

Roll Packages from 431ea69a42e2 to e55e7ac30b8c (6 revisions) flutter/flutter#193351

Merged

Navigation Menu

Repository navigation

[pigeon] Fix NSnumber edge cases and null value bug - #12997

[pigeon] Fix NSnumber edge cases and null value bug#12997
auto-submit[bot] merged 6 commits into
flutter:mainfrom
tarrinneal:check-for-bools-in-swift-numbers

tarrinneal commented Sep 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Sep 24, 2026

Uh oh!

gemini-code-assist Bot Sep 24, 2026

Uh oh!

stuartmorgan-g left a comment

Uh oh!

stuartmorgan-g Sep 24, 2026

Uh oh!

tarrinneal Sep 24, 2026

Uh oh!

stuartmorgan-g Sep 24, 2026

Uh oh!

tarrinneal Sep 24, 2026

Uh oh!

Uh oh!

auto-submit Bot commented Sep 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

tarrinneal commented Sep 24, 2026

Pre-Review Checklist

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

tarrinneal Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

tarrinneal Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

auto-submit Bot commented Sep 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants