Skip to content

[pigeon] Report a clear error for enhanced enums#11880

Open
theprantadutta wants to merge 2 commits into
flutter:mainfrom
theprantadutta:fix/pigeon-enhanced-enum-error
Open

[pigeon] Report a clear error for enhanced enums#11880
theprantadutta wants to merge 2 commits into
flutter:mainfrom
theprantadutta:fix/pigeon-enhanced-enum-error

Conversation

@theprantadutta

Copy link
Copy Markdown

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 visitEnumDeclaration and reports a clear, source-located error:

Pigeon doesn't support enhanced enums ("Enum1"). Use a plain enum without a constructor, fields, methods, or arguments on its values.

Implementation notes:

  • The enum is still registered by its value names so that classes referencing it don't produce additional cascading "Unknown type" errors — the user sees exactly one actionable message, and recording the error already prevents code generation.
  • The visitor no longer walks the enhanced enum's body. Its class-like members aren't expected outside of a class by the rest of the visitor; in particular, a getter inside an enum previously crashed the parser on a null parameter list (visitMethodDeclaration's node.parameters!). One of the new tests covers this case.
  • Plain enums (including the legal trailing-semicolon form 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

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 full pigeon_lib_test.dart suite (113 tests) and version_test.dart pass locally.

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

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

Comment on lines +1760 to +1764
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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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;

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[pigeon] Enhanced enums fields are coalesced into the next class

1 participant