-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
report: remove unnecessary return in setters #26614
Conversation
The value is accessible when using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Barring shenanigans like Object.getOwnPropertyDescriptor(), return values from a setter function will always be inaccessible. Remove the `return` statements as they can be misleading, suggesting that the return value is accessible and perhaps used somewhere.
Updated the commit message to acknowledge this potential for shenanigans. Rebased so re-running CI out of abundance of caution: https://ci.nodejs.org/job/node-test-pull-request/21479/ |
Landed in d78d33d |
Barring shenanigans like Object.getOwnPropertyDescriptor(), return values from a setter function will always be inaccessible. Remove the `return` statements as they can be misleading, suggesting that the return value is accessible and perhaps used somewhere. PR-URL: nodejs#26614 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Barring shenanigans like Object.getOwnPropertyDescriptor(), return values from a setter function will always be inaccessible. Remove the `return` statements as they can be misleading, suggesting that the return value is accessible and perhaps used somewhere. PR-URL: nodejs#26614 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Barring shenanigans like Object.getOwnPropertyDescriptor(), return values from a setter function will always be inaccessible. Remove the `return` statements as they can be misleading, suggesting that the return value is accessible and perhaps used somewhere. PR-URL: #26614 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Barring shenanigans such as using Object.getOwnPropertyDescriptor(), return values from a setter function will always be inaccessible. Remove
the
return
statements as they can be misleading, suggesting that thereturn value is accessible and perhaps used somewhere.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes