[pigeon] Adds non-null object fields#1191
Conversation
4a46dd9 to
57782e5
Compare
|
I'm confused by the description here; non-nullable field support was added in #549. What exactly is this PR doing? |
| 'final Map<Object$nullTag, Object$nullTag> pigeonMap = <Object$nullTag, Object$nullTag>{};', | ||
| ); | ||
| for (final NamedType field in klass.fields) { | ||
| final String nullsafe = field.type.isNullable ? '?' : ''; |
There was a problem hiding this comment.
Everywhere else in the code we use isNullSafe to declare operators, then switch on wether something is nullable or not. That's reversed here which is a bit harder to follow.
The name "nullsafe" isn't helpful.
There was a problem hiding this comment.
I've made some changes about it. Could be that?
I have the follow ideia: |
8acce59 to
ec74255
Compare
I'm not sure how that is going to get us objc and java coverage. We need to do what you did here to verify that non-null objects work correctly on the objc and java generators. |
Objc and java generators already test nullable fields. I thought: If all variables be nullable when the argument |
|
What's the status of this PR? Is it waiting for more review, or updates based on the discussion above? |
I don't know about the need of java coverage then it would be a revision if the implementation is enough |
|
Looks like the PR #1543 implements the same thing. |
|
I see. I didn't realize that the problem only existed with the Dart generator. I'll work with stuart to get his PR merged. What neither PR had was platform tests which we should have. |
This PR supports non-nullable fields of classes and enums.
Issue flutter/flutter#59118
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).