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

Added location, type, and privacy to completed-docs #2095

Merged
merged 1 commit into from Feb 7, 2017
Merged

Added location, type, and privacy to completed-docs #2095

merged 1 commit into from Feb 7, 2017

Conversation

JoshuaKGoldberg
Copy link
Contributor

Also adds interfaces, enums, types, and a few other doc-able contructs.

Fixes #1713.
Lays the groundwork for #1921.

export type All = typeof Rule.ALL;

export type DocType = All
| typeof Rule.ARGUMENT_CLASSES
Copy link
Contributor

@nchen63 nchen63 Jan 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't typeof Rule.ARGUMENT_CLASSES "string"? In my IDE, DocType resolves to string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What version of TypeScript are you on? For me (VS Code, TypeScript 2.1.X) typeof Rule.ARGUMENT_CLASSES correctly resolves to the "classes" literal.

Historically, before support for discriminated literal types, I liked having this explicit typing - both to inform the user and for future-proofing the type declarations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using VS Code, TS 2.1.4. So you're saying that const a: Visibility = "s"; causes a compilation error for you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you meant to use const instead of public static? In that case, I see the string literal type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're totally right. Thanks for the catch! I've switched them to const.

@nchen63
Copy link
Contributor

nchen63 commented Jan 28, 2017

I think you need to rebase

@JoshuaKGoldberg
Copy link
Contributor Author

Rebase should be good to go.

@nchen63
Copy link
Contributor

nchen63 commented Jan 30, 2017

I think this may be considered a breaking change if someone already specifies "all" because they'll see new errors. Thoughts?

Also adds interfaces, enums, types, and a few other doc-able contructs.
@JoshuaKGoldberg
Copy link
Contributor Author

Agreed. Changed it to use the current default, so that the new locations are opt-in. For the next TSLint release we could consider expanding default coverage.

Copy link
Contributor

@nchen63 nchen63 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@FDiskas
Copy link

FDiskas commented Jun 5, 2017

🎽 when it will be released?

@adidahiya
Copy link
Contributor

@FDiskas it's available since v4.5. you can find out by yourself by clicking through to the squashed commit:

image

and look at the tags list:

image

@Viktor-Bredihin
Copy link

what's the status? I still can't get it work

@JoshuaKGoldberg
Copy link
Contributor Author

@Viktor-Bredihin what is your tslint.json config, and what are you trying to lint? Can you post a minimum reproduction we can take a look at? What problems are you seeing?

@Viktor-Bredihin
Copy link

@JoshuaKGoldberg
I'm trying to make this rule work '"completed-docs": true', it's not showing any errors in webstorm, however works fine when I use tslint in the console

@JoshuaKGoldberg
Copy link
Contributor Author

Ah - that's not an issue with this rule. That's an issue with the Webstorm TSLint extension.

completed-docs is a "type checked" rule (link to docs). That means it uses more expensive APIs from TypeScript. Many extensions don't support this kind of rule for performance reasons.

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

Successfully merging this pull request may close these issues.

None yet

5 participants