Update: add new schema to no-restricted-syntax (fixes #8298) #8357
Conversation
vitorbal
added
accepted
|
@vitorbal, thanks for your PR! By analyzing the history of the files in this pull request, we identified @not-an-aardvark, @BYK and @scriptdaemon to be potential reviewers. |
|
LGTM |
|
@platinumazure yeah, sounds like a good idea! I'll add it 👍
…On Wed, Mar 29, 2017 at 8:06 PM Kevin Partington ***@***.***> wrote:
***@***.**** commented on this pull request.
LGTM.
Just one docs question: Should we have an example (or some prose) which
indicates that strings and objects can be freely mixed in config?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8357 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAmNdunK7QjjywfZ-Bee8al4xjuuuTxsks5rqp2mgaJpZM4MtLB7>
.
|
lib/rules/no-restricted-syntax.js
Outdated
| context.report({ | ||
| node, | ||
| message: hasCustomMessage ? selectorOrObject.message : "Using '{{selector}}' is not allowed.", | ||
| data: { selector } |
There was a problem hiding this comment.
If a custom message contains the substring {{selector}}, this will cause it to get replaced with the current selector. I doubt this will be a common problem in practice, but do you think it's worth documenting this or updating the logic to avoid it?
There was a problem hiding this comment.
Very true! I updated the logic to take this into account and added a test for it. Thanks!
docs/rules/no-restricted-syntax.md
Outdated
| @@ -12,7 +12,7 @@ This rule disallows specified (that is, user-defined) syntax. | |||
|
|
|||
| ## Options | |||
|
|
|||
| This rule takes a list of strings: | |||
| This rule takes a list of strings, where each string is a node type or AST selector: | |||
There was a problem hiding this comment.
Nitpick: Technically, any node type is also an AST selector, so this sentence is redundant. However, I'm fine with keeping it if you think it makes things clearer.
There was a problem hiding this comment.
Good point. I changed it to say only "where each string is an AST selector".
|
LGTM |
vitorbal
force-pushed
the
enhance-no-restricted-syntax
branch
from
d137f14
to
ab745a4
Compare
|
LGTM |
vitorbal
force-pushed
the
enhance-no-restricted-syntax
branch
from
ab745a4
to
5c38842
Compare
|
LGTM |
eslint-deprecated
bot
added
the
archived due to age
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
This PR updates the
no-restricted-syntaxrule to support custom messages, like discussed in #8298.This is a non-breaking enhancement to the rule.
Is there anything you'd like reviewers to focus on?
Any suggestions to make the docs clearer are welcome!