Skip to content

Commit

Permalink
ESLint Plugin: Add rule dependencies-docblocks
Browse files Browse the repository at this point in the history
  • Loading branch information
aduth committed Feb 8, 2019
1 parent bc23a2e commit b3edc7f
Show file tree
Hide file tree
Showing 7 changed files with 234 additions and 3 deletions.
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/dependency-group`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/dependency-group.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](/packages/eslint-plugin/docs/rules/no-unused-vars-before-return.md)|Disallow assigning variable values if unused before a return
[dependency-group](/packages/eslint-plugin/docs/rules/dependency-group.md)|Enforce dependencies docblocks formatting

### 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 @@ -3,6 +3,7 @@ module.exports = {
'@wordpress',
],
rules: {
'@wordpress/dependency-group': 'error',
'@wordpress/no-unused-vars-before-return': 'error',
'no-restricted-syntax': [
'error',
Expand Down
36 changes: 36 additions & 0 deletions packages/eslint-plugin/docs/rules/dependency-group.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Enforce dependencies docblocks formatting (dependency-group)

Ensures that all top-level package imports adhere to the dependencies grouping conventions as outlined in the [Coding Guidelines](https://github.com/WordPress/gutenberg/blob/master/docs/contributors/coding-guidelines.md#imports).

Specifically, this ensures that:

- An import is preceded by "External dependencies", "WordPress dependencies", or "Internal dependencies" as appropriate by the import source.

## Rule details

Examples of **incorrect** code for this rule:

```js
import { get } from 'lodash';
import { Component } from '@wordpress/element';
import edit from './edit';
```

Examples of **correct** code for this rule:

```js
/*
* External dependencies
*/
import { get } from 'lodash';

/*
* WordPress dependencies
*/
import { Component } from '@wordpress/element';

/*
* Internal dependencies
*/
import edit from './edit';
```
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ To avoid wastefully computing the result of a function call when assigning a var

## Rule details

The following patterns are considered warnings:
Examples of **incorrect** code for this rule:

```js
function example( number ) {
Expand All @@ -17,7 +17,7 @@ function example( number ) {
}
```

The following patterns are not considered warnings:
Examples of **correct** code for this rule:

```js
function example( number ) {
Expand Down
52 changes: 52 additions & 0 deletions packages/eslint-plugin/rules/__tests__/dependencies-docblocks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* External dependencies
*/
import { RuleTester } from 'eslint';

/**
* Internal dependencies
*/
import rule from '../dependency-group';

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

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';`,
},
],
invalid: [
{
code: `
import { get } from 'lodash';
import { Component } from '@wordpress/element';
import edit from './edit';`,
errors: [
{ message: 'Expected preceding "External dependencies" comment block' },
{ message: 'Expected preceding "WordPress dependencies" comment block' },
{ message: 'Expected preceding "Internal dependencies" comment block' },
],
},
],
} );
140 changes: 140 additions & 0 deletions packages/eslint-plugin/rules/dependency-group.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
module.exports = {
meta: {
type: 'layout',
docs: {
description: 'Enforce dependencies docblocks formatting',
url: 'https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/dependency-group.md',
},
schema: [],
},
create( context ) {
const comments = context.getSourceCode().getAllComments();

/**
* Locality classification of an import, one of "External",
* "WordPress", "Internal".
*
* @typedef {string} WPPackageLocality
*/

/**
* Given an import source string, returns the locality classification
* of the import sort.
*
* @param {string} source Import source string.
*
* @return {WPPackageLocality} Package locality.
*/
function getPackageLocality( source ) {
if ( source.startsWith( '.' ) ) {
return 'Internal';
} else if ( source.startsWith( '@wordpress/' ) ) {
return 'WordPress';
}

return 'External';
}

/**
* Returns true if the given comment node satisfies a desired locality,
* or false otherwise.
*
* @param {espree.Node} node Comment node to check.
* @param {WPPackageLocality} locality Desired package locality.
*
* @return {boolean} Whether comment node satisfies locality.
*/
function isLocalityDependencyBlock( node, locality ) {
const { type, value } = node;
if ( type !== 'Block' ) {
return false;
}

// (Temporary) Tolerances:
// - Normalize `/**` and `/*`
// - Case insensitive "Dependencies" vs. "dependencies"
// - Ending period
// - "Node" dependencies as an alias for External

if ( locality === 'External' ) {
locality = '(External|Node)';
}

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

/**
* Returns true if the given node occurs prior in code to a reference,
* or false otherwise.
*
* @param {espree.Node} node Node to test being before reference.
* @param {espree.Node} reference Node against which to compare.
*
* @return {boolean} Whether node occurs before reference.
*/
function isBefore( node, reference ) {
return node.start < reference.start;
}

/**
* Returns true if a given node is preceded by a comment block
* satisfying a locality requirement, or false otherwise.
*
* @param {espree.Node} node Node to test.
* @param {WPPackageLocality} locality Desired package locality.
*
* @return {boolean} Whether node preceded by locality comment block.
*/
function isPrecededByDependencyBlock( node, locality ) {
return comments.some( ( comment ) => {
return isLocalityDependencyBlock( comment, locality ) && isBefore( comment, node );
} );
}

/**
* Given an import source string and node occurrence from which the
* source is derived, tests that the package source is satisfied by a
* preceding comment block of appropriate locality.
*
* @param {string} source Import source string.
* @param {espree.Node} node Node from which import source derived.
*/
function testPackage( source, node ) {
const locality = getPackageLocality( source );
if ( ! isPrecededByDependencyBlock( node, locality ) ) {
context.report( {
node,
message: `Expected preceding "${ locality } dependencies" comment block`,
} );
}
}

return {
Program( node ) {
// Since we only care to enforce imports which occur at the
// top-level scope, match on Program and test its children,
// rather than matching the import nodes directly.
node.body.forEach( ( child ) => {
switch ( child.type ) {
case 'ImportDeclaration':
testPackage( child.source.value, child );
break;

case 'CallExpression':
const { callee, arguments: args } = child;
if (
callee.name === 'require' &&
args.length === 1 &&
args[ 0 ].type === 'Literal' &&
typeof args[ 0 ].value === 'string'
) {
testPackage( args[ 0 ].value, child );
}
break;
}
} );
},
};
},
};

0 comments on commit b3edc7f

Please sign in to comment.