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

Implement missing RegExp properties #1304

Closed
8 tasks done
RageKnify opened this issue Jun 6, 2021 · 5 comments
Closed
8 tasks done

Implement missing RegExp properties #1304

RageKnify opened this issue Jun 6, 2021 · 5 comments
Assignees
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@RageKnify
Copy link
Contributor

RageKnify commented Jun 6, 2021

ECMASCript feature
The description of the RegExp Constructor is here: https://tc39.es/ecma262/#sec-regexp-constructor

Example code
MDN should have enough examples to test, https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp

  • get RegExp [ @@species ]
  • RegExp.prototype.constructor
  • RegExp.prototype [ @@match ] ( string )
  • RegExp.prototype [ @@matchAll ] ( string )
  • RegExp.prototype [ @@replace ] ( string, replaceValue )
  • RegExp.prototype [ @@search ] ( string )
  • get RegExp.prototype.source
  • RegExp.prototype [ @@split ] ( string, limit )

Part of the code for @@replace is in String.prototype.replace and should be removed from there.

pub(crate) fn replace(this: &Value, args: &[Value], context: &mut Context) -> Result<Value> {

@RageKnify RageKnify added enhancement New feature or request good first issue Good for newcomers builtins PRs and Issues related to builtins/intrinsics labels Jun 6, 2021
@raskad
Copy link
Member

raskad commented Jun 15, 2021

I am currently working on this.

@raskad
Copy link
Member

raskad commented Jul 19, 2021

Depending on how we want to define this issue, I think we could mark some items on the list as done or close this issue.
The following is the current state of the built-ins/RegExp/prototype test suite:

Test Suite Total Passed
RegExp.prototype 880 713
Symbol.species 8 8
Symbol.match 102 100
Symbol.matchAll 52 34
Symbol.replace 134 105
Symbol.search 46 38
source 24 12
Symbol.split 88 54

My feeling is that a big part of the failing tests is based on missing features, either somewhere in boa or in regress. For the most part, I think the basic features are implemented.

@RageKnify
Copy link
Contributor Author

Depending on how we want to define this issue, I think we could mark some items on the list as done or close this issue.

My opinion is that if a function is spec compliant we can consider it done.

Tests that depend on regress implementing more things could be open as separate issues and include a link to regress's issue so that we have something tracking it here.

If the failing tests depend on things being implemented elsewhere in boa (for example Reflect is used in a lot of tests outside its folder) then we can also consider that function done, and it might be a good idea to open an issue for the missing feature if none exist.

@raskad
Copy link
Member

raskad commented Jul 24, 2021

I've looked trough all of the failing tests for the RegExp/prototype tests. Here is a list of the related feature / bugfix that is necessary for the tests to pass.

Feature / Bug Failing test
ComputedPropertyName in ObjectLiteral get
  • Symbol.matchAll/isregexp-this-throws
  • Symbol.matchAll/isregexp-called-once
  • Symbol.matchAll/species-constructor-get-species-throws
new Object("a").toString() does not return 'a'
  • exec/S15.10.6.2_A1_T3
RegExp Match Indices #1428
  • flags/return-order
  • flags/rethrow
  • flags/coercion-hasIndices
  • flags/get-order
  • flags/this-val-regexp
Implementing the eval function #948
  • source/value
  • source/value-line-terminator
  • source/value-slash
  • source/value-empty
  • source/value-u
Realm related?
  • Symbol.split/splitter-proto-from-ctor-realm
  • ignoreCase/cross-realm
  • dotAll/cross-realm
  • sticky/cross-realm
  • multiline/cross-realm
  • source/cross-realm
  • unicode/cross-realm
  • hasIndices/cross-realm
  • global/cross-realm
Related to assigning this in function?
  • Symbol.replace/fn-invoke-this-no-strict
UTF16 surrogate not supported #736
  • Symbol.match/builtin-infer-unicode
  • Symbol.replace/coerce-unicode
var initialized hoist with undefined currently not implemented
  • exec/S15.10.6.2_A4_T5
  • exec/S15.10.6.2_A1_T20
  • test/S15.10.6.3_A1_T20

@jedel1043
Copy link
Member

jedel1043 commented Sep 8, 2021

Closing because #1434 solved most, if not all, the differences from the spec and we have all properties implemented for RegExp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants