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

sourceCode.isSpaceBetweenTokens() and multiline comments #12263

Closed
mdjermanovic opened this issue Sep 12, 2019 · 7 comments
Closed

sourceCode.isSpaceBetweenTokens() and multiline comments #12263

mdjermanovic opened this issue Sep 12, 2019 · 7 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features Hacktoberfest Recommended issue for those participating in Hacktoberfest https://hacktoberfest.digitalocean.com/ help wanted The team would welcome a contribution from the community for this issue

Comments

@mdjermanovic
Copy link
Member

mdjermanovic commented Sep 12, 2019

Noticed this while working on #12259

sourceCode.isSpaceBetweenTokens() sees newlines inside multiline block comments as spaces.

Observable:

/* eslint space-before-function-paren:["error", "never"]*/

function foo/*
*/(){}

Demo link

The above is an error because of the newline (function foo/* */(){} is not an error for the same rule.)

Is it intentional or a bug?

There is no such test for isSpaceBetweenTokens() in source-code tests.

Also, when I add the s flag to the /\/\*.*?\*\//gu regex in isSpaceBetweenTokens and run npm test everything still works.

@mdjermanovic mdjermanovic added bug ESLint is working incorrectly core Relates to ESLint's core APIs and features question This issue asks a question about ESLint labels Sep 12, 2019
@kaicataldo
Copy link
Member

Seems like a bug to me.

@mysticatea
Copy link
Member

Sounds a bug to me.

Maybe, the function has been used to check two tokens which are on the same line in most cases. Because spacing rules do nothing if two tokens are on different lines in general.

@mysticatea mysticatea added accepted There is consensus among the team that this change meets the criteria for inclusion and removed question This issue asks a question about ESLint labels Sep 13, 2019
@kaicataldo
Copy link
Member

I will try to work on this this weekend!

@kaicataldo
Copy link
Member

@mdjermanovic Though if you need to fix it for your work, please feel free to take it on. I don’t want to block you!

@mdjermanovic
Copy link
Member Author

mdjermanovic commented Sep 13, 2019

@kaicataldo Feel free to claim this :)

The effects of this change can be also observed in the current version of template-tag-spacing, it will have more and fewer errors in some cases.

@kaicataldo
Copy link
Member

Sorry I haven't gotten to this yet. It's in my backlog, but if someone can get to it before I can, please feel free!

@kaicataldo kaicataldo added Hacktoberfest Recommended issue for those participating in Hacktoberfest https://hacktoberfest.digitalocean.com/ help wanted The team would welcome a contribution from the community for this issue labels Oct 1, 2019
@mdjermanovic
Copy link
Member Author

Fixed in #12407

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Apr 22, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Apr 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features Hacktoberfest Recommended issue for those participating in Hacktoberfest https://hacktoberfest.digitalocean.com/ help wanted The team would welcome a contribution from the community for this issue
Projects
None yet
Development

No branches or pull requests

3 participants