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:

boa/core/engine/src/value/mod.rs

Lines 62 to 81 in 848103a

	pub enum JsValue {
	/// `null` - A null value, for when a value doesn't exist.
	Null,
	/// `undefined` - An undefined value, for when a field or index doesn't exist.
	Undefined,
	/// `boolean` - A `true` / `false` value, for if a certain criteria is met.
	Boolean(bool),
	/// `String` - A UTF-16 string, such as `"Hello, world"`.
	String(JsString),
	/// `Number` - A 64-bit floating point number, such as `3.1415`
	Rational(f64),
	/// `Number` - A 32-bit integer, such as `42`.
	Integer(i32),
	/// `BigInt` - holds any arbitrary large signed integer.
	BigInt(JsBigInt),
	/// `Object` - An object, such as `Math`, represented by a binary tree of string keys to Javascript values.
	Object(JsObject),
	/// `Symbol` - A Symbol Primitive type.
	Symbol(JsSymbol),
	}

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 <#3761 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AYQMU3JRGTZX4IGZKCEBCH3Y4LXCBAVCNFSM6AAAAABFFCKW2WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBTGUYTMNJXGM> . You are receiving this because you were assigned.Message ID: ***@***.***>