diff --git a/.gitignore b/.gitignore index 69efcd8..e6ac55c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,6 @@ node_modules rules/*.js rules/*.d.ts -test/fixes/spec.bad.tofix.ts +test/fixers/spec.bad.tofix.ts *.js *.d.ts \ No newline at end of file diff --git a/.travis.yml b/.travis.yml index ceed399..405ac64 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,6 @@ script: - npm run build - npm run lint - npm test -- "./test/fixes/test.sh" deploy: provider: npm email: hadrien.milano@gmail.com diff --git a/package.json b/package.json index b6e2b82..358e896 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "description": "tslint support for an override keyword", "main": "tslint-override.json", "scripts": { - "test": "tslint --test test && node_modules/.bin/ts-node -P test/tsconfig.json test/test.ts && tslint -p test test/test.ts && tslint -p test/exclude-interfaces", + "test": "./test.sh", "lint": "tslint -p .", "tslint": "tslint", "build": "npm run clean && tsc -p .", diff --git a/rules/explicitOverrideRule.ts b/rules/explicitOverrideRule.ts index 1d6b13e..9cadfbe 100644 --- a/rules/explicitOverrideRule.ts +++ b/rules/explicitOverrideRule.ts @@ -27,6 +27,11 @@ const OPTION_DECORATOR = 'decorator'; const OPTION_JSDOC_TAG = 'jsdoc'; const OPTION_EXCLUDE_INTERFACES = 'exclude-interfaces'; +const MESSAGE_EXTRA_CONSTRUCTOR = 'Extraneous override keyword: constructors always override the parent'; +const MESSAGE_EXTRA_STATIC = 'Extraneous override keyword: static members cannot override'; +const MESSAGE_EXTRA_NO_OVERRIDE = 'Member with @override keyword does not override any base class member'; +const MESSAGE_MISSING_OVERRIDE = 'Member is overriding a base member. Use the @override keyword if this override is intended'; + interface IOptions { useJsdocTag: boolean; useDecorator: boolean; @@ -121,16 +126,14 @@ class Walker extends Lint.AbstractWalker { private checkNonOverrideableElementDeclaration(node: AllClassElements) { const foundKeyword = this.checkNodeForOverrideKeyword(node); if (foundKeyword !== undefined) { - this.addFailureAtNode(foundKeyword, 'Extraneous override keyword', - Lint.Replacement.deleteText(foundKeyword.getStart(), foundKeyword.getWidth())); + this.addFailureAtNode(foundKeyword, 'Extraneous override keyword', fixRemoveOverrideKeyword(foundKeyword)); } } private checkConstructorDeclaration(node: ts.ConstructorDeclaration) { const foundKeyword = this.checkNodeForOverrideKeyword(node); if (foundKeyword !== undefined) { - this.addFailureAtNode(foundKeyword, 'Extraneous override keyword: constructors always override the parent', - Lint.Replacement.deleteText(foundKeyword.getStart(), foundKeyword.getWidth())); + this.addFailureAtNode(foundKeyword, MESSAGE_EXTRA_CONSTRUCTOR, fixRemoveOverrideKeyword(foundKeyword)); } } @@ -139,12 +142,16 @@ class Walker extends Lint.AbstractWalker { if (isStaticMember(node)) { if (foundKeyword !== undefined) { - this.addFailureAtNode(foundKeyword, 'Extraneous override keyword: static members cannot override', - Lint.Replacement.deleteText(foundKeyword.getStart(), foundKeyword.getWidth())); + this.addFailureAtNode(foundKeyword, MESSAGE_EXTRA_STATIC, fixRemoveOverrideKeyword(foundKeyword)); } return; } + if (!ts.isPropertyDeclaration(node) && node.body === undefined) { + // Special case if this is just an overload signature + return; + } + const parent = node.parent; if (parent == null || !isClassType(parent)) { return; @@ -152,13 +159,9 @@ class Walker extends Lint.AbstractWalker { const base = this.checkHeritageChain(parent, node); if (foundKeyword !== undefined && base === undefined) { - this.addFailureAtNode(node.name, 'Member with @override keyword does not override any base class member', - Lint.Replacement.deleteText(foundKeyword.getStart(), foundKeyword.getWidth())); + this.addFailureAtNode(node.name, MESSAGE_EXTRA_NO_OVERRIDE, fixRemoveOverrideKeyword(foundKeyword)); } else if (foundKeyword === undefined && base !== undefined) { - const fix = this.fixAddOverrideKeyword(node); - this.addFailureAtNode(node.name, - 'Member is overriding a base member. Use the @override keyword if this override is intended', - fix); + this.addFailureAtNode(node.name, MESSAGE_MISSING_OVERRIDE, this.fixAddOverrideKeyword(node)); } } @@ -328,6 +331,10 @@ class Walker extends Lint.AbstractWalker { } } +function fixRemoveOverrideKeyword(keyword: OverrideKeyword) { + return Lint.Replacement.deleteText(keyword.getStart(), keyword.getWidth()); +} + function isStaticMember(node: ts.Node): boolean { return (ts.getCombinedModifierFlags(node) & ts.ModifierFlags.Static) !== 0; } diff --git a/test.sh b/test.sh new file mode 100755 index 0000000..ad65518 --- /dev/null +++ b/test.sh @@ -0,0 +1,23 @@ +#!/bin/sh -xe + +# Test the lint rule + +tslint --test test +node_modules/.bin/ts-node -P test/tsconfig.json test/test.ts +tslint -p test test/test.ts +tslint -p test/exclude-interfaces + +# Test the fixers + +FIX_TEST_DIR="test/fixers" + +cp "${FIX_TEST_DIR}/spec.bad.ts" "${FIX_TEST_DIR}/spec.bad.tofix.ts" # Make a copy to keep the original untouched +npm run tslint -- --fix -p "${FIX_TEST_DIR}" # Fix +diff "${FIX_TEST_DIR}/spec.fixed.jsdoc.ts" "${FIX_TEST_DIR}/spec.bad.tofix.ts" # Check the result against the reference +node_modules/.bin/ts-node -P "${FIX_TEST_DIR}/tsconfig.json" "${FIX_TEST_DIR}/spec.bad.tofix.ts" # Check that the result is valid for tsc + +# Repeat for the decorator-based fixer +cp "${FIX_TEST_DIR}/spec.bad.ts" "${FIX_TEST_DIR}/spec.bad.tofix.ts" +npm run tslint -- --fix -c ${FIX_TEST_DIR}/tslint.decorator.json -p "${FIX_TEST_DIR}" +diff "${FIX_TEST_DIR}/spec.fixed.decorator.ts" "${FIX_TEST_DIR}/spec.bad.tofix.ts" +node_modules/.bin/ts-node -P "${FIX_TEST_DIR}/tsconfig.json" "${FIX_TEST_DIR}/spec.bad.tofix.ts" diff --git a/test/fixes/spec.bad.ts b/test/fixers/spec.bad.ts similarity index 78% rename from test/fixes/spec.bad.ts rename to test/fixers/spec.bad.ts index 5805fa2..5532bf5 100644 --- a/test/fixes/spec.bad.ts +++ b/test/fixers/spec.bad.ts @@ -1,3 +1,5 @@ +import '../../register'; + abstract class First { } @@ -7,11 +9,17 @@ class Foo extends First { /** doc comment **/ public bar(): void { } + public bar2(): void { } + public overrideMe(arg0: string): number { return 0; } public overrideMe2(arg0: string): number { return 0; } public overrideMe3(arg0: string): number { return 0; } public notToOverride(): void { } + + public overloadedMethod(): void; + public overloadedMethod(v: string): void; + public overloadedMethod(v?: string): void { } } export class Baz extends Foo { @@ -60,5 +68,9 @@ export class Baz extends Foo { public notToOverride(): void { } @override - @override public bar(): void { } + @override public bar2(): void { } + + public overloadedMethod(): void; + public overloadedMethod(v: string): void; + public overloadedMethod(v?: string): void { } } diff --git a/test/fixes/spec.fixed.decorator.ts b/test/fixers/spec.fixed.decorator.ts similarity index 78% rename from test/fixes/spec.fixed.decorator.ts rename to test/fixers/spec.fixed.decorator.ts index 47a290f..91fa8a0 100644 --- a/test/fixes/spec.fixed.decorator.ts +++ b/test/fixers/spec.fixed.decorator.ts @@ -1,3 +1,5 @@ +import '../../register'; + abstract class First { } @@ -7,11 +9,17 @@ class Foo extends First { /** doc comment **/ public bar(): void { } + public bar2(): void { } + public overrideMe(arg0: string): number { return 0; } public overrideMe2(arg0: string): number { return 0; } public overrideMe3(arg0: string): number { return 0; } public notToOverride(): void { } + + public overloadedMethod(): void; + public overloadedMethod(v: string): void; + public overloadedMethod(v?: string): void { } } export class Baz extends Foo { @@ -59,5 +67,9 @@ export class Baz extends Foo { @override public notToOverride(): void { } - @override public bar(): void { } + @override public bar2(): void { } + + public overloadedMethod(): void; + public overloadedMethod(v: string): void; + @override public overloadedMethod(v?: string): void { } } diff --git a/test/fixes/spec.fixed.jsdoc.ts b/test/fixers/spec.fixed.jsdoc.ts similarity index 77% rename from test/fixes/spec.fixed.jsdoc.ts rename to test/fixers/spec.fixed.jsdoc.ts index 9276f28..58fcb5d 100644 --- a/test/fixes/spec.fixed.jsdoc.ts +++ b/test/fixers/spec.fixed.jsdoc.ts @@ -1,3 +1,5 @@ +import '../../register'; + abstract class First { } @@ -7,11 +9,17 @@ class Foo extends First { /** doc comment **/ public bar(): void { } + public bar2(): void { } + public overrideMe(arg0: string): number { return 0; } public overrideMe2(arg0: string): number { return 0; } public overrideMe3(arg0: string): number { return 0; } public notToOverride(): void { } + + public overloadedMethod(): void; + public overloadedMethod(v: string): void; + public overloadedMethod(v?: string): void { } } export class Baz extends Foo { @@ -62,5 +70,9 @@ export class Baz extends Foo { /** @override */ public notToOverride(): void { } - @override public bar(): void { } + @override public bar2(): void { } + + public overloadedMethod(): void; + public overloadedMethod(v: string): void; + /** @override */ public overloadedMethod(v?: string): void { } } diff --git a/test/fixers/tsconfig.json b/test/fixers/tsconfig.json new file mode 100644 index 0000000..e1e366e --- /dev/null +++ b/test/fixers/tsconfig.json @@ -0,0 +1,10 @@ +{ + "compilerOptions": { + "target": "es2015", + "experimentalDecorators": true, + "strict": true + }, + "files": [ + "spec.bad.tofix.ts" + ] + } \ No newline at end of file diff --git a/test/fixes/tslint.decorator.json b/test/fixers/tslint.decorator.json similarity index 100% rename from test/fixes/tslint.decorator.json rename to test/fixers/tslint.decorator.json diff --git a/test/fixes/tslint.json b/test/fixers/tslint.json similarity index 100% rename from test/fixes/tslint.json rename to test/fixers/tslint.json diff --git a/test/fixes/test.sh b/test/fixes/test.sh deleted file mode 100755 index aa0c8a8..0000000 --- a/test/fixes/test.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/bin/sh -xe - -FIX_TEST_DIR="test/fixes" - -cp "${FIX_TEST_DIR}/spec.bad.ts" "${FIX_TEST_DIR}/spec.bad.tofix.ts" -npm run tslint -- --fix -p "${FIX_TEST_DIR}" -diff "${FIX_TEST_DIR}/spec.fixed.jsdoc.ts" "${FIX_TEST_DIR}/spec.bad.tofix.ts" - -cp "${FIX_TEST_DIR}/spec.bad.ts" "${FIX_TEST_DIR}/spec.bad.tofix.ts" -npm run tslint -- --fix -c ${FIX_TEST_DIR}/tslint.decorator.json -p "${FIX_TEST_DIR}" -diff "${FIX_TEST_DIR}/spec.fixed.decorator.ts" "${FIX_TEST_DIR}/spec.bad.tofix.ts" diff --git a/test/fixes/tsconfig.json b/test/fixes/tsconfig.json deleted file mode 100644 index 141faaf..0000000 --- a/test/fixes/tsconfig.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "compilerOptions": { - "target": "es2015" - }, - "files": [ - "spec.bad.tofix.ts" - ] - } \ No newline at end of file diff --git a/test/spec.ts.lint b/test/spec.ts.lint index 9c94df6..bf462af 100644 --- a/test/spec.ts.lint +++ b/test/spec.ts.lint @@ -27,6 +27,9 @@ class Base extends ABase { public propertyNotToOverride = 1; public methodToOverride(): void { } public methodNotToOverride(): void { } + public overloadedMethod(a: number): void; + public overloadedMethod(a: string): void; + public overloadedMethod(a: number | string): void { } } const DynBase = class extends ABase { @@ -115,6 +118,11 @@ class Example1 extends Base { /** @override */ public aMethodToOverride(): void { } public aMethodNotToOverride(): void { } ~~~~~~~~~~~~~~~~~~~~ [Member is overriding a base member. Use the @override keyword if this override is intended] + + public overloadedMethod(a: number): void; + public overloadedMethod(a: string): void; + public overloadedMethod(a: number | string): void { } + ~~~~~~~~~~~~~~~~ [Member is overriding a base member. Use the @override keyword if this override is intended] } const Example2 = class extends Base { diff --git a/test/tslint.json b/test/tslint.json index 761dbdd..883163a 100644 --- a/test/tslint.json +++ b/test/tslint.json @@ -1,3 +1,4 @@ { - "extends": "../tslint-override.json" + "extends": "../tslint-override.json", + "one-line": true } \ No newline at end of file