Skip to content

feat(eslint-plugin): [block-spacing] extending base rule for TS related blocks #6195

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
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
12 changes: 12 additions & 0 deletions packages/eslint-plugin/docs/rules/block-spacing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
description: 'Disallow or enforce spaces inside of blocks after opening block and before closing block.'
---

> 🛑 This file is source code, not the primary documentation location! 🛑
>
> See **https://typescript-eslint.io/rules/block-spacing** for documentation.

## Examples

This rule extends the base [`eslint/block-spacing`](https://eslint.org/docs/rules/block-spacing) rule.
This version adds support for TypeScript related blocks (interfaces, object type literals and enums).
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/configs/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ export = {
'@typescript-eslint/ban-ts-comment': 'error',
'@typescript-eslint/ban-tslint-comment': 'error',
'@typescript-eslint/ban-types': 'error',
'block-spacing': 'off',
'@typescript-eslint/block-spacing': 'error',
'brace-style': 'off',
'@typescript-eslint/brace-style': 'error',
'@typescript-eslint/class-literal-property-style': 'error',
Expand Down
163 changes: 163 additions & 0 deletions packages/eslint-plugin/src/rules/block-spacing.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
import type { TSESTree } from '@typescript-eslint/utils';
import { AST_TOKEN_TYPES } from '@typescript-eslint/utils';

import * as util from '../util';
import { getESLintCoreRule } from '../util/getESLintCoreRule';

const baseRule = getESLintCoreRule('block-spacing');

export type Options = util.InferOptionsTypeFromRule<typeof baseRule>;
export type MessageIds = util.InferMessageIdsTypeFromRule<typeof baseRule>;

export default util.createRule<Options, MessageIds>({
name: 'block-spacing',
meta: {
type: 'layout',
docs: {
description:
'Disallow or enforce spaces inside of blocks after opening block and before closing block',
recommended: false,
extendsBaseRule: true,
},
fixable: 'whitespace',
hasSuggestions: baseRule.meta.hasSuggestions,
schema: baseRule.meta.schema,
messages: baseRule.meta.messages,
},
defaultOptions: ['always'],

create(context, [whenToApplyOption]) {
const sourceCode = context.getSourceCode();
const baseRules = baseRule.create(context);
const always = whenToApplyOption !== 'never';
const messageId = always ? 'missing' : 'extra';
/**
* Gets the open brace token from a given node.
* @returns The token of the open brace.
*/
function getOpenBrace(
node: TSESTree.TSEnumDeclaration,
): TSESTree.PunctuatorToken {
// guaranteed for enums
// This is the only change made here from the base rule
return sourceCode.getFirstToken(node, {
filter: token =>
token.type === AST_TOKEN_TYPES.Punctuator && token.value === '{',
}) as TSESTree.PunctuatorToken;
}

/**
* Checks whether or not:
* - given tokens are on same line.
* - there is/isn't a space between given tokens.
* @param left A token to check.
* @param right The token which is next to `left`.
* @returns
* When the option is `"always"`, `true` if there are one or more spaces between given tokens.
* When the option is `"never"`, `true` if there are not any spaces between given tokens.
* If given tokens are not on same line, it's always `true`.
*/
function isValid(left: TSESTree.Token, right: TSESTree.Token): boolean {
return (
!util.isTokenOnSameLine(left, right) ||
sourceCode.isSpaceBetween!(left, right) === always
);
}

/**
* Checks and reports invalid spacing style inside braces.
*/
function checkSpacingInsideBraces(node: TSESTree.TSEnumDeclaration): void {
// Gets braces and the first/last token of content.
const openBrace = getOpenBrace(node);
const closeBrace = sourceCode.getLastToken(node)!;
const firstToken = sourceCode.getTokenAfter(openBrace, {
includeComments: true,
})!;
const lastToken = sourceCode.getTokenBefore(closeBrace, {
includeComments: true,
})!;

// Skip if the node is invalid or empty.
if (
openBrace.type !== AST_TOKEN_TYPES.Punctuator ||
openBrace.value !== '{' ||
closeBrace.type !== AST_TOKEN_TYPES.Punctuator ||
closeBrace.value !== '}' ||
firstToken === closeBrace
) {
return;
}

// Skip line comments for option never
if (!always && firstToken.type === AST_TOKEN_TYPES.Line) {
return;
}

if (!isValid(openBrace, firstToken)) {
let loc = openBrace.loc;

if (messageId === 'extra') {
loc = {
start: openBrace.loc.end,
end: firstToken.loc.start,
};
}

context.report({
node,
loc,
messageId,
data: {
location: 'after',
token: openBrace.value,
},
fix(fixer) {
if (always) {
return fixer.insertTextBefore(firstToken, ' ');
}

return fixer.removeRange([openBrace.range[1], firstToken.range[0]]);
},
});
}
if (!isValid(lastToken, closeBrace)) {
let loc = closeBrace.loc;

if (messageId === 'extra') {
loc = {
start: lastToken.loc.end,
end: closeBrace.loc.start,
};
}
context.report({
node,
loc,
messageId,
data: {
location: 'before',
token: closeBrace.value,
},
fix(fixer) {
if (always) {
return fixer.insertTextAfter(lastToken, ' ');
}

return fixer.removeRange([lastToken.range[1], closeBrace.range[0]]);
},
});
}
}
return {
...baseRules,

// This code worked "out of the box" for interface and type literal
// Enums were very close to match as well, the only reason they are not is that was that enums don't have a body node in the parser
// So the opening brace punctuator starts in the middle of the node - `getFirstToken` in
// the base rule did not filter for the first opening brace punctuator
TSInterfaceBody: baseRules.BlockStatement as never,
TSTypeLiteral: baseRules.BlockStatement as never,
TSEnumDeclaration: checkSpacingInsideBraces,
Comment on lines +158 to +160
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kept the base rule for interface and type literal, though changing to checkSpacingInsideBraces will work as well

};
},
});
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import awaitThenable from './await-thenable';
import banTsComment from './ban-ts-comment';
import banTslintComment from './ban-tslint-comment';
import banTypes from './ban-types';
import blockSpacing from './block-spacing';
import braceStyle from './brace-style';
import classLiteralPropertyStyle from './class-literal-property-style';
import commaDangle from './comma-dangle';
Expand Down Expand Up @@ -137,6 +138,7 @@ export default {
'ban-ts-comment': banTsComment,
'ban-tslint-comment': banTslintComment,
'ban-types': banTypes,
'block-spacing': blockSpacing,
'brace-style': braceStyle,
'class-literal-property-style': classLiteralPropertyStyle,
'comma-dangle': commaDangle,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/util/getESLintCoreRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const isESLintV8 = semver.major(version) >= 8;
interface RuleMap {
/* eslint-disable @typescript-eslint/consistent-type-imports -- more concise to use inline imports */
'arrow-parens': typeof import('eslint/lib/rules/arrow-parens');
'block-spacing': typeof import('eslint/lib/rules/block-spacing');
'brace-style': typeof import('eslint/lib/rules/brace-style');
'comma-dangle': typeof import('eslint/lib/rules/comma-dangle');
'dot-notation': typeof import('eslint/lib/rules/dot-notation');
Expand Down
145 changes: 145 additions & 0 deletions packages/eslint-plugin/tests/rules/block-spacing.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
import { AST_NODE_TYPES } from '@typescript-eslint/utils';

import rule from '../../src/rules/block-spacing';
import type { InvalidTestCase, ValidTestCase } from '../RuleTester';
import { RuleTester } from '../RuleTester';

const ruleTester = new RuleTester({
parser: '@typescript-eslint/parser',
});

type InvalidBlockSpacingTestCase = InvalidTestCase<
'missing' | 'extra',
['always' | 'never']
>;

const options = ['always', 'never'] as const;
const typeDeclarations = [
{
nodeType: AST_NODE_TYPES.TSInterfaceBody,
stringPrefix: 'interface Foo ',
},
{
nodeType: AST_NODE_TYPES.TSTypeLiteral,
stringPrefix: 'type Foo = ',
},
{
nodeType: AST_NODE_TYPES.TSEnumDeclaration,
stringPrefix: 'enum Foo ',
},
{
nodeType: AST_NODE_TYPES.TSEnumDeclaration,
stringPrefix: 'const enum Foo ',
},
];
const emptyBlocks = ['{}', '{ }'];
const singlePropertyBlocks = ['{bar: true}', '{ bar: true }'];
const blockComment = '/* comment */';

ruleTester.run('block-spacing', rule, {
valid: [
// Empty blocks don't apply
...options.flatMap(option =>
typeDeclarations.flatMap(typeDec =>
emptyBlocks.map<ValidTestCase<['always' | 'never']>>(blockType => ({
code: typeDec.stringPrefix + blockType,
options: [option],
})),
),
),
...typeDeclarations.flatMap<ValidTestCase<['always' | 'never']>>(
typeDec => {
const property =
typeDec.nodeType === AST_NODE_TYPES.TSEnumDeclaration
? 'bar = 1'
: 'bar: true;';
return [
{
code: `${typeDec.stringPrefix}{ /* comment */ ${property} /* comment */ } // always`,
options: ['always'],
},
{
code: `${typeDec.stringPrefix}{/* comment */ ${property} /* comment */} // never`,
options: ['never'],
},
{
code: `${typeDec.stringPrefix}{ //comment\n ${property}}`,
options: ['never'],
},
];
},
),
],
invalid: [
...options.flatMap(option =>
typeDeclarations.flatMap(typeDec => {
return singlePropertyBlocks.flatMap<InvalidBlockSpacingTestCase>(
(blockType, blockIndex) => {
// These are actually valid, so filter them out
if (
(option === 'always' && blockType.startsWith('{ ')) ||
(option === 'never' && blockType.startsWith('{bar'))
) {
return [];
}
const reverseBlockType = singlePropertyBlocks[1 - blockIndex];
let code = `${typeDec.stringPrefix}${blockType}; /* ${option} */`;
let output = `${typeDec.stringPrefix}${reverseBlockType}; /* ${option} */`;
if (typeDec.nodeType === AST_NODE_TYPES.TSEnumDeclaration) {
output = output.replace(':', '=');
code = code.replace(':', '=');
}

return {
code,
options: [option],
output,
errors: [
{
type: typeDec.nodeType,
messageId: option === 'always' ? 'missing' : 'extra',
data: { location: 'after', token: '{' },
},
{
type: typeDec.nodeType,
messageId: option === 'always' ? 'missing' : 'extra',
data: { location: 'before', token: '}' },
},
],
};
},
);
}),
),
// With block comments
...options.flatMap(option =>
typeDeclarations.flatMap<InvalidBlockSpacingTestCase>(typeDec => {
const property =
typeDec.nodeType === AST_NODE_TYPES.TSEnumDeclaration
? 'bar = 1'
: 'bar: true;';
const alwaysSpace = option === 'always' ? '' : ' ';
const neverSpace = option === 'always' ? ' ' : '';
return [
{
code: `${typeDec.stringPrefix}{${alwaysSpace}${blockComment}${property}${blockComment}${alwaysSpace}} /* ${option} */`,
output: `${typeDec.stringPrefix}{${neverSpace}${blockComment}${property}${blockComment}${neverSpace}} /* ${option} */`,
options: [option],
errors: [
{
type: typeDec.nodeType,
messageId: option === 'always' ? 'missing' : 'extra',
data: { location: 'after', token: '{' },
},
{
type: typeDec.nodeType,
messageId: option === 'always' ? 'missing' : 'extra',
data: { location: 'before', token: '}' },
},
],
},
];
}),
),
],
});
15 changes: 15 additions & 0 deletions packages/eslint-plugin/typings/eslint-rules.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,21 @@ declare module 'eslint/lib/rules/quotes' {
export = rule;
}

declare module 'eslint/lib/rules/block-spacing' {
import type { TSESLint, TSESTree } from '@typescript-eslint/utils';

const rule: TSESLint.RuleModule<
'missing' | 'extra',
['always' | 'never'],
{
BlockStatement(node: TSESTree.BlockStatement): void;
StaticBlock(node: TSESTree.StaticBlock): void;
SwitchStatement(node: TSESTree.SwitchStatement): void;
}
>;
export = rule;
}

declare module 'eslint/lib/rules/brace-style' {
import type { TSESLint, TSESTree } from '@typescript-eslint/utils';

Expand Down