jscs-dev / node-jscs

:arrow_heading_up: JavaScript Code Style checker (unmaintained)
https://jscs-dev.github.io
MIT License
4.96k stars 513 forks source link

New Rule: disallowArrayDestructuringReturn #2149

Closed fxmaxvl closed 8 years ago

fxmaxvl commented 8 years ago

Requires object destructuring for multiple return values, not array destructuring.

Fixes #2073

markelog commented 8 years ago

What about assignment cases?

fxmaxvl commented 8 years ago

what exactly?

markelog commented 8 years ago

This pull fully covers the return cases i.e. #2073. I'm asking if you would like to fully implement the

// bad
const [left, __, top] = processInput(input);

// good
const { left, right } = processInput(input);

From https://github.com/airbnb/javascript#5.3.

Or maybe just detect and forbid array destructuring en masse.

fxmaxvl commented 8 years ago

disallowArrayDestructuringReturn - works only for return cases, assignment cases is not part of this rule - if I right understand this :

5.3 Use object destructuring for multiple return values, not array destructuring. from https://github.com/airbnb/javascript#5.3.

Maybe we should create a new rule for dissallow array destructuring globally? (disallowArrayDestructuring) By the way, in https://github.com/airbnb/javascript#5.2 - assignment with array destructuring is considered as a good case.

markelog commented 8 years ago

When i think it of it of thoroughly, isn't this rule forbids any return statement with array value, without any check if that value is used in destruction assignment?

It seems check for assignment is mandatory and even should show the error only for assignment.

fxmaxvl commented 8 years ago

how we can check if that value is used in destruction assignment in cases when function declaration and function call with assignment destruction is placed in different modules, files etc?

markelog commented 8 years ago

If in different modules then we are out of luck (actually, not really, but it sounds a bit complicated to implement for one rule), but we can "easily" detect such violations if both assignment and FE is in the same rule.

fxmaxvl commented 8 years ago

so, what the final behavior for this rule? if we can detect both assignment and function - show error only for assignment, whilst when we can detect only a function - show error for return statement? But I think, display error for both cases (assignment and return) it will be more handy.

markelog commented 8 years ago

whilst when we can detect only a function - show error for return statement?

The thing is, if function returns an array, it doesn't mean it would be used in destructuring assignment. So we can show error only for the assignment.

markelog commented 8 years ago

Also, we were thinking to release today, but we can push it, if you intendent to finish this soon

fxmaxvl commented 8 years ago

oh, today i cant finish it, but probably tomorrow it will be done.

markelog commented 8 years ago

Ready for review?

fxmaxvl commented 8 years ago

yes, sure, I rewrote mostly all. I removed the logic that detects function declarations and them returns - because it not make sense for us, only assignment and call expressions are important.

markelog commented 8 years ago

Thank you!