Skip to content

docs(eslint-plugin): clearly document extension rules #1456

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

Merged
merged 5 commits into from
Jan 16, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions docs/getting-started/linting/FAQ.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- [My linting seems really slow](#my-linting-seems-really-slow)
- [I get errors telling me "The file must be included in at least one of the projects provided"](#i-get-errors-telling-me-"the-file-must-be-included-in-at-least-one-of-the-projects-provided")
- [I use a framework (like Vue) that requires custom file extensions, and I get errors like "You should add `parserOptions.extraFileExtensions` to your config"](<#i-use-a-framework-(like-vue)-that-requires-custom-file-extensions,-and-i-get-errors-like-"you-should-add-`parserOptions.extraFileExtensions`-to-your-config">)
- [I am using a rule from ESLint core, and it doesn't work correctly with TypeScript code](#i-am-using-a-rule-from-eslint-core,-and-it-doesn't-work-correctly-with-typescript-code)

---

Expand Down Expand Up @@ -81,3 +82,22 @@ You can use `parserOptions.extraFileExtensions` to specify an array of non-TypeS
```

---

## I am using a rule from ESLint core, and it doesn't work correctly with TypeScript code

This is a pretty common thing because TypeScript adds new features that ESLint doesn't know about.

The first step is to [check our list of "extension" rules here](../../../packages/eslint-plugin/README.md#extension-rules). An extension rule is simply a rule which extends the base ESLint rules to support TypeScript syntax. If you find it in there, give it a go to see if it works for you. You can configure it by disabling the base rule, and turning on the extension rule. Here's an example with the `semi` rule:

```json
{
"rules": {
"semi": "off",
"@typescript-eslint/semi": "error"
}
}
```

If you don't find an existing extension rule, or the extension rule doesn't work for your case, then you can go ahead and check our issues. [The contributing guide outlines the best way to raise an issue](../../../CONTRIBUTING.md#raising-issues).

---
61 changes: 38 additions & 23 deletions packages/eslint-plugin/README.md

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
"@typescript-eslint/require-await": "error",
"@typescript-eslint/restrict-plus-operands": "error",
"@typescript-eslint/restrict-template-expressions": "error",
"no-return-await": "off",
"@typescript-eslint/return-await": "error",
"semi": "off",
"@typescript-eslint/semi": "error",
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/brace-style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export default createRule<Options, MessageIds>({
description: 'Enforce consistent brace style for blocks',
category: 'Stylistic Issues',
recommended: false,
extendsBaseRule: true,
},
messages: baseRule.meta.messages,
fixable: baseRule.meta.fixable,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/camelcase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export default util.createRule<Options, MessageIds>({
description: 'Enforce camelCase naming convention',
category: 'Stylistic Issues',
recommended: 'error',
extendsBaseRule: true,
},
deprecated: true,
replacedBy: ['naming-convention'],
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/default-param-last.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export default createRule({
description: 'Enforce default parameters to be last',
category: 'Best Practices',
recommended: false,
extendsBaseRule: true,
},
schema: [],
messages: {
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/func-call-spacing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export default util.createRule<Options, MessageIds>({
'Require or disallow spacing between function identifiers and their invocations',
category: 'Stylistic Issues',
recommended: false,
extendsBaseRule: true,
},
fixable: 'whitespace',
schema: {
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/indent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export default util.createRule<Options, MessageIds>({
category: 'Stylistic Issues',
// too opinionated to be recommended
recommended: false,
extendsBaseRule: true,
},
fixable: 'whitespace',
schema: baseRule.meta.schema,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/no-array-constructor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export default util.createRule({
description: 'Disallow generic `Array` constructors',
category: 'Stylistic Issues',
recommended: 'error',
extendsBaseRule: true,
},
fixable: 'code',
messages: {
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/no-empty-function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export default util.createRule<Options, MessageIds>({
description: 'Disallow empty functions',
category: 'Best Practices',
recommended: 'error',
extendsBaseRule: true,
},
schema: [schema],
messages: baseRule.meta.messages,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/no-extra-parens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export default util.createRule<Options, MessageIds>({
description: 'Disallow unnecessary parentheses',
category: 'Possible Errors',
recommended: false,
extendsBaseRule: true,
},
fixable: 'code',
schema: baseRule.meta.schema,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/no-extra-semi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export default util.createRule<Options, MessageIds>({
description: 'Disallow unnecessary semicolons',
category: 'Possible Errors',
recommended: false,
extendsBaseRule: true,
},
fixable: 'code',
schema: baseRule.meta.schema,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/no-magic-numbers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export default util.createRule<Options, MessageIds>({
description: 'Disallow magic numbers',
category: 'Best Practices',
recommended: false,
extendsBaseRule: true,
},
// Extend base schema with additional property to ignore TS numeric literal types
schema: [
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/no-unused-expressions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export default util.createRule({
description: 'Disallow unused expressions',
category: 'Best Practices',
recommended: false,
extendsBaseRule: true,
},
schema: baseRule.meta.schema,
messages: {},
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/no-unused-vars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export default util.createRule({
description: 'Disallow unused variables',
category: 'Variables',
recommended: 'warn',
extendsBaseRule: true,
},
schema: baseRule.meta.schema,
messages: baseRule.meta.messages,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/no-use-before-define.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ export default util.createRule<Options, MessageIds>({
description: 'Disallow the use of variables before they are defined',
category: 'Variables',
recommended: 'error',
extendsBaseRule: true,
},
messages: {
noUseBeforeDefine: "'{{name}}' was used before it was defined.",
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/no-useless-constructor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export default util.createRule<Options, MessageIds>({
description: 'Disallow unnecessary constructors',
category: 'Best Practices',
recommended: false,
extendsBaseRule: true,
},
schema: baseRule.meta.schema,
messages: baseRule.meta.messages,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/quotes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export default util.createRule<Options, MessageIds>({
'Enforce the consistent use of either backticks, double, or single quotes',
category: 'Stylistic Issues',
recommended: false,
extendsBaseRule: true,
},
fixable: 'code',
messages: baseRule.meta.messages,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/require-await.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export default util.createRule<Options, MessageIds>({
category: 'Best Practices',
recommended: 'error',
requiresTypeChecking: true,
extendsBaseRule: true,
},
schema: baseRule.meta.schema,
messages: baseRule.meta.messages,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/return-await.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export default util.createRule({
category: 'Best Practices',
recommended: false,
requiresTypeChecking: true,
extendsBaseRule: 'no-return-await',
},
type: 'problem',
messages: {
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/semi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export default util.createRule<Options, MessageIds>({
category: 'Stylistic Issues',
// too opinionated to be recommended
recommended: false,
extendsBaseRule: true,
},
fixable: 'code',
schema: baseRule.meta.schema,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export default util.createRule<Options, MessageIds>({
description: 'Enforces consistent spacing before function parenthesis',
category: 'Stylistic Issues',
recommended: false,
extendsBaseRule: true,
},
fixable: 'whitespace',
schema: [
Expand Down
50 changes: 44 additions & 6 deletions packages/eslint-plugin/tests/configs.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
import rules from '../src/rules';
import plugin from '../src/index';

const RULE_NAME_PREFIX = '@typescript-eslint/';
const EXTENSION_RULES = Object.entries(rules)
.filter(([, rule]) => rule.meta.docs.extendsBaseRule)
.map(
([ruleName, rule]) =>
[
`${RULE_NAME_PREFIX}${ruleName}`,
typeof rule.meta.docs.extendsBaseRule === 'string'
? rule.meta.docs.extendsBaseRule
: ruleName,
] as const,
);

function entriesToObject<T = unknown>(value: [string, T][]): Record<string, T> {
return value.reduce<Record<string, T>>((accum, [k, v]) => {
accum[k] = v;
Expand All @@ -14,10 +27,27 @@ function filterRules(values: Record<string, string>): [string, string][] {
);
}

const RULE_NAME_PREFIX = '@typescript-eslint/';
function itHasBaseRulesOverriden(
unfilteredConfigRules: Record<string, string>,
): void {
it('has the base rules overriden by the appropriate extension rules', () => {
const ruleNames = new Set(Object.keys(unfilteredConfigRules));
EXTENSION_RULES.forEach(([ruleName, extRuleName]) => {
if (ruleNames.has(ruleName)) {
// this looks a little weird, but it provides the cleanest test output style
expect(unfilteredConfigRules).toMatchObject({
...unfilteredConfigRules,
[extRuleName]: 'off',
});
Comment on lines +37 to +41
Copy link
Member Author

Choose a reason for hiding this comment

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

image

}
});
});
}

describe('all.json config', () => {
const configRules = filterRules(plugin.configs.all.rules);
const unfilteredConfigRules: Record<string, string> =
plugin.configs.all.rules;
const configRules = filterRules(unfilteredConfigRules);
// note: exclude deprecated rules, this config is allowed to change between minor versions
const ruleConfigs = Object.entries(rules)
.filter(([, rule]) => !rule.meta.deprecated)
Expand All @@ -26,10 +56,14 @@ describe('all.json config', () => {
it('contains all of the rules, excluding the deprecated ones', () => {
expect(entriesToObject(ruleConfigs)).toEqual(entriesToObject(configRules));
});

itHasBaseRulesOverriden(unfilteredConfigRules);
});

describe('recommended.json config', () => {
const configRules = filterRules(plugin.configs.recommended.rules);
const unfilteredConfigRules: Record<string, string> =
plugin.configs.recommended.rules;
const configRules = filterRules(unfilteredConfigRules);
// note: include deprecated rules so that the config doesn't change between major bumps
const ruleConfigs = Object.entries(rules)
.filter(
Expand All @@ -45,12 +79,14 @@ describe('recommended.json config', () => {
it("contains all recommended rules that don't require typechecking, excluding the deprecated ones", () => {
expect(entriesToObject(ruleConfigs)).toEqual(entriesToObject(configRules));
});

itHasBaseRulesOverriden(unfilteredConfigRules);
});

describe('recommended-requiring-type-checking.json config', () => {
const configRules = filterRules(
plugin.configs['recommended-requiring-type-checking'].rules,
);
const unfilteredConfigRules: Record<string, string> =
plugin.configs['recommended-requiring-type-checking'].rules;
const configRules = filterRules(unfilteredConfigRules);
// note: include deprecated rules so that the config doesn't change between major bumps
const ruleConfigs = Object.entries(rules)
.filter(
Expand All @@ -66,4 +102,6 @@ describe('recommended-requiring-type-checking.json config', () => {
it('contains all recommended rules that require type checking, excluding the deprecated ones', () => {
expect(entriesToObject(ruleConfigs)).toEqual(entriesToObject(configRules));
});

itHasBaseRulesOverriden(unfilteredConfigRules);
});
55 changes: 42 additions & 13 deletions packages/eslint-plugin/tests/docs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ function createRuleLink(ruleName: string): string {
return `[\`@typescript-eslint/${ruleName}\`](./docs/rules/${ruleName}.md)`;
}

function parseReadme(): marked.Tokens.Table {
function parseReadme(): {
base: marked.Tokens.Table;
extension: marked.Tokens.Table;
} {
const readmeRaw = fs.readFileSync(
path.resolve(__dirname, '../README.md'),
'utf8',
Expand All @@ -22,14 +25,17 @@ function parseReadme(): marked.Tokens.Table {
});

// find the table
const rulesTable = readme.find(
const rulesTables = readme.filter(
(token): token is marked.Tokens.Table => token.type === 'table',
);
if (!rulesTable) {
throw Error('Could not find the rules table in README.md');
if (rulesTables.length !== 2) {
throw Error('Could not find both rules tables in README.md');
}

return rulesTable;
return {
base: rulesTables[0],
extension: rulesTables[1],
};
}

describe('Validating rule docs', () => {
Expand Down Expand Up @@ -90,24 +96,47 @@ describe('Validating rule metadata', () => {
});

describe('Validating README.md', () => {
const rulesTable = parseReadme().cells;
const notDeprecated = rulesData.filter(
([, rule]) => rule.meta.deprecated !== true,
const rulesTables = parseReadme();
const notDeprecated = rulesData.filter(([, rule]) => !rule.meta.deprecated);
const baseRules = notDeprecated.filter(
([, rule]) => !rule.meta.docs.extendsBaseRule,
);
const extensionRules = notDeprecated.filter(
([, rule]) => rule.meta.docs.extendsBaseRule,
);

it('All non-deprecated base rules should have a row in the base rules table, and the table should be ordered alphabetically', () => {
const baseRuleNames = baseRules
.map(([ruleName]) => ruleName)
.sort()
.map(createRuleLink);

it('All non-deprecated rules should have a row in the table, and the table should be ordered alphabetically', () => {
const ruleNames = notDeprecated
expect(rulesTables.base.cells.map(row => row[0])).toStrictEqual(
baseRuleNames,
);
});
it('All non-deprecated extension rules should have a row in the base rules table, and the table should be ordered alphabetically', () => {
const extensionRuleNames = extensionRules
.map(([ruleName]) => ruleName)
.sort()
.map(createRuleLink);

expect(rulesTable.map(row => row[0])).toStrictEqual(ruleNames);
expect(rulesTables.extension.cells.map(row => row[0])).toStrictEqual(
extensionRuleNames,
);
});

for (const [ruleName, rule] of notDeprecated) {
describe(`Checking rule ${ruleName}`, () => {
const ruleRow =
rulesTable.find(row => row[0].includes(`/${ruleName}.md`)) ?? [];
const ruleRow: string[] | undefined = (rule.meta.docs.extendsBaseRule
? rulesTables.extension.cells
: rulesTables.base.cells
).find(row => row[0].includes(`/${ruleName}.md`));
if (!ruleRow) {
// rule is in the wrong table, the first two tests will catch this, so no point in creating noise;
// these tests will ofc fail in that case
return;
}

it('Link column should be correct', () => {
expect(ruleRow[0]).toEqual(createRuleLink(ruleName));
Expand Down
Loading