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

[completed-docs] exported variable parsed as internal variable #2335

Closed
bumbleblym opened this issue Mar 13, 2017 · 6 comments
Closed

[completed-docs] exported variable parsed as internal variable #2335

bumbleblym opened this issue Mar 13, 2017 · 6 comments

Comments

@bumbleblym
Copy link
Contributor

Bug Report

  • TSLint version: 4.5.1
  • TypeScript version: 2.2.1
  • Running TSLint via: (pick one) CLI / Node.js API / VSCode / grunt-tslint / Atom / Visual Studio / etc

TypeScript code being linted

export const Null = null

with tslint.json configuration:

{
  "rules": {
    "completed-docs": [true, {
      "variables": {
        "visibilities": [
          "exported"
        ]
      }
    }]
  }
}

Actual behavior

"all": lint error
"exported": no lint error
"internal": lint error

Expected behavior

"all": lint error
"exported": lint error
"internal": no lint error

exported variable is being parsed as internal. repro

Also, documented visibilities type is inconsistent:
schema: enum
example: array enum
implementation: array enum

perhaps #629 should consider docs as well.

@JoshuaKGoldberg
Copy link
Contributor

I'll take a look after #2415 goes in.

@reduckted
Copy link
Contributor

This bug is caused by the fact that the VariableDeclaration does not have modifiers. The modifiers are on the VariableStatement which is a grandparent of a VariableDeclaration.

VariableStatement               <-- The relevant modifiers are here.
 └─ VariableDeclarationList
     └─ VariableDeclaration     <-- This node is checked.

I've made a fix for this, but it requires some changes that I've made in PR #2911 to allow the modifiers from a different node to be used. Once that PR is merged, I'll submit another PR for this bug.

@JoshuaKGoldberg
Copy link
Contributor

Bump @reduckted - still interested in a followup PR?

@reduckted
Copy link
Contributor

Doh! Totally forgot about this. Yes, I'll take a look into it again. :)

@reduckted
Copy link
Contributor

reduckted commented Nov 14, 2017

Hmm, looks like I already fixed this in #2950. ¯\_(ツ)_/¯

Edit: Which, for the record, was released in v5.6.0

@ajafff
Copy link
Contributor

ajafff commented Nov 14, 2017

Looking at the tests added in that PR I can confirm that this is fixed.

@ajafff ajafff closed this as completed Nov 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants