Skip to content

Commit 761a9d3

Browse files
committed
fix(nitrogen): match Kotlin's is prefix rule for JVM accessors
Kotlin only shortens a property's JVM accessor names when `is` is not followed by a lowercase letter, and it applies that rule to every type, not just booleans (`JvmAbi.startsWithIsPrefix`). Nitrogen used a plain `startsWith('is')` check that was additionally gated on the property being a boolean, so for names such as `isolatedBoolean` or for a non-boolean `isTextValue` it generated JNI lookups for methods the Kotlin class does not expose, crashing at runtime on Android. Also covers the two shapes with `isolatedBoolean` and `isTextValue` on `SharedTestObjectProps` plus assertions in `getTests.ts`.
1 parent 94e72e6 commit 761a9d3

17 files changed

Lines changed: 163 additions & 37 deletions

‎example/src/getTests.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,22 @@ export function getTests(
551551
.didNotThrow()
552552
.equals(true)
553553
),
554+
createTest('set + get isolatedBoolean', () =>
555+
it(() => {
556+
testObject.isolatedBoolean = true
557+
return testObject.isolatedBoolean
558+
})
559+
.didNotThrow()
560+
.equals(true)
561+
),
562+
createTest('set + get isTextValue', () =>
563+
it(() => {
564+
testObject.isTextValue = 'hello'
565+
return testObject.isTextValue
566+
})
567+
.didNotThrow()
568+
.equals('hello')
569+
),
554570
createTest('set optionalCallback, then undefined', () =>
555571
it(() => {
556572
testObject.optionalCallback = () => {}

‎packages/nitrogen/src/syntax/Property.ts‎

Lines changed: 30 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ export interface PropertyBody {
1414

1515
export type LanguageEnvironment = 'jvm' | 'swift' | 'other'
1616

17+
// Kotlin only shortens accessors when `is` is not followed by a lowercase letter
18+
// (see `JvmAbi.startsWithIsPrefix`), and it does that for every type - not just booleans.
19+
function hasJvmIsPrefix(name: string): boolean {
20+
if (!name.startsWith('is') || name.length === 2) return false
21+
const next = name.charAt(2)
22+
return next < 'a' || next > 'z'
23+
}
24+
1725
export interface PropertyModifiers {
1826
/**
1927
* The name of the class that defines this C++ property getter/setter method.
@@ -72,49 +80,34 @@ export class Property implements CodeNode {
7280
}
7381

7482
getGetterName(environment: LanguageEnvironment): string {
75-
if (this.type.kind === 'boolean') {
76-
// Boolean accessors where the property starts with "is" or "has" are renamed in JVM and Swift
77-
switch (environment) {
78-
case 'jvm':
79-
if (this.name.startsWith('is')) {
80-
// isSomething -> isSomething()
81-
return this.name
82-
} else {
83-
break
84-
}
85-
case 'swift':
86-
if (this.name.startsWith('is')) {
87-
// isSomething -> isSomething()
88-
return this.name
89-
} else if (this.name.startsWith('has')) {
90-
// hasSomething -> hasSomething()
91-
return this.name
92-
} else {
93-
break
94-
}
95-
default:
96-
break
97-
}
83+
switch (environment) {
84+
case 'jvm':
85+
if (hasJvmIsPrefix(this.name)) {
86+
// isSomething -> isSomething()
87+
return this.name
88+
}
89+
break
90+
case 'swift':
91+
// Swift only shortens boolean accessors that start with "is" or "has"
92+
if (
93+
this.type.kind === 'boolean' &&
94+
(this.name.startsWith('is') || this.name.startsWith('has'))
95+
) {
96+
// isSomething -> isSomething()
97+
return this.name
98+
}
99+
break
100+
default:
101+
break
98102
}
99103
// isSomething -> getIsSomething()
100104
return `get${capitalizeName(this.name)}`
101105
}
102106

103107
getSetterName(environment: LanguageEnvironment): string {
104-
if (this.type.kind === 'boolean') {
105-
// Boolean accessors where the property starts with "is" are renamed in JVM
106-
switch (environment) {
107-
case 'jvm':
108-
if (this.name.startsWith('is')) {
109-
// isSomething -> setSomething()
110-
const cleanName = this.name.replace('is', '')
111-
return `set${capitalizeName(cleanName)}`
112-
} else {
113-
break
114-
}
115-
default:
116-
break
117-
}
108+
if (environment === 'jvm' && hasJvmIsPrefix(this.name)) {
109+
// isSomething -> setSomething()
110+
return `set${capitalizeName(this.name.slice(2))}`
118111
}
119112
// isSomething -> setIsSomething()
120113
return `set${capitalizeName(this.name)}`

‎packages/react-native-nitro-test/android/src/main/java/com/margelo/nitro/test/HybridTestObjectKotlin.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ class HybridTestObjectKotlin : HybridTestObjectSwiftKotlinSpec() {
4343
override val isBoolean = false
4444
override var hasBooleanWritable = false
4545
override var isBooleanWritable = false
46+
override var isolatedBoolean = false
47+
override var isTextValue = ""
4648

4749
override fun simpleFunc() {
4850
// do nothing

‎packages/react-native-nitro-test/cpp/HybridTestObjectCpp.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,22 @@ void HybridTestObjectCpp::setHasBooleanWritable(bool hasBooleanWritable) {
183183
_hasBooleanWritable = hasBooleanWritable;
184184
}
185185

186+
bool HybridTestObjectCpp::getIsolatedBoolean() {
187+
return _isolatedBoolean;
188+
}
189+
190+
void HybridTestObjectCpp::setIsolatedBoolean(bool isolatedBoolean) {
191+
_isolatedBoolean = isolatedBoolean;
192+
}
193+
194+
std::string HybridTestObjectCpp::getIsTextValue() {
195+
return _isTextValue;
196+
}
197+
198+
void HybridTestObjectCpp::setIsTextValue(const std::string& isTextValue) {
199+
_isTextValue = isTextValue;
200+
}
201+
186202
void HybridTestObjectCpp::setOptionalCallback(const std::optional<std::function<void(double)>>& callback) {
187203
_optionalCallback = callback;
188204
}

‎packages/react-native-nitro-test/cpp/HybridTestObjectCpp.hpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ class HybridTestObjectCpp : public HybridTestObjectCppSpec {
4545
std::optional<std::function<void(double)>> _optionalCallback;
4646
bool _hasBooleanWritable;
4747
bool _isBooleanWritable;
48+
bool _isolatedBoolean;
49+
std::string _isTextValue;
4850

4951
private:
5052
static inline uint64_t calculateFibonacci(int count) noexcept {
@@ -103,6 +105,10 @@ class HybridTestObjectCpp : public HybridTestObjectCppSpec {
103105
void setIsBooleanWritable(bool isBooleanWritable) override;
104106
bool getHasBooleanWritable() override;
105107
void setHasBooleanWritable(bool hasBooleanWritable) override;
108+
bool getIsolatedBoolean() override;
109+
void setIsolatedBoolean(bool isolatedBoolean) override;
110+
std::string getIsTextValue() override;
111+
void setIsTextValue(const std::string& isTextValue) override;
106112

107113
public:
108114
// Methods

‎packages/react-native-nitro-test/ios/HybridTestObjectSwift.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ class HybridTestObjectSwift: HybridTestObjectSwiftKotlinSpec {
4949
let isBoolean = false
5050
var hasBooleanWritable = false
5151
var isBooleanWritable = false
52+
var isolatedBoolean = false
53+
var isTextValue = ""
5254

5355
var thisObject: any HybridTestObjectSwiftKotlinSpec {
5456
return self

‎packages/react-native-nitro-test/nitrogen/generated/android/c++/JHybridTestObjectSwiftKotlinSpec.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,24 @@ namespace margelo::nitro::test {
364364
static const auto method = _javaPart->javaClassStatic()->getMethod<void(jboolean /* isBooleanWritable */)>("setBooleanWritable");
365365
method(_javaPart, isBooleanWritable);
366366
}
367+
bool JHybridTestObjectSwiftKotlinSpec::getIsolatedBoolean() {
368+
static const auto method = _javaPart->javaClassStatic()->getMethod<jboolean()>("getIsolatedBoolean");
369+
auto __result = method(_javaPart);
370+
return static_cast<bool>(__result);
371+
}
372+
void JHybridTestObjectSwiftKotlinSpec::setIsolatedBoolean(bool isolatedBoolean) {
373+
static const auto method = _javaPart->javaClassStatic()->getMethod<void(jboolean /* isolatedBoolean */)>("setIsolatedBoolean");
374+
method(_javaPart, isolatedBoolean);
375+
}
376+
std::string JHybridTestObjectSwiftKotlinSpec::getIsTextValue() {
377+
static const auto method = _javaPart->javaClassStatic()->getMethod<jni::local_ref<jni::JString>()>("isTextValue");
378+
auto __result = method(_javaPart);
379+
return __result->toStdString();
380+
}
381+
void JHybridTestObjectSwiftKotlinSpec::setIsTextValue(const std::string& isTextValue) {
382+
static const auto method = _javaPart->javaClassStatic()->getMethod<void(jni::alias_ref<jni::JString> /* isTextValue */)>("setTextValue");
383+
method(_javaPart, jni::make_jstring(isTextValue));
384+
}
367385
std::variant<double, std::string> JHybridTestObjectSwiftKotlinSpec::getSomeVariant() {
368386
static const auto method = _javaPart->javaClassStatic()->getMethod<jni::local_ref<JVariant_Double_String>()>("getSomeVariant");
369387
auto __result = method(_javaPart);

‎packages/react-native-nitro-test/nitrogen/generated/android/c++/JHybridTestObjectSwiftKotlinSpec.hpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ namespace margelo::nitro::test {
8787
void setHasBooleanWritable(bool hasBooleanWritable) override;
8888
bool getIsBooleanWritable() override;
8989
void setIsBooleanWritable(bool isBooleanWritable) override;
90+
bool getIsolatedBoolean() override;
91+
void setIsolatedBoolean(bool isolatedBoolean) override;
92+
std::string getIsTextValue() override;
93+
void setIsTextValue(const std::string& isTextValue) override;
9094
std::variant<double, std::string> getSomeVariant() override;
9195
void setSomeVariant(const std::variant<double, std::string>& someVariant) override;
9296

‎packages/react-native-nitro-test/nitrogen/generated/android/kotlin/com/margelo/nitro/test/HybridTestObjectSwiftKotlinSpec.kt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,18 @@ abstract class HybridTestObjectSwiftKotlinSpec: HybridObject() {
161161
@set:Keep
162162
abstract var isBooleanWritable: Boolean
163163

164+
@get:DoNotStrip
165+
@get:Keep
166+
@set:DoNotStrip
167+
@set:Keep
168+
abstract var isolatedBoolean: Boolean
169+
170+
@get:DoNotStrip
171+
@get:Keep
172+
@set:DoNotStrip
173+
@set:Keep
174+
abstract var isTextValue: String
175+
164176
@get:DoNotStrip
165177
@get:Keep
166178
@set:DoNotStrip

‎packages/react-native-nitro-test/nitrogen/generated/ios/c++/HybridTestObjectSwiftKotlinSpecSwift.hpp‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,19 @@ namespace margelo::nitro::test {
262262
inline void setIsBooleanWritable(bool isBooleanWritable) noexcept override {
263263
_swiftPart.setIsBooleanWritable(std::forward<decltype(isBooleanWritable)>(isBooleanWritable));
264264
}
265+
inline bool getIsolatedBoolean() noexcept override {
266+
return _swiftPart.isolatedBoolean();
267+
}
268+
inline void setIsolatedBoolean(bool isolatedBoolean) noexcept override {
269+
_swiftPart.setIsolatedBoolean(std::forward<decltype(isolatedBoolean)>(isolatedBoolean));
270+
}
271+
inline std::string getIsTextValue() noexcept override {
272+
auto __result = _swiftPart.getIsTextValue();
273+
return __result;
274+
}
275+
inline void setIsTextValue(const std::string& isTextValue) noexcept override {
276+
_swiftPart.setIsTextValue(isTextValue);
277+
}
265278
inline std::variant<double, std::string> getSomeVariant() noexcept override {
266279
auto __result = _swiftPart.getSomeVariant();
267280
return __result;

0 commit comments

Comments
 (0)