Skip to content

Commit

Permalink
Add custom eslint rule for usage of GUTENBERG_PHASE
Browse files Browse the repository at this point in the history
  • Loading branch information
talldan committed Jan 31, 2019
1 parent 6b35d5e commit b7d2a5e
Show file tree
Hide file tree
Showing 6 changed files with 288 additions and 1 deletion.
1 change: 1 addition & 0 deletions packages/eslint-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
### New Features

- New Rule: [`@wordpress/no-unused-vars-before-return`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/no-unused-vars-before-return.md)
- New Rule: [`@wordpress/gutenberg-phase`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/gutenberg-phase.md)

## 1.0.0 (2018-12-12)

Expand Down
3 changes: 2 additions & 1 deletion packages/eslint-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ The granular rulesets will not define any environment globals. As such, if they

Rule|Description
---|---
[no-unused-vars-before-return](docs/rules/no-unused-vars-before-return.md)|Disallow assigning variable values if unused before a return
[no-unused-vars-before-return](docs/rules/no-unused-vars-before-return.md)|Disallow assigning variable values if unused before a return.
[gutenberg-phase](docs/rules/gutenberg-phase.md)|Governs the use of the `window.GUTENBERG_PHASE` constant.

### Legacy

Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/configs/custom.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = {
],
rules: {
'@wordpress/no-unused-vars-before-return': 'error',
'@wordpress/gutenberg-phase': 'error',
'no-restricted-syntax': [
'error',
{
Expand Down
62 changes: 62 additions & 0 deletions packages/eslint-plugin/docs/rules/gutenberg-phase.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# The `GUTENBERG_PHASE` global (gutenberg-phase)

To enable the use of feature flags in Gutenberg, the GUTENBERG_PHASE global constant was introduced. This constant is replaced with a number value at build time using webpack's define plugin.

There are a few rules around using this constant:

- Only access `GUTENBERG_PHASE` via the window object, e.g. `window.GUTENBERG_PHASE`. This ensures that environments that do not inject the variable at build time do not encounter an error due to an undefined global variable (`window.GUTENBERG_PHASE` evaluates as undefined, while `GUTENBERG_PHASE` throws an error.). The webpack configuration will also only replace exact matches of `window.GUTENBERG_PHASE`.
- The `GUTENBERG_PHASE` variable should only be used in a strict equality comparison with a number, e.g. `window.GUTENBERG_PHASE === 2` or `window.GUTENBERG_PHASE !== 2`. The value of the injected variable should always be a number, so this ensures the correct evaluation of the expression. Furthermore, when `GUTENBERG_PHASE` is undefined this comparison still returns either true (for `!==`) or false (for `===`), whereas both the `<` and `>` operators will always return false.
- `GUTENBERG_PHASE` should only be used within the condition of an if statement, e.g. `if ( window.GUTENBERG_PHASE === 2 ) { // implement feature here }`. This rule ensure that where the expression `window.GUTENBERG_PHASE === 2` resolves to false, the entire if statement and its body is removed through dead code elimination.


## Rule details

The following patterns are considered warnings:

```js
if ( GUTENBERG_PHASE === 2 ) {
// implement feature here.
}
```

```js
if ( window[ 'GUTENBERG_PHASE' ] === 2 ) {
// implement feature here.
}
```

```js
if ( window.GUTENBERG_PHASE === '2' ) {
// implement feature here.
}
```

```js
if ( window.GUTENBERG_PHASE > 2 ) {
// implement feature here.
}
```

```js
if ( true || window.GUTENBERG_PHASE > 2 ) {
// implement feature here.
}
```

```js
const isMyFeatureActive = window.GUTENBERG_PHASE === 2;
```

The following patterns are not considered warnings:

```js
if ( window.GUTENBERG_PHASE === 2 ) {
// implement feature here.
}
```

```js
if ( window.GUTENBERG_PHASE !== 2 ) {
return;
}
```
58 changes: 58 additions & 0 deletions packages/eslint-plugin/rules/__tests__/gutenberg-phase.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* External dependencies
*/
import { RuleTester } from 'eslint';

/**
* Internal dependencies
*/
import rule from '../gutenberg-phase';

const ruleTester = new RuleTester( {
parserOptions: {
ecmaVersion: 6,
},
} );

const ACCESS_ERROR = 'The `GUTENBERG_PHASE` constant should only be accessed as a property of the `window` object using dot notation.';
const EQUALITY_ERROR = 'The `GUTENBERG_PHASE` constant should only be used in a strict equality comparison with a primitive number.';
const IF_ERROR = 'The `GUTENBERG_PHASE` constant should only be used as part of an expression that is the only condition of an if statement.';

ruleTester.run( 'gutenberg-phase', rule, {
valid: [
{ code: `if ( window.GUTENBERG_PHASE === 2 ) {}` },
{ code: `if ( window.GUTENBERG_PHASE !== 2 ) {}` },
{ code: `if ( 2 === window.GUTENBERG_PHASE ) {}` },
{ code: `if ( 2 !== window.GUTENBERG_PHASE ) {}` },
],
invalid: [
{
code: `if ( GUTENBERG_PHASE === 1 ) {}`,
errors: [ { message: ACCESS_ERROR } ],
},
{
code: `if ( window[ 'GUTENBERG_PHASE' ] === 1 ) {}`,
errors: [ { message: ACCESS_ERROR } ],
},
{
code: `if ( window.GUTENBERG_PHASE > 1 ) {}`,
errors: [ { message: EQUALITY_ERROR } ],
},
{
code: `if ( window.GUTENBERG_PHASE === '2' ) {}`,
errors: [ { message: EQUALITY_ERROR } ],
},
{
code: `if ( true ) { window.GUTENBERG_PHASE === 2 }`,
errors: [ { message: IF_ERROR } ],
},
{
code: `if ( true || window.GUTENBERG_PHASE === 2 ) {}`,
errors: [ { message: IF_ERROR } ],
},
{
code: `const isFeatureActive = window.GUTENBERG_PHASE === 2;`,
errors: [ { message: IF_ERROR } ],
},
],
} );
164 changes: 164 additions & 0 deletions packages/eslint-plugin/rules/gutenberg-phase.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
/**
* Traverse up through the chain of parent AST nodes returning the first parent
* the predicate returns a truthy value for.
*
* @param {Object} sourceNode The AST node to search from.
* @param {function} predicate A predicate invoked for each parent.
*
* @return {?Object } The first encountered parent node where the predicate
* returns a truthy value.
*/
function findParent( sourceNode, predicate ) {
if ( ! sourceNode.parent ) {
return;
}

if ( predicate( sourceNode.parent ) ) {
return sourceNode.parent;
}

return findParent( sourceNode.parent, predicate );
}

/**
* Tests whether the GUTENBERG_PHASE variable is accessed via the global window
* object, triggering a violation if not.
*
* @example
* ```js
* // good
* if ( window.GUTENBERG_PHASE === 2 ) {
*
* // bad
* if ( GUTENBERG_PHASE === 2 ) {
* ```
*
* @param {Object} node The GUTENBERG_PHASE identifier node.
* @param {Object} context The eslint context object.
*/
function testIsAccessedViaWindowObject( node, context ) {
const parent = node.parent;

if (
parent &&
node.type === 'Identifier' &&
parent.type === 'MemberExpression' &&
parent.object.name === 'window' &&
! parent.computed

) {
return;
}

context.report(
node,
'The `GUTENBERG_PHASE` constant should only be accessed as a property of the `window` object using dot notation.',
);
}

/**
* Tests whether the GUTENBERG_PHASE variable is used in a strict binary
* equality expression in a comparison with a number, triggering a
* violation if not.
*
* @example
* ```js
* // good
* if ( window.GUTENBERG_PHASE === 2 ) {
*
* // bad
* if ( window.GUTENBERG_PHASE >= '2' ) {
* ```
*
* @param {Object} node The GUTENBERG_PHASE identifier node.
* @param {Object} context The eslint context object.
*/
function testIsUsedInStrictBinaryExpression( node, context ) {
const parent = findParent( node, ( candidate ) => candidate.type === 'BinaryExpression' );

if ( parent ) {
const comparisonNode = node.parent.type === 'MemberExpression' ? node.parent : node;

// Test for window.GUTENBERG_PHASE === <number> or <number> === window.GUTENBERG_PHASE
const hasCorrectOperator = [ '===', '!==' ].includes( parent.operator );
const hasCorrectOperands = (
( parent.left === comparisonNode && typeof parent.right.value === 'number' ) ||
( parent.right === comparisonNode && typeof parent.left.value === 'number' )
);

if ( hasCorrectOperator && hasCorrectOperands ) {
return;
}
}

context.report(
node,
'The `GUTENBERG_PHASE` constant should only be used in a strict equality comparison with a primitive number.',
);
}

/**
* Tests whether the GUTENBERG_PHASE variable is used as the condition for an
* if statement, triggering a violation if not.
*
* @example
* ```js
* // good
* if ( window.GUTENBERG_PHASE === 2 ) {
*
* // bad
* const isFeatureActive = window.GUTENBERG_PHASE === 2;
* ```
*
* @param {Object} node The GUTENBERG_PHASE identifier node.
* @param {Object} context The eslint context object.
*/
function testIsUsedInIfStatement( node, context ) {
const ifParent = findParent( node, ( candidate ) => candidate.type === 'IfStatement' );
const binaryParent = findParent( node, ( candidate ) => candidate.type === 'BinaryExpression' );

if ( ifParent &&
binaryParent &&
ifParent.test &&
ifParent.test.start === binaryParent.start &&
ifParent.test.end === binaryParent.end
) {
return;
}

context.report(
node,
'The `GUTENBERG_PHASE` constant should only be used as part of an expression that is the only condition of an if statement.',
);
}

module.exports = {
meta: {
type: 'problem',
schema: [],
},
create( context ) {
return {
Identifier( node ) {
// Bypass any identifiers with a node name different to `GUTENBERG_PHASE`.
if ( node.name !== 'GUTENBERG_PHASE' ) {
return;
}

testIsAccessedViaWindowObject( node, context );
testIsUsedInStrictBinaryExpression( node, context );
testIsUsedInIfStatement( node, context );
},
Literal( node ) {
// Bypass any identifiers with a node value different to `GUTENBERG_PHASE`.
if ( node.value !== 'GUTENBERG_PHASE' ) {
return;
}

if ( node.parent && node.parent.type === 'MemberExpression' ) {
testIsAccessedViaWindowObject( node, context );
}
},
};
},
};

0 comments on commit b7d2a5e

Please sign in to comment.