[pigeon] Report a clear error for enhanced enums#11880
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates Pigeon to version 27.0.1 and introduces detection for unsupported enhanced enums in Dart input files. When an enhanced enum (defined by having members or constant arguments) is detected, Pigeon now emits a clear error message and avoids visiting its children to prevent parser crashes. Feedback on these changes suggests expanding the enhanced enum detection to also check for type parameters, mixins, and implemented interfaces to ensure all unsupported cases are caught.
| final bool hasEnumMembers = node.body.members.isNotEmpty; | ||
| final bool hasConstantArguments = node.body.constants.any( | ||
| (dart_ast.EnumConstantDeclaration e) => e.arguments != null, | ||
| ); | ||
| final bool isEnhancedEnum = hasEnumMembers || hasConstantArguments; |
There was a problem hiding this comment.
Enhanced enums in Dart can also declare type parameters, implement interfaces, or use mixins (using with or implements clauses). The current check only detects enhanced enums with members or constant arguments. To fully prevent unsupported enhanced enums from causing issues, we should also check for node.typeParameters, node.withClause, and node.implementsClause.
final bool hasEnumMembers = node.body.members.isNotEmpty;
final bool hasConstantArguments = node.body.constants.any(
(dart_ast.EnumConstantDeclaration e) => e.arguments != null,
);
final bool isEnhancedEnum = hasEnumMembers ||
hasConstantArguments ||
node.typeParameters != null ||
node.withClause != null ||
node.implementsClause != null;c4afe8d to
b1639ce
Compare
|
Rebased onto latest Re-ran the suite after rebasing —
Ready for review whenever you have a chance. |
|
Gentle bump @tarrinneal — this was rebased onto main (pigeon v27.1.1) on June 17 with all local tests passing, and I believe it's just waiting on triage/CI now. Happy to rebase again if it has drifted in the meantime. Thanks! |
|
autosubmit label was removed for flutter/packages/11880, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.
|
|
@stuartmorgan-g for 2nd |
|
Thanks for the review @tarrinneal — and for applying the comment trims directly; I kept your commit as-is. On top of that I rebased onto main and bumped to 27.1.2, since 27.1.1 was taken by #11894 landing yesterday (CHANGELOG entries stacked accordingly). Analyzer clean and the enhanced-enum tests pass after the rebase. |
Pigeon's input parser silently ignored the extra syntax in enhanced enums (constructors, fields, methods, or arguments on values), reading only the value names. This produced confusing generated output — the enum's fields were effectively coalesced into the next class — with no indication that the input was unsupported. Detect enhanced enums in visitEnumDeclaration and report a clear, source-located error instead. The enum is still registered by its value names so that classes referencing it don't produce additional cascading 'Unknown type' errors, and the visitor no longer walks the enum's body (whose class-like members it doesn't expect; a getter there previously crashed the parser on a null parameter list). Fixes flutter/flutter#160827
…implements clauses as enhanced enums These forms were previously still silently reduced to their value names. The error message now lists the full set of unsupported syntax.
Reduce comment length
8337f77 to
c231afb
Compare
|
autosubmit label was removed for flutter/packages/11880, because - The status or check suite Mac_x64 ios_build_all_packages master has failed. Please fix the issues identified (or deflake) before re-applying this label. |
…er#189387) flutter/packages@20928d5...ad2eab1 2026-07-13 35750184+motucraft@users.noreply.github.com [camera] Guard CameraController value updates after dispose (flutter/packages#11861) 2026-07-13 dkwingsmt@users.noreply.github.com [material_ui] Remove unused example files (flutter/packages#12179) 2026-07-13 puneetkukreja98@gmail.com [google_maps_flutter_platform_interface] Adds support for mapTypeControlEnabled, fullscreenControlEnabled, and streetViewControlEnabled for web (flutter/packages#12191) 2026-07-13 44747303+theprantadutta@users.noreply.github.com [pigeon] Report a clear error for enhanced enums (flutter/packages#11880) 2026-07-13 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 6.1.0 to 6.2.0 in the all-github-actions group (flutter/packages#12190) 2026-07-13 stuartmorgan@google.com [shared_preferences] Convert legacy codepath to Kotlin (flutter/packages#12139) 2026-07-12 engine-flutter-autoroll@skia.org Roll Flutter from f7b66f3 to cf9e8af (11 revisions) (flutter/packages#12188) 2026-07-11 44525804+sailendrabathi@users.noreply.github.com [video_player] Improve seek performance on Android (flutter/packages#11810) 2026-07-11 stuartmorgan@google.com [pigeon] Disable iOS test parallelization (flutter/packages#12177) 2026-07-11 dkwingsmt@users.noreply.github.com [material_ui, cupertino_ui] Migrate snippet TODOs to `<callout-box>` (flutter/packages#12146) 2026-07-11 dkwingsmt@users.noreply.github.com [cupertino_ui] Fix example path (flutter/packages#12151) 2026-07-10 engine-flutter-autoroll@skia.org Roll Flutter from dc2a870 to f7b66f3 (12 revisions) (flutter/packages#12175) 2026-07-10 engine-flutter-autoroll@skia.org Roll Flutter (stable) from f94f4fc to ee80f08 (3 revisions) (flutter/packages#12171) 2026-07-10 engine-flutter-autoroll@skia.org Manual roll Flutter from 91939cc to dc2a870 (50 revisions) (flutter/packages#12169) 2026-07-09 jessiewong401@gmail.com Update `legacy` test to be in Warn Range (flutter/packages#12168) 2026-07-09 dkwingsmt@users.noreply.github.com [material_ui, cupertino_ui] Migrate dartpad TODOs to `<callout-box>` (flutter/packages#12120) 2026-07-09 73310711+shrabanti722@users.noreply.github.com [video_player_avfoundation] Implement preventsDisplaySleepDuringVideoPlayback (flutter/packages#11547) 2026-07-09 44747303+theprantadutta@users.noreply.github.com [pigeon] Add usage docs to generated Dart event channel methods (flutter/packages#11894) 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
Pigeon's input parser silently ignored the extra syntax in enhanced enums (constructors, fields, methods, or arguments on values), reading only the value names. This produced confusing generated output — as described in the linked issue, the enum's fields were effectively coalesced into the next class — with no indication that the input was unsupported.
This change detects enhanced enums in
visitEnumDeclarationand reports a clear, source-located error:Implementation notes:
visitMethodDeclaration'snode.parameters!). One of the new tests covers this case.enum E { a, b; }) are unaffected; a test guards against false positives.The scope follows the approach discussed and agreed in the issue thread (silent failure → clear error; no attempt to actually support enhanced enums).
Fixes flutter/flutter#160827
Pre-Review Checklist
[shared_preferences]///).Four new tests in
pigeon_lib_test.dart: enhanced enum with constructor+fields, enhanced enum with only arguments on values, enhanced enum with a getter (the previous parser-crash case), and a plain enum with a trailing semicolon (false-positive guard). The fullpigeon_lib_test.dartsuite (113 tests) andversion_test.dartpass locally.