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

Problematic code for no-sequences showing and being flagged as non-problematic #5536

Closed
brettz9 opened this issue Mar 10, 2016 · 7 comments
Closed
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 documentation Relates to ESLint's documentation enhancement This change enhances an existing feature of ESLint

Comments

@brettz9
Copy link
Contributor

brettz9 commented Mar 10, 2016

What version of ESLint are you using?

The ESLint demo (and looking at docs in master).

What configuration and parser (Espree, Babel-ESLint, etc.) are you using?

The ESLint demo (and looking at docs in master).

What did you do? Please include the actual source code causing the issue.

I noticed that the following code in no-sequences:

(0,eval)("doSomething();");

...is listed in the introductory block referring to the problem of sequences as well as in a non-problematic section.

What did you expect to happen?

I'd expect that the code would not appear in the non-problematic block in the docs but would be flagged in the demo. I'd also expect a non-problematic equivalent to be documented.

What actually happened? Please include the actual, raw output from ESLint.

The code appears in a non-problematic block and was not flagged as a problem (with no-sequences enabled) in the demo.

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Mar 10, 2016
@nzakas
Copy link
Member

nzakas commented Mar 10, 2016

Sequences are allowed when enclosed in parentheses. See http://eslint.org/docs/rules/no-sequences#rule-details. The rule is behaving as expected.

The introductory paragraph and code example does not say all of the code is considered bad, it is just showing examples of code sequences. We can maybe make that clearer.

@nzakas nzakas added enhancement This change enhances an existing feature of ESLint documentation Relates to ESLint's documentation accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Mar 10, 2016
@brettz9
Copy link
Contributor Author

brettz9 commented Mar 10, 2016

Ok, sure--maybe a problematic equivalent too, since there's already a non-problematic version:

0, eval("doSomething();");

@kaicataldo
Copy link
Member

Very minor tweak to increase clarity of docs, including the problematic version of the code above. Thoughts?

Edit: Just to clarify why I only changed one word: "its use" is not very clear - it's hard to tell if it's referring to the rule or sequences as a pattern. This change should make it clearer to the reader.

@nzakas
Copy link
Member

nzakas commented Mar 22, 2016

I like it.

@kaicataldo
Copy link
Member

Kinda goofed by pushing to my fork first and tried to delete it. Here's the actual PR: #5650 Sorry for the confusion 😅

kaicataldo added a commit that referenced this issue Mar 23, 2016
Docs: Update no-sequences rule docs for clarity (fixes #5536)
@brettz9
Copy link
Contributor Author

brettz9 commented Mar 23, 2016

Sweet, thanks, I think that was a very good way to resolve it...

@kaicataldo
Copy link
Member

@brettz9 Glad to hear it - thanks for reporting the issue!

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@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 Feb 6, 2018
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 documentation Relates to ESLint's documentation enhancement This change enhances an existing feature of ESLint
Projects
None yet
Development

No branches or pull requests

4 participants