[pigeon] allow empty class#12181
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for empty data classes in Pigeon across Dart, Java, Kotlin, and Objective-C generators, and bumps the package version to 27.2.0. Feedback on the changes suggests adding a blank line between the constructor and the serialization method in the generated Dart code for empty classes to improve readability.
| if (classDefinition.fields.isNotEmpty) { | ||
| indent.newln(); | ||
| for (final NamedType field in getFieldsInSerializationOrder(classDefinition)) { | ||
| addDocumentationComments(indent, field.documentationComments, docCommentSpec); | ||
|
|
||
| final String datatype = addGenericTypes(field.type); | ||
| indent.writeln('$datatype ${field.name};'); | ||
| indent.newln(); | ||
| } | ||
| } |
There was a problem hiding this comment.
When generating Dart code for an empty class, there is no blank line written between the constructor and the _writeToList method. Adding a blank line here improves the readability of the generated code and aligns with Dart style guidelines for class member spacing.
| if (classDefinition.fields.isNotEmpty) { | |
| indent.newln(); | |
| for (final NamedType field in getFieldsInSerializationOrder(classDefinition)) { | |
| addDocumentationComments(indent, field.documentationComments, docCommentSpec); | |
| final String datatype = addGenericTypes(field.type); | |
| indent.writeln('$datatype ${field.name};'); | |
| indent.newln(); | |
| } | |
| } | |
| if (classDefinition.fields.isNotEmpty) { | |
| indent.newln(); | |
| for (final NamedType field in getFieldsInSerializationOrder(classDefinition)) { | |
| addDocumentationComments(indent, field.documentationComments, docCommentSpec); | |
| final String datatype = addGenericTypes(field.type); | |
| indent.writeln('$datatype ${field.name};'); | |
| indent.newln(); | |
| } | |
| } else { | |
| indent.newln(); | |
| } |
|
Looks like Linux, Windows, and Obj-C tests are all sad. |
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
LGTM with a couple of small tweaks.
Some of the output is a little silly (like the string concatenation of Foo( and ) as separate steps with nothing in between for to-string methods), but nothing that's worth adding complexity to the generators for other than the one case noted below where it's creating an unused variable.
| (nullable NSDictionary<NSNumber *, FLTAllNullableTypesWithoutRecursion *> *) | ||
| nullableClassMap; | ||
| nullableClassMap | ||
| anEmptyClass:(nullable FLTAnEmptyClass *)anEmptyClass; |
There was a problem hiding this comment.
The Obj-C test failure looks like a call site wasn't updated for this change.
| if (![object isKindOfClass:[self class]]) { | ||
| return NO; | ||
| } | ||
| FLTAnEmptyClass *other = (FLTAnEmptyClass *)object; |
There was a problem hiding this comment.
This shouldn't be output when there are no fields, since it'll trigger unused variable warnings in some projects.
| List? testList; | ||
| } | ||
|
|
||
| class AnEmptyClass {} |
There was a problem hiding this comment.
You need to move this before AllClassesWrapper because of flutter/flutter#128330. That may well be all you need to fix Linux and Windows.
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
(Now with the right radio button selected.)
2 participants
instead of preventing it from being defined, I just made it work