Skip to content

[pigeon] updates toString and isNullish methods#11625

Open
tarrinneal wants to merge 2 commits intoflutter:mainfrom
tarrinneal:toString
Open

[pigeon] updates toString and isNullish methods#11625
tarrinneal wants to merge 2 commits intoflutter:mainfrom
tarrinneal:toString

Conversation

@tarrinneal
Copy link
Copy Markdown
Contributor

prequel pr to #11352 to land non NI changes to simplify pr.

Copy link
Copy Markdown

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

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 26.4.0, introducing automatic generation of toString (or equivalent) methods for data classes across all supported languages. It also refines the Swift isNullish utility to handle nested optional values. Review feedback highlights a critical logic error in the C++ generator that results in unbalanced braces and corrupted code, along with a compilation issue when handling C++ enums. Additionally, for Swift, the feedback identifies a redundant protocol conformance error in subclasses and suggests using String(describing:) to prevent compiler warnings during string interpolation of optional fields.

Comment thread packages/pigeon/lib/src/cpp/cpp_generator.dart Outdated
Comment on lines +1161 to +1171
indent.writeln('ss << *$name;');
}
});
indent.nest(1, () {
indent.writeln('ss << "null";');
});
} else {
if (field.type.isClass) {
indent.writeln('ss << $name.ToString();');
} else {
indent.writeln('ss << $name;');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

C++ enum class types do not support the << operator for std::ostream by default. If a data class contains an enum field, the generated ToString method will fail to compile. You should check if the field is an enum and cast it to its underlying integer type before appending it to the stream.

Comment thread packages/pigeon/lib/src/swift/swift_generator.dart Outdated
final Iterable<String> fieldStrings = classDefinition.fields.map((
NamedType field,
) {
return '${field.name}: \\(${field.name})';
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

In Swift, string interpolation of optional values triggers a compiler warning (e.g., "Expression implicitly coerced from 'String?' to 'Any'"). It is recommended to wrap optional fields in String(describing:) to silence this warning and provide a consistent string representation.

Suggested change
return '${field.name}: \\(${field.name})';
return '${field.name}: \(String(describing: ${field.name}))';

@github-actions github-actions Bot removed the CICD Run CI/CD label May 1, 2026
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.

1 participant