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

fix(rome_js_analyze): Add constructor support #4605

Merged
merged 1 commit into from
Jun 30, 2023
Merged

fix(rome_js_analyze): Add constructor support #4605

merged 1 commit into from
Jun 30, 2023

Conversation

denbezrukov
Copy link
Contributor

Summary

Add test cases:

class A {
	constructor(a, a) {}
}
class A {
	constructor(private a, a) {}
}
class A {
	constructor(a, readonly a) {}
}
class A {
	constructor(private a, private a) {}
}
class A {
	constructor(readonly a, private a) {}
}

Test Plan

cargo test -p rome_js_analyze

Changelog

  • The PR requires a changelog line

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Jun 23, 2023 •
edited
Loading

✅ Deploy Preview for docs-rometools canceled.

Name Link
🔨 Latest commit 2febb7b
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/649e9bb796050b0008045d7b

@denbezrukov denbezrukov requested a review from Conaclos June 23, 2023 19:38
@github-actions github-actions bot added the A-Linter Area: linter label Jun 23, 2023
@denbezrukov
Copy link
Contributor Author

I'm wondering if we have the same problem for other rules.

Conaclos
Conaclos approved these changes Jun 24, 2023
View reviewed changes
Copy link
Contributor

@Conaclos Conaclos left a comment

Choose a reason for hiding this comment

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

Good catch!

@denbezrukov denbezrukov force-pushed the fix/no-duplicate-parameters branch from 6a6f367 to 8d99986 Compare June 28, 2023 13:26
denbezrukov added a commit that referenced this pull request Jun 28, 2023
@denbezrukov
fix(rome_js_analyze): Add constructor support #4605
d16788c
@github-actions github-actions bot added A-Formatter Area: formatter A-Parser Area: parser labels Jun 28, 2023
@denbezrukov
Copy link
Contributor Author

Could you please rereview?
I've change query node from:

pub(crate) AnyJsFunctionAndMethod = JsArrowFunctionExpression| JsFunctionDeclaration| JsFunctionExportDefaultDeclaration | JsFunctionExpression | JsMethodClassMember | JsMethodObjectMember

to

declare_node_union! {
    pub AnyJsParameters = JsParameters | JsConstructorParameters
}

@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 48863 48863 0
Passed 47810 47810 0
Failed 1053 1053 0
Panics 0 0 0
Coverage 97.84% 97.84% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6212 6212 0
Passed 1764 1764 0
Failed 4448 4448 0
Panics 0 0 0
Coverage 28.40% 28.40% 0.00%

ts/babel

Test result main count This PR count Difference
Total 639 639 0
Passed 573 573 0
Failed 66 66 0
Panics 0 0 0
Coverage 89.67% 89.67% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 17224 17224 0
Passed 13121 13121 0
Failed 4103 4103 0
Panics 0 0 0
Coverage 76.18% 76.18% 0.00%

@denbezrukov denbezrukov force-pushed the fix/no-duplicate-parameters branch from d16788c to 2bd00dd Compare June 28, 2023 19:36
ematipico
ematipico approved these changes Jun 29, 2023
View reviewed changes
crates/rome_js_syntax/src/parameter_ext.rs
@@ -0,0 +1,130 @@
use crate::{
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that these methods are part of the user facing API, we should document them and add doctests. That's what we've been trying to do for the whole crate.

Conaclos
Conaclos approved these changes Jun 29, 2023
View reviewed changes
Copy link
Contributor

@Conaclos Conaclos 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 :)

@denbezrukov denbezrukov force-pushed the fix/no-duplicate-parameters branch 5 times, most recently from fdd962f to 3a3b784 Compare June 30, 2023 08:09
@denbezrukov denbezrukov force-pushed the fix/no-duplicate-parameters branch from 3a3b784 to 2febb7b Compare June 30, 2023 09:09
@denbezrukov denbezrukov merged commit 08645a8 into main Jun 30, 2023
@denbezrukov denbezrukov deleted the fix/no-duplicate-parameters branch June 30, 2023 09:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ematipico ematipico ematipico approved these changes

@Conaclos Conaclos Conaclos approved these changes

Assignees
No one assigned
Labels
A-Formatter Area: formatter A-Linter Area: linter A-Parser Area: parser
Projects
None yet
Milestone
No milestone
Development

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