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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(unbound-method): create rule #765

Merged
merged 12 commits into from Mar 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintignore
@@ -1,3 +1,4 @@
coverage/
lib/
!.eslintrc.js
src/rules/__tests__/fixtures/
29 changes: 27 additions & 2 deletions README.md
Expand Up @@ -128,7 +128,7 @@ installations requiring long-term consistency.

## Rules

<!-- begin rules list -->
<!-- begin base rules list -->

| Rule | Description | Configurations | Fixable |
| ---------------------------------------------------------------------------- | --------------------------------------------------------------- | ---------------- | ------------ |
Expand Down Expand Up @@ -173,7 +173,32 @@ installations requiring long-term consistency.
| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | ![recommended][] | |
| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | ![recommended][] | ![fixable][] |

<!-- end rules list -->
<!-- end base rules list -->

## TypeScript Rules
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something about needing to install the upstream plugin as well? Possibly in the table if we'll have othe rrules in the future that do not use type info

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly in the table if we'll have othe rrules in the future that do not use type info

You mean if we have other rules that don't extend base rules.


You're completely right - I don't expect us to make any more rules that extend base rules, only more rules that use type information; so I might leave the table as is for now and just add a sentence saying that unbound-method is an extension rule (as well as updating its own docs).


In addition to the above rules, this plugin also includes a few advanced rules
that are powered by type-checking information provided by TypeScript.

In order to use these rules, you must be using `@typescript-eslint/parser` &
adjust your eslint config as outlined
[here](https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/TYPED_LINTING.md)

Note that unlike the type-checking rules in `@typescript-eslint/eslint-plugin`,
the rules here will fallback to doing nothing if type information is not
available, meaning its safe to include them in shared configs that could be used
on JavaScript and TypeScript projects.

Also note that `unbound-method` depends on `@typescript-eslint/eslint-plugin`,
as it extends the original `unbound-method` rule from that plugin.

<!-- begin type rules list -->

| Rule | Description | Configurations | Fixable |
| ---------------------------------------------- | ------------------------------------------------------------- | -------------- | ------- |
| [unbound-method](docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | | |

<!-- end type rules list -->

## Credit

Expand Down
1 change: 1 addition & 0 deletions babel.config.js
Expand Up @@ -7,4 +7,5 @@ module.exports = {
'@babel/preset-typescript',
['@babel/preset-env', { targets: { node: 10 } }],
],
ignore: ['src/**/__tests__/fixtures/**'],
};
54 changes: 54 additions & 0 deletions docs/rules/unbound-method.md
@@ -0,0 +1,54 @@
# Enforces unbound methods are called with their expected scope (`unbound-method`)
G-Rath marked this conversation as resolved.
Show resolved Hide resolved

## Rule Details

This rule extends the base [`@typescript-eslint/unbound-method`][original-rule]
rule, meaning you must depend on `@typescript-eslint/eslint-plugin` for it to
work. It adds support for understanding when it's ok to pass an unbound method
to `expect` calls.

See the [`@typescript-eslint` documentation][original-rule] for more details on
the `unbound-method` rule.

Note that while this rule requires type information to work, it will fail
silently when not available allowing you to safely enable it on projects that
are not using TypeScript.

## How to use

```json5
{
parser: '@typescript-eslint/parser',
parserOptions: {
project: 'tsconfig.json',
ecmaVersion: 2020,
sourceType: 'module',
},
overrides: [
{
files: ['test/**'],
extends: ['jest'],
rules: {
// you should turn the original rule off *only* for test files
'@typescript-eslint/unbound-method': 'off',
'jest/unbound-method': 'error',
},
},
],
rules: {
'@typescript-eslint/unbound-method': 'error',
},
}
```

This rule should be applied to your test files in place of the original rule,
which should be applied to the rest of your codebase.

## Options

See [`@typescript-eslint/unbound-method`][original-rule] options.

<sup>Taken with 鉂わ笍 [from `@typescript-eslint` core][original-rule]</sup>

[original-rule]:
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/unbound-method.md
9 changes: 8 additions & 1 deletion package.json
Expand Up @@ -62,7 +62,8 @@
"displayName": "test",
"testEnvironment": "node",
"testPathIgnorePatterns": [
"<rootDir>/lib/.*"
"<rootDir>/lib/.*",
"<rootDir>/src/rules/__tests__/fixtures/*"
]
},
{
Expand Down Expand Up @@ -121,8 +122,14 @@
"typescript": "^4.0.0"
},
"peerDependencies": {
"@typescript-eslint/eslint-plugin": ">= 4",
"eslint": ">=5"
},
"peerDependenciesMeta": {
"@typescript-eslint/eslint-plugin": {
"optional": true
}
},
"engines": {
"node": ">=10"
},
Expand Down
1 change: 1 addition & 0 deletions src/__tests__/__snapshots__/rules.test.ts.snap
Expand Up @@ -46,6 +46,7 @@ Object {
"jest/prefer-todo": "error",
"jest/require-to-throw-message": "error",
"jest/require-top-level-describe": "error",
"jest/unbound-method": "error",
"jest/valid-describe": "error",
"jest/valid-expect": "error",
"jest/valid-expect-in-promise": "error",
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/rules.test.ts
Expand Up @@ -2,7 +2,7 @@ import { existsSync } from 'fs';
import { resolve } from 'path';
import plugin from '../';

const numberOfRules = 44;
const numberOfRules = 45;
const ruleNames = Object.keys(plugin.rules);
const deprecatedRules = Object.entries(plugin.rules)
.filter(([, rule]) => rule.meta.deprecated)
Expand Down
13 changes: 13 additions & 0 deletions src/rules/__tests__/fixtures/class.ts
@@ -0,0 +1,13 @@
// used by no-throw-literal test case to validate custom error
export class Error {}

// used by unbound-method test case to test imports
export const console = { log() {} };

// used by prefer-reduce-type-parameter to test native vs userland check
export class Reducable {
reduce() {}
}

// used by no-implied-eval test function imports
export class Function {}
Empty file.
1 change: 1 addition & 0 deletions src/rules/__tests__/fixtures/foo.ts
@@ -0,0 +1 @@
export type T = number;