From 8bd0a2a8903713049738fe5090312be1fd031f3c Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Sun, 1 Jan 2017 17:50:58 -0800 Subject: [PATCH 1/2] Add `no-interface-constructor` rule --- docs/_data/rules.json | 11 +++ docs/rules/misused-new/index.html | 13 +++ .../rules/no-interface-constructor/index.html | 14 ++++ src/rules/misusedNewRule.ts | 81 +++++++++++++++++++ test/rules/misused-new/test.ts.lint | 32 ++++++++ test/rules/misused-new/tslint.json | 5 ++ 6 files changed, 156 insertions(+) create mode 100644 docs/rules/misused-new/index.html create mode 100644 docs/rules/no-interface-constructor/index.html create mode 100644 src/rules/misusedNewRule.ts create mode 100644 test/rules/misused-new/test.ts.lint create mode 100644 test/rules/misused-new/tslint.json diff --git a/docs/_data/rules.json b/docs/_data/rules.json index 05c801c263f..4b93072bd7c 100644 --- a/docs/_data/rules.json +++ b/docs/_data/rules.json @@ -475,6 +475,17 @@ "type": "typescript", "typescriptOnly": true }, + { + "ruleName": "misused-new", + "description": "Warns on apparent attempts to define constructors for interfaces or `new` for classes.", + "optionsDescription": "Not configurable.", + "options": null, + "optionExamples": [ + "true" + ], + "type": "functionality", + "typescriptOnly": true + }, { "ruleName": "new-parens", "description": "Requires parentheses when invoking a constructor via the `new` keyword.", diff --git a/docs/rules/misused-new/index.html b/docs/rules/misused-new/index.html new file mode 100644 index 00000000000..2d47e30bc7b --- /dev/null +++ b/docs/rules/misused-new/index.html @@ -0,0 +1,13 @@ +--- +ruleName: misused-new +description: Warns on apparent attempts to define constructors for interfaces or `new` for classes. +optionsDescription: Not configurable. +options: null +optionExamples: + - 'true' +type: functionality +typescriptOnly: true +layout: rule +title: 'Rule: misused-new' +optionsJSON: 'null' +--- \ No newline at end of file diff --git a/docs/rules/no-interface-constructor/index.html b/docs/rules/no-interface-constructor/index.html new file mode 100644 index 00000000000..b2f5778f784 --- /dev/null +++ b/docs/rules/no-interface-constructor/index.html @@ -0,0 +1,14 @@ +--- +ruleName: no-interface-constructor +description: Warns on apparent attempts to define constructors for interfaces. +rationale: '`interface I { new(): I }` declares a type where for `x: I`, `new x()` is also of type `I`.' +optionsDescription: Not configurable. +options: null +optionExamples: + - 'true' +type: functionality +typescriptOnly: true +layout: rule +title: 'Rule: no-interface-constructor' +optionsJSON: 'null' +--- \ No newline at end of file diff --git a/src/rules/misusedNewRule.ts b/src/rules/misusedNewRule.ts new file mode 100644 index 00000000000..742c800cebb --- /dev/null +++ b/src/rules/misusedNewRule.ts @@ -0,0 +1,81 @@ +/** + * @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 * 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: "misused-new", + description: "Warns on apparent attempts to define constructors for interfaces or `new` for classes.", + optionsDescription: "Not configurable.", + options: null, + optionExamples: ["true"], + type: "functionality", + typescriptOnly: true, + }; + /* tslint:enable:object-literal-sort-keys */ + + public static FAILURE_STRING_INTERFACE = "Interfaces cannot be constructed, only classes. Did you mean `declare class`?"; + public static FAILURE_STRING_CLASS = '`new` in a class is a method named "new". Did you mean `constructor`?'; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithWalker(new Walker(sourceFile, this.getOptions())); + } +} + +class Walker extends Lint.RuleWalker { + public visitMethodSignature(node: ts.MethodSignature) { + if (nameIs(node.name, "constructor")) { + this.addFailureAtNode(node, Rule.FAILURE_STRING_INTERFACE); + } + } + + public visitMethodDeclaration(node: ts.MethodDeclaration) { + if (node.body === undefined && nameIs(node.name, "new") && + returnTypeMatchesParent(node.parent as ts.ClassLikeDeclaration, node)) { + this.addFailureAtNode(node, Rule.FAILURE_STRING_CLASS); + } + } + + public visitConstructSignature(node: ts.ConstructSignatureDeclaration) { + if (returnTypeMatchesParent(node.parent as ts.InterfaceDeclaration, node)) { + this.addFailureAtNode(node, Rule.FAILURE_STRING_INTERFACE); + } + } +} + +function nameIs(name: ts.PropertyName, text: string): boolean { + return name.kind === ts.SyntaxKind.Identifier && name.text === text; +} + +function returnTypeMatchesParent(parent: { name?: ts.Identifier }, decl: ts.SignatureDeclaration): boolean { + if (parent.name === undefined) { + return false; + } + + const name = parent.name.text; + const type = decl.type; + if (type === undefined || type.kind !== ts.SyntaxKind.TypeReference) { + return false; + } + + const typeName = (type as ts.TypeReferenceNode).typeName; + return typeName.kind === ts.SyntaxKind.Identifier && (typeName as ts.Identifier).text === name; +} diff --git a/test/rules/misused-new/test.ts.lint b/test/rules/misused-new/test.ts.lint new file mode 100644 index 00000000000..f1ed5ca1120 --- /dev/null +++ b/test/rules/misused-new/test.ts.lint @@ -0,0 +1,32 @@ +interface I { + new(): I; + ~~~~~~~~~ [0] + // OK if return type is not the interface. + new(): {}; + constructor(): void; + ~~~~~~~~~~~~~~~~~~~~ [0] +} + +// Works for generic type. +interface G { + new(): G; + ~~~~~~~~~~~~~~~ [0] +} + +type T = { + // `new` OK in type literal (we don't know the type name) + new(): T; + // `constructor` still flagged. + constructor(): void; + ~~~~~~~~~~~~~~~~~~~~ [0] +} + +class C { + new(): C; + ~~~~~~~~~ [1] + // OK if there's a body + new() {} +} + +[0]: Interfaces cannot be constructed, only classes. Did you mean `declare class`? +[1]: `new` in a class is a method named "new". Did you mean `constructor`? diff --git a/test/rules/misused-new/tslint.json b/test/rules/misused-new/tslint.json new file mode 100644 index 00000000000..2f6e310624a --- /dev/null +++ b/test/rules/misused-new/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "misused-new": true + } +} From 5af075f30e8d08d9f1cb3647c05c2cc86234f05b Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 4 Jan 2017 17:54:31 -0800 Subject: [PATCH 2/2] Rename to no-misused-new --- docs/_data/rules.json | 22 +++++++++---------- .../index.html | 4 ++-- ...{misusedNewRule.ts => noMisusedNewRule.ts} | 2 +- test/rules/misused-new/tslint.json | 5 ----- .../test.ts.lint | 0 test/rules/no-misused-new/tslint.json | 5 +++++ 6 files changed, 19 insertions(+), 19 deletions(-) rename docs/rules/{misused-new => no-misused-new}/index.html (82%) rename src/rules/{misusedNewRule.ts => noMisusedNewRule.ts} (98%) delete mode 100644 test/rules/misused-new/tslint.json rename test/rules/{misused-new => no-misused-new}/test.ts.lint (100%) create mode 100644 test/rules/no-misused-new/tslint.json diff --git a/docs/_data/rules.json b/docs/_data/rules.json index 4b93072bd7c..f6a6e3a620e 100644 --- a/docs/_data/rules.json +++ b/docs/_data/rules.json @@ -475,17 +475,6 @@ "type": "typescript", "typescriptOnly": true }, - { - "ruleName": "misused-new", - "description": "Warns on apparent attempts to define constructors for interfaces or `new` for classes.", - "optionsDescription": "Not configurable.", - "options": null, - "optionExamples": [ - "true" - ], - "type": "functionality", - "typescriptOnly": true - }, { "ruleName": "new-parens", "description": "Requires parentheses when invoking a constructor via the `new` keyword.", @@ -791,6 +780,17 @@ "type": "maintainability", "typescriptOnly": true }, + { + "ruleName": "no-misused-new", + "description": "Warns on apparent attempts to define constructors for interfaces or `new` for classes.", + "optionsDescription": "Not configurable.", + "options": null, + "optionExamples": [ + "true" + ], + "type": "functionality", + "typescriptOnly": true + }, { "ruleName": "no-namespace", "description": "Disallows use of internal `module`s and `namespace`s.", diff --git a/docs/rules/misused-new/index.html b/docs/rules/no-misused-new/index.html similarity index 82% rename from docs/rules/misused-new/index.html rename to docs/rules/no-misused-new/index.html index 2d47e30bc7b..771e8ed5754 100644 --- a/docs/rules/misused-new/index.html +++ b/docs/rules/no-misused-new/index.html @@ -1,5 +1,5 @@ --- -ruleName: misused-new +ruleName: no-misused-new description: Warns on apparent attempts to define constructors for interfaces or `new` for classes. optionsDescription: Not configurable. options: null @@ -8,6 +8,6 @@ type: functionality typescriptOnly: true layout: rule -title: 'Rule: misused-new' +title: 'Rule: no-misused-new' optionsJSON: 'null' --- \ No newline at end of file diff --git a/src/rules/misusedNewRule.ts b/src/rules/noMisusedNewRule.ts similarity index 98% rename from src/rules/misusedNewRule.ts rename to src/rules/noMisusedNewRule.ts index 742c800cebb..e5bdad0f269 100644 --- a/src/rules/misusedNewRule.ts +++ b/src/rules/noMisusedNewRule.ts @@ -22,7 +22,7 @@ import * as Lint from "../index"; export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { - ruleName: "misused-new", + ruleName: "no-misused-new", description: "Warns on apparent attempts to define constructors for interfaces or `new` for classes.", optionsDescription: "Not configurable.", options: null, diff --git a/test/rules/misused-new/tslint.json b/test/rules/misused-new/tslint.json deleted file mode 100644 index 2f6e310624a..00000000000 --- a/test/rules/misused-new/tslint.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "rules": { - "misused-new": true - } -} diff --git a/test/rules/misused-new/test.ts.lint b/test/rules/no-misused-new/test.ts.lint similarity index 100% rename from test/rules/misused-new/test.ts.lint rename to test/rules/no-misused-new/test.ts.lint diff --git a/test/rules/no-misused-new/tslint.json b/test/rules/no-misused-new/tslint.json new file mode 100644 index 00000000000..87f36324069 --- /dev/null +++ b/test/rules/no-misused-new/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-misused-new": true + } +}