Skip to content

Commit

Permalink
fixing bug with global-scope detection, closes #7
Browse files Browse the repository at this point in the history
  • Loading branch information
getify committed Mar 12, 2019
1 parent 97e66d5 commit c841b84
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 14 deletions.
24 changes: 14 additions & 10 deletions README.md
Expand Up @@ -267,11 +267,11 @@ The **proper-arrows**/*this* rule can be configured in three modes: `"nested"` (

* [`"nested"`](#rule-this-configuration-nested) (default) permits a `this` to appear lower in a nested `=>` arrow function (i.e., `x = y => z => this.foo(z)`), as long as there is no non-arrow function boundary crossed.

Additionally, include the `"no-global"` option to forbid `this`-containing `=>` arrow functions from the global scope (where they would inherit the global object as `this`).
Additionally, include the `"no-global"` option to forbid `this`-containing `=>` arrow functions from the top-level scope (where they might inherit the global object as `this`).

* [`"always"`](#rule-this-configuration-always) is more strict, requiring every single `=>` arrow function to have its own `this` reference.

Additionally, include the `"no-global"` option to forbid `this`-containing `=>` arrow functions from the global scope (where they would inherit the global object as `this`).
Additionally, include the `"no-global"` option to forbid `this`-containing `=>` arrow functions from the top-level scope (where they might inherit the global object as `this`).

* [`"never"`](#rule-this-configuration-never) is the reverse of the rule, which is that all `=>` arrow functions are forbidden from using `this`.

Expand All @@ -293,7 +293,7 @@ To configure this rule mode (default):

This rule mode allows a `this` to appear either in the `=>` arrow function, or nested deeper in a chain of `=>` arrow functions, as long as there is **no non-arrow function boundary crossed in the nesting**.

If `"no-global"` **has not** been specified for this mode, these statements will all pass the rule:
If the `"no-global"` flag **has not** been set for this mode, these statements will all pass the rule:

```js
var a = b => this.foo(b);
Expand All @@ -303,7 +303,7 @@ var c = d => e => this.foo(e);
var f = (g = h => this.foo(h)) => g;
```

And if `"no-global"` **has not** been specified for this mode, these statements will each fail the rule:
But these statements will each fail this rule:

```js
var a = b => foo(b);
Expand All @@ -317,7 +317,9 @@ var h = i => function(j){ this.foo(j); };
var k = l => function(){ return m => this.foo(m); };
```

But if `"no-global"` **has** been specified for this mode, these statements will all pass the rule:
##### `"no-global"` Flag

If the `"no-global"` flag **has** been set for this mode, these statements will all pass the rule:

```js
function foo() { return a => this.bar(a); }
Expand All @@ -329,7 +331,7 @@ function foo(cb = a => this.bar(a)) { .. }
function foo(cb = a => b => this.bar(a,b)) { .. }
```

And if `"no-global"` **has** been specified for this mode, these statments will each fail the rule:
And these statments will each fail this `"no-global"`-flagged rule:

```js
var foo = a => this.bar(a);
Expand Down Expand Up @@ -359,7 +361,7 @@ To configure this rule mode:

This rule mode requires a `this` reference to appear in every single `=>` arrow function (e.g., nested `this` is not sufficient).

If `"no-global"` **has not** been specified for this mode, these statements will all pass the rule:
If the `"no-global"` flag **has not** been set for this mode, these statements will all pass the rule:

```js
var a = b => this.foo(b);
Expand All @@ -369,7 +371,7 @@ var c = d => this.foo(e => this.bar(e));
var f = (g = h => this.foo(h)) => this.bar(g);
```

And if `"no-global"` **has not** been specified for this mode, these statements will each fail the rule:
But these statements will each fail this rule:

```js
var a = b => foo(b);
Expand All @@ -383,15 +385,17 @@ var i = (j = k => k) => this.foo(j);

**Note:** In each of the above examples, at least one of the `=>` arrow functions does not have its own `this`, hence the mode's rule failure (doesn't consider nested `this`).

But if `"no-global"` **has** been specified for this mode, these statements will all pass the rule:
##### `"no-global"` Flag

If the `"no-global"` flag **has** been set for this mode, these statements will all pass the rule:

```js
function foo() { return a => this.bar(a); }

function foo(cb = a => this.bar(a)) { .. }
```

And if `"no-global"` **has** been specified for this mode, these statements will each fail the rule:
And these statements will each fail this `"no-global"`-flagged rule:

```js
var foo = a => this.bar(a);
Expand Down
18 changes: 15 additions & 3 deletions lib/index.js
Expand Up @@ -89,13 +89,13 @@ module.exports = {
},
},
create(context) {
var sourceTypeScript = context.parserOptions.sourceType !== "module";
var parserOptions = context.parserOptions;
var extraGlobalScope = parserOptions.ecmaFeatures && context.parserOptions.ecmaFeatures.globalReturn;

var nestedThis = (context.options[0] === "nested" || !("0" in context.options));
var alwaysThis = (context.options[0] === "always");
var neverThis = (context.options[0] === "never");
var noGlobal = (
sourceTypeScript &&
["always","nested",].includes(context.options[0]) &&
context.options[1] === "no-global"
);
Expand All @@ -108,7 +108,19 @@ module.exports = {
},
"ArrowFunctionExpression:exit": function exit(node) {
var scope = context.getScope();
var inGlobalScope = sourceTypeScript && scope.upper.upper == null;
var inGlobalScope = (
(
extraGlobalScope &&
scope.upper &&
scope.upper.upper &&
scope.upper.upper.type == "global"
) ||
(
!extraGlobalScope &&
scope.upper &&
scope.upper.type == "global"
)
);
var foundThis = thisFoundIn.has(node);

if (foundThis && noGlobal && inGlobalScope) {
Expand Down
2 changes: 1 addition & 1 deletion package.json
@@ -1,6 +1,6 @@
{
"name": "@getify/eslint-plugin-proper-arrows",
"version": "1.0.0",
"version": "1.0.1",
"description": "ESLint rules to ensure proper arrow function definitions",
"main": "./lib/index.js",
"scripts": {
Expand Down
4 changes: 4 additions & 0 deletions scripts/node-tests.js
Expand Up @@ -44,6 +44,10 @@ global.thisAlwaysNoGlobalOptions = {
parserOptions: { ecmaVersion: 2015, },
rules: { "@getify/proper-arrows/this": ["error","always","no-global",], },
};
global.thisAlwaysNoGlobalNodeCommonJSOptions = {
parserOptions: { ecmaVersion: 2015, ecmaFeatures: { globalReturn: true, }, },
rules: { "@getify/proper-arrows/this": ["error","always","no-global",], },
};
global.thisNestedNoGlobalOptions = {
parserOptions: { ecmaVersion: 2015, },
rules: { "@getify/proper-arrows/this": ["error","nested","no-global",], },
Expand Down
14 changes: 14 additions & 0 deletions tests/tests.this.js
Expand Up @@ -219,6 +219,20 @@ QUnit.test( "THIS: property arrow, this (always + no-global)", function test(ass
assert.strictEqual( messageId, "noGlobal", "messageId" );
} );

QUnit.test( "THIS: outer arrow, this (always + no-global + node/commonjs)", function test(assert){
var code = `
var x = y => this.foo(y);
`;

var results = eslinter.verify( code, thisAlwaysNoGlobalNodeCommonJSOptions );
var [{ ruleId, messageId, } = {},] = results || [];

assert.expect( 3 );
assert.strictEqual( results.length, 1, "only 1 error" );
assert.strictEqual( ruleId, "@getify/proper-arrows/this", "ruleId" );
assert.strictEqual( messageId, "noGlobal", "messageId" );
} );


// **********************************************

Expand Down

0 comments on commit c841b84

Please sign in to comment.