Notice: This Wiki is now read only and edits are no longer possible. Please see: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/wikis/Wiki-shutdown-plan for the plan.
Difference between revisions of "Orion/ESLint"
(add in rule problem text rather than a link (where possible)) |
(Use rule reported text rather than link, where possible) |
||
Line 226: | Line 226: | ||
| Warning | | Warning | ||
|style="text-align:center;"| 11.0 | |style="text-align:center;"| 11.0 | ||
− | | | + | | ''Getter is not present'' or ''Setter is not present'' |
| It's a common mistake in JavaScript to create an object with just a setter for a property but never have a corresponding getter defined for it. Without a getter, you cannot read the property, so it ends up not being used. | | It's a common mistake in JavaScript to create an object with just a setter for a property but never have a corresponding getter defined for it. Without a getter, you cannot read the property, so it ends up not being used. | ||
|style="text-align:center;" | | |style="text-align:center;" | | ||
Line 269: | Line 269: | ||
| Warning | | Warning | ||
|style="text-align:center;"| 11.0 | |style="text-align:center;"| 11.0 | ||
− | | | + | | ''Unexpected 'else' after 'return'.'' |
| If an if block contains a return statement, the else block becomes unnecessary. Its contents can be placed outside of the block. | | If an if block contains a return statement, the else block becomes unnecessary. Its contents can be placed outside of the block. | ||
|style="text-align:center;" | | |style="text-align:center;" | | ||
Line 277: | Line 277: | ||
| Error | | Error | ||
|style="text-align:center;"| 11.0 | |style="text-align:center;"| 11.0 | ||
− | | | + | | ''Unexpected label "{${0}}"'' |
| Labeled statements are only used in conjunction with labeled break and continue statements. ECMAScript has no goto statement. | | Labeled statements are only used in conjunction with labeled break and continue statements. ECMAScript has no goto statement. | ||
|style="text-align:center;" | | |style="text-align:center;" | | ||
Line 285: | Line 285: | ||
| Error | | Error | ||
|style="text-align:center;"| 11.0 | |style="text-align:center;"| 11.0 | ||
− | | | + | | ''Use ‘===’ to compare with ‘null’.'' or ''Use ‘!==’ to compare with ‘null’.'' |
| Comparing to null without a type-checking operator (== or !=), can have unintended results as the comparison will evaluate to true when comparing to not just a null, but also an undefined value. | | Comparing to null without a type-checking operator (== or !=), can have unintended results as the comparison will evaluate to true when comparing to not just a null, but also an undefined value. | ||
|style="text-align:center;" | | |style="text-align:center;" | | ||
Line 295: | Line 295: | ||
| '''eval' function calls are discouraged.'' | | '''eval' function calls are discouraged.'' | ||
| Warns when the <code>eval()</code> function is used | | Warns when the <code>eval()</code> function is used | ||
+ | | | ||
| | | | ||
|- | |- | ||
Line 366: | Line 367: | ||
| Error | | Error | ||
|style="text-align:center;"| 11.0 | |style="text-align:center;"| 11.0 | ||
− | | | + | | ''Comparing to itself is potentially pointless.'' |
| Comparing a variable against itself is usually an error, either an typo or refactoring error. It is confusing to the reader and may potentially introduce a runtime error. | | Comparing a variable against itself is usually an error, either an typo or refactoring error. It is confusing to the reader and may potentially introduce a runtime error. | ||
|style="text-align:center;" | | |style="text-align:center;" | |
Revision as of 18:02, 12 January 2016
Orion 5.0 introduced ESLint as the validator that ships with Orion. This page captures ongoing issues.
Contents
Rules
The following sections describe all of the rules currently available in Orion and ones that we plan to add in the future.
Potential Programming Problems
These rules report about problems with your code that might have undesired or incorrect results. Warnings from these rules should be addressed.
Rule | Default Severity | Available Since | Problem Message | Details | Quick Fix? | Fix All? |
---|---|---|---|---|---|---|
no-comma-dangle | Ignore | 8.0 | Trailing commas in object expressions are discouraged. | Flags trailing commas in object expressions. | ✔ | ✔ |
no-cond-assign | Error | 8.0 | Expected a conditional expression and instead saw an assignment. | Flags assignment in an if/while/do..while/for condition.
|
Future | |
no-console | Warning | 8.0 | Discouraged use of console in browser-based code. | Flags the use of console in browser-based code | ||
no-constant-condition | Error | 8.0 | Discouraged use of constant as a conditional expression. | Flags the use of a constant as a conditional expression | ||
no-control-regex | Error | 11.0 | Unexpected control character in regular expression | Control characters are special, invisible characters in the ASCII range 0-31. These characters are rarely used in JavaScript strings so a regular expression containing these characters is most likely a mistake. | ||
no-debugger | Warning | 6.0 | debugger' statement use is discouraged. | Warns when the debugger keyword is used
|
Future | |
no-dupe-keys | Error | 6.0 | Duplicate object key '{key}'. | Flags object literals that contain the same key more than once. | Future | |
no-duplicate-case | Error | 11.0 | Duplicate case label. | A switch statements with duplicate case labels is normally an indication of a programmer error. | ||
no-empty-block | Warning | 7.0 | Empty block should be removed or commented. | Flags an empty block like if (something) { } .
|
✔ | |
no-empty-character-class | Error | 11.0 | Empty class. | Empty character classes in regular expressions do not match anything and can result in code that may not work as intended. | ||
no-extra-boolean-cast | Error | 11.0 | no-extra-boolean-cast | In contexts such as an if statement's test where the result of the expression will already be coerced to a Boolean, casting to a Boolean via double negation (!!) is unnecessary. | ||
no-extra-parens | Warning | 11.0 | Gratuitous parentheses around expression. | This rule restricts the use of parentheses to only where they are necessary. It may be restricted to report only function expressions. | ||
no-extra-semi | Warning | 5.0 | Unnecessary semicolon. | Warns when a semicolon is found where it is not needed | ✔ | ✔ |
no-fallthrough | Error | 6.0 | Switch case may be entered by falling through previous case. If intended, add a new comment //$FALL-THROUGH$ on the line above | Flags a fallthrough case within a switch statement, unless it is explicitly commented.
|
✔ | |
no-invalid-regexp | Error | 11.0 | no-invalid-regexp | This rule validates string arguments passed to the RegExp constructor. | ||
no-irregular-whitespace | Ignore | 11.0 | Irregular whitespace not allowed. | Invalid or irregular whitespace causes issues with ECMAScript 5 parsers and also makes code harder to debug in a similar nature to mixed tabs and spaces. | ||
no-negated-in-lhs | Error | 11.0 | no-negated-in-lhs | Disallow negated left operand of in operator | ||
no-obj-calls | Error | 11.0 | '{${0}}' is not a function. | ECMAScript provides several global objects that are intended to be used as-is. Some of these objects look as if they could be constructors due their capitalization (such as Math and JSON) but will throw an error if you try to execute them as functions. | ||
no-regex-spaces | Error | 8.0 | Avoid using multiple spaces in regular expressions. Use ' {${0}}' instead. | Flags regular expressions that have two or more subsequent spaces. | ||
no-reserved-keys | Error | 8.0 | Reserved words should not be used as property keys. | Checks to see if a JavaScript reserved word is being used as a property key | ✔ | Future |
no-sparse-arrays | Warning | 8.0 | Sparse array declarations should be avoided. | Flags sparse arrays with null elements like var arr = [1, ,2]
|
✔ | Future |
no-unreachable | Error | 6.0 | Unreachable code. | Flags statements that occur after a return , throw , etc.
|
✔ | |
use-isnan | Error | 6.0 | Use the isNaN function to compare with NaN. | Warns when a comparison is done against NaN without using the isNaN function
|
✔ | Future |
valid-typeof | Error | 7.0 | Invalid typeof comparison. | Warns when a typeof comparison uses an invalid type. |
Best Practices
These rules are not necessarily problem with your code, but are warnings that you might be doing something you probably should not be.
Rule | Default Severity | Available Since | Problem Message | Details | Quick Fix? | Fix All? |
---|---|---|---|---|---|---|
accessor-pairs | Warning | 11.0 | Getter is not present or Setter is not present | It's a common mistake in JavaScript to create an object with just a setter for a property but never have a corresponding getter defined for it. Without a getter, you cannot read the property, so it ends up not being used. | ||
block-scoped-var | Error | - | '{variable}' used outside of binding context. |
Occurs when a function-scoped variable (var) is treated as if it was block scoped. |
||
curly | Ignore | 6.0 | Statements should be enclosed in braces. | Warns when statements should be enclosed in braces, for example if(something) foo();
|
Future | Future |
eqeqeq | Warning | 5.0 |
|
Warns when == or != is used
|
✔ | ✔ |
no-caller | Error | 9.0 | arguments.{callee, caller} is deprecated. | Flags references to arguments.callee and arguments.caller .
|
||
no-else-return | Warning | 11.0 | Unexpected 'else' after 'return'. | If an if block contains a return statement, the else block becomes unnecessary. Its contents can be placed outside of the block. | ||
no-empty-label | Error | 11.0 | Unexpected label "{${0}}" | Labeled statements are only used in conjunction with labeled break and continue statements. ECMAScript has no goto statement. | ||
no-eq-null | Error | 11.0 | Use ‘===’ to compare with ‘null’. or Use ‘!==’ to compare with ‘null’. | Comparing to null without a type-checking operator (== or !=), can have unintended results as the comparison will evaluate to true when comparing to not just a null, but also an undefined value. | ||
no-eval | Ignore | 6.0 | 'eval' function calls are discouraged. | Warns when the eval() function is used
|
||
no-implied-eval | Ignore | 6.0 | Implicit 'eval' function calls are discouraged. | Flags calls to the string-argument form of setTimeout and setInterval , which implicitly perform eval . This rule logic was merged to be part of the no-eval rule.
|
||
no-iterator | Error | 8.0 | Discouraged __iterator__ property use. | Flags use of __iterator__ as an identifier name or property key.
|
||
no-new-array | Warning | 6.0 | Use the array literal notation []. | Flags new Array() . ESLint calls this rule no-array-constructor
|
Future | Future |
no-new-func | Warning | 6.0 | The Function constructor is eval. | Flags new Function() .
|
||
no-new-object | Warning | 6.0 | Use the object literal notation {}. | Flags new Object() .
|
||
no-new-wrappers | Warning | 6.0 | Do not use {String, Number, Boolean, Math, JSON} as a constructor. | Flags new applied to any of those.
|
Future | |
no-proto | Error | 9.0 |
|
Flags use of the property __proto__ as object keys or in assignments
|
||
no-redeclare | Warning | 5.0 | '{a}' is already defined. | Usually results from having two for loops in the same function that share a loop variable declaration like var i=... .
|
Future | |
no-self-compare | Error | 11.0 | Comparing to itself is potentially pointless. | Comparing a variable against itself is usually an error, either an typo or refactoring error. It is confusing to the reader and may potentially introduce a runtime error. | ||
no-shadow | Warning | 8.0 | '{a}' is already declared in the upper scope. | Flags variables that have the same name as a variable declared in an upper scope. | Future | |
no-shadow-global | Warning | 9.0 | '{a}' shadows a global member. | Flags variables that have the same name as a variable declared in the global scope or specified environments. | Future | |
no-throw-literal | Warning | 9.0 | Throw an Error instead. | Flags code that throws a literal, eg. throw "a problem occurred";
|
✔ | Future |
no-undef | Error | 5.0 | '{variable}' is not defined. |
|
✔ | |
no-unused-params | Warning | 6.0 | Parameter '{param}' is not used. | Flags parameters in function declarations / expressions if they are not being used | ✔ | |
no-unused-vars | Warning | 5.0 |
|
Warns when a variable is created and not used (read) or a variable that is read-only is assigned to. | ✔ | |
no-use-before-define | Warning | 5.0 | '{a}' was used before it was defined. | Warns when a variable or function is used before it is defined | ||
no-with | Warning | 9.0 | Expected an identifier and instead saw 'with'. | Treated as fatal parse error in JSLint. | ||
radix | Warning | 8.0 | Missing radix parameter. | Warns when parseInt() called without the 2nd parameter (radix). | ||
no-self-assign | Error | 11.0 | Assigning to itself is pointless. | Warns when a variable is being assigned to itself | Future | Future |
Code Style
These rules are not problems in any way but will warn based on certain accepted styles for coding.
Rule | Default Severity | Available Since | Problem Message | Details | Quick Fix? | Fix All? |
---|---|---|---|---|---|---|
missing-doc | Ignore | 5.0 | Missing documentation for function '{name}'. | Flags missing documentation nodes on function declarations and function expressions when they appear as object properties | Future | Future |
missing-doc-items | Ignore | - | Missing '{a}' tag for '{b}'. | Flags missing documentation for parameters, returns, throws, etc on function declarations and function expressions when they appear as object properties | ||
missing-nls | Ignore | 9.0 | Non-externalized string literal '{a}'. | Flags String literals that are not properly NLS'd | ✔ | Future |
unnecessary-nls | Ignore | 11.0 | Unnecessary $NON-NLS$ tag. | Flags $NON-NLS$ comments that are not needed to exclude a string literal | ✔ | ✔ |
new-parens | Error | 6.0 | Missing parentheses invoking constructor. | Flags new Whatever
|
Future | |
no-jslint | Warning | 7.0 | The 'jslint' directive is unsupported, please use eslint-env. | Flags use of the /* jslint */ directive | Future | |
no-mixed-returns | Error | - | Mixed return types should be avoided. | Flags functions that return more than one kind of item, for example returning String and Object | ||
no-undef-init | Warning | 9.0 | Avoid explicitly initializing variables to 'undefined'. | Warns when an explicit assignment to undefined is done, for example var foo = undefined;
|
Future | Future |
semi | Warning | 5.0 | Missing semicolon. | Warns when a semicolon should be used but is not | ✔ | ✔ |
Implementing a new rule
This section explains how to implement a new linting rule. For brevity, all paths are given relative to bundles/org.eclipse.orion.client.javascript/web/ in the client repo.
First, create the tests and rule:
- Create a new describe("{rule-name}") block in: /js-tests/javascript/eslintRuleTests.js. Add several tests to it.
- Run the JS bundle tests and ensure that your new tests fail.
- Implement the actual logic for your rule in /eslint/lib/load-rules-async.js.
- Run the tests again. This time they should all pass.
To get your rule running in the Orion product's validator, there are additional steps:
- Create strings for the validation message(s) generated by your rule in /javascript/nls/root/problems.js.
- Create a short string describing what your rule does in /javascript/nls/root/messages.js.
- Add an entry to the orion.core.setting declaration in /javascript/plugins/javascriptPlugin.js. This allows your rule to be configured from the JS validator settings page. Make sure it references the same nameKey used in messages.js.
- Add a config entry for your rule to /javascript/ruleData.js.
- Add your rule to the #updated() handler in /javascript/validator.js.
Reload the JS plugin, and your rule should now be configurable from the Settings page, and properly set up for translation too.
Tests
- Every rule needs extensive unit tests.
- Unit tests should use Mocha.
Running the tests
- To run the ESLint tests as part of the main JavaScript tests suite load {orion-server-url}/javascript/js-tests/JsMochaSuite.html in your web browser.
- To run just the ESLint rule tests, load {orion-server-url}/js-tests/javascript/JsMochaSuite.html?grep=ESLint%20Rule%20Tests in your browser.
- To run just the ESLint core tests, load {orion-server-url}/js-tests/javascript/JsMochaSuite.html?grep=ESLint%20Core%20Tests in your browser.
- To run only a subset of the tests, add ?grep=ESLint%20Rule%20Tests onto the test page URL.
See Mocha usage options for more options.
User interface
-
Should we have a UI for configuring what rules are active?bug 424268. - Should we try to support
.eslintrc
? This would be an ideal project-scope setting, and is being tracked in bug 461228. -
Should we try to honor equivalent JSLint/JSHint flags when possible? For example/*jslint eqeqeq:false */
could disable the eqeqeq rule on a per-file basis.- ESLint now has its own syntax for this:
/*eslint ..*/
, which we are using instead.
- ESLint now has its own syntax for this:
i18n
We need to support i18n. Pre-req is bug 422278 (orion.edit.validator support for i18n).