Restrict Direct Construction of `JsValue` to Ensure Invariants

[HalidOdat]

The problem lies in the ability for users to manually construct a JsValue::Rational from a f64 value that fits within an i32, as JsValue is a public enum.

A proposed solution is to introduce a struct that encapsulates the JsValue enum. By doing so, construction of JsValue::Rational would be limited to its respective constructor functions.

This would allow us to remove many checks that are done to see if a JsValue::Rational f64 value can fit in a i32.

Additional Information

Related to #1373 , something similar was done in #1830 here

[MartiPresa]

Is this issue still available?

[jedel1043]

@MartiPresa Sure! Let me assign it to you.

[jedel1043]

As a small guidance, the enum you need to work on is here: https://github.com/boa-dev/boa/blob/848103ae3fbf47b4224679adcb14350344920140/core/engine/src/value/mod.rs#L62-L81

Ideally, the new repr of JsValue would be:

#[derive(Trace, Finalize, Debug, Clone)]
struct JsValue {
    inner: ValueRepr
}

#[derive(Finalize, Debug, Clone)]
enum ValueRepr {
    Null,
    Undefined,
    /* ... rest of the variants ... */
}

Then, JsValue would need to expose a method to get an enum of the inner variant:

impl JsValue {
    pub variant(&self) -> JsValueVariant<'_> {
        /// Convert from &ValueRepr to JsValueVariant<'_>
    }
}

The rest is fixing all the compilation errors related to the new API, like replacing direct matches of JsValue variables to calls to variant() which match on JsValueVariant.

[MartiPresa]

I will start checking on this. Thanx!

El lun, 8 abr 2024 a la(s) 4:40 p.m., José Julián Espina ( @.***) escribió:

As a small guidance, the enum you need to work on is here:

https://github.com/boa-dev/boa/blob/848103ae3fbf47b4224679adcb14350344920140/core/engine/src/value/mod.rs#L62-L81

Ideally, the new repr of JsValue would be:

#[derive(Trace, Finalize, Debug, Clone)]struct JsValue { inner: ValueRepr} #[derive(Finalize, Debug, Clone)]enum ValueRepr { Null, Undefined, /* ... rest of the variants ... */}

Then, JsValue would need to expose a method to get an enum of the inner variant:

impl JsValue { pub variant(&self) -> JsValueVariant<'>{ /// Convert from ValueRepr to JsValueVariant<'> }}

The rest is fixing all the compilation errors related to the new API, like replacing direct matches of JsValue variables to matches on JsValueVariant and calls to variant().

— Reply to this email directly, view it on GitHub https://github.com/boa-dev/boa/issues/3761#issuecomment-2043516573, or unsubscribe https://github.com/notifications/unsubscribe-auth/AYQMU3JRGTZX4IGZKCEBCH3Y4LXCBAVCNFSM6AAAAABFFCKW2WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBTGUYTMNJXGM . You are receiving this because you were assigned.Message ID: @.***>