Giter VIP home page Giter VIP logo

tslint-override's Introduction

TSLint override rule

Finally brings support for the override keyword to TypeScript!

Build Status

What

export class Basic {
    public overridePlease(): void { }
    public doNotOverride(): void { }
}

export class Child extends Basic {
    public doNotOverride(): void { } // ERROR: Method Child#doNotOverride is overriding Basic#doNotOverride. Use the @override JSDoc tag if the override is intended

    // Make it explicit that you intend to override this member
    /** @override */ public overridePlease(): void { }

    // Alternatively, you can use the decorator syntax
    @override public overridePlease(): void { }

    // Typos won't bother you anymore
    /** @override */ public overidePlease(): void { } // ERROR: Method with @override tag is not overriding anything
}

Why

Most modern object oriented languages provide an override keyword to prevent misuse of the override mechanism. However, support for the override keyword in TypeScript is nowhere in sight, and in the meantime, TypeScript programmers are left with no ideal solution to this problem.

Here are some reasons to use this rule:

  • You may want to override a method, but introduce a typo in the method name and end up creating a new method by accident.
  • You accidentally override a method of a base class because the method you just added shares the same name and has a compatible signature.
  • A library author introduces a new method to a base class, which gets accidentally overridden by a method you wrote in a subclass in the past.
  • ...

How

npm install --save-dev tslint-override

Then, in your tslint.json, extend the tslint-override configuration.

{
    "extends": [
        "tslint-override"
    ]
}

Excluding interfaces

By default, tslint-override checks all members inherited or defined by any object in the heritage chain. You can opt out of interface checking. In that case, tslint-override will only look at parent classes.

{
    "extends": [
        "tslint-override"
    ],
    "rules": {
        "explicit-override": [ true, "exclude-interfaces" ]
    }
}

Using decorators

If you want to use the decorator syntax, you will need the override decorator in your scope. There are two ways to do this:

Let tslint-override do the job

In your application entry point, include the following import.

import 'tslint-override/register';

You can then use @override anywhere else in your code:

class Foo extends Basic {
    @override public doStuff() { }
}

Define it yourself

Alternatively, you can define it yourself, but make sure it is in scope where you need it.

function override(_target: any, _propertyKey: string, _descriptor?: PropertyDescriptor) { /* noop */ }

If your love for java will never die, you can define this with a capital 'O' and use @Override in your code instead.

Enforcing either jsdoc tags or decorators

Both jsdoc tags and decorators are accepted by default. If you want to force one and ignore the other, specify either the decorator or jsdoc parameter like this:

    "rules": {
        "explicit-override": [ true, "decorator" ]
    }

By default, the fixer adds jsdoc tags. Specifying the decorator option will also change that to use decorators.

PascalCase @Override fixer

Linting will accept both @override and @Override jsdoc tags or decorators, but the fixer defaults to adding @override. This can be changed with the pascal-case-fixer option.

{
    "extends": [
        "tslint-override"
    ],
    "rules": {
        "explicit-override": [ true, "pascal-case-fixer" ]
    }
}

Note: tslint-override/register does not support the PascalCase @Override decorator, so you will have to define it yourself.

Enforcing a new line in the fixer

If you want the fixer to put a new line after the tag use the new-line-after-decorators-and-tags option.

{
    "extends": [
        "tslint-override"
    ],
    "rules": {
        "explicit-override": [ true, "new-line-after-decorators-and-tags" ]
    }
}

or

{
    "extends": [
        "tslint-override"
    ],
    "rules": {
        "explicit-override": [ true, "decorator", "new-line-after-decorators-and-tags" ]
    }
}

Angular-friendly syntax (decorator with parenthesis)

tslint-override offers an angular-friendly decorator syntax, where the decorator is called with a pair of parenthesis.

import 'tslint-override/angular-register';

You can then use @Override() or @override() anywhere in your code:

class Foo extends Basic {
    @Override() public doStuff() { }
}

Add the setting angular-syntax-fixer to let tslint know to use parenthesis when fixing code.

{
    "extends": [
        "tslint-override"
    ],
    "rules": {
        "explicit-override": [ true, "decorator", "angular-syntax-fixer", "pascal-case-fixer" ]
    }
}

IDE support

This rule requires type information. If you are still using vscode-tslint you should switch over to using the tslint-language-service because vscode-tslint won't show the errors reported by tslint-override.

Example in VSCode: preview

Contributing, Credits, etc

Created and maintained by @hmil.
Contributions:

  • @Yuudaari - Added pascal-case-fixer option.
  • @stasberkov - Added exclude-interfaces option.
  • @mzyil - Added new-line-after-decorators-and-tags option.
  • @wSedlacek - Added angular-friendly decorator and fixer, various bug fixes.

License: MIT

Please star this repo if you like it, and submit code and issues if something doesn't work.

tslint-override's People

Contributors

chirivulpes avatar hmil avatar mzyil avatar stasberkov avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar

tslint-override's Issues

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.

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() {}

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.

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
};

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.

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>) {

"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 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!

`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.

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.

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.

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.

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)

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

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?

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.