Skip to content

Commit

Permalink
✨ Support comments in-between properties
Browse files Browse the repository at this point in the history
  • Loading branch information
@coyotte508
coyotte508 committed Dec 15, 2022
1 parent 5e951ba commit b12c530
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 36 deletions.
113 changes: 77 additions & 36 deletions packages/eslint-plugin/src/rules/key-spacing.ts
View file
Expand Up @@ -45,15 +45,30 @@ export default util.createRule<Options, MessageIds>({
return sourceCode.getTokenBefore(colonToken)!;
}

/**
* Relevant nodes to our rule
*
* node.typeAnnotation will aways be defined, but no way to enforce that and keep
* the type compatible with TSEstree.Node (except maybe TS 4.9' "satisfies" keyword)
*/
type KeyTypeNode =
| TSESTree.TSIndexSignature
| TSESTree.TSPropertySignature
| TSESTree.PropertyDefinition;

function isKeyTypeNode(node: TSESTree.Node): node is KeyTypeNode {
return (
(node.type === AST_NODE_TYPES.TSPropertySignature ||
node.type === AST_NODE_TYPES.TSIndexSignature ||
node.type === AST_NODE_TYPES.PropertyDefinition) &&
!!node.typeAnnotation
);
}

/**
* To handle index signatures, to get the whole text for the parameters
*/
function getKeyText(
node:
| TSESTree.TSIndexSignature
| TSESTree.TSPropertySignature
| TSESTree.PropertyDefinition,
): string {
function getKeyText(node: KeyTypeNode): string {
if ('key' in node) {
return sourceCode.getText(node.key);
}
Expand Down Expand Up @@ -85,10 +100,7 @@ export default util.createRule<Options, MessageIds>({
}

function checkBeforeColon(
node:
| TSESTree.TSIndexSignature
| TSESTree.TSPropertySignature
| TSESTree.PropertyDefinition,
node: KeyTypeNode,
nBeforeColon: number,
mode: 'strict' | 'minimum',
): void {
Expand All @@ -113,10 +125,7 @@ export default util.createRule<Options, MessageIds>({
}

function checkAfterColon(
node:
| TSESTree.TSIndexSignature
| TSESTree.TSPropertySignature
| TSESTree.PropertyDefinition,
node: KeyTypeNode,
nAfterColon: number,
mode: 'strict' | 'minimum',
): void {
Expand All @@ -140,6 +149,53 @@ export default util.createRule<Options, MessageIds>({
}
}

// adapted from https://github.com/eslint/eslint/blob/ba74253e8bd63e9e163bbee0540031be77e39253/lib/rules/key-spacing.js#L356
function continuesAlignGroup(
lastMember: TSESTree.Node,
candidate: TSESTree.Node,
): boolean {
const groupEndLine = lastMember.loc.start.line;
const candidateValueStartLine = (
isKeyTypeNode(candidate) ? candidate.typeAnnotation! : candidate
).loc.start.line;

if (candidateValueStartLine === groupEndLine) {
return false;
}

if (candidateValueStartLine - groupEndLine === 1) {
return true;
}

/*
* Check that the first comment is adjacent to the end of the group, the
* last comment is adjacent to the candidate property, and that successive
* comments are adjacent to each other.
*/
const leadingComments = sourceCode.getCommentsBefore(candidate);

if (
leadingComments.length &&
leadingComments[0].loc.start.line - groupEndLine <= 1 &&
candidateValueStartLine -
leadingComments[leadingComments.length - 1].loc.end.line <=
1
) {
for (let i = 1; i < leadingComments.length; i++) {
if (
leadingComments[i].loc.start.line -
leadingComments[i - 1].loc.end.line >
1
) {
return false;
}
}
return true;
}

return false;
}

function checkAlignGroup(group: TSESTree.Node[]): void {
let alignColumn = 0;
const align =
Expand Down Expand Up @@ -175,17 +231,12 @@ export default util.createRule<Options, MessageIds>({
: options.mode) ?? 'strict';

for (const node of group) {
if (
(node.type === AST_NODE_TYPES.TSPropertySignature ||
node.type === AST_NODE_TYPES.TSIndexSignature ||
node.type === AST_NODE_TYPES.PropertyDefinition) &&
node.typeAnnotation
) {
if (isKeyTypeNode(node)) {
alignColumn = Math.max(
alignColumn,
align === 'colon'
? getKeyLocEnd(node).column + nBeforeColon
: node.typeAnnotation.loc.start.column +
: node.typeAnnotation!.loc.start.column +
':'.length +
nAfterColon +
nBeforeColon,
Expand All @@ -194,16 +245,11 @@ export default util.createRule<Options, MessageIds>({
}

for (const node of group) {
if (
(node.type === AST_NODE_TYPES.TSPropertySignature ||
node.type === AST_NODE_TYPES.TSIndexSignature ||
node.type === AST_NODE_TYPES.PropertyDefinition) &&
node.typeAnnotation
) {
if (isKeyTypeNode(node)) {
const start =
align === 'colon'
? node.typeAnnotation.loc.start.column
: node.typeAnnotation.typeAnnotation.loc.start.column;
? node.typeAnnotation!.loc.start.column
: node.typeAnnotation!.typeAnnotation.loc.start.column;

if (start !== alignColumn) {
context.report({
Expand Down Expand Up @@ -263,12 +309,7 @@ export default util.createRule<Options, MessageIds>({
? options.multiLine.mode
: options.mode) ?? 'strict';

if (
(node.type === AST_NODE_TYPES.TSPropertySignature ||
node.type === AST_NODE_TYPES.TSIndexSignature ||
node.type === AST_NODE_TYPES.PropertyDefinition) &&
node.typeAnnotation
) {
if (isKeyTypeNode(node)) {
checkBeforeColon(node, nBeforeColon, mode);
checkAfterColon(node, nAfterColon, mode);
}
Expand Down Expand Up @@ -296,7 +337,7 @@ export default util.createRule<Options, MessageIds>({
? currentAlignGroup[currentAlignGroup.length - 1]
: null;

if (prevNode?.loc.start.line === node.loc.start.line - 1) {
if (prevNode && continuesAlignGroup(prevNode, node)) {
currentAlignGroup.push(node);
} else if (prevNode?.loc.start.line === node.loc.start.line) {
if (currentAlignGroup.length) {
Expand Down
17 changes: 17 additions & 0 deletions packages/eslint-plugin/tests/rules/key-spacing.test.ts
View file
Expand Up @@ -16,6 +16,18 @@ ruleTester.run('key-spacing', rule, {
code: 'interface X {\n a: number;\n abc: string\n};',
options: [{ align: 'value' }],
},
{
code: 'interface X {\n a: number;\n // Some comment\n abc: string\n};',
options: [{ align: 'value' }],
},
{
code: 'interface X {\n a: number;\n // Some comment\n // on multiple lines\n abc: string\n};',
options: [{ align: 'value' }],
},
{
code: 'interface X {\n a: number;\n /**\n * Doc comment\n */\n abc: string\n};',
options: [{ align: 'value' }],
},
{
code: 'interface X {\n a: number;\n\n abc: string\n};',
options: [{ align: 'value' }],
Expand Down Expand Up @@ -124,6 +136,11 @@ ruleTester.run('key-spacing', rule, {
options: [{ align: 'value' }],
errors: [{ messageId: 'extraValue' }, { messageId: 'extraKey' }],
},
{
code: 'interface X {\n a: number;\n // Some comment\n\n // interrupted in the middle\n abc: string\n};',
options: [{ align: 'value' }],
errors: [{ messageId: 'extraValue' }],
},
// align: colon
{
code: 'interface X {\n a : number;\n abc: string\n};',
Expand Down

0 comments on commit b12c530

Please sign in to comment.