Tslint: completed-docs false positive when entire doc comment consists of only tags

Created on 23 Dec 2016  路  6Comments  路  Source: palantir/tslint

Bug Report

  • __TSLint version__: 4.2.0
  • __TypeScript version__: 2.1.4
  • __Running TSLint via__: CLI

TypeScript code being linted

/**
 * @ignore
 * @see foo
 */
private static bar () : number {
    return 4;
}

with tslint.json configuration:

{
    "rules": {
        "completed-docs": true
    }
}

Actual behavior

Methods must have documentation.

Expected behavior

No error, as with TS 2.0.10 and tslint 4.1.1.

Rationale: we currently have /** @ignore */ on all private properties and methods (which will be the case until we have some extra bandwidth to spend on documenting all that stuff), and in some cases where there isn't anything more to add on top of a class description we have public properties whose docs are just /** @see $ClassName */.

P2 Fixed Bug

Most helpful comment

While it would be nice to have the ability to fine tune which tags are whitelisted as suggested above, I would think that "@inheritdoc" in particular should automatically and always bypass this rule, due to the inherent meaning of that tag. Without this, this rule is pretty much unusable for me right now, which is a shame since I really need it.

All 6 comments

A good solution to this could be changing the rule config to allow for conditions that mark a construct as not needing to be documented. Some conditions:

  • Not on the whitelist of types (what's currently implemented)
  • Has tags but no textual description (project may or may not consider that acceptable)
  • Has a tag that indicates it shouldn't be documented, such as @ignore or @inheritdoc

Proposal: extend the rule options to have global and per-construct settings.

  • If settings is the string "all", enable it for every possible construct.
  • If settings is an object, apply the IConstructDocsSettings logic to every possible constructs.
  • If settings is an array, for each member:

    1. If it's a string "all": enable all constructs

    2. If it's a string: enable it for that construct

    3. If it's an object, for each [key, value]:



      • The key is either "all" for all constructors or the name of an individual construct


      • The value is either false to disable, true to enable completely, or an IConstructDocsSettings



interface IConstructDocsSettings {
    /**
     * Tags that indicate a block is ok.
     */
    "ok-tags"?: boolean;

    /**
     * Having tags without text is ok.
     */
    "tags-without-text"?: boolean;
}

Thoughts?

Just to add another usage example. I tend to use @inheritdoc when implementing an interface or abstract class. However, I'm currently getting the "Documentation must exist for public methods" warning when doing so.

/**
 * An abstract class to illustrate my point.
 */
abstract class Foo {
  /**
   * A demo method to illustrate my point.
   * @param message - Do something with it.
   */
  public abstract bar(message: string): void;
}

/**
 * A concrete class that extends the Foo base class.
 */
class ConcreteFoo extends Foo {

  /**
   * @inheritdoc
   */
  public bar(message: string): void {  // Documentation must exist for public methods
    console.log(message);
  }
}

:anchor: :+1:

While it would be nice to have the ability to fine tune which tags are whitelisted as suggested above, I would think that "@inheritdoc" in particular should automatically and always bypass this rule, due to the inherent meaning of that tag. Without this, this rule is pretty much unusable for me right now, which is a shame since I really need it.

This does not handle single-line doc comments.

  /**
   * @inheritdoc
   */
  public test1(): void { }

  /** @inheritdoc */
  public test2(): void { } // Documentation must exist for public methods

See #3365.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rajinder-yadav picture rajinder-yadav  路  3Comments

Ne-Ne picture Ne-Ne  路  3Comments

ypresto picture ypresto  路  3Comments

jacob-robertson picture jacob-robertson  路  3Comments

denkomanceski picture denkomanceski  路  3Comments