Add eslint to ensure correct usage of feature flag (https://github.com/woocommerce/woocommerce-blocks/pull/1696)

* introduce feature flags

* move config to webpack-helper

* remove package default

* add gating to frontendConfig and coreConfig

* add feature gating to PHP

* add flag to start command

* move code to Bootstrap.php

* remove flag from npm start

* add eslint rule

* add strict equal only rule

* update messages to use tokens

* update strictBinary to whitelistedFlag and add all tests

* update to correct messageId key

* add more tests

* highlight wrong flag
This commit is contained in:
Seghir Nadir 2020-02-10 14:24:15 +01:00 committed by GitHub
parent a39426495d
commit ce190a9c72
4 changed files with 442 additions and 1 deletions

View File

@ -10,6 +10,7 @@ module.exports = {
rules: {
'@wordpress/dependency-group': 'off',
'woocommerce/dependency-group': 'error',
'woocommerce/feature-flag': 'error',
'valid-jsdoc': 'off',
radix: 'error',
yoda: [ 'error', 'never' ],

View File

@ -1,6 +1,6 @@
{
"name": "eslint-plugin-woocommerce",
"version": "0.0.1",
"version": "0.0.2",
"main": "index.js",
"devDependencies": {
"eslint": "6.8.0"

View File

@ -0,0 +1,188 @@
/**
* External dependencies
*/
import { RuleTester } from 'eslint';
/**
* Internal dependencies
*/
import rule from '../feature-flag';
const ruleTester = new RuleTester( {
parserOptions: {
sourceType: 'module',
ecmaVersion: 6,
},
} );
ruleTester.run( 'feature-flag', rule, {
valid: [
{
code: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
},
],
invalid: [
{
code: `
if ( WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
errors: [
{
messageId: 'accessedViaEnv',
},
],
output: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
},
{
code: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE !== 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
errors: [
{
messageId: 'equalOperator',
},
],
output: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
},
{
code: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE == 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
errors: [
{
messageId: 'equalOperator',
},
],
output: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
},
{
code: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE > 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
errors: [
{
messageId: 'equalOperator',
},
],
output: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
},
{
code: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE < 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
errors: [
{
messageId: 'equalOperator',
},
],
output: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
},
{
code: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE >= 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
errors: [
{
messageId: 'equalOperator',
},
],
output: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
},
{
code: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE <= 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
errors: [
{
messageId: 'equalOperator',
},
],
output: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
},
{
code: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE == 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
errors: [
{
messageId: 'equalOperator',
},
],
output: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
},
{
code: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE > 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
errors: [
{
messageId: 'equalOperator',
},
],
output: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
},
{
code: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'core' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
errors: [
{
messageId: 'whiteListedFlag',
},
],
output: `
if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
registerBlockType( 'woocommerce/checkout', settings );
}`,
},
{
code: `
const featureFlag = process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental'`,
errors: [
{
messageId: 'noTernary',
},
],
},
],
} );

View File

@ -0,0 +1,252 @@
/**
* 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 WOOCOMMERCE_BLOCKS_PHASE variable is accessed via
* `process.env.WOOCOMMERCE_BLOCKS_PHASE`.
*
* @example
* ```js
* // good
* if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
*
* // bad
* if ( WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
* ```
*
* @param {Object} node The WOOCOMMERCE_BLOCKS_PHASE identifier node.
* @param {Object} context The eslint context object.
*/
function testIsAccessedViaProcessEnv( node, context ) {
let parent = node.parent;
if (
parent &&
parent.type === 'MemberExpression' &&
context.getSource( parent ) === 'process.env.WOOCOMMERCE_BLOCKS_PHASE'
) {
return;
}
if ( parent && parent.type === 'BinaryExpression' ) {
if ( parent.left.type === 'Identifier' ) {
parent = parent.left;
} else {
parent = parent.right;
}
}
context.report( {
node,
messageId: 'accessedViaEnv',
fix( fixer ) {
return fixer.replaceText(
parent,
'process.env.WOOCOMMERCE_BLOCKS_PHASE'
);
},
} );
}
/**
* Tests whether the WOOCOMMERCE_BLOCKS_PHASE strict binary comparison
* is strict equal only
*
* @example
* ```js
* // good
* if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
* if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'stable' ) {
*
* // bad
* if ( process.env.WOOCOMMERCE_BLOCKS_PHASE !== 'experimental' ) {
* if ( process.env.WOOCOMMERCE_BLOCKS_PHASE !== 'stable' ) {
* ```
*
* @param {Object} node The WOOCOMMERCE_BLOCKS_PHASE identifier node.
* @param {Object} context The eslint context object.
*/
function testBinaryExpressionOperatorIsEqual( node, context ) {
const sourceCode = context.getSourceCode();
node = node.parent.parent;
if ( node.type === 'BinaryExpression' ) {
const operatorToken = sourceCode.getFirstTokenBetween(
node.left,
node.right,
( token ) => token.value === node.operator
);
if ( operatorToken.value === '===' ) {
return;
}
context.report( {
node,
loc: operatorToken.loc,
messageId: 'equalOperator',
fix( fixer ) {
return fixer.replaceText( operatorToken, '===' );
},
} );
}
}
/**
* Tests whether the WOOCOMMERCE_BLOCKS_PHASE variable is used in a strict binary
* equality expression in a comparison with a enum of string flags, triggering a
* violation if not.
*
* @example
* ```js
* // good
* if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
* if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'stable' ) {
*
* // bad
* if ( process.env.WOOCOMMERCE_BLOCKS_PHASE == 'experimental' ) {
* if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'core' ) {
* ```
*
* @param {Object} node The WOOCOMMERCE_BLOCKS_PHASE identifier node.
* @param {Object} context The eslint context object.
*/
function testIsUsedInStrictBinaryExpression( node, context ) {
const parent = findParent(
node,
( candidate ) => candidate.type === 'BinaryExpression'
);
const flags = [ 'experimental', 'stable' ];
let providedFlag;
if ( parent ) {
const comparisonNode =
node.parent.type === 'MemberExpression' ? node.parent : node;
const hasCorrectOperands =
( parent.left === comparisonNode &&
flags.includes( parent.right.value ) ) ||
( parent.right === comparisonNode &&
flags.includes( parent.left.value ) );
if (
parent.left === comparisonNode &&
typeof parent.right.value === 'string'
) {
providedFlag = parent.right;
} else {
providedFlag = parent.left;
}
if ( hasCorrectOperands ) {
return;
}
}
context.report( {
node,
loc: providedFlag.loc,
messageId: 'whiteListedFlag',
data: {
flags: flags.join( ', ' ),
},
fix( fixer ) {
return fixer.replaceText( providedFlag, "'experimental'" );
},
} );
}
/**
* Tests whether the WOOCOMMERCE_BLOCKS_PHASE variable is used as the condition for an
* if statement, triggering a violation if not.
*
* @example
* ```js
* // good
* if ( process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental' ) {
*
* // bad
* const isFeatureActive = process.env.WOOCOMMERCE_BLOCKS_PHASE === 'experimental';
* ```
*
* @param {Object} node The WOOCOMMERCE_BLOCKS_PHASE identifier node.
* @param {Object} context The eslint context object.
*/
function testIsUsedInIfOrTernary( node, context ) {
const conditionalParent = findParent( node, ( candidate ) =>
[ 'IfStatement', 'ConditionalExpression' ].includes( candidate.type )
);
const binaryParent = findParent(
node,
( candidate ) => candidate.type === 'BinaryExpression'
);
if (
conditionalParent &&
binaryParent &&
conditionalParent.test &&
conditionalParent.test.start === binaryParent.start &&
conditionalParent.test.end === binaryParent.end
) {
return;
}
context.report( {
node,
messageId: 'noTernary',
} );
}
module.exports = {
meta: {
type: 'problem',
schema: [],
fixable: 'code',
messages: {
accessedViaEnv:
'The `WOOCOMMERCE_BLOCKS_PHASE` constant should be accessed using `process.env.WOOCOMMERCE_BLOCKS_PHASE`.',
whiteListedFlag:
'The `WOOCOMMERCE_BLOCKS_PHASE` constant should only be used in a strict equality comparison with a predefined flag of: {{ flags }}.',
equalOperator:
'The `WOOCOMMERCE_BLOCKS_PHASE` comparison should only be a strict equal `===`, if you need `!==` try switching the flag ',
noTernary:
'The `WOOCOMMERCE_BLOCKS_PHASE` constant should only be used as part of the condition in an if statement or ternary expression.',
},
},
create( context ) {
return {
Identifier( node ) {
// Bypass any identifiers with a node name different to `WOOCOMMERCE_BLOCKS_PHASE`.
if ( node.name !== 'WOOCOMMERCE_BLOCKS_PHASE' ) {
return;
}
testIsAccessedViaProcessEnv( node, context );
testIsUsedInStrictBinaryExpression( node, context );
testBinaryExpressionOperatorIsEqual( node, context );
testIsUsedInIfOrTernary( node, context );
},
Literal( node ) {
// Bypass any identifiers with a node value different to `WOOCOMMERCE_BLOCKS_PHASE`.
if ( node.value !== 'WOOCOMMERCE_BLOCKS_PHASE' ) {
return;
}
if ( node.parent && node.parent.type === 'MemberExpression' ) {
testIsAccessedViaProcessEnv( node, context );
}
},
};
},
};