Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

[unnecessary-constructor] Incorrectly flagging accessibility-restricting constructors #4511

Closed
ericbf opened this issue Feb 7, 2019 · 3 comments · Fixed by #4619
Closed

Comments

@ericbf
Copy link
Contributor

ericbf commented Feb 7, 2019

Bug Report

  • TSLint version: 5.12.1
  • TypeScript version: 3.1.6
  • Running TSLint via: Atom

TypeScript code being linted

class SomethingNotConstructible {
	private constructor() {}
}

with tslint.json configuration:

{
	"rules": {
		"unnecessary-constructor": true
	}
}

Actual behavior

The private constructor is flagged as unnecessary.

Expected behavior

The private constructor is not flagged.

There are cases where a class shouldn't be constructible (factory classes or something) where it would make sense to have an empty constructor with the private/etc modifier to prevent it from being constructed externally. Right now, this private empty constructor is flagged by the unnecessary-constructor rule. In my estimation, this constructor is necessary, because it performs a task that would not performed were it not included (makes the constructor inaccessible).

@ericbf ericbf changed the title Rule unnecessary-constructor should not flag empty private constructor [unnecessary-constructor] Should not flag empty private constructor Feb 7, 2019
@ericbf ericbf changed the title [unnecessary-constructor] Should not flag empty private constructor [unnecessary-constructor] Incorrectly flagging accessibility-restricting constructors Feb 7, 2019
@JoshuaKGoldberg
Copy link
Contributor

Agreed, thanks for the catch @ericbf!

This should be a pretty straightforward change: the rule should also check if there's a privacy modifier on the constructor other than public.

@nirajgeorgian
Copy link

hello, i am new to this open source world but have been coding in js and typescript for around 2-3 years now. i can look into this bug and if possible can make necessary changes to it.
so can i start working on it ?

@ericbf
Copy link
Contributor Author

ericbf commented Feb 18, 2019

@nirajgeorgian Absolutely

JoshuaKGoldberg pushed a commit that referenced this issue Apr 1, 2019
This commit checks the modifiers of each constructor declaration. If it has any modifier that isn't `public`, then it's actually necessary and shouldn't be flagged by unnecessary-constructor rule.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants