packages
packages copied to clipboard
[pigeon] Fixes double prefixes added to enum names for Objc HostApis and FlutterApis
In some areas of the Objc generator, the the name created from _objcTypeForDartType is being passed to _enumName and both of these methods add a prefix to enums. The locations are when they are used in HostApi or FlutterApi methods, but the name of the generated enum would be correct. e.g. FLTEnumName vs FLTFLTEnumName.
This fixes the locations where this is happening by passing the AST name to _enumName instead.
Pre-launch Checklist
- [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
- [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use
dart format.) - [x] I signed the CLA.
- [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g.
[shared_preferences] - [ ] I linked to at least one issue that this PR fixes in the description above.
- [x] I updated
pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. - [x] I updated
CHANGELOG.mdto add a description of the change, following repository CHANGELOG style. - [x] I updated/added relevant documentation (doc comments with
///). - [x] I added new tests to check the change I am making, or this PR is test-exempt.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
What scenario is missing from our integration tests that would have triggered this?
@tarrinneal It looks like the scenario is a combination of:
- Using a primitive enum as a method parameter or return value
- Generates Objective-c code with a prefix
It looks like CoreTests.gen.h doesn't use a prefix. Although scenario 1 does occur for it.
Would it be a good idea to update core tests with a prefix?
Would it be a good idea to update core tests with a prefix?
yup
@tarrinneal Adding the prefix to core tests also found a location where the prefix wasn't added to the enum. I think a better solution might be to actually get rid of the _enumName method altogether and just use _ObjcType instead. It seems redundant to _ObjcType and it seems to be error prone to have both. I will try and explore this option. What are your thoughts?
Actually it looks like the problem is bigger than enums. Objective-c prefixes may need a bigger fix.
I can take this over if you'd like.
@stuartmorgan this is ready for final review