Skip to content

Spec alignment create method property#5013

Merged
jedel1043 merged 8 commits intoboa-dev:mainfrom
MayankRaj435:spec-alignment-create-method-property
Mar 12, 2026
Merged

Spec alignment create method property#5013
jedel1043 merged 8 commits intoboa-dev:mainfrom
MayankRaj435:spec-alignment-create-method-property

Conversation

@MayankRaj435
Copy link
Contributor

This PR aligns Boa more closely with the ECMAScript specification by implementing missing abstract operations and centralizing them in operations.rs.

1. Implement CreateMethodProperty (§7.3.5)

Added the create_method_property helper to core/engine/src/object/operations.rs.

Before:

// core/engine/src/object/operations.rs
// todo: CreateMethodProperty

After:

pub(crate) fn create_method_property<K, V>(
    &self,
    key: K,
    value: V,
    context: &mut InternalMethodPropertyContext<'_>,
) -> JsResult<bool>
where
    K: Into<PropertyKey>,
    V: Into<JsValue>,
{
    let new_desc = PropertyDescriptor::builder()
        .value(value)
        .writable(true)
        .enumerable(false)
        .configurable(true);
    self.__define_own_property__(&key.into(), new_desc.into(), context)
}

2. Move CopyDataProperties (§7.3.26)

Moved the implementation from jsobject.rs to operations.rs to center abstract operations as suggested by internal todo comments.

Before:

  • jsobject.rs: Contained the implementation of copy_data_properties.
  • operations.rs: Contained // todo: CopyDataProperties.

After:

  • jsobject.rs: Implementation removed.
  • operations.rs:
pub fn copy_data_properties<K>(
    &self,
    source: &JsValue,
    excluded_keys: Vec<K>,
    context: &mut Context,
) -> JsResult<()> {
    // ... (Implementation moved here)
}

Tests

  • Verified with cargo check -p boa_engine.
  • Confirmed formatting with cargo fmt.

Your Name added 3 commits March 12, 2026 11:11
Implemented entries, keys, values, @@iterator, and toString wrapper methods
on JsTypedArray and its subclasses. Added unit tests for verification.

Resolves boa-dev#3252
@MayankRaj435 MayankRaj435 requested a review from a team as a code owner March 12, 2026 13:57
@github-actions
Copy link

github-actions bot commented Mar 12, 2026

Test262 conformance changes

Test result main count PR count difference
Total 52,963 52,963 0
Passed 49,902 49,902 0
Ignored 2,222 2,222 0
Failed 839 839 0
Panics 0 0 0
Conformance 94.22% 94.22% 0.00%

Tested main commit: cf48c75d45aeaece9ada406250030e28184b1acd
Tested PR commit: a384eb57e246180d1329f361ed86b6a8ec2a0e15
Compare commits: cf48c75...a384eb5

alienx5499 and others added 2 commits March 12, 2026 21:29
This Pull Request fixes/closes boa-dev#4981.

It changes the following:

* Make `JsNativeError::kind` a private field instead of `pub`.
* Add a `JsNativeError::kind(&self) -> &JsNativeErrorKind` accessor as
the stable API.
* Update internal usages (engine tests, runtime tests, URI tests, tester
helpers, docs/examples) to call `kind()` instead of reading the field
directly.

Testing:

* `cargo test -p boa_engine`
This Pull Request fixes/closes
[boa-dev#4986](boa-dev#4986)

It changes the following:
- Trace a new call frame whenever a new frame is pushed onto the stack.
- Ensure each code block’s compiled output is printed only once.
@jedel1043 jedel1043 added A-Internal Changes that don't modify execution behaviour Waiting On Author Waiting on PR changes from the author labels Mar 12, 2026
… it in VM opcodes

Address PR review feedback:
- Rename create_method_property to define_method_property per ECMAScript 2026 spec (10.2.8)
- Update return type to JsResult<()> since the spec returns 'unused'
- Use define_method_property in DefineClassMethodByName, DefineClassStaticMethodByName,
  and DefineClassMethodByValue VM opcodes instead of manual PropertyDescriptor construction
- Remove unrelated TypedArray wrapper methods (entries, keys, values, iterator, to_string)
  and associated tests from jstypedarray.rs
- Revert pub(crate) visibility changes in builtin.rs
- Move CopyDataProperties from jsobject.rs to operations.rs
@MayankRaj435 MayankRaj435 force-pushed the spec-alignment-create-method-property branch from b59b0f5 to 5ea7a2b Compare March 12, 2026 19:21
@MayankRaj435
Copy link
Contributor Author

@jedel1043 I have done the changes ,please review it once and do lemme know ,thanks

@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 82.85714% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.47%. Comparing base (6ddc2b4) to head (a384eb5).
⚠️ Report is 816 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/object/operations.rs 88.46% 3 Missing ⚠️
core/engine/src/vm/opcode/define/class/method.rs 66.66% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5013       +/-   ##
===========================================
+ Coverage   47.24%   58.47%   +11.22%     
===========================================
  Files         476      559       +83     
  Lines       46892    61450    +14558     
===========================================
+ Hits        22154    35930    +13776     
- Misses      24738    25520      +782     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MayankRaj435 MayankRaj435 requested a review from jedel1043 March 12, 2026 19:37
Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks!

@jedel1043 jedel1043 added this pull request to the merge queue Mar 12, 2026
Merged via the queue into boa-dev:main with commit 57ebbbd Mar 12, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Internal Changes that don't modify execution behaviour Waiting On Author Waiting on PR changes from the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spec alignment: Implement CreateMethodProperty and organize CopyDataProperties

4 participants