Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Add 'type-literal-delimiter' rule #2374

Merged
merged 3 commits into from
May 14, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions src/configs/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export const rules = {
"method": "never",
"named": "never",
}],
"type-literal-delimiter": true,
"variable-name": [
true,
"ban-keywords",
Expand Down
2 changes: 1 addition & 1 deletion src/rules/adjacentOverloadSignaturesRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export function getOverloadKey(node: ts.SignatureDeclaration): string | undefine
return (computed ? "0" : "1") + (isStatic ? "0" : "1") + name;
}

function getOverloadInfo(node: ts.SignatureDeclaration): string | { name: string; computed?: boolean } | undefined {
function getOverloadInfo(node: ts.SignatureDeclaration): string | { name: string, computed?: boolean } | undefined {
switch (node.kind) {
case ts.SyntaxKind.ConstructSignature:
case ts.SyntaxKind.Constructor:
Expand Down
76 changes: 76 additions & 0 deletions src/rules/typeLiteralDelimiterRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/**
* @license
* Copyright 2017 Palantir Technologies, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { isTypeLiteralNode } from "tsutils";
import * as ts from "typescript";

import * as Lint from "../index";

export class Rule extends Lint.Rules.AbstractRule {
/* tslint:disable:object-literal-sort-keys */
public static metadata: Lint.IRuleMetadata = {
ruleName: "type-literal-delimiter",
description: Lint.Utils.dedent`
Checks that type literal members are separated by commas.
Does not check the last member, as that is done by 'trailing-comma'.`,
optionsDescription: "Not configurable.",
options: null,
optionExamples: [true],
type: "style",
typescriptOnly: true,
};
/* tslint:enable:object-literal-sort-keys */

public static FAILURE_STRING_MISSING =
"Expected type literal to use ',' to separate members.";
public static FAILURE_STRING_SEMICOLON =
"Expected type literal to use ',' instead of ';'.";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithFunction(sourceFile, walk);
}
}

function walk(ctx: Lint.WalkContext<void>): void {
const { sourceFile } = ctx;
ts.forEachChild(sourceFile, function cb(node: ts.Node): void {
if (isTypeLiteralNode(node)) {
check(node);
}
ts.forEachChild(node, cb);
});

function check({ members }: ts.TypeLiteralNode): void {
members.forEach((member, idx) => {
const end = member.end - 1;
const delimiter = sourceFile.text[end];
switch (delimiter) {
case ",":
break;
case ";":
ctx.addFailureAt(end, 1, Rule.FAILURE_STRING_SEMICOLON);
break;
default:
// Allow missing trailing ','
if (idx !== members.length - 1) {
ctx.addFailureAt(end, 1, Rule.FAILURE_STRING_MISSING);
}
break;
}
});
}
}
28 changes: 28 additions & 0 deletions test/rules/type-literal-delimiter/test.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
type T = {
Copy link
Contributor

Choose a reason for hiding this comment

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

does it handle space before/after delimiter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not currently, no. I think that would be more appropriate for the whitespace rule.

Copy link
Contributor

@nchen63 nchen63 May 13, 2017

Choose a reason for hiding this comment

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

I'm asking if it would erroneously flag { a: b , c: d } or { a: b, c: d }. Should probably have a unit test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, gotcha. Added a test.

x: number
~ [MISSING]
y: string
}

type T = {
x: number,
// Doesn't care about missing final comma; trailing-comma will handle.
y: string
}

// Does care about semicolon.
type T = {
x: number,
y: string;
~ [SEMI]
}

type T = {
x: number;
~ [SEMI]
y: string
}


[MISSING]: Expected type literal to use ',' to separate members.
[SEMI]: Expected type literal to use ',' instead of ';'.
5 changes: 5 additions & 0 deletions test/rules/type-literal-delimiter/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"type-literal-delimiter": true
}
}