Skip to content

Commit

Permalink
ESLint Plugin: Restrict tolerances as fixable error
Browse files Browse the repository at this point in the history
  • Loading branch information
aduth committed Feb 11, 2019
1 parent 8b0c491 commit 1cf75fa
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 22 deletions.
9 changes: 6 additions & 3 deletions packages/eslint-plugin/rules/__tests__/dependency-group.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ ruleTester.run( 'dependency-group', rule, {
valid: [
{
code: `
/*
/**
* External dependencies
*/
import { get } from 'lodash';
import classnames from 'classnames';
/*
/**
* WordPress dependencies
*/
import { Component } from '@wordpress/element';
/*
/**
* Internal dependencies
*/
import edit from './edit';`,
Expand All @@ -41,6 +41,9 @@ import edit from './edit';`,
code: `
import { get } from 'lodash';
import classnames from 'classnames';
/*
* wordpress dependencies.
*/
import { Component } from '@wordpress/element';
import edit from './edit';`,
errors: [
Expand Down
94 changes: 75 additions & 19 deletions packages/eslint-plugin/rules/dependency-group.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,28 @@ module.exports = {
* @typedef {string} WPPackageLocality
*/

/**
* Object describing a dependency block correction to be made.
*
* @property {?espree.Node} comment Comment node on which to replace
* value, if one can be salvaged.
* @property {string} value Expected comment node value.
*
* @typedef {Object} WPDependencyBlockCorrection
*/

/**
* Given a desired locality, generates the expected comment node value
* property.
*
* @param {WPPackageLocality} locality Desired package locality.
*
* @return {string} Expected comment node value.
*/
function getCommentValue( locality ) {
return `*\n * ${ locality } dependencies\n `;
}

/**
* Given an import source string, returns the locality classification
* of the import sort.
Expand Down Expand Up @@ -51,7 +73,7 @@ module.exports = {
return false;
}

// (Temporary) Tolerances:
// Tolerances:
// - Normalize `/**` and `/*`
// - Case insensitive "Dependencies" vs. "dependencies"
// - Ending period
Expand All @@ -61,7 +83,7 @@ module.exports = {
locality = '(External|Node)';
}

const pattern = new RegExp( `^\\*?\\n \\* ${ locality } [dD]ependencies\\.?\\n $` );
const pattern = new RegExp( `^\\*?\\n \\* ${ locality } dependencies\\.?\\n $`, 'i' );
return pattern.test( value );
}

Expand All @@ -79,18 +101,44 @@ module.exports = {
}

/**
* Returns true if a given node is preceded by a comment block
* satisfying a locality requirement, or false otherwise.
* Tests source comments to determine whether a comment exists which
* satisfies the desired locality. If a match is found and requires no
* updates, the function returns undefined. Otherwise, it will return
* a WPDependencyBlockCorrection object describing a correction.
*
* @param {espree.Node} node Node to test.
* @param {WPPackageLocality} locality Desired package locality.
*
* @return {boolean} Whether node preceded by locality comment block.
* @return {?WPDependencyBlockCorrection} Correction, if applicable.
*/
function isPrecededByDependencyBlock( node, locality ) {
return comments.some( ( comment ) => {
return isLocalityDependencyBlock( comment, locality ) && isBefore( comment, node );
} );
function getDependencyBlockCorrection( node, locality ) {
const value = getCommentValue( locality );

let comment;
for ( let i = 0; i < comments.length; i++ ) {
comment = comments[ i ];

if ( ! isBefore( comment, node ) ) {
// Exhausted options.
break;
}

if ( ! isLocalityDependencyBlock( comment, locality ) ) {
// Not usable (either not an block comment, or not one
// matching a tolerable pattern).
continue;
}

if ( comment.value === value ) {
// No change needed. (OK)
return;
}

// Found a comment needing correction.
return { comment, value };
}

return { value };
}

return {
Expand All @@ -103,7 +151,7 @@ module.exports = {
*
* @type {Set<WPPackageLocality>}
*/
const reported = new Set();
const verified = new Set();

// Since we only care to enforce imports which occur at the
// top-level scope, match on Program and test its children,
Expand Down Expand Up @@ -133,23 +181,31 @@ module.exports = {
}

const locality = getPackageLocality( source );
if (
reported.has( locality ) ||
isPrecededByDependencyBlock( child, locality )
) {
if ( verified.has( locality ) ) {
return;
}

reported.add( locality );
// Avoid verifying any other imports for the locality,
// regardless whether a correction must be made.
verified.add( locality );

// Determine whether a correction must be made.
const correction = getDependencyBlockCorrection( child, locality );
if ( ! correction ) {
return;
}

context.report( {
node: child,
message: `Expected preceding "${ locality } dependencies" comment block`,
fix( fixer ) {
return fixer.insertTextBefore(
child,
`/**\n * ${ locality } dependencies\n */\n`
);
const { comment, value } = correction;
const text = `/*${ value }*/`;
if ( comment ) {
return fixer.replaceText( comment, text );
}

return fixer.insertTextBefore( child, text + '\n' );
},
} );
} );
Expand Down

0 comments on commit 1cf75fa

Please sign in to comment.