+
Skip to content
This repository was archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_js_semantic):should push a new scope when meet a function expression #2973

Merged
merged 8 commits into from
Aug 1, 2022
Merged

fix(rome_js_semantic):should push a new scope when meet a function expression #2973

merged 8 commits into from
Aug 1, 2022

Conversation

IWANABETHATGUY
Copy link
Contributor

@IWANABETHATGUY IWANABETHATGUY commented Jul 30, 2022 •
edited
Loading

Summary

  1. Fix missing push scope event when meeting a function expression.
  2. e.g.
var test = function test(a, b, c) {
//              ^^^^^^^^^^^^^^^^^^^ should push a new scope
}

https://play.rome.tools/?code=var+test+%3D+function+test%28a%2C+b%2C+c%29+%7B%0A%0A%7D&lineWidth=80&indentStyle=tab&quoteStyle=double&indentWidth=2&sourceType=module&typescript=false&jsx=false#dgBhAHIAIAB0AGUAcwB0ACAAPQAgAGYAdQBuAGMAdABpAG8AbgAgAHQAZQBzAHQAKABhACwAIABiACwAIABjACkAIAB7AAoACgB9AA==

Test Plan

  1. Should pass newly added semantic test case.

@IWANABETHATGUY
Copy link
Contributor Author

!bench_analyzer

@IWANABETHATGUY IWANABETHATGUY mentioned this pull request Jul 30, 2022
Merged
@github-actions
Copy link

Analyzer Benchmark Results

group                     main                                   pr
-----                     ----                                   --
analyzer/css.js           1.00   1974.6±3.72µs     5.9 MB/sec    1.00   1983.0±5.22µs     5.9 MB/sec
analyzer/index.js         1.00      4.7±0.01ms     6.8 MB/sec    1.01      4.8±0.01ms     6.7 MB/sec
analyzer/lint.ts          1.00      2.7±0.03ms    15.6 MB/sec    1.00      2.7±0.01ms    15.6 MB/sec
analyzer/parser.ts        1.00      6.3±0.01ms     7.7 MB/sec    1.01      6.3±0.01ms     7.6 MB/sec
analyzer/router.ts        1.00      4.5±0.00ms    13.7 MB/sec    1.01      4.5±0.07ms    13.6 MB/sec
analyzer/statement.ts     1.00      6.0±0.07ms     5.9 MB/sec    1.01      6.0±0.04ms     5.9 MB/sec
analyzer/typescript.ts    1.00      8.8±0.02ms     6.2 MB/sec    1.01      8.9±0.01ms     6.1 MB/sec

@xunilrj
Copy link
Contributor

xunilrj commented Jul 30, 2022

Mind creating some extra tests?

var f/*#F1*/ = function f/*#F2*/() {console.log(f/*READ F2*/);}",
var f/*#F1*/ = function () {console.log(f/*READ F1*/);}",
let f/*#F1*/ = 1; let g = function f/*#F2*/() {console.log(2, f/*READ F2*/);}",
let f/*#F*/ = function g/*#G*/(){}; g/*?*/();",

The last one needs a new assertion "?" that matches UnresolvedReference

@IWANABETHATGUY
Copy link
Contributor Author

Mind creating some extra tests?

var f/*#F1*/ = function f/*#F2*/() {console.log(f/*READ F2*/);}",
var f/*#F1*/ = function () {console.log(f/*READ F1*/);}",
let f/*#F1*/ = 1; let g = function f/*#F2*/() {console.log(2, f/*READ F2*/);}",
let f/*#F*/ = function g/*#G*/(){}; g/*?*/();",

The last one needs a new assertion "?" that matches UnresolvedReference

Done

@xunilrj
Copy link
Contributor

xunilrj commented Jul 30, 2022

Approved.

@leops leops merged commit 1cf50b0 into rome:main Aug 1, 2022
@IWANABETHATGUY IWANABETHATGUY deleted the fix/new-scope-function-expression branch August 1, 2022 13:33
IWANABETHATGUY added a commit to IWANABETHATGUY/tools that referenced this pull request Aug 22, 2022
@IWANABETHATGUY
fix(rome_js_semantic):should push a new scope when meet a function ex…
4b8c399 
…pression (rome#2973)

* fix: 🐛 function expression should push new scope

* fix: 🐛 semantic scope

* fix: 🐛 remove mainn

* fix: 🐛 pop scope when leave function expr

* style: 💄 fmt

* chore: 🤖 revert

* chore: 🤖 rever

t

* test: 💍 more read test case
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@xunilrj xunilrj xunilrj approved these changes

@leops leops leops approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@IWANABETHATGUY @xunilrj @leops
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载