search

ljharb / es-abstract

ECMAScript spec abstract operations.
MIT License
115 stars 30 forks source link

[Fix] `ES2018+`: fix `GetSubstitution` with named captures #121

Closed EmilSV closed 3 years ago

EmilSV commented 3 years ago

line https://github.com/es-shims/es-abstract/blob/2b693d9d4f6c84011191018c1b6a18a4705c5958/2020/GetSubstitution.js#L115

has some code the looks buggy as

'>'.length

will be executed first leading it to always be 1 and so you get

'$<' + groupName + 1

most likely ('$<' + groupName + '>').length

would be more correct

ljharb commented 3 years ago

Hmm, you may be right! Could you provide a regression test, so it won't break in the future?

ljharb commented 3 years ago

Although you've definitely spotted a bug, unfortunately this isn't the correct or entire fix. I'll update this PR with that, and land it. Thanks for bringing it to my attention!

codecov[bot] commented 3 years ago

Codecov Report

Merging #121 (9ce2a77) into master (2b693d9) will increase coverage by 0.26%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #121      +/-   ##
==========================================
+ Coverage   90.94%   91.20%   +0.26%     
==========================================
  Files         828      828              
  Lines       11404    11404              
  Branches     2597     2597              
==========================================
+ Hits        10371    10401      +30     
+ Misses       1033     1003      -30
Impacted Files Coverage Δ
2018/GetSubstitution.js 96.25% <100.00%> (+12.50%) :arrow_up:
2019/GetSubstitution.js 96.25% <100.00%> (+12.50%) :arrow_up:
2020/GetSubstitution.js 96.25% <100.00%> (+12.50%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2b693d9...9ce2a77. Read the comment docs.

EmilSV commented 3 years ago

glad i could help