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

Lost use case with `no-reserved-keys` to `quote-props` merge #3381

Closed
freejosh opened this Issue Aug 12, 2015 · 10 comments

Comments

Projects
None yet
4 participants
@freejosh

freejosh commented Aug 12, 2015

Previously I had quote-props turned off and wanted errors for no-reserved-keys but that now seems impossible. In other words, I don't care about property quoting, except if it's a keyword.

Another way to look at it is wanting as-needed to have an option to ignore unnecessarily quoted properties.

@eslintbot

This comment has been minimized.

Show comment
Hide comment
@eslintbot

eslintbot Aug 12, 2015

Thanks for the issue! We get a lot of issues, so this message is automatically posted to each one to help you check that you've included all of the information we need to help you.

Reporting a bug? Please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. The source code that caused the problem
  3. The configuration you're using (for the rule or your entire config file)
  4. The actual ESLint output complete with line numbers

Requesting a new rule? Please be sure to include:

  1. The use case for the rule - what is it trying to prevent or flag?
  2. Whether the rule is trying to prevent an error or is purely stylistic
  3. Why you believe this rule is generic enough to be included

Requesting a feature? Please be sure to include:

  1. The problem you want to solve (don't mention the solution)
  2. Your take on the correct solution to problem

Including this information in your issue helps us to triage it and get you a response as quickly as possible.

Thanks!

eslintbot commented Aug 12, 2015

Thanks for the issue! We get a lot of issues, so this message is automatically posted to each one to help you check that you've included all of the information we need to help you.

Reporting a bug? Please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. The source code that caused the problem
  3. The configuration you're using (for the rule or your entire config file)
  4. The actual ESLint output complete with line numbers

Requesting a new rule? Please be sure to include:

  1. The use case for the rule - what is it trying to prevent or flag?
  2. Whether the rule is trying to prevent an error or is purely stylistic
  3. Why you believe this rule is generic enough to be included

Requesting a feature? Please be sure to include:

  1. The problem you want to solve (don't mention the solution)
  2. Your take on the correct solution to problem

Including this information in your issue helps us to triage it and get you a response as quickly as possible.

Thanks!

@eslintbot eslintbot added the triage label Aug 12, 2015

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Aug 14, 2015

Member

I'm not quite sure what you're saying. Can you show an example of what you'd like to warn and not warn?

Member

nzakas commented Aug 14, 2015

I'm not quite sure what you're saying. Can you show an example of what you'd like to warn and not warn?

@freejosh

This comment has been minimized.

Show comment
Hide comment
@freejosh

freejosh Aug 14, 2015

/*eslint quote-props: [2, "as-needed", {"keywords": true}] */
var obj = {
    'unnecessary': 1,
    if: 2
};

Gives me 2 errors, but I only want the one about the reserved key and not the unnecessarily quoted property.

freejosh commented Aug 14, 2015

/*eslint quote-props: [2, "as-needed", {"keywords": true}] */
var obj = {
    'unnecessary': 1,
    if: 2
};

Gives me 2 errors, but I only want the one about the reserved key and not the unnecessarily quoted property.

@ilyavolodin

This comment has been minimized.

Show comment
Hide comment
@ilyavolodin

ilyavolodin Aug 15, 2015

Member

@freejosh Can you paste ESLint errors that you are getting for the code above? Also, what version of ESLint are you using and what parser?

Member

ilyavolodin commented Aug 15, 2015

@freejosh Can you paste ESLint errors that you are getting for the code above? Also, what version of ESLint are you using and what parser?

@freejosh

This comment has been minimized.

Show comment
Hide comment
@freejosh

freejosh Aug 16, 2015

I'm using v1.1.0, default parser. For my code above I get:

  3:5  error  Unnecessarily quoted property `unnecessary` found  quote-props
  4:5  error  Unquoted reserved word `if` used as key            quote-props

Which is correct for the current version, I'm just saying that I've lost the pre-1.0 behaviour where I could get the second error only.

For example, with v0.24.1 I would have used the rules /*eslint quote-props: 0, no-reserved-keys: 2 */ to get the output:

  2:10  error  Reserved word 'if' used as key  no-reserved-keys

freejosh commented Aug 16, 2015

I'm using v1.1.0, default parser. For my code above I get:

  3:5  error  Unnecessarily quoted property `unnecessary` found  quote-props
  4:5  error  Unquoted reserved word `if` used as key            quote-props

Which is correct for the current version, I'm just saying that I've lost the pre-1.0 behaviour where I could get the second error only.

For example, with v0.24.1 I would have used the rules /*eslint quote-props: 0, no-reserved-keys: 2 */ to get the output:

  2:10  error  Reserved word 'if' used as key  no-reserved-keys
@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Aug 17, 2015

Member

Okay, so you only want to enforce quotes for properties if a keyword is used, and ignore all others?

Member

nzakas commented Aug 17, 2015

Okay, so you only want to enforce quotes for properties if a keyword is used, and ignore all others?

@freejosh

This comment has been minimized.

Show comment
Hide comment
@freejosh

freejosh commented Aug 18, 2015

Yep!

@nzakas nzakas added enhancement rule accepted and removed triage labels Aug 19, 2015

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Aug 19, 2015

Member

So maybe what we need is:

quote-props: [2, "ignore", { keywords: true }]

"ignore" is a weird term to use, but since we already have keywords:true, I can't think of another way.

Member

nzakas commented Aug 19, 2015

So maybe what we need is:

quote-props: [2, "ignore", { keywords: true }]

"ignore" is a weird term to use, but since we already have keywords:true, I can't think of another way.

@freejosh

This comment has been minimized.

Show comment
Hide comment
@freejosh

freejosh Aug 19, 2015

I was thinking maybe something like:

quote-props: [2, "as-needed", { keywords: true, unnecessary: false }]

To me it's still doing the as-needed behavior, but we're turning off a subset of results (same way as keywords turned on a subset)

freejosh commented Aug 19, 2015

I was thinking maybe something like:

quote-props: [2, "as-needed", { keywords: true, unnecessary: false }]

To me it's still doing the as-needed behavior, but we're turning off a subset of results (same way as keywords turned on a subset)

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Aug 20, 2015

Member

Ah! Good call, I like that.

Member

nzakas commented Aug 20, 2015

Ah! Good call, I like that.

BYK added a commit that referenced this issue Aug 23, 2015

BYK added a commit that referenced this issue Aug 24, 2015

BYK added a commit that referenced this issue Aug 24, 2015

@nzakas nzakas closed this in #3494 Aug 25, 2015

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.