Skip to content
This repository has been archived by the owner. It is now read-only.

4594 - fix unnecessary-constructor parameters for super call #4813

Conversation

tanmoyopenroot
Copy link
Contributor

@tanmoyopenroot tanmoyopenroot commented Jul 27, 2019

PR checklist

Overview of change:

Add check-super-calls option to unnecessary-constructor

{
  "rules": {
    "unnecessary-constructor": [
        true,
        { "check-super-calls": true }
    ]
  }
}

Is there anything you'd like reviewers to focus on?

Not sure if the option name check-super-calls is correct.

CHANGELOG.md entry:

[new-rule-option] check-super-calls option for unnecessary-constructor rule

Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

A good start! 🎊
Left some comments on requested structural changes and missing test changes.

src/rules/unnecessaryConstructorRule.ts Outdated Show resolved Hide resolved
src/rules/unnecessaryConstructorRule.ts Outdated Show resolved Hide resolved
src/rules/unnecessaryConstructorRule.ts Outdated Show resolved Hide resolved
@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Jul 27, 2019

Looks like you have some lint failures you'll need to clean up too:

This type is not allowed in the 'if' condition because it could be undefined. Allowed types are boolean or boolean-or-undefined. (strict-boolean-expressions)
  87 | 
  88 | const isEmptyOrContainsOnlySuper = (node: ts.ConstructorDeclaration, options: Options): boolean => {
> 89 |     if (node.body) {
     |        ^
  90 |         const { unnecessarySuperCall } = options;
  91 | 
  92 |         if (!containsStatements(node.body.statements)) {

@tanmoyopenroot
Copy link
Contributor Author

tanmoyopenroot commented Jul 28, 2019

Looks like you have some lint failures you'll need to clean up too:

This type is not allowed in the 'if' condition because it could be undefined. Allowed types are boolean or boolean-or-undefined. (strict-boolean-expressions)
  87 | 
  88 | const isEmptyOrContainsOnlySuper = (node: ts.ConstructorDeclaration, options: Options): boolean => {
> 89 |     if (node.body) {
     |        ^
  90 |         const { unnecessarySuperCall } = options;
  91 | 
  92 |         if (!containsStatements(node.body.statements)) {

Done

@tanmoyopenroot
Copy link
Contributor Author

tanmoyopenroot commented Aug 10, 2019

A good start!
Left some comments on requested structural changes and missing test changes.

Done

@tanmoyopenroot
Copy link
Contributor Author

tanmoyopenroot commented Aug 23, 2019

@JoshuaKGoldberg @adidahiya ???

@adidahiya
Copy link
Member

adidahiya commented Sep 10, 2019

testNext failure is unreleated. I'd like to release this and improve it in future PRs if necessary

@adidahiya adidahiya merged commit b6c8b0c into palantir:master Sep 10, 2019
13 of 14 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unnecessary-constructor parameters for super call
3 participants