Skip to content
This repository has been archived by the owner on Mar 25, 2021. 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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

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

Done

@tanmoyopenroot
Copy link
Contributor Author

@JoshuaKGoldberg @adidahiya ???

@adidahiya
Copy link
Contributor

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