Skip to content

Commit

Permalink
properly fix #174
Browse files Browse the repository at this point in the history
  • Loading branch information
oscartbeaumont committed Nov 23, 2023
1 parent 72c8ee7 commit 467e2b0
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 12 deletions.
6 changes: 6 additions & 0 deletions src/lang/ts/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ pub enum ExportError {
InvalidName(NamedLocation, ExportPath, String),
#[error("Attempted to export '{0}' with tagging but the type is not tagged.")]
InvalidTagging(ExportPath),
#[error("Attempted to export '{0}' with internal tagging but the variant is a tuple struct.")]
InvalidTaggedVariantContainingTupleStruct(ExportPath),
#[error("Unable to export type named '{0}' from locations '{:?}' '{:?}'", .1.as_str(), .2.as_str())]
DuplicateTypeName(Cow<'static, str>, ImplLocation, ImplLocation),
#[error("IO error: {0}")]
Expand All @@ -63,6 +65,10 @@ impl PartialEq for ExportError {
l0 == r0 && l1 == r1 && l2 == r2
}
(Self::InvalidTagging(l0), Self::InvalidTagging(r0)) => l0 == r0,
(
Self::InvalidTaggedVariantContainingTupleStruct(l0),
Self::InvalidTaggedVariantContainingTupleStruct(r0),
) => l0 == r0,
(Self::DuplicateTypeName(l0, l1, l2), Self::DuplicateTypeName(r0, r1, r2)) => {
l0 == r0 && l1 == r1 && l2 == r2
}
Expand Down
95 changes: 87 additions & 8 deletions src/lang/ts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,15 +528,24 @@ fn enum_datatype(ctx: ExportContext, e: &EnumType, type_map: &TypeMap) -> Output
format!("{{ {tag}: {sanitised_name} }}")
}
(EnumRepr::Internal { tag }, EnumVariants::Unnamed(tuple)) => {
let mut typ = unnamed_fields_datatype(
ctx.clone(),
&skip_fields(tuple.fields()).collect::<Vec<_>>(),
type_map,
)?;
let fields = skip_fields(tuple.fields()).collect::<Vec<_>>();

// This field is only required for `{ty}` not `[...]` so we only need to check when there one field
let dont_join_ty = if tuple.fields().len() == 1 {
let (_, ty) = fields.first().expect("checked length above");
validate_type_for_tagged_intersection(
ctx.clone(),
(**ty).clone(),
type_map,
)?
} else {
false
};

// TODO: This `null` check is a bad fix for an internally tagged type with a `null` variant being exported as `{ type: "A" } & null` (which is `never` in TS)
// TODO: Move this check into the macros so it can apply to any language cause it should (it's just hard to do in the macros)
if typ == "null" {
let mut typ =
unnamed_fields_datatype(ctx.clone(), &fields, type_map)?;

if dont_join_ty {
format!("({{ {tag}: {sanitised_name} }})")
} else {
// We wanna be sure `... & ... | ...` becomes `... & (... | ...)`
Expand Down Expand Up @@ -690,6 +699,76 @@ pub(crate) fn sanitise_type_name(ctx: ExportContext, loc: NamedLocation, ident:
Ok(ident.to_string())
}

fn validate_type_for_tagged_intersection(
ctx: ExportContext,
ty: DataType,
type_map: &TypeMap,
) -> Result<bool> {
match ty {
DataType::Any
| DataType::Unknown
| DataType::Primitive(_)
// `T & null` is `never` but `T & (U | null)` (this variant) is `T & U` so it's fine.
| DataType::Nullable(_)
| DataType::List(_)
| DataType::Map(_)
| DataType::Result(_)
| DataType::Generic(_) => Ok(false),
DataType::Literal(v) => match v {
LiteralType::None => Ok(true),
_ => Ok(false),
},
DataType::Struct(v) => match v.fields {
StructFields::Unit => Ok(true),
StructFields::Unnamed(_) => {
Err(ExportError::InvalidTaggedVariantContainingTupleStruct(
ctx.export_path()
))
}
StructFields::Named(fields) => {
// Prevent `{ tag: "{tag}" } & Record<string | never>`
if fields.tag.is_none() && fields.fields.len() == 0 {

Check warning on line 730 in src/lang/ts/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

length comparison to zero

warning: length comparison to zero --> src/lang/ts/mod.rs:730:44 | 730 | if fields.tag.is_none() && fields.fields.len() == 0 { | ^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `fields.fields.is_empty()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero = note: `#[warn(clippy::len_zero)]` implied by `#[warn(clippy::all)]`
return Ok(true);
}

return Ok(false);

Check warning on line 734 in src/lang/ts/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

unneeded `return` statement

warning: unneeded `return` statement --> src/lang/ts/mod.rs:734:17 | 734 | return Ok(false); | ^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return note: the lint level is defined here --> src/lib.rs:3:9 | 3 | #![warn(clippy::all, clippy::unwrap_used, clippy::panic)] // TODO: missing_docs | ^^^^^^^^^^^ = note: `#[warn(clippy::needless_return)]` implied by `#[warn(clippy::all)]` help: remove `return` | 734 - return Ok(false); 734 + Ok(false) |
}
},
DataType::Enum(v) => {
match v.repr {
EnumRepr::Untagged => {
Ok(v.variants.iter().any(|(_, v)| match &v.inner {
// `{ .. } & null` is `never`
EnumVariants::Unit => true,
// `{ ... } & Record<string, never>` is not useful
EnumVariants::Named(v) => v.tag.is_none() && v.fields().len() == 0,

Check warning on line 744 in src/lang/ts/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

length comparison to zero

warning: length comparison to zero --> src/lang/ts/mod.rs:744:70 | 744 | EnumVariants::Named(v) => v.tag.is_none() && v.fields().len() == 0, | ^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `v.fields().is_empty()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
EnumVariants::Unnamed(_) => false,
}))
},
// All of these repr's are always objects.
EnumRepr::Internal { .. } | EnumRepr::Adjacent { .. } | EnumRepr::External => Ok(false),
}
}
DataType::Tuple(v) => {
// Empty tuple is `null`
if v.elements.len() == 0 {

Check warning on line 754 in src/lang/ts/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

length comparison to zero

warning: length comparison to zero --> src/lang/ts/mod.rs:754:16 | 754 | if v.elements.len() == 0 { | ^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `v.elements.is_empty()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
return Ok(true);
}

Ok(false)
}
DataType::Reference(r) => validate_type_for_tagged_intersection(
ctx,
type_map
.get(r.sid)
.expect("TypeMap should have been populated by now")
.inner
.clone(),
type_map,
),
}
}

const ANY: &str = "any";
const UNKNOWN: &str = "unknown";
const NUMBER: &str = "number";
Expand Down
2 changes: 1 addition & 1 deletion tests/remote_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@ fn typescript_types_bevy_ecs() {
ts::export::<bevy_ecs::entity::Entity>(
&ExportConfig::default().bigint(BigIntExportBehavior::Number)
),
Ok("export Entity = number;".into())
Ok("export type Entity = number".into())
);
}
66 changes: 64 additions & 2 deletions tests/serde/empty_enum.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use specta::Type;
use specta::{
ts::{ExportError, ExportPath},
Type,
};

use crate::ts::assert_ts;

Expand All @@ -18,11 +21,70 @@ enum C {}
#[specta(export = false, untagged)]
enum D {}

#[derive(Type)]
#[specta(export = false)]
pub struct Inner;

#[derive(Type)]
#[specta(export = false)]
pub struct Inner2 {}

#[derive(Type)]
#[specta(export = false)]
pub struct Inner3();

#[derive(Type)]
#[specta(export = false, tag = "a")]
enum E {
A(Inner),
B(Inner),
}

#[derive(Type)]
#[specta(export = false, tag = "a")]
enum F {
A(Inner2),
B(Inner2),
}

#[derive(Type)]
#[specta(export = false, tag = "a")]
enum G {
A(Inner3),
B(Inner3),
}

#[derive(Type)]
#[specta(export = false, tag = "a")]
enum H {
#[specta(skip)]
A(Inner3),
B(Inner2),
}

#[derive(Type)]
#[specta(transparent)]
pub struct Demo(());

#[derive(Type)]
#[specta(export = false, tag = "a")]
enum I {
A(Demo),
B(Demo),
}

// https://github.com/oscartbeaumont/specta/issues/174
#[test]
fn empty_enums() {
// `never & { tag = "a" }` would collease to `never` so we don't need to include it.
// `never & { tag = "a" }` would coalesce to `never` so we don't need to include it.
assert_ts!(A, "never");
assert_ts!(B, "never");
assert_ts!(C, "never");
assert_ts!(D, "never");

assert_ts!(E, "({ a: \"A\" }) | ({ a: \"B\" })");
assert_ts!(F, "({ a: \"A\" }) | ({ a: \"B\" })");
assert_ts!(error; G, ExportError::InvalidTaggedVariantContainingTupleStruct(ExportPath::new_unsafe("G")));
assert_ts!(H, "({ a: \"B\" })");
assert_ts!(I, "({ a: \"A\" }) | ({ a: \"B\" })");
}
1 change: 1 addition & 0 deletions tests/serde/empty_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ struct A {}
#[specta(export = false, tag = "a")]
struct B {}

// https://github.com/oscartbeaumont/specta/issues/174
#[test]
fn empty_enums() {
assert_ts!(A, "Record<string, never>");
Expand Down
2 changes: 1 addition & 1 deletion tests/serde/internally_tagged.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn internally_tagged() {
assert_ts!(E, "({ type: \"A\" })");
assert_ts!(F, "({ type: \"A\" } & FInner)");
assert_ts!(error; G, SerdeError::InvalidInternallyTaggedEnum);
assert_ts!(H, "({ type: \"A\" } & HInner)");
assert_ts!(H, "({ type: \"A\" })");
assert_ts!(error; I, SerdeError::InvalidInternallyTaggedEnum);
assert_ts!(L, "({ type: \"A\" } & ({ type: \"A\" } | { type: \"B\" }))");
assert_ts!(M, "({ type: \"A\" })");
Expand Down

0 comments on commit 467e2b0

Please sign in to comment.