Skip to content
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

Handle exclude-interfaces bug #13

Merged
merged 1 commit into from
Nov 26, 2018
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
41 changes: 32 additions & 9 deletions rules/explicitOverrideRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const MESSAGE_EXTRA_CONSTRUCTOR = 'Extraneous override keyword: constructors alw
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';
const MESSAGE_EXTRA = 'Extraneous override keyword';

interface IOptions {
useJsdocTag: boolean;
Expand Down Expand Up @@ -85,6 +86,11 @@ export class Rule extends Lint.Rules.TypedRule {
const OVERRIDE_KEYWORD = 'override';
const OVERRIDE_DECORATOR_MATCHER = /^@[oO]verride(\(\s*\))?$/;

type HeritageChainCheckResult = {
baseClass?: ts.Type;
baseInterface?: ts.Type;
} | undefined;

class Walker extends Lint.AbstractWalker<IOptions> {

constructor(
Expand Down Expand Up @@ -126,7 +132,7 @@ class Walker extends Lint.AbstractWalker<IOptions> {
private checkNonOverrideableElementDeclaration(node: AllClassElements) {
const foundKeyword = this.checkNodeForOverrideKeyword(node);
if (foundKeyword !== undefined) {
this.addFailureAtNode(foundKeyword, 'Extraneous override keyword', fixRemoveOverrideKeyword(foundKeyword));
this.addFailureAtNode(foundKeyword, MESSAGE_EXTRA, fixRemoveOverrideKeyword(foundKeyword));
}
}

Expand Down Expand Up @@ -158,9 +164,21 @@ class Walker extends Lint.AbstractWalker<IOptions> {
}
const base = this.checkHeritageChain(parent, node);

if (foundKeyword !== undefined && base === undefined) {
if (
// This member declares @override
foundKeyword !== undefined &&
// But no base symbol was found
(base === undefined || base.baseClass === undefined && base.baseInterface === undefined)
) {
this.addFailureAtNode(node.name, MESSAGE_EXTRA_NO_OVERRIDE, fixRemoveOverrideKeyword(foundKeyword));
} else if (foundKeyword === undefined && base !== undefined) {
} else if (
// This member does not declare @override
foundKeyword === undefined &&
// And something was found
base !== undefined &&
// And that thing is either a base class, or an interface and we are not excluding interface
(base.baseClass !== undefined || base.baseInterface !== undefined && !this._options.excludeInterfaces)
) {
this.addFailureAtNode(node.name, MESSAGE_MISSING_OVERRIDE, this.fixAddOverrideKeyword(node));
}
}
Expand Down Expand Up @@ -304,7 +322,10 @@ class Walker extends Lint.AbstractWalker<IOptions> {
}

private checkHeritageChain(declaration: ts.ClassDeclaration | ts.ClassExpression, node: OverrideableElement)
: ts.Type | undefined {
: HeritageChainCheckResult {

let baseInterface: ts.Type | undefined;
let baseClass: ts.Type | undefined;

const currentDeclaration = declaration;
if (currentDeclaration === undefined) {
Expand All @@ -315,19 +336,21 @@ class Walker extends Lint.AbstractWalker<IOptions> {
return;
}
for (const clause of clauses) {
if (this.options.excludeInterfaces && clause.token === ts.SyntaxKind.ImplementsKeyword) {
continue;
}
const isInterface = clause.token === ts.SyntaxKind.ImplementsKeyword;
for (const typeNode of clause.types) {
const type = this.checker.getTypeAtLocation(typeNode);
for (const symb of type.getProperties()) {
if (symb.name === node.name.getText()) {
return type;
if (isInterface) {
baseInterface = type;
} else {
baseClass = type;
}
}
}
}
}
return undefined;
return { baseInterface, baseClass };
}
}

Expand Down
3 changes: 3 additions & 0 deletions test/exclude-interfaces/spec.exclude-interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ interface TestInterface2 {
}

abstract class AbstractClass2 implements TestInterface2 {
/**
* @override
*/
func1() {
}
func2() {
Expand Down