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

prefer-string-starts-ends-with: Fix missing parentheses for some cases #976

Merged

Conversation

fisker
Copy link
Collaborator

@fisker fisker commented Dec 31, 2020

Noticed this bug during #970

/^a/.test(foo + bar);

is fixed to

foo + bar.startsWith('a');

Should be

(foo + bar).startsWith('a');

I didn't add to many cases in needAddParenthesesToMemberExpressionObject, for now, only some nodes can be string. I believe there are other rules need this, we'll add more when we need.

@fisker fisker force-pushed the prefer-string-starts-ends-with-parens branch from 1194a0f to 1011896 Compare December 31, 2020 08:00
@fisker fisker force-pushed the prefer-string-starts-ends-with-parens branch 3 times, most recently from 35b9e29 to 5b3df26 Compare December 31, 2020 08:59
@fisker fisker force-pushed the prefer-string-starts-ends-with-parens branch from 5b3df26 to b27b35c Compare December 31, 2020 09:01
@fisker fisker marked this pull request as ready for review December 31, 2020 09:02
@fisker fisker force-pushed the prefer-string-starts-ends-with-parens branch from 2cc4b85 to bc58ccd Compare December 31, 2020 09:14
@fisker fisker marked this pull request as draft December 31, 2020 09:52
@fisker fisker force-pushed the prefer-string-starts-ends-with-parens branch from d3a7065 to f0bf5d6 Compare December 31, 2020 10:11
@fisker fisker marked this pull request as ready for review December 31, 2020 10:21

case 'Literal': {
/* istanbul ignore next */
if (typeof node.value === 'number' && /^\d+$/.test(node.raw)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Won't this fail on 1.1 or 1e+2 and more? If this is intentional, it would be good with a code comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, 1.1.toString(); an 2e+2.toString(); are valid.

Copy link
Collaborator Author

@fisker fisker Jan 2, 2021

Choose a reason for hiding this comment

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

fisker and others added 2 commits January 2, 2021 11:35
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
@fisker fisker marked this pull request as draft January 2, 2021 03:38
@fisker fisker force-pushed the prefer-string-starts-ends-with-parens branch from 1c061de to 5b524af Compare January 2, 2021 03:55
@fisker fisker marked this pull request as ready for review January 2, 2021 07:44
@sindresorhus sindresorhus merged commit e2f94fe into sindresorhus:master Jan 6, 2021
@fisker fisker deleted the prefer-string-starts-ends-with-parens branch January 6, 2021 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants