[no-useless-constructor] / [no-empty-function] with mixed JS & TS codebase

[scottohara]

(This issue was originally raised in eslint/typescript-eslint-parser#418; and it was suggested that it would be more appropriately logged here, as it is not something that the parser can handle)

What version of TypeScript are you using?
2.6.2

What version of typescript-eslint-parser are you using?
9.0.0

What code were you trying to parse?

export default class Foo {
  constructor(private name: string) {}

  get greeting() : string {
    return `Hello ${this.name}`;
  }
}

What did you expect to happen?
No errors/warnings about empty constructor.

What happened?

  2:2   error  Useless constructor           no-useless-constructor
  2:36  error  Unexpected empty constructor  no-empty-function

For now, I have turned off these two rules in my .ts projects; but for projects that mix *.ts and *.js code, it would be nice to be able to have these rules enabled to catch any useless constructors/empty functions in *.js code without it also warning about valid *.ts constructors.

[macklinu]

I'm curious what the proper solution is here. Should these rules be implemented in this plugin to support TypeScript's constructor syntax, since constructor(private name: string) {} is the equivalent of:

class Foo {
  private name: string
  constructor(name: string) {
    this.name = name
  }
}

I'd like to help out, but I'm not sure I understand what the next steps should be.

[corbinu]

@macklinu

Yes the eslint rule should be forked into this plugin and fixed to understand TypeScript just like https://github.com/nzakas/eslint-plugin-typescript/blob/master/docs/rules/no-unused-vars.md

[macklinu]

Cool, thanks for that info, @corbinu!

[corbinu]

Really glad your helping out!

[JamesHenry]

Please try out 1.2.0 and see if it helps - it's possible that no-empty-function is not yet taken care of, but if that's the case please open a dedicate issue. Many thanks!

[armano2]

this issue is still valid for no-empty-function

  2:36  error  Unexpected empty constructor  no-empty-function

is still going to error

[scottohara]

The new @typescript-eslint/no-useless-constructor rule works as described, and no longer warns for code as per the original issue description. (no-empty-function still warns, as expected).

Unfortunately the new rule won't help in mixed JS & TS codebases, as per the issue title.

I was hoping that the no-useless-constructor rule from eslint could remain enabled (to catch useless constructors in vanilla JS code), and that the new @typescript-eslint/no-useless-constructor rule would simply 'undo' any false positives in TS code that had been flagged by the core rule.

(I'm guessing that this simply isn't possible, and that once an error has been flagged by a rule, there's no way for that error to be 'unflagged'?)

A similar issue exists with a number of other @typescript-eslint/* rules that wrap an existing eslint rule (e.g. indent is another like this), where the core rule in eslint has to be disabled to prevent false positives.

I appreciate that mixed JS & TS codebases may not be the primary use case for most, but having to disable the older rules means developers working in mixed codebases (or are still in the process of migrating from JS to TS) have to choose which language they want these rules to apply to, and not both.

[armano2]

@scottohara can you give me example of issue with @typescript-eslint/no-useless-constructor for js file?

[scottohara]

{
  "parser": "@typescript-eslint/parser",
  "plugins": ["@typescript-eslint"]
  "rules": {
    "no-useless-constructor": "error",
    "@typescript-eslint/no-useless-constructor": "error"
  }
}

// no-useless-constructor.js

class Foo {
  constructor() {}
}

// no-useless-constructor.ts

class Foo {
  constructor(private name: string) {}
}

no-useless-constructor.js
  5:3  error  Useless constructor  no-useless-constructor
  5:3  error  Useless constructor  @typescript-eslint/no-useless-constructor

no-useless-constructor.ts
  5:3  error  Useless constructor  no-useless-constructor

The expectation here is that the constructor in the *.js file is useless (and should be warned about; but the one in the *.ts file is not useless. Therefore, there should be 1 error in total.

With both rules enabled, we get 3 errors:

• 2 x errors for the JS file
• 1 x error for the TS file

[armano2]

we are not able to augment how eslint rules are working, you should use only @typescript-eslint/no-useless-constructor -> this rule works for js and ts files

https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/tests/lib/rules/no-useless-constructor.js

[JamesHenry]

So I believe closing this was correct, and that a new focused issue should be created for no-empty-function?

[armano2]

i didn't see any issues for no-empty-function and i didn't wanted to forget about this one, if you feel its better to close it, i'm ok with that

[bradzacher]

Spawned off into #426 so it's clearly tracked