-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
feat: directive prologue detection and autofix condition in quotes
#17284
Conversation
I personally think there's no need to document edge cases where rules don't autofix the code, but we could add some "correct" examples for this rule with the |
I think it's helpful to add a note to the documentation because there isn't any mention of directives right now. Something along the lines of, "This rule is aware of directive prologues such as "use strict" and will not flag or autofix them unless doing so will not change how the directive prologue is interpreted." |
Thanks for your feedback @mdjermanovic @nzakas. I've updated the documentation as suggested (in 6f7b899). |
docs/src/rules/quotes.md
Outdated
|
||
Many codebases require strings to be defined in a consistent manner. | ||
|
||
## Rule Details | ||
|
||
This rule enforces the consistent use of either backticks, double, or single quotes. | ||
|
||
This rule is aware of directive prologues such as `"use strict";` and will not flag or autofix them unless doing so will not change how the directive prologue is interpreted. |
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.
A little difficult to parse, maybe:
This rule is aware of directive prologues such as `"use strict";` and will not flag or autofix them unless doing so will not change how the directive prologue is interpreted. | |
This rule is aware of directive prologues such as `"use strict"` and will not flag or autofix them if doing so will change how the directive prologue is interpreted. |
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.
Thanks! I thought that the semicolon in "use strict";
would make it clearer that what is shown is a directive as opposed to just a string, but it that's bad for readability we can leave it out.
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.
Yeah, dangling semicolons are a little visually confusing. :)
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
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. Would like @mdjermanovic to review before merging.
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, thanks!
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.42.0` -> `8.43.0`](https://renovatebot.com/diffs/npm/eslint/8.42.0/8.43.0) | --- ### Release Notes <details> <summary>eslint/eslint (eslint)</summary> ### [`v8.43.0`](https://github.com/eslint/eslint/releases/tag/v8.43.0) [Compare Source](eslint/eslint@v8.42.0...v8.43.0) #### Features - [`14581ff`](eslint/eslint@14581ff) feat: directive prologue detection and autofix condition in `quotes` ([#​17284](eslint/eslint#17284)) (Francesco Trotta) - [`e50fac3`](eslint/eslint@e50fac3) feat: add declaration loc to message in block-scoped-var ([#​17252](eslint/eslint#17252)) (Milos Djermanovic) - [`1b7faf0`](eslint/eslint@1b7faf0) feat: add `skipJSXText` option to `no-irregular-whitespace` rule ([#​17182](eslint/eslint#17182)) (Azat S) #### Bug Fixes - [`5338b56`](eslint/eslint@5338b56) fix: normalize `cwd` passed to `ESLint`/`FlatESLint` constructor ([#​17277](eslint/eslint#17277)) (Milos Djermanovic) - [`54383e6`](eslint/eslint@54383e6) fix: Remove `no-extra-parens` autofix for potential directives ([#​17022](eslint/eslint#17022)) (Francesco Trotta) #### Documentation - [`8b855ea`](eslint/eslint@8b855ea) docs: resubmit pr17061 doc changes ([#​17292](eslint/eslint#17292)) (唯然) - [`372722e`](eslint/eslint@372722e) docs: resubmit pr17012 doc changes ([#​17293](eslint/eslint#17293)) (唯然) - [`67e7af3`](eslint/eslint@67e7af3) docs: resubmit custom-rules doc changes ([#​17294](eslint/eslint#17294)) (唯然) - [`9e3d77c`](eslint/eslint@9e3d77c) docs: Resubmit Fix formatting in Custom Rules docs ([#​17281](eslint/eslint#17281)) (Milos Djermanovic) - [`503647a`](eslint/eslint@503647a) docs: Resubmit markVariableAsUsed docs ([#​17280](eslint/eslint#17280)) (Nicholas C. Zakas) - [`e0cf0d8`](eslint/eslint@e0cf0d8) docs: Custom rule & plugin tutorial ([#​170...
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
see #17022
What changes did you make? (Give an overview)
This PR implements two changes in the
quotes
rule:"use strict";
from("use strict");
. This can cause the rule to report new problems in certain cases.no-extra-parens
: unparenthesized string literals that are direct children of anExpressionStatement
will not be autofixed.Both changes were discussed in #17022, but we decided to postpone them to a new PR.
Is there anything you'd like reviewers to focus on?
Shall we add a note to the documentation, like in no-extra-parens?