Skip to content

Commit

Permalink
fix: Fix optional enums in Nitrogen (#130)
Browse files Browse the repository at this point in the history
* fix: Fix optional enums in Nitrogen

* fix: Fix also for non-enum variants

* fix: Fix for variants
  • Loading branch information
mrousavy authored Sep 16, 2024
1 parent 51c8769 commit f317ea2
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 21 deletions.
17 changes: 11 additions & 6 deletions packages/nitrogen/src/syntax/createType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,18 @@ export function createType(type: TSMorphType, isOptional: boolean): Type {
)
return new EnumType(typename, enumDeclaration)
} else if (type.isUnion()) {
// It is some kind of union - either full of strings (then it's an enum, or different types, then it's a Variant)
const isEnumUnion = type.getUnionTypes().every((t) => t.isStringLiteral())
// It is some kind of union;
// - of string literals (then it's an enum)
// - of type `T | undefined` (then it's just optional `T`)
// - of different types (then it's a variant `A | B | C`)
const types = type.getUnionTypes()
const nonNullTypes = types.filter(
(t) => !t.isNull() && !t.isUndefined() && !t.isVoid()
)
const isEnumUnion = nonNullTypes.every((t) => t.isStringLiteral())
if (isEnumUnion) {
// It consists only of string literaly - that means it's describing an enum!
const symbol = type.getAliasSymbol()
const symbol = type.getNonNullableType().getAliasSymbol()
if (symbol == null) {
// If there is no alias, it is an inline union instead of a separate type declaration!
throw new Error(
Expand Down Expand Up @@ -258,9 +265,7 @@ export function createType(type: TSMorphType, isOptional: boolean): Type {
`Anonymous objects cannot be represented in C++! Extract "${type.getText()}" to a separate interface/type declaration.`
)
} else if (type.isStringLiteral()) {
throw new Error(
`String literal ${type.getText()} cannot be represented in C++ because it is ambiguous between a string and a discriminating union enum.`
)
return new StringType()
} else {
throw new Error(
`The TypeScript type "${type.getText()}" cannot be represented in C++!`
Expand Down
33 changes: 18 additions & 15 deletions packages/nitrogen/src/syntax/types/EnumType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,24 +50,27 @@ export class EnumType implements Type {
} else {
// It's a TS union '..' | '..'
this.jsType = 'union'
this.enumMembers = declaration.getUnionTypes().map((t, i) => {
if (t.isStringLiteral()) {
const literalValue = t.getLiteralValueOrThrow()
if (typeof literalValue !== 'string')
this.enumMembers = declaration
.getNonNullableType()
.getUnionTypes()
.map((t, i) => {
if (t.isStringLiteral()) {
const literalValue = t.getLiteralValueOrThrow()
if (typeof literalValue !== 'string')
throw new Error(
`${enumName}: Value "${literalValue}" is not a string - it is ${typeof literalValue}!`
)
return {
name: escapeCppName(literalValue).toUpperCase(),
value: i,
stringValue: literalValue,
}
} else {
throw new Error(
`${enumName}: Value "${literalValue}" is not a string - it is ${typeof literalValue}!`
`${enumName}: Value "${t.getText()}" is not a string literal - it cannot be represented in a C++ enum!`
)
return {
name: escapeCppName(literalValue).toUpperCase(),
value: i,
stringValue: literalValue,
}
} else {
throw new Error(
`${enumName}: Value "${t.getText()}" is not a string literal - it cannot be represented in a C++ enum!`
)
}
})
})
this.declarationFile = createCppUnion(enumName, this.enumMembers)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,5 +176,9 @@ namespace margelo::nitro::image {
static const auto method = _javaPart->getClass()->getMethod<void(jni::alias_ref<JFunc_void_Person::javaobject> /* callback */)>("addOnPersonBornListener");
method(_javaPart, JFunc_void_Person::fromCpp(callback));
}
void JHybridKotlinTestObjectSpec::something1(std::optional<Powertrain> optional) {
static const auto method = _javaPart->getClass()->getMethod<void(jni::alias_ref<JPowertrain> /* optional */)>("something1");
method(_javaPart, optional.has_value() ? JPowertrain::fromCpp(optional.value()) : nullptr);
}

} // namespace margelo::nitro::image
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ namespace margelo::nitro::image {
std::future<void> asyncTest() override;
std::shared_ptr<AnyMap> createMap() override;
void addOnPersonBornListener(const std::function<void(const Person& /* p */)>& callback) override;
void something1(std::optional<Powertrain> optional) override;

private:
friend HybridBase;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ abstract class HybridKotlinTestObjectSpec: HybridObject() {
val result = addOnPersonBornListener(callback.toLambda())
return result
}

@DoNotStrip
@Keep
abstract fun something1(optional: Powertrain?): Unit

private external fun initHybrid(): HybridData

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -556,5 +556,13 @@ namespace margelo::nitro::image::bridge::swift {
inline std::shared_ptr<Func_void_Person_Wrapper> share_Func_void_Person(const Func_void_Person& value) {
return std::make_shared<Func_void_Person_Wrapper>(value);
}

/**
* Specialized version of `std::optional<Powertrain>`.
*/
using std__optional_Powertrain_ = std::optional<Powertrain>;
inline std::optional<Powertrain> create_std__optional_Powertrain_(const Powertrain& value) {
return std::optional<Powertrain>(value);
}

} // namespace margelo::nitro::image::bridge::swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace margelo::nitro::image {
prototype.registerHybridMethod("asyncTest", &HybridKotlinTestObjectSpec::asyncTest);
prototype.registerHybridMethod("createMap", &HybridKotlinTestObjectSpec::createMap);
prototype.registerHybridMethod("addOnPersonBornListener", &HybridKotlinTestObjectSpec::addOnPersonBornListener);
prototype.registerHybridMethod("something1", &HybridKotlinTestObjectSpec::something1);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ namespace NitroModules { class ArrayBuffer; }
namespace NitroModules { class AnyMap; }
// Forward declaration of `Person` to properly resolve imports.
namespace margelo::nitro::image { struct Person; }
// Forward declaration of `Powertrain` to properly resolve imports.
namespace margelo::nitro::image { enum class Powertrain; }

#include <optional>
#include <vector>
Expand All @@ -32,6 +34,7 @@ namespace margelo::nitro::image { struct Person; }
#include <NitroModules/AnyMap.hpp>
#include <functional>
#include "Person.hpp"
#include "Powertrain.hpp"

namespace margelo::nitro::image {

Expand Down Expand Up @@ -75,6 +78,7 @@ namespace margelo::nitro::image {
virtual std::future<void> asyncTest() = 0;
virtual std::shared_ptr<AnyMap> createMap() = 0;
virtual void addOnPersonBornListener(const std::function<void(const Person& /* p */)>& callback) = 0;
virtual void something1(std::optional<Powertrain> optional) = 0;

protected:
// Hybrid Setup
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,5 +167,7 @@ export interface KotlinTestObject extends HybridObject<{ android: 'kotlin' }> {

addOnPersonBornListener(callback: (p: Person) => void): void

something1(optional?: Powertrain): void

someRecord: Record<string, string>
}

0 comments on commit f317ea2

Please sign in to comment.