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

Parameter Count seems failed in destructuring object parameters #365

Open
zymbth opened this issue Sep 30, 2024 · 8 comments · May be fixed by #448
Open

Parameter Count seems failed in destructuring object parameters #365

zymbth opened this issue Sep 30, 2024 · 8 comments · May be fixed by #448
Assignees
Labels
bug Something isn't working

Comments

@zymbth
Copy link

zymbth commented Sep 30, 2024

Describe the bug
Parameter Count seems failed in destructuring object parameters

To Reproduce
Steps to reproduce the behavior:

  1. What is the exact code you are analyzing?
function dummyFuncOne({ param1, param2, param3, param4, param5 }) {
  return 'One'
}

const dummyFuncTwo = ({ param1, param2, param3, param4 }) => {
  return 'Two'
}
  1. What is the output you are getting?
-  rrd ~ parameter count
   src\views\Test\parameter-count.js
   👉 Max number of function parameters should be 3. See: https://vue-mess-detector.webmania.cc/rules/rrd/parameter-count.html
   function dummyFuncOne has 5 parameters 🚨

   src\views\Test\parameter-count.js
   👉 Max number of function parameters should be 3. See: https://vue-mess-detector.webmania.cc/rules/rrd/parameter-count.html
   function dummyFuncTwo has 4 parameters 🚨

Expected behavior
Correctly count the destructured object parameters.

Screenshots
image

Used version number of vue-mess-detector: 0.57.0

Used version number of node & yarn: [email protected], [email protected]

@zymbth zymbth added the bug Something isn't working label Sep 30, 2024
@David-Pena
Copy link
Collaborator

Failing test added for reference

@rrd108
Copy link
Owner

rrd108 commented Sep 30, 2024

@David-Pena it is like that by design, isn't it?

From the code maintainability viewpoint they have 4 and 5 parameters.

I understand the issue, but I want to be sure how we think about this and clearly state that in the docs.

@David-Pena
Copy link
Collaborator

it is like that by design, isn't it?

I don't think so, right now I don't remember completely but I think its just misssing the correct logic to identify destructured params

From the code maintainability viewpoint they have 4 and 5 parameters.

I think destructured props shouldnt be limited. In this case its only one param so it shouldnt be reported, what do you think? @rrd108

@rrd108
Copy link
Owner

rrd108 commented Sep 30, 2024

When the param is an object - I think still there is a reasonable limit of non optional properties

but for now I am ok handling it as one

@David-Pena
Copy link
Collaborator

When the param is an object - I think still there is a reasonable limit of non optional properties

Lets create a separate issue for that change to the rule

@David-Pena
Copy link
Collaborator

Do you have any progress in here? @rrd108

@zymbth would you like to send a PR?

@zymbth
Copy link
Author

zymbth commented Oct 8, 2024

@David-Pena Sorry for the late reply, just got back from vacation. I've been a bit busy lately, try to contribute in the future.

@David-Pena
Copy link
Collaborator

This is solved in bbded51

@David-Pena David-Pena self-assigned this Nov 4, 2024
@David-Pena David-Pena linked a pull request Nov 4, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants