Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adjacent-overload-signatures typescript-eslint rule #1250

Closed
jessegreenberg opened this issue May 19, 2022 · 4 comments
Closed

adjacent-overload-signatures typescript-eslint rule #1250

jessegreenberg opened this issue May 19, 2022 · 4 comments

Comments

@jessegreenberg
Copy link
Contributor

jessegreenberg commented May 19, 2022

From to #1114, see https://typescript-eslint.io/rules/adjacent-overload-signatures/.
Currently 258 errors in the project. Should we chip-away and enable or ignore this rule? To be discussed during dev meeting.

@jessegreenberg
Copy link
Contributor Author

It requires getters and setters to be next to each other. Like this

  getHintResponse(): ResolvedResponse {
    return ResponsePacket.getResponseText( this._hintResponse );
  }

  get hintResponse(): ResolvedResponse { return this.getHintResponse(); }

  setHintResponse( hintResponse: VoicingResponse ): void {
    this._hintResponse = hintResponse;
  }

  set hintResponse( hintResponse: VoicingResponse ) { this.setHintResponse( hintResponse ); }

The get hintResponse and set hintResponse must be together.

The reason this may be useful is because it could prevent accidental overloading since overloaded functions must be grouped.

SR volunteered to to give it a try, if it doesn't take too long to fix existing errors it is worth having this rule enabled.

@samreid
Copy link
Member

samreid commented May 26, 2022

We reviewed the cases and decided to divide and conquer. I'll do cases in scenery. @marlitas will do the other cases.

marlitas added a commit to phetsims/axon that referenced this issue May 26, 2022
marlitas added a commit to phetsims/scenery-phet that referenced this issue May 26, 2022
marlitas added a commit to phetsims/kite that referenced this issue May 26, 2022
samreid added a commit to phetsims/scenery that referenced this issue May 26, 2022
@samreid
Copy link
Member

samreid commented May 26, 2022

OK I finished in scenery. When the rest of the cases are complete, @marlitas can you please enable the rule in chipper/eslint/.eslintrc.js and commit/push? Self-unassigning.

@samreid samreid removed their assignment May 26, 2022
marlitas added a commit to phetsims/utterance-queue that referenced this issue May 26, 2022
marlitas added a commit to phetsims/tambo that referenced this issue May 26, 2022
marlitas added a commit to phetsims/sun that referenced this issue May 26, 2022
@marlitas
Copy link
Contributor

lint rule enabled runs clean. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants