Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compile-time checked #[ts(optional)] #367

Merged

Conversation

NyxCode
Copy link
Collaborator

@NyxCode NyxCode commented Nov 11, 2024

Builds on top of #366.

  • Get's rid of all string/path manipulation in the macro
    • As a consequence, it's impossible to break this with type aliases in all cases.
  • Produces a compile-time error when #[ts(optional)] is used on a non-Option field. The error is not great yet - it's probably possible to improve this though.
    #[ts(optional = nullable)]
    d: i32,
error[E0308]: mismatched types
  --> ts-rs/tests/integration/optional_field.rs:94:10
   |
94 | #[derive(TS)]
   |          ^^
   |          |
   |          expected `PhantomData<Option<_>>`, found `PhantomData<i32>`
   |          expected due to this
   |
   = note: expected struct `PhantomData<std::option::Option<_>>`
              found struct `PhantomData<i32>`

@NyxCode
Copy link
Collaborator Author

NyxCode commented Nov 11, 2024

This is achieved (technically semver-breaking) by adding an associated type, OptionInnerType to the TS trait. This type is set to Self for all types, except for Option<T>, where it is set to T.

With that, the behaviour of #[ts(optional)] never errors by itself. The mechanism to produce an error if it's used on a non-Option field is completely separate.

This is done by emitting this code:

use std::marker::PhantomData;
let actual: PhantomData<#ty> = PhantomData;
let must: PhantomData<Option<_>> = actual;

If #ty is not an Option, that code fails to compile, and produces the error as seen above.

@NyxCode
Copy link
Collaborator Author

NyxCode commented Nov 11, 2024

Alternative way to provoke a compile-time error:

fn a<T>(_: std::option::Option<T>) {}
fn b(x: #ty) { a(x) }

@NyxCode
Copy link
Collaborator Author

NyxCode commented Nov 11, 2024

Notably, what unfortunately does not work is something like this:

let true = <#ty as TS>::IS_OPTION;

@NyxCode
Copy link
Collaborator Author

NyxCode commented Nov 11, 2024

I massively improved the error. For

#[derive(TS)]
#[ts(export, export_to = "optional_field/", optional)]
struct OptionalStruct {
    #[ts(optional = nullable)]
    d: i32,
}

we now get

error[E0277]: `#[ts(optional)]` can only be used on fields of type `Option`
   --> ts-rs/tests/integration/optional_field.rs:104:5
    |
104 |     #[ts(optional = nullable)]
    |     ^
    |
    = help: the trait `IsOption` is not implemented for `i32`
    = note: `#[ts(optional)]` was used on a field of type i32, which is not permitted
    = help: the trait `IsOption` is implemented for `std::option::Option<T>`

There is a weird note attached to the error though, which I don't know how to get rid of:

note: required by a bound in `<OptionalStruct as TS>::inline::check_that_field_is_option`
   --> ts-rs/tests/integration/optional_field.rs:94:10
    |
94  | #[derive(TS)]
    |          ^^ required by this bound in `check_that_field_is_option`
...
104 |     #[ts(optional = nullable)]
    |     - required by a bound in this function
    = note: this error originates in the derive macro `TS` (in Nightly builds, run with -Z macro-backtrace for more info)

I think that's what rust-lang/rust#51992 will be for.

Copy link
Collaborator

@gustavo-shigueo gustavo-shigueo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dude, this is awesome! You fixed all the issues we had with Option in the first place. I love that you got the compiler to check #[ts(optional)] with a great error message

@gustavo-shigueo gustavo-shigueo merged commit dcc7a46 into add_ts_optional_to_struct Nov 11, 2024
13 of 18 checks passed
@gustavo-shigueo gustavo-shigueo deleted the struct-level-optional-checked branch November 11, 2024 01:20
@gustavo-shigueo
Copy link
Collaborator

There is a weird note attached to the error though, which I don't know how to get rid of:

note: required by a bound in `<OptionalStruct as TS>::inline::check_that_field_is_option`
   --> ts-rs/tests/integration/optional_field.rs:94:10
    |
94  | #[derive(TS)]
    |          ^^ required by this bound in `check_that_field_is_option`
...
104 |     #[ts(optional = nullable)]
    |     - required by a bound in this function
    = note: this error originates in the derive macro `TS` (in Nightly builds, run with -Z macro-backtrace for more info)

With how good the error message is overall, I don't think this is a problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants