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

completed-docs should have an option to check every method/function overload #4416

Closed
captain-yossarian opened this issue Dec 28, 2018 · 7 comments · Fixed by #4563
Closed

Comments

@captain-yossarian
Copy link

Feature request for completed-docs rule

Is it possible to enforce (add option if does not exist) jsDoc comment for function/method overloading?

@JoshuaKGoldberg
Copy link
Contributor

@SerhiiBilyk sorry, what do you mean by "overloading"? Could you post a code snippet or two?

@captain-yossarian
Copy link
Author

@JoshuaKGoldberg

class Foo {

    /*
     * Enforce jsDoc comment here
     */
    emit(event: string, data: number) => void;

    /*
     * Enforce jsDoc comment  here
     */
    emit(event:string, data: string) => void;
 
    emit(event:string, data:any) {
        // ....... some code
    }
}

@adidahiya
Copy link
Contributor

this is not currently possible but seems like a reasonable request. currently TSLint will report 3 failures here but adding a jsdoc comment to any overload will make them go away:

  class Foo {
      emit(event: string, data: number): void;
-     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for methods.]
      emit(event: string, data: string): void;
-     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for methods.]
      emit(event: string, data: string | number) {
-     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for methods.]
          // ....... some code
      }
  }

@adidahiya adidahiya changed the title completed-docs enable for method/function overload completed-docs should have an option to check every method/function overload Feb 6, 2019
@0815fox
Copy link

0815fox commented Feb 25, 2019

Did you do something on this? Because since my update from 5.12.x to 5.13.0 today, i get new reports for exactly that and I definitly do not want to write a separate comment for each overload ;-) If so: is there a not yet documented option to turn this back to "one documentation for all overloads required"?

@adidahiya
Copy link
Contributor

hmm, that could have happened as a result of #3557. @JoshuaKGoldberg could you take a look?

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Mar 2, 2019

Yes, the behavior is now to require one on all overloads.

{
  "rules": {
    "completed-docs": [
      true,
      {
        "methods": true
      }
    ]
  }
}
class Foo {
    // completed-docs: Documentation must exist for methods
    emit(event: string, data: number): void;

    // completed-docs: Documentation must exist for methods
    emit(event:string, data: string): void;
 
    /**
     * Exists in one place
     */
    emit(event:string, data:any) {
        // ....... some code
    }
}

I'll look at making this an opt-in behavior...

@0815fox
Copy link

0815fox commented Mar 5, 2019

Thumbs up for configurable. Suggestion:

"overloads": false

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.

4 participants