Skip to content

Categorize more rules #52

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 17, 2015
Merged

Categorize more rules #52

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 89 additions & 3 deletions lib/checks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,112 @@
var checkCategoryMapping = {
"accessor-pairs": "Bug Risk",
"block-scoped-var": "Bug Risk",
"callback-return": "Bug Risk",
"comma-dangle": "Bug Risk",
"complexity": "Complexity",
"no-unused-vars": "Bug Risk",
"no-empty": "Bug Risk",
"consistent-return": "Bug Risk",
"curly": "Clarity",
"default-case": "Bug Risk",
"dot-location": "Clarity",
"dot-notation": "Clarity",
"eqeqeq": "Bug Risk",
"global-require": "Clarity",
"guard-for-in": "Bug Risk",
"handle-callback-err": "Bug Risk",
"init-declarations": "Clarity",
"no-alert": "Bug Risk",
"no-caller": "Compatibility",
"no-case-declarations": "Bug Risk",
"no-catch-shadow": "Bug Risk",
"no-cond-assign": "Bug Risk",
"no-console": "Bug Risk",
"no-constant-condition": "Bug Risk",
"no-control-regex": "Bug Risk",
"no-debugger": "Bug Risk",
"no-delete-var": "Bug Risk",
"no-div-regex": "Bug Risk",
"no-dupe-args": "Bug Risk",
"no-dupe-keys": "Bug Risk",
"no-duplicate-case": "Bug Risk",
"no-else-return": "Clarity",
"no-empty": "Bug Risk",
"no-empty-character-class": "Bug Risk",
"no-empty-label": "Bug Risk",
"no-empty-pattern": "Bug Risk",
"no-eq-null": "Bug Risk",
"no-eval": "Security",
"no-ex-assign": "Bug Risk",
"no-extend-native": "Bug Risk",
"no-extra-bind": "Bug Risk",
"no-extra-boolean-cast": "Bug Risk",
"no-extra-parens": "Bug Risk",
"no-extra-semi": "Bug Risk",
"no-fallthrough": "Bug Risk",
"no-floating-decimal": "Clarity",
"no-func-assign": "Bug Risk",
"no-implicit-coercion": "Bug Risk",
"no-implied-eval": "Security",
"no-inner-declarations": "Compatibility",
"no-invalid-regexp": "Bug Risk",
"no-invalid-this": "Bug Risk",
"no-irregular-whitespace": "Compatibility",
"no-iterator": "Compatibility",
"no-label-var": "Bug Risk",
"no-labels": "Bug Risk",
"no-lone-blocks": "Bug Risk",
"no-loop-func": "Bug Risk",
"no-magic-numbers": "Clarity",
"no-mixed-requires": "Clarity",
"no-multi-spaces": "Bug Risk",
"no-multi-str": "Compatibility",
"no-native-reassign": "Bug Risk",
"no-negated-in-lhs": "Bug Risk",
"no-new": "Bug Risk",
"no-new-func": "Clarity",
"no-new-require": "Clarity",
"no-new-wrappers": "Bug Risk",
"no-obj-calls": "Bug Risk",
"no-octal": "Compatibility",
"no-octal-escape": "Compatibility",
"no-param-reassign": "Bug Risk",
"no-path-concat": "Bug Risk",
"no-process-env": "Bug Risk",
"no-process-exit": "Bug Risk",
"no-proto": "Compatibility",
"no-redeclare": "Bug Risk",
"no-regex-spaces": "Bug Risk",
"no-restricted-modules": "Security",
"no-return-assign": "Bug Risk",
"no-script-url": "Security",
"no-self-compare": "Bug Risk",
"no-sequences": "Bug Risk",
"no-shadow": "Bug Risk",
"no-shadow-restricted-names": "Bug Risk",
"no-sparse-arrays": "Bug Risk",
"no-sync": "Bug Risk",
"no-throw-literal": "Clarity",
"no-undef": "Bug Risk",
"no-undef-init": "Bug Risk",
"no-undefined": "Compatibility",
"no-unexpected-multiline": "Bug Risk",
"no-unreachable": "Bug Risk",
"no-unused-expressions": "Bug Risk",
"no-unused-vars": "Bug Risk",
"no-use-before-define": "Compatibility",
"no-useless-call": "Bug Risk",
"no-useless-concat": "Bug Risk",
"no-void": "Compatibility",
"no-warning-comments": "Bug Risk",
"no-with": "Compatibility",
"np-dupe-keys": "Bug Risk",
"radix": "Bug Risk",
"use-isnan": "Bug Risk",
"valid-typeof": "Bug Risk"
"use-strict": "Bug Risk",
"valid-jsdoc": "Clarity",
"valid-typeof": "Bug Risk",
"vars-on-top": "Clarity",
"wrap-iife": "Clarity",
"yoda": "Clarity"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would argue yoda conditionals are to avoid bug risks (accidental = instead of ==) more than they are for clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A yoda conditional is when a literal value is being compared to a variable in that order. See http://eslint.org/docs/rules/yoda

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes: I'm arguing that their stylistic purpose is to avoid a bug, because if you accidentally type = it's a compile/runtime error instead of a "now this variable has the wrong value and nobody knows why" bug. At least, that's always why I've used them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't think you follow me. It's not about assignment (accidentally typing =), it's about the order used in properly constructed comparisons. From the docs I referenced:

if ("red" === color) {
    // ...
}

This is called a Yoda condition because it reads as, "red is the color", similar to the way the Star Wars character Yoda speaks. Compare to the other way of arranging the operands:

if (color === "red") {
    // ...
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not following each other very well. :)

I understand what Yoda conditionals are: I personally tend to use them. The reason I, personally, use Yoda conditionals is to avoid the bug risk of accidental =. So I would consider not using a Yoda conditional a bug risk, not a clarity decision. That is:

if (color == "red") { ... }

is, in my eyes, a bug risk (because it's far too easy to miss/delete one of the = and not realize it). (The risk is somewhat reduced by the fact that almost everybody requires === in JS nowadays, but still.)

That being said, "Bug Risk" may not be an appropriate classification because it only makes sense if your eslintrc requires people to use yoda conditionals. If your eslintrc requires them not to be used, "Clarity" does make more sense. So since either configuration is possible, perhaps "Clarity" is more broadly applicable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, okay, I'm with you now. Agree "Clarity" is probably better. Tying vote is that the default for the rule is "never."

};

var categories = function(checkName) {
Expand Down