hmil/tslint-override

TSLint plugin bringing support for the override keyword

tslint-override's Issues

Replace tslint with typescript-eslint ?

The TypeScript project has announced that they're shifting focus to eslint from tslint

Is there any chance you can look into switching to eslint as well? I think this plugin would be very beneficial! I don't like to switch to typescript-eslint before I found an override-implementation.

Add support for changing the casing of the decorator for use in the fixer

The rest of the decorators we use in my project are capitalised. It's nice that this tslint rule still allows those to pass, but it'd be nicer if the fixer had a way to work with that style as well.

Maybe support passing a string option that must start with @, and that is the name of the decorator? IE:

"explicit-override": [true, "decorator", "exclude-interfaces", "@Override"],

But I guess then someone could change it to something meaningless like @Foo, maybe you wouldn't want to do it that way for that reason

release to npm

Confusing `exclude-interfaces` behaviour

I was expecting that adding "excludes-interfaces" would prevent lint errors if I was missing an @override marker, but not produce any errors if I did choose to correctly add an @override marker.

I've since realised that @override markers for implementing an interface are less useful, as you'll get a compile error if you typo a method name, etc. (as you won't now correctly implement the interface). A place where it is still useful is abstract classes, but that's a bit more of a corner-case.

But either way it might at least be worth clarifying in the docs so people know what to expect?

Add license

Ignore methods with 'super'

Would be great to have an option to ignore (not highlight) overrides that call super. In such cases developer obviously is aware of the override.

Support decorator syntax

Allow opting out variables

Can we opt out variables? I don't want to see a warning/error when I use an overridden variable, only methods.

Allow opting out of certain function names

For example, the render function is used extensively in React, and if you extend React.Component, tslint-override will trigger, and 'fix' it to have an override comment.

Add an option to add a new line after jsDoc style fixes

We would like to have an option for fixer to add a new line after jsdoc style fixing.
"/** @override */\n" + indentation instead of just "/** @override */ "

So this:

export class ChildMyClass extends MyClass
{
    /** @override */ public method(parameters: string[]): void {
        parameters.forEach(console.error);
    }
}

becomes this:

export class ChildMyClass extends MyClass
{
    /** @override */
    public method(parameters: string[]): void {
        parameters.forEach(console.error);
    }
}

A crude implementation would be:

else {
    // No Jsdoc found, create a new one with just the tag
    let text = node.getFullText();
    const tokenStart = text.indexOf(node.getText());
    text = text.substr(0, tokenStart);
    const lastNL = text.lastIndexOf("\n");
    const indent = text.substr(lastNL + 1);
    return Lint.Replacement.appendText(node.getStart(), `/** @${this.getKeyword()} */\n` + indent);
}

Checking seems to work OK regardless of the new line.

Is this possible?
Thanks!

Rule warns on overloads, and "fixes" overloads to make an *actual* error

/**
 * Remove the context menu from this element
 */
public setContextMenu(): void; // lint warn on method name
/**
 * Set the context menu for this element
 */
public setContextMenu(generator: () => IContextMenu): void; // lint warn on method name
@Bound 
public setContextMenu(generatorOrContextMenu?: GetterOfOr<IContextMenu>) { // lint warn on method name

After fixer:

/**
 * Remove the context menu from this element
 */
@override public setContextMenu(): void; // ts error cause decorators here are invalid
/**
 * Set the context menu for this element
 */
@override public setContextMenu(generator: () => IContextMenu): void; // ts error cause decorators here are invalid
@override
@Bound
public setContextMenu(generatorOrContextMenu?: GetterOfOr<IContextMenu>) {

The override decorator is required on anonymous classes, even though they're invalid

Decorators are invalid in anonymous classes. As a result, when this rule is configured to use decorator-style override marking, this rule will warn & and try to make a fix that causes an error.

There probably should be some kind of additional option for anonymous classes that only has an effect when "decorator" is provided. For example, maybe an "anonymous-classes-jsdoc" or "anonymous-classes-ignore", defaulting to one or the other. (I would lean towards just "ignore" as default, personally)

example code:

abstract class Foo {
	public bar = 0;
}

const Fiz = new class extends Foo {
	public bar = 1;
};

Fixes to:

const Fiz = new class extends Foo {
	@Override public bar = 1; // errors
};

Add a "@final" rule?

(or @sealed)

Example:

class Foo {
  /**
   * @final
   */
  public bar() {}
}

class Bar extends Foo {
  /**
   * @override
   */
  public bar() {} // errors, "bar is marked as 'final', it should not be overridden"
}

Caveats For Days:

1. I'm guessing it wouldn't be possible to implement this with decorators, since the presence of decorators isn't "exported" by Typescript (I'm not too sure the terminology of TS's innards). I'm guessing that TSLint doesn't get that information either, then. And that means you'd have to be able to look at the source code of the superclass method to check if it's decorated with @final or not, which might not even be possible if the superclass method is in another file.
   • Even if looking directly at the source code of other files is possible, this might be impossible to implement in an efficient way, if you have to look at the source of superclasses for every method (when those superclasses would probably be in other files).
2. Would this even be within this project's scope? They're related checks, but this project wasn't made with the "final" rule in mind.
   • Note: The efficiency of the "final" rule (if it existed) theoretically could be improved by piggybacking off the existence of stuff marked @override. So maybe that makes them a little more linked?

Note: Does implementing final in TSLint make more sense than having it as a keyword in Typescript? I think in a way it could be more helpful, because consumers could still override if they really wanted to and knew what they were doing, vs being locked out completely (unless you were to like manually assign the method or sth)

`window` cannot be found inside of Web Workers

When registering the decorators it is assumed that if global is undefined then window must be defined. However this is not always the case. In Web Workers global and window throw the following error (at least under TypeScript)

Error: apps/samples/basic/src/app/app.worker.ts:3:1 - error TS2304: Cannot find name 'window'.
Error: apps/samples/basic/src/app/app.worker.ts:4:1 - error TS2304: Cannot find name 'global'.

globalThis is defined however.

I know we did look at globalThis back when setting up the angular-register and we decided not to because of backwards compatibility. Maybe we should look at using/creating a globalThis polyfill so we can use globalThis instead of window or global and support Web Workers

Here is one such pollyfill I found: https://www.npmjs.com/package/globalthis

Another solution may be to simply offer an import that isn't attached to the global for cases like this.

Separate rules for 'check-overrides' and 'explicit-overrides'

Firstly, thank you for the plugin, I've been really missing this functionality in TS!

It would be great if the checking of existing @OverRide markers was split out into a separate rule from checking whether a method should have an @OverRide marker added to it.
Essentially the first is enforcing the thing that should really be done by the compiler. Whilst the second is enforcing consistent code style/good coding practice, the job of the linter.

My use case for having separate rules, rather than just config options, is that:

1. If something is explicitly marked as @OverRide, which doesn't override a parent, then you want a severity:error, and the build to fail.
2. If there isn't an @OverRide on a parameter that does override a parent, then you want a lower severity, so that you know about it, without the build failing.

"new-line-after-decorators-and-tags" not working

"new-line-after-decorators-and-tags" not working.

I use the override decorator:

 "explicit-override": [
      true,
      "exclude-interfaces",
      "decorator",
      "new-line-after-decorators-and-tags"
    ],

All files pass linting despite the following:

@override myFunction(): void {

Add examples to readme

npm install does not install rules directory

npm install does not install rules directory as result feature does not work.

Support of Project Reference

Is it possible to run this rule in CLI when base and dependent classes are in different projects?
We have base project with common logic and several dependent projects (though TS 3.0 feature - project reference). So, base and dependent classes are in different typescript projects.
In VS2017 this rule works fine.
But I don't understand is it possible to check it with CLI. In TSLint CLI you can use only one project in --project argument. But in this case referenced projects are not used and I get a lot of false errors, that my override methods do not correspond to any base class. If I don't use --project argument I get warning that this rule requires type checking and I should use --project arg.

Allow opting out of checking for interfaces

This has been discussed in microsoft/TypeScript#2000 , but I will only implement it upon request or if a PR is initiated.

Angular like syntax - @Override()

In Angular, and NestJS (which is Angular like) decorators use the syntax of @Decorator() instead of @decorator and is on it's own line above the class member it is modifying. From my research this is the legacy es7 proposal decorator styling, never the less Angular is still using this styling so it would be nice if tslint-ovrride would offer an option for this styling.

I have made a modified version of register.ts to allow this when parsing without issue however in addition to this I would like the option to fix any errors using this same styling rather then in a comment.

I do see #18 added support for @Override, I presume adding support for this Angular like syntax would just be an extension of the changes made in that pull request.

angular-register.ts

declare var global: any;
declare global {
  /**
   * Specifies that this member must override a parent member.
   */
  const Override: () => (
    _target: any,
    _propertyKey: string,
    _descriptor?: PropertyDescriptor
  ) => void;

  interface Window {
    Override: (_target: any, _propertyKey: string, _descriptor?: PropertyDescriptor) => void;
  }

  namespace NodeJS {
    interface Global {
      Override: () => (
        _target: any,
        _propertyKey: string,
        _descriptor?: PropertyDescriptor
      ) => void;
    }
  }
}

export const ctx = typeof window !== 'undefined' ? window : global;

/**
 * Specifies that this member must override a parent member.
 */
export function Override(_target: any, _propertyKey: string, _descriptor?: PropertyDescriptor) {
  // noop
}

ctx.Override = () => Override;

Usage:

@Override()
public ngOnInit() {}

Improve fixers

Current fixers just prepend a tag to the function declaration. It would be great if they checked for existing jsDoc and add the tag in there.