Skip to content

Commit

Permalink
docs(eslint-plugin): clearly document extension rules (#1456)
Browse files Browse the repository at this point in the history
  • Loading branch information
bradzacher committed Jan 16, 2020
1 parent bb0a846 commit 9f9a5c5
Show file tree
Hide file tree
Showing 26 changed files with 187 additions and 65 deletions.
20 changes: 20 additions & 0 deletions docs/getting-started/linting/FAQ.md
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
@@ -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',
});
}
});
});
}

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
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

0 comments on commit 9f9a5c5

Please sign in to comment.