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: Add rule to prohibit unsafe APIs #27301

Merged
merged 15 commits into from
Nov 27, 2020
4 changes: 4 additions & 0 deletions packages/eslint-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### New Feature

- Add `no-unsafe-wp-apis` rule to discourage usage of unsafe APIs ([#27301](https://github.com/WordPress/gutenberg/pull/27301)).

### Documentation

- Include a note about the minimum version required for `node` (10.0.0) and `npm` (6.9.0).
Expand Down
43 changes: 43 additions & 0 deletions packages/eslint-plugin/docs/rules/no-unsafe-wp-apis.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Prevent unsafe API usage (no-unsafe-wp-apis)

Prevent unsafe APIs from `@wordpress/*` packages from being imported.

This includes experimental and unstable APIs which are expected to change and likely to cause issues in application code.
See the [documentation](https://github.com/WordPress/gutenberg/blob/master/docs/contributors/coding-guidelines.md#experimental-and-unstable-apis).

> **There is no support commitment for experimental and unstable APIs.** They can and will be removed or changed without advance warning, including as part of a minor or patch release. As an external consumer, you should avoid these APIs.
> …
>
> - An **experimental API** is one which is planned for eventual public availability, but is subject to further experimentation, testing, and discussion.
> - An **unstable API** is one which serves as a means to an end. It is not desired to ever be converted into a public API.

## Rule details

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

```js
import { __experimentalFeature } from '@wordpress/foo';
import { __unstableFeature } from '@wordpress/bar';
```

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

```js
import { registerBlockType } from '@wordpress/blocks';
```

## Options

The rule can be configured via an object.
This should be an object where the keys are import package names and the values are arrays of allowed unsafe imports.

#### Example configuration

```json
{
"@wordpress/no-unsafe-wp-apis": [
"error",
{ "@wordpress/block-editor": [ "__experimentalBlock" ] }
]
}
```
119 changes: 119 additions & 0 deletions packages/eslint-plugin/rules/__tests__/no-unsafe-wp-apis.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/**
* External dependencies
*/
import { RuleTester } from 'eslint';

/**
* Internal dependencies
*/
import rule from '../no-unsafe-wp-apis';

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

const options = [
{ '@wordpress/package': [ '__experimentalSafe', '__unstableSafe' ] },
];

ruleTester.run( 'no-unsafe-wp-apis', rule, {
valid: [
{ code: "import _ from 'lodash';", options },
{ code: "import { map } from 'lodash';", options },
{ code: "import { __experimentalFoo } from 'lodash';", options },
{ code: "import { __unstableFoo } from 'lodash';", options },
{ code: "import _, { __unstableFoo } from 'lodash';", options },
{ code: "import * as _ from 'lodash';", options },

{ code: "import _ from './x';", options },
{ code: "import { map } from './x';", options },
{ code: "import { __experimentalFoo } from './x';", options },
{ code: "import { __unstableFoo } from './x';", options },
{ code: "import _, { __unstableFoo } from './x';", options },
{ code: "import * as _ from './x';", options },

{ code: "import s from '@wordpress/package';", options },
{ code: "import { feature } from '@wordpress/package';", options },
{
code: "import { __experimentalSafe } from '@wordpress/package';",
options,
},
{
code: "import { __unstableSafe } from '@wordpress/package';",
options,
},
{
code:
"import { feature, __experimentalSafe } from '@wordpress/package';",
options,
},
{
code: "import s, { __experimentalSafe } from '@wordpress/package';",
options,
},
{ code: "import * as s from '@wordpress/package';", options },
],

invalid: [
{
code: "import { __experimentalUnsafe } from '@wordpress/package';",
options,
errors: [
{
message: `Usage of \`__experimentalUnsafe\` from \`@wordpress/package\` is not allowed.
See https://developer.wordpress.org/block-editor/contributors/develop/coding-guidelines/#experimental-and-unstable-apis for details.`,
type: 'ImportSpecifier',
},
],
},
{
code: "import { __experimentalSafe } from '@wordpress/unsafe';",
options,
errors: [
{
message: `Usage of \`__experimentalSafe\` from \`@wordpress/unsafe\` is not allowed.
See https://developer.wordpress.org/block-editor/contributors/develop/coding-guidelines/#experimental-and-unstable-apis for details.`,
type: 'ImportSpecifier',
},
],
},
{
code:
"import { feature, __experimentalSafe } from '@wordpress/unsafe';",
options,
errors: [
{
message: `Usage of \`__experimentalSafe\` from \`@wordpress/unsafe\` is not allowed.
See https://developer.wordpress.org/block-editor/contributors/develop/coding-guidelines/#experimental-and-unstable-apis for details.`,
type: 'ImportSpecifier',
},
],
},
{
code:
"import s, { __experimentalUnsafe } from '@wordpress/package';",
options,
errors: [
{
message: `Usage of \`__experimentalUnsafe\` from \`@wordpress/package\` is not allowed.
See https://developer.wordpress.org/block-editor/contributors/develop/coding-guidelines/#experimental-and-unstable-apis for details.`,
type: 'ImportSpecifier',
},
],
},
{
code: "import { __unstableFeature } from '@wordpress/package';",
options,
errors: [
{
message: `Usage of \`__unstableFeature\` from \`@wordpress/package\` is not allowed.
See https://developer.wordpress.org/block-editor/contributors/develop/coding-guidelines/#experimental-and-unstable-apis for details.`,
type: 'ImportSpecifier',
},
],
},
],
} );
5 changes: 3 additions & 2 deletions packages/eslint-plugin/rules/dependency-group.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/** @typedef {import('estree').Comment} Comment */
/** @typedef {import('estree').Node} Node */

module.exports = /** @type {import('eslint').Rule.RuleModule} */ ( {
/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
sirreal marked this conversation as resolved.
Show resolved Hide resolved
meta: {
type: 'layout',
docs: {
Expand Down Expand Up @@ -254,4 +255,4 @@ module.exports = /** @type {import('eslint').Rule.RuleModule} */ ( {
},
};
},
} );
};
88 changes: 88 additions & 0 deletions packages/eslint-plugin/rules/no-unsafe-wp-apis.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
type: 'problem',
meta: {
schema: [
{
type: 'object',
additionalProperties: false,
patternProperties: {
'^@wordpress\\/[a-zA-Z0-9_-]+$': {
type: 'array',
uniqueItems: true,
minItems: 1,
items: {
type: 'string',
pattern: '^(?:__experimental|__unstable)',
},
},
},
},
],
},
create( context ) {
/** @type {AllowedImportsMap} */
const allowedImports =
( context.options &&
typeof context.options[ 0 ] === 'object' &&
context.options[ 0 ] ) ||
{};
const reporter = makeListener( { allowedImports, context } );

return { ImportDeclaration: reporter };
},
};

/**
* @param {Object} _
* @param {AllowedImportsMap} _.allowedImports
* @param {import('eslint').Rule.RuleContext} _.context
*
* @return {(node: Node) => void} Listener function
*/
function makeListener( { allowedImports, context } ) {
return function reporter( node ) {
if ( node.type !== 'ImportDeclaration' ) {
return;
}
if ( typeof node.source.value !== 'string' ) {
return;
}

const sourceModule = node.source.value.trim();

// Ignore non-WordPress packages
if ( ! sourceModule.startsWith( '@wordpress/' ) ) {
return;
}

const allowedImportNames = allowedImports[ sourceModule ] || [];

node.specifiers.forEach( ( specifierNode ) => {
if ( specifierNode.type !== 'ImportSpecifier' ) {
return;
}

const importedName = specifierNode.imported.name;

if (
! importedName.startsWith( '__unstable' ) &&
! importedName.startsWith( '__experimental' )
) {
return;
}

if ( allowedImportNames.includes( importedName ) ) {
return;
}

context.report( {
message: `Usage of \`${ importedName }\` from \`${ sourceModule }\` is not allowed.\nSee https://developer.wordpress.org/block-editor/contributors/develop/coding-guidelines/#experimental-and-unstable-apis for details.`,
node: specifierNode,
} );
} );
};
}

/** @typedef {import('estree').Node} Node */
/** @typedef {Record<string, string[]|undefined>} AllowedImportsMap */
5 changes: 2 additions & 3 deletions packages/eslint-plugin/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"module": "CommonJS",
"rootDir": "rules",
"declarationDir": "build-types"
},
// NOTE: This package is being progressively typed. You are encouraged to
// expand this array with files which can be type-checked. At some point in
// the future, this can be simplified to an `includes` of `src/**/*`.
"files": [
"rules/dependency-group.js"
]
"files": [ "rules/dependency-group.js", "rules/no-unsafe-wp-apis.js" ]
}