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

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

Merged
adidahiya merged 2 commits into
palantir:masterfrom
tanmoyopenroot:fix-unnecessary-constructor-parameters-for-super-call
Sep 10, 2019
Merged

4594 - fix unnecessary-constructor parameters for super call#4813
adidahiya merged 2 commits into
palantir:masterfrom
tanmoyopenroot:fix-unnecessary-constructor-parameters-for-super-call

Conversation

@tanmoyopenroot

@tanmoyopenroot tanmoyopenroot commented Jul 27, 2019

Copy link
Copy Markdown
Contributor

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

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread src/rules/unnecessaryConstructorRule.ts Outdated
Comment thread src/rules/unnecessaryConstructorRule.ts Outdated
Comment thread src/rules/unnecessaryConstructorRule.ts Outdated
@JoshuaKGoldberg

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

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

Done

@tanmoyopenroot

Copy link
Copy Markdown
Contributor Author

@JoshuaKGoldberg @adidahiya ???

@adidahiya

Copy link
Copy Markdown
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