Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESLint Plugin: Restrict tolerances as fixable error #13785

Merged
merged 2 commits into from
Feb 11, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
ESLint Plugin: Restrict tolerances as fixable error
  • Loading branch information
aduth committed Feb 11, 2019
commit 1cf75fae0feab0a3745bd3743f430d8b0f2d94a2
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