jscs-dev / node-jscs

:arrow_heading_up: JavaScript Code Style checker (unmaintained)
https://jscs-dev.github.io
MIT License
4.96k stars 513 forks source link

Suggestion: requireCamelCaseOrUpperCaseIdentifiers should ignore pre-existing #1902

Closed bturk closed 8 years ago

bturk commented 8 years ago

When using requireCamelCaseOrUpperCaseIdentifiers it should only look at variables or property names I create, not ones I use. I've had this rule trigger a couple of times because of external API's.

For example when using Twitter oauth:

var myObj = {
   createdAt: twitterObj.created_at
}

Should not trigger an error.

var myObj = {
   created_at: twitterObj.created_at
}

Should trigger an error.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

hzoo commented 8 years ago

As a workaround you can use the ignoreProperties option would looks like would work in your case?

bturk commented 8 years ago

Yes, that's what I am using now, just would be a nice to have feature to only check variables being created vs being used.

hzoo commented 8 years ago

That could be a new option. We would need to check all variable declarations / use escope type api. Would you like to try making a PR for it? Most of the team will be busy working on 3.0 issues instead of adding new rules/options.

bturk commented 8 years ago

I'll take a look this weekend, just looked at the code for the rule, looks like there is a bit of a learning curve to go though.

hzoo commented 8 years ago

Yeah I think having to check for all declared variables is more involved than most rules. You can check https://github.com/jscs-dev/node-jscs/blob/master/lib/rules/disallow-unused-params.js for the use of escope.

Otherwise I would check out other rules, check out our CONTRIBUTING.md, and use http://astexplorer.net/ to view the AST.

dharkness commented 8 years ago

+1 for ignoring usage as above and when calling functions. Ideally, I just want to see the source of the problem flagged--the declaration. All other references follow from it.

fluky commented 8 years ago

Any updates on this? This rule is basically unusable for ES6 as Promise, Reflect, etc trigger the rule.

Never mind, just realized I could use allExcept for those, though I must admit that may not be the most elegant solution. For a simple yeoman generator with a handful of files (3 source, 3 unit test and a gulpfile.js) I had to add the following to make it work:

"allExcept": [ "Error", "Object", "Boolean", "Date", "Reflect", "Promise", "Server", "Db", "Assertion", "MongoClient", "Base" ]

The first 6 are all standard Javascript types. The rest are for MongoDB, Chai and Yeoman.

markelog commented 8 years ago

If anyone would like to add standard ones, it would be most appreciated

fluky commented 8 years ago

For the standard component you'd need everything capitalized from this list:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects

markelog commented 8 years ago

Yeah

hzoo commented 8 years ago

We released 3.0 recently! We also wrote a post about how 3.0 will be our last major release (since we are deprecating the project) and that we are merging with ESLint!

Thus we won't be working on any new features moving forward which includes new rules/options. We will continue to fix bugs and work on CST/better autofixing for current rules in the upcoming months both to help JSCS users and to gain experience that we can hopefully use in ESLint. But otherwise we both teams will be working on getting ESLint feature parity with JSCS and just a nice transition for our users.

I'm putting an orphaned label to note this issue isn't planned to be worked on, but might be a good contribution as a custom rule (not in JSCS core itself).

I would encourage anyone who wants the rule to try writing it themselves: I would either write a custom rule or submit an issue to ESLint and if accepted a PR. Otherwise a custom rule that won't be in core repo itself.

JSCS rules are also very similar to ESLint rules: here are some resources to get started:

bturk commented 8 years ago

So...very...disappointing. I'm not sure that ESLint is going to benefit from getting a crew that abandons a project with many hundreds of open and ignored issues. Not good news for the ESLint crew as I see it.

hzoo commented 8 years ago

@bturk Not sure why you think we are abandoning the project and have been ignoring issues - both teams decided to merge for the benefit of the community as a whole since we are doing the same things.

I don't think you have to read the post to understand that everyone here and most people in open source do this in their free time. I'm just closing most requests for new rules/options so we can focus on bug fixes and helping people move to ESLint. If you think this is a bug I can reopen.