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
1 change: 1 addition & 0 deletions packages/eslint-plugin/configs/custom.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module.exports = {
'@wordpress/no-global-active-element': 'error',
'@wordpress/no-global-get-selection': 'error',
'@wordpress/no-global-event-listener': 'warn',
'@wordpress/no-unsafe-wp-apis': 'error',
},
overrides: [
{
Expand Down
44 changes: 44 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,44 @@
# 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 experimental imports.
Note that `unstable` features are not intended for public use and the rule cannot be configured to allow them.
sirreal marked this conversation as resolved.
Show resolved Hide resolved

#### Example configuration

```json
{
"wpcalypso/wp-no-unsafe-features": [
"error",
{ "@wordpress/block-editor": [ "__experimentalBlock" ] }
]
}
```
112 changes: 112 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,112 @@
/**
* 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/safe': [ '__experimentalSafe' ] } ];

ruleTester.run( 'wp-no-unsafe-features', 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/safe';", options },
{ code: "import { feature } from '@wordpress/safe';", options },
{
code: "import { __experimentalSafe } from '@wordpress/safe';",
options,
},
{
code:
"import { feature, __experimentalSafe } from '@wordpress/safe';",
options,
},
{
code: "import s, { __experimentalSafe } from '@wordpress/safe';",
options,
},
{ code: "import * as s from '@wordpress/safe';", options },
],

invalid: [
{
code: "import { __experimentalUnsafe } from '@wordpress/safe';",
options,
errors: [
{
message:
'Usage of `__experimentalUnsafe` from `@wordpress/safe` is not allowed',
type: 'ImportSpecifier',
},
],
},
{
code: "import { __experimentalSafe } from '@wordpress/unsafe';",
options,
errors: [
{
message:
'Usage of `__experimentalSafe` from `@wordpress/unsafe` is not allowed',
type: 'ImportSpecifier',
},
],
},
{
code:
"import { feature, __experimentalSafe } from '@wordpress/unsafe';",
options,
errors: [
{
message:
'Usage of `__experimentalSafe` from `@wordpress/unsafe` is not allowed',
type: 'ImportSpecifier',
},
],
},
{
code: "import s, { __experimentalUnsafe } from '@wordpress/safe';",
options,
errors: [
{
message:
'Usage of `__experimentalUnsafe` from `@wordpress/safe` is not allowed',
type: 'ImportSpecifier',
},
],
},
{
code: "import { __unstableIsNeverSafe } from '@wordpress/safe';",
options,
errors: [
{
message:
'Usage of `__unstableIsNeverSafe` from `@wordpress/safe` is not allowed',
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} */ ( {
},
};
},
} );
};
95 changes: 95 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,95 @@
/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
type: 'problem',
meta: {
messages: {
noUnsafeFeatures:
'Usage of `{{importedName}}` from `{{sourceModule}}` is not allowed',
},
schema: [
{
type: 'object',
additionalProperties: false,
patternProperties: {
'^@wordpress\\/[a-zA-Z0-9_-]+$': {
type: 'array',
uniqueItems: true,
minItems: 1,
items: {
type: 'string',
pattern: '^__experimental',
},
},
},
},
],
},
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();

// Only interested in @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;

const data = {
sourceModule,
importedName,
};

if (
// Unstable is never allowed
importedName.startsWith( '__unstable' ) ||
// Experimental may be allowed
( importedName.startsWith( '__experimental' ) &&
! allowedImportNames.includes( importedName ) )
) {
context.report( {
messageId: 'noUnsafeFeatures',
node: specifierNode,
data,
} );
}
} );
};
}

/** @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" ]
}