Repository navigation
[jnigen] Getters and setters - #3231
Conversation
PR HealthBreaking changes ✔️
This check can be disabled by tagging the PR with Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with
API leaks
|
| Package | Leaked API symbol | Leaking sources |
|---|---|---|
| jni | $JIterator | core_bindings.dart::JIterator::implementIn::$impl core_bindings.dart::JIterator::implement::$impl |
| jni | $JCollection | core_bindings.dart::JCollection::implementIn::$impl core_bindings.dart::JCollection::implement::$impl |
| jni | $JList | core_bindings.dart::JList::implementIn::$impl core_bindings.dart::JList::implement::$impl |
| jni | $JMap$JEntry | core_bindings.dart::JMap$JEntry::implementIn::$impl core_bindings.dart::JMap$JEntry::implement::$impl |
| jni | $JMap | core_bindings.dart::JMap::implementIn::$impl core_bindings.dart::JMap::implement::$impl |
| jni | $JSet | core_bindings.dart::JSet::implementIn::$impl core_bindings.dart::JSet::implement::$impl |
This check can be disabled by tagging the PR with skip-leaking-check.
liamappelbe
changed the title
| .constructorId("(Ljava/lang/String;)V") | ||
| .call(fileInputStreamClass, [file.toJString()]); | ||
| final pdDoc = PDDocument.load$6(inputFile)!; | ||
| int pages = pdDoc.getNumberOfPages(); |
There was a problem hiding this comment.
oh gosh, I forgot how verbose Java is 😆
|
|
||
| bool _isVoid(ReferredType t) => t is PrimitiveType && t.name == 'void'; | ||
|
|
||
| final _getterRegExp = RegExp(r'^get[A-Z]'); |
There was a problem hiding this comment.
Are there any other patterns that we should match?
Is it common in Java to have isEven() that we want do do get isEven?
LGTM to the patterns we have.
There was a problem hiding this comment.
Good point. This is a super common pattern in Java, and I think it makes sense to transform these ones too.
| } | ||
|
|
||
| enum MethodPropertyKind { | ||
| none, |
There was a problem hiding this comment.
is this a normal method instead of none?
(And I guess we don't do operator which is the 4th type of member.)
| expect(e.number, equals(0)); | ||
| expect(e.isUp, true); | ||
| expect(e.codename!.toDartString(), equals('achilles')); | ||
| e.number = 1; |
There was a problem hiding this comment.
Immediately feels like Dart 😄
| test('Misc. instance methods', () { | ||
| final e = Example(); | ||
| final rand = e.getRandom(); | ||
| final rand = e.random; |
There was a problem hiding this comment.
this one is weird, because I suspect this is not a getter but an actual generator.
But you could argue it's misnamed in Java. it should have been named nextRandom().
There was a problem hiding this comment.
I think eventually we should make this configurable. In the same way users can rename things, they should also be able to convert methods to be getters/setters, or not.
propertyKindfield toMethod, which can begetter,setter, ornone._MethodRenamer(best viewed with whitespace changes hidden)_isGetterand_isSettershow the rules for the when the transformation happens.propertyNamesis for.propertySigis the deduping key.propertyKind.idName. The old logic for naming the private_id_variables for each method caused naming collisions since getters and setters can have the same name. So the id name needs to incorperate the property kind.Fixes #612