Skip to content

Commit

Permalink
Update: add ESLint core Node.js and CommonJS rules (#206)
Browse files Browse the repository at this point in the history
  • Loading branch information
kaicataldo authored Feb 18, 2020
1 parent b8f9945 commit 8788a11
Show file tree
Hide file tree
Showing 31 changed files with 3,233 additions and 0 deletions.
164 changes: 164 additions & 0 deletions docs/rules/callback-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
# node/callback-return
> require `return` statements after callbacks
The callback pattern is at the heart of most I/O and event-driven programming
in JavaScript.

```js
function doSomething(err, callback) {
if (err) {
return callback(err);
}
callback();
}
```

To prevent calling the callback multiple times it is important to `return` anytime the callback is triggered outside
of the main function body. Neglecting this technique often leads to issues where you do something more than once.
For example, in the case of an HTTP request, you may try to send HTTP headers more than once leading Node.js to `throw`
a `Can't render headers after they are sent to the client.` error.

## 📖 Rule Details

This rule is aimed at ensuring that callbacks used outside of the main function block are always part-of or immediately
preceding a `return` statement. This rule decides what is a callback based on the name of the function being called.

### Options

The rule takes a single option - an array of possible callback names - which may include object methods. The default callback names are `callback`, `cb`, `next`.

#### Default callback names

Examples of **incorrect** code for this rule with the default `["callback", "cb", "next"]` option:

```js
/*eslint callback-return: "error"*/

function foo(err, callback) {
if (err) {
callback(err);
}
callback();
}
```

Examples of **correct** code for this rule with the default `["callback", "cb", "next"]` option:

```js
/*eslint callback-return: "error"*/

function foo(err, callback) {
if (err) {
return callback(err);
}
callback();
}
```

#### Supplied callback names

Examples of **incorrect** code for this rule with the option `["done", "send.error", "send.success"]`:

```js
/*eslint callback-return: ["error", ["done", "send.error", "send.success"]]*/

function foo(err, done) {
if (err) {
done(err);
}
done();
}

function bar(err, send) {
if (err) {
send.error(err);
}
send.success();
}
```

Examples of **correct** code for this rule with the option `["done", "send.error", "send.success"]`:

```js
/*eslint callback-return: ["error", ["done", "send.error", "send.success"]]*/

function foo(err, done) {
if (err) {
return done(err);
}
done();
}

function bar(err, send) {
if (err) {
return send.error(err);
}
send.success();
}
```

### Known Limitations

Because it is difficult to understand the meaning of a program through static analysis, this rule has limitations:

* *false negatives* when this rule reports correct code, but the program calls the callback more than one time (which is incorrect behavior)
* *false positives* when this rule reports incorrect code, but the program calls the callback only one time (which is correct behavior)

#### Passing the callback by reference

The static analysis of this rule does not detect that the program calls the callback if it is an argument of a function (for example, `setTimeout`).

Example of a *false negative* when this rule reports correct code:

```js
/*eslint callback-return: "error"*/

function foo(err, callback) {
if (err) {
setTimeout(callback, 0); // this is bad, but WILL NOT warn
}
callback();
}
```

#### Triggering the callback within a nested function

The static analysis of this rule does not detect that the program calls the callback from within a nested function or an immediately-invoked function expression (IIFE).

Example of a *false negative* when this rule reports correct code:

```js
/*eslint callback-return: "error"*/

function foo(err, callback) {
if (err) {
process.nextTick(function() {
return callback(); // this is bad, but WILL NOT warn
});
}
callback();
}
```

#### If/else statements

The static analysis of this rule does not detect that the program calls the callback only one time in each branch of an `if` statement.

Example of a *false positive* when this rule reports incorrect code:

```js
/*eslint callback-return: "error"*/

function foo(err, callback) {
if (err) {
callback(err); // this is fine, but WILL warn
} else {
callback(); // this is fine, but WILL warn
}
}
```

## 🔎 Implementation

- [Rule source](../../lib/rules/callback-return.js)
- [Test source](../../tests/lib/rules/callback-return.js)
91 changes: 91 additions & 0 deletions docs/rules/global-require.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# node/global-require
> require `require()` calls to be placed at top-level module scope
In Node.js, module dependencies are included using the `require()` function, such as:

```js
var fs = require("fs");
```

While `require()` may be called anywhere in code, some style guides prescribe that it should be called only in the top level of a module to make it easier to identify dependencies. For instance, it's arguably harder to identify dependencies when they are deeply nested inside of functions and other statements:

```js
function foo() {

if (condition) {
var fs = require("fs");
}
}
```

Since `require()` does a synchronous load, it can cause performance problems when used in other locations.

Further, ES6 modules mandate that `import` and `export` statements can only occur in the top level of the module's body.

## 📖 Rule Details

This rule requires all calls to `require()` to be at the top level of the module, similar to ES6 `import` and `export` statements, which also can occur only at the top level.

Examples of **incorrect** code for this rule:

```js
/*eslint global-require: "error"*/
/*eslint-env es6*/

// calling require() inside of a function is not allowed
function readFile(filename, callback) {
var fs = require('fs');
fs.readFile(filename, callback)
}

// conditional requires like this are also not allowed
if (DEBUG) { require('debug'); }

// a require() in a switch statement is also flagged
switch(x) { case '1': require('1'); break; }

// you may not require() inside an arrow function body
var getModule = (name) => require(name);

// you may not require() inside of a function body as well
function getModule(name) { return require(name); }

// you may not require() inside of a try/catch block
try {
require(unsafeModule);
} catch(e) {
console.log(e);
}
```

Examples of **correct** code for this rule:

```js
/*eslint global-require: "error"*/

// all these variations of require() are ok
require('x');
var y = require('y');
var z;
z = require('z').initialize();

// requiring a module and using it in a function is ok
var fs = require('fs');
function readFile(filename, callback) {
fs.readFile(filename, callback)
}

// you can use a ternary to determine which module to require
var logger = DEBUG ? require('dev-logger') : require('logger');

// if you want you can require() at the end of your module
function doSomethingA() {}
function doSomethingB() {}
var x = require("x"),
z = require("z");
```

## 🔎 Implementation

- [Rule source](../../lib/rules/global-require.js)
- [Test source](../../tests/lib/rules/global-require.js)
78 changes: 78 additions & 0 deletions docs/rules/handle-callback-err.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# node/handle-callback-err
> require error handling in callbacks
In Node.js, a common pattern for dealing with asynchronous behavior is called the callback pattern.
This pattern expects an `Error` object or `null` as the first argument of the callback.
Forgetting to handle these errors can lead to some really strange behavior in your application.

```js
function loadData (err, data) {
doSomething(); // forgot to handle error
}
```

## 📖 Rule Details

This rule expects that when you're using the callback pattern in Node.js you'll handle the error.

### Options

The rule takes a single string option: the name of the error parameter. The default is `"err"`.

Examples of **incorrect** code for this rule with the default `"err"` parameter name:

```js
/*eslint handle-callback-err: "error"*/

function loadData (err, data) {
doSomething();
}

```

Examples of **correct** code for this rule with the default `"err"` parameter name:

```js
/*eslint handle-callback-err: "error"*/

function loadData (err, data) {
if (err) {
console.log(err.stack);
}
doSomething();
}

function generateError (err) {
if (err) {}
}
```

Examples of **correct** code for this rule with a sample `"error"` parameter name:

```js
/*eslint handle-callback-err: ["error", "error"]*/

function loadData (error, data) {
if (error) {
console.log(error.stack);
}
doSomething();
}
```

#### Regular Expression

Sometimes (especially in big projects) the name of the error variable is not consistent across the project,
so you need a more flexible configuration to ensure that the rule reports all unhandled errors.

If the configured name of the error variable begins with a `^` it is considered to be a regexp pattern.

* If the option is `"^(err|error|anySpecificError)$"`, the rule reports unhandled errors where the parameter name can be `err`, `error` or `anySpecificError`.
* If the option is `"^.+Error$"`, the rule reports unhandled errors where the parameter name ends with `Error` (for example, `connectionError` or `validationError` will match).
* If the option is `"^.*(e|E)rr"`, the rule reports unhandled errors where the parameter name matches any string that contains `err` or `Err` (for example, `err`, `error`, `anyError`, `some_err` will match).


## 🔎 Implementation

- [Rule source](../../lib/rules/handle-callback-err.js)
- [Test source](../../tests/lib/rules/handle-callback-err.js)
Loading

0 comments on commit 8788a11

Please sign in to comment.