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

Update: Add exceptions option to camelcase. #9684

Closed
wants to merge 1 commit into from
Closed

Update: Add exceptions option to camelcase. #9684

wants to merge 1 commit into from

Conversation

coreyfarrell
Copy link

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:

please explain:
What rule do you want to change?
camelcase

Does this change cause the rule to produce more or fewer warnings?
It can cause fewer warnings to be produced if the new option is used.

How will the change be implemented? (New option, new default behavior, etc.)?
A new option "exceptions" can be provided to the camelcase rule.

Please provide some example code that this change will affect:

const mysql = require('mysql');
const pool = mysql.createPool();

const account_id = 'value';
const message_cause = 'dbas like underscores';
pool.query('INSERT INTO account_log SET ?', {account_id, message_cause}, err => {
  if (err) {
    throw err;
  }
  pool.end();
});

What does the rule currently do for this code?
The camelcase rule must be disabled when dealing with outside code that requires use of underscored names.

What will the rule do after it's changed?
After the change the example above would be able to include the option "exceptions": ["account_id", "message_cause"] in the camelcase options object instead of disabling it entirely.

What changes did you make? (Give an overview)
Add support for "exceptions" to be provided in the camelcase options object, ignore any identifiers listed in the object.

Is there anything you'd like reviewers to focus on?
Nothing I can think of.

@j-f1
Copy link
Contributor

j-f1 commented Dec 4, 2017

For your example, you can use a regular object literal

const mysql = require('mysql');
const pool = mysql.createPool();

const accountId = 'value';
const messageCause = 'dbas like underscores';
pool.query('INSERT INTO account_log SET ?', {account_id: accountId, message_cause: messageCause}, err => {
  if (err) {
    throw err;
  }
  pool.end();
});

which will avoid the issue altogether.

@coreyfarrell
Copy link
Author

@j-f1 I just tried your code, it still produces errors when camelcase is enabled.

@BrianRosamilia
Copy link

BrianRosamilia commented Dec 5, 2017

Why can't this just be made to be more generic and have an option called obj or params (objparams?) where it won't check for camel case names in an object initializer? Having to add an array of string exceptions seems kludgey

To clarify, this rule would be the exact same as always for the camelcase rule, but the last example

var obj = {
    my_pref: 1
};

is allowed

https://eslint.org/docs/rules/camelcase#always

I actually came here to make a feature request for this and found this issue. It seems much needed.

Edit: Wow I'm dumb, this already exists, thanks @j-f1 -- sorry for the hijack

@j-f1
Copy link
Contributor

j-f1 commented Dec 5, 2017

That’s what the properties: never option does @BrianRosamilia.

@aladdin-add aladdin-add added enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules labels Dec 5, 2017
@ilyavolodin
Copy link
Member

As @j-f1 mentioned, this should pass with properties: never flag. Do we need to add another option that basically satisfies the example provided?

@coreyfarrell
Copy link
Author

The point of my patch is to allow specific camel case symbols to be used anywhere. If I'm dealing with a database which uses time_id I don't care where time_id is used, variables, symbols or parameters. I don't want to allow all camel case property names, only the names from the database.

@platinumazure
Copy link
Member

@coreyfarell Maybe you want to take a look at the id-match rule? That rule lets you define a regex representing a valid identifier. I think you could theoretically use a regex like "^(?:[^_]+|time_id)$" to get what you want here (of course, you may need to tweak).

@coreyfarrell
Copy link
Author

In theory the id-match rule could work. It looks difficult to use but I'm going to close this PR because it feels like a waste of time.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jun 24, 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 Jun 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants