Skip to content

Commit

Permalink
Add SyntaxErrors in GlobalDeclarationInstantiation (#2908)
Browse files Browse the repository at this point in the history 
* Add SyntaxErrors in GlobalDeclarationInstantiation

* Fix tests

* Apply suggestions
  • Loading branch information
@raskad
raskad committed May 7, 2023
1 parent 484cc16 commit 719b5a1
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 78 deletions.
18 changes: 9 additions & 9 deletions boa_engine/src/builtins/string/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -569,25 +569,25 @@ fn split_with_symbol_split_method() {
TestAction::run_harness(),
TestAction::assert_eq(
indoc! {r#"
let sep = {};
sep[Symbol.split] = function(s, limit) { return s + limit.toString(); };
'hello'.split(sep, 10)
let sep_a = {};
sep_a[Symbol.split] = function(s, limit) { return s + limit.toString(); };
'hello'.split(sep_a, 10)
"#},
"hello10",
),
TestAction::assert(indoc! {r#"
let sep = {};
sep[Symbol.split] = undefined;
let sep_b = {};
sep_b[Symbol.split] = undefined;
arrayEquals(
'hello'.split(sep),
'hello'.split(sep_b),
['hello']
)
"#}),
TestAction::assert_native_error(
indoc! {r#"
let sep = {};
sep[Symbol.split] = 10;
'hello'.split(sep, 10);
let sep_c = {};
sep_c[Symbol.split] = 10;
'hello'.split(sep_c, 10);
"#},
JsNativeErrorKind::Type,
"value returned for property of object is not a function",
Expand Down
40 changes: 34 additions & 6 deletions boa_engine/src/bytecompiler/declarations.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,44 @@ impl ByteCompiler<'_, '_> {
&mut self,
script: &StatementList,
) -> JsResult<()> {
// Note: Step 1-4 Are currently handled while parsing.
// 1. Let lexNames be the LexicallyDeclaredNames of script.
let lex_names = top_level_lexically_declared_names(script);

// 2. Let varNames be the VarDeclaredNames of script.
let var_names = top_level_var_declared_names(script);

// 3. For each element name of lexNames, do
// a. If env.HasVarDeclaration(name) is true, throw a SyntaxError exception.
// b. If env.HasLexicalDeclaration(name) is true, throw a SyntaxError exception.
// c. Let hasRestrictedGlobal be ? env.HasRestrictedGlobalProperty(name).
// d. If hasRestrictedGlobal is true, throw a SyntaxError exception.
for name in lex_names {
// Note: Our implementation differs from the spec here.
// a. If env.HasVarDeclaration(name) is true, throw a SyntaxError exception.

// b. If env.HasLexicalDeclaration(name) is true, throw a SyntaxError exception.
if self.has_binding(name) {
return Err(JsNativeError::syntax()
.with_message("duplicate lexical declaration")
.into());
}

// c. Let hasRestrictedGlobal be ? env.HasRestrictedGlobalProperty(name).
let has_restricted_global = self.context.has_restricted_global_property(name)?;

// d. If hasRestrictedGlobal is true, throw a SyntaxError exception.
if has_restricted_global {
return Err(JsNativeError::syntax()
.with_message("cannot redefine non-configurable global property")
.into());
}
}

// 4. For each element name of varNames, do
// a. If env.HasLexicalDeclaration(name) is true, throw a SyntaxError exception.
for name in var_names {
// a. If env.HasLexicalDeclaration(name) is true, throw a SyntaxError exception.
if self.has_binding(name) {
return Err(JsNativeError::syntax()
.with_message("duplicate lexical declaration")
.into());
}
}

// 5. Let varDeclarations be the VarScopedDeclarations of script.
// Note: VarScopedDeclarations for a Script node is TopLevelVarScopedDeclarations.
Expand Down
29 changes: 29 additions & 0 deletions boa_engine/src/context/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,35 @@ impl Context<'_> {
// 9. Return unused.
Ok(())
}

/// `HasRestrictedGlobalProperty ( N )`
///
/// More information:
/// - [ECMAScript reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-hasrestrictedglobalproperty
pub(crate) fn has_restricted_global_property(&mut self, name: Identifier) -> JsResult<bool> {
// 1. Let ObjRec be envRec.[[ObjectRecord]].
// 2. Let globalObject be ObjRec.[[BindingObject]].
let global_object = self.realm().global_object().clone();

// 3. Let existingProp be ? globalObject.[[GetOwnProperty]](N).
let name = PropertyKey::from(self.interner().resolve_expect(name.sym()).utf16());
let existing_prop = global_object.__get_own_property__(&name, self)?;

// 4. If existingProp is undefined, return false.
let Some(existing_prop) = existing_prop else {
return Ok(false);
};

// 5. If existingProp.[[Configurable]] is true, return false.
if existing_prop.configurable() == Some(true) {
return Ok(false);
}

// 6. Return true.
Ok(true)
}
}

impl<'host> Context<'host> {
Expand Down
76 changes: 38 additions & 38 deletions boa_engine/src/tests/operators.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,25 @@ fn short_circuit_evaluation() {
// short circuiting OR.
TestAction::assert_eq(
indoc! {r#"
function add_one(counter) {
function add_one_a(counter) {
counter.value += 1;
return true;
}
let counter = { value: 0 };
let _ = add_one(counter) || add_one(counter);
counter.value
let counter_a = { value: 0 };
add_one_a(counter_a) || add_one_a(counter_a);
counter_a.value
"#},
1,
),
TestAction::assert_eq(
indoc! {r#"
function add_one(counter) {
function add_one_b(counter) {
counter.value += 1;
return false;
}
let counter = { value: 0 };
let _ = add_one(counter) || add_one(counter);
counter.value
let counter_b = { value: 0 };
add_one_b(counter_b) || add_one_b(counter_b);
counter_b.value
"#},
2,
),
Expand All @@ -55,25 +55,25 @@ fn short_circuit_evaluation() {
// short circuiting AND
TestAction::assert_eq(
indoc! {r#"
function add_one(counter) {
function add_one_c(counter) {
counter.value += 1;
return true;
}
let counter = { value: 0 };
let _ = add_one(counter) && add_one(counter);
counter.value
let counter_c = { value: 0 };
add_one_c(counter_c) && add_one_c(counter_c);
counter_c.value
"#},
2,
),
TestAction::assert_eq(
indoc! {r#"
function add_one(counter) {
function add_one_d(counter) {
counter.value += 1;
return false;
}
let counter = { value: 0 };
let _ = add_one(counter) && add_one(counter);
counter.value
let counter_d = { value: 0 };
add_one_d(counter_d) && add_one_d(counter_d);
counter_d.value
"#},
1,
),
Expand Down Expand Up @@ -114,24 +114,24 @@ fn assign_operator_precedence() {
fn unary_pre() {
run_test_actions([
TestAction::assert_eq("{ let a = 5; ++a; a }", 6),
TestAction::assert_eq("{ let a = 5; --a; a }", 4),
TestAction::assert_eq("{ const a = { b: 5 }; ++a.b; a['b'] }", 6),
TestAction::assert_eq("{ const a = { b: 5 }; --a['b']; a.b }", 4),
TestAction::assert_eq("{ let a = 5; ++a }", 6),
TestAction::assert_eq("{ let a = 5; --a }", 4),
TestAction::assert_eq("{ let a = 2147483647; ++a }", 2_147_483_648_i64),
TestAction::assert_eq("{ let a = -2147483648; --a }", -2_147_483_649_i64),
TestAction::assert_eq("{ let b = 5; --b; b }", 4),
TestAction::assert_eq("{ const c = { a: 5 }; ++c.a; c['a'] }", 6),
TestAction::assert_eq("{ const d = { a: 5 }; --d['a']; d.a }", 4),
TestAction::assert_eq("{ let e = 5; ++e }", 6),
TestAction::assert_eq("{ let f = 5; --f }", 4),
TestAction::assert_eq("{ let g = 2147483647; ++g }", 2_147_483_648_i64),
TestAction::assert_eq("{ let h = -2147483648; --h }", -2_147_483_649_i64),
TestAction::assert_eq(
indoc! {r#"
let a = {[Symbol.toPrimitive]() { return 123; }};
++a
let i = {[Symbol.toPrimitive]() { return 123; }};
++i
"#},
124,
),
TestAction::assert_eq(
indoc! {r#"
let a = {[Symbol.toPrimitive]() { return 123; }};
++a
let j = {[Symbol.toPrimitive]() { return 123; }};
++j
"#},
124,
),
Expand Down Expand Up @@ -210,24 +210,24 @@ fn typeofs() {
fn unary_post() {
run_test_actions([
TestAction::assert_eq("{ let a = 5; a++; a }", 6),
TestAction::assert_eq("{ let a = 5; a--; a }", 4),
TestAction::assert_eq("{ const a = { b: 5 }; a.b++; a['b'] }", 6),
TestAction::assert_eq("{ const a = { b: 5 }; a['b']--; a.b }", 4),
TestAction::assert_eq("{ let a = 5; a++ }", 5),
TestAction::assert_eq("{ let a = 5; a-- }", 5),
TestAction::assert_eq("{ let a = 2147483647; a++; a }", 2_147_483_648_i64),
TestAction::assert_eq("{ let a = -2147483648; a--; a }", -2_147_483_649_i64),
TestAction::assert_eq("{ let b = 5; b--; b }", 4),
TestAction::assert_eq("{ const c = { a: 5 }; c.a++; c['a'] }", 6),
TestAction::assert_eq("{ const d = { a: 5 }; d['a']--; d.a }", 4),
TestAction::assert_eq("{ let e = 5; e++ }", 5),
TestAction::assert_eq("{ let f = 5; f-- }", 5),
TestAction::assert_eq("{ let g = 2147483647; g++; g }", 2_147_483_648_i64),
TestAction::assert_eq("{ let h = -2147483648; h--; h }", -2_147_483_649_i64),
TestAction::assert_eq(
indoc! {r#"
let a = {[Symbol.toPrimitive]() { return 123; }};
a++
let i = {[Symbol.toPrimitive]() { return 123; }};
i++
"#},
123,
),
TestAction::assert_eq(
indoc! {r#"
let a = {[Symbol.toPrimitive]() { return 123; }};
a--
let j = {[Symbol.toPrimitive]() { return 123; }};
j--
"#},
123,
),
Expand Down
45 changes: 21 additions & 24 deletions boa_tester/src/exec/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ impl Test {
(true, value.display().to_string())
}
Outcome::Negative {
phase: Phase::Parse | Phase::Early,
phase: Phase::Parse,
error_type,
} => {
assert_eq!(
Expand All @@ -259,18 +259,12 @@ impl Test {

if self.is_module() {
match context.parse_module(source) {
Ok(module_item_list) => match context.compile_module(&module_item_list) {
Ok(_) => (false, "ModuleItemList compilation should fail".to_owned()),
Err(e) => (true, format!("Uncaught {e:?}")),
},
Ok(_) => (false, "ModuleItemList parsing should fail".to_owned()),
Err(e) => (true, format!("Uncaught {e}")),
}
} else {
match context.parse_script(source) {
Ok(statement_list) => match context.compile_script(&statement_list) {
Ok(_) => (false, "StatementList compilation should fail".to_owned()),
Err(e) => (true, format!("Uncaught {e:?}")),
},
Ok(_) => (false, "StatementList parsing should fail".to_owned()),
Err(e) => (true, format!("Uncaught {e}")),
}
}
Expand All @@ -290,30 +284,33 @@ impl Test {
if let Err(e) = self.set_up_env(harness, context, AsyncResult::default()) {
return (false, e);
}
let code = if self.is_module() {
match context
.parse_module(source)
.map_err(Into::into)
.and_then(|stmts| context.compile_module(&stmts))
{

let e = if self.is_module() {
let module = match context.parse_module(source) {
Ok(code) => code,
Err(e) => return (false, format!("Uncaught {e}")),
}
} else {
};
match context
.parse_script(source)
.map_err(Into::into)
.and_then(|stmts| context.compile_script(&stmts))
.compile_module(&module)
.and_then(|code| context.execute(code))
{
Ok(_) => return (false, "Module execution should fail".to_owned()),
Err(e) => e,
}
} else {
let script = match context.parse_script(source) {
Ok(code) => code,
Err(e) => return (false, format!("Uncaught {e}")),
};
match context
.compile_script(&script)
.and_then(|code| context.execute(code))
{
Ok(_) => return (false, "Script execution should fail".to_owned()),
Err(e) => e,
}
};

let e = match context.execute(code) {
Ok(res) => return (false, res.display().to_string()),
Err(e) => e,
};
if let Ok(e) = e.try_native(context) {
match &e.kind {
JsNativeErrorKind::Syntax if error_type == ErrorType::SyntaxError => {}
Expand Down
1 change: 0 additions & 1 deletion boa_tester/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,6 @@ impl<'de> Deserialize<'de> for TestFlags {
#[serde(rename_all = "lowercase")]
enum Phase {
Parse,
Early,
Resolution,
Runtime,
}
Expand Down

0 comments on commit 719b5a1

Please sign in to comment.