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

completed-docs: Added option to check get and set accessors. #3497

Merged
merged 2 commits into from
Nov 30, 2017
Merged

completed-docs: Added option to check get and set accessors. #3497

merged 2 commits into from
Nov 30, 2017

Conversation

reduckted
Copy link
Contributor

@reduckted reduckted commented Nov 18, 2017

PR checklist

Overview of change:

The "properties" for the completed-docs rule now checks GetAccessor and SetAccessor node types.

CHANGELOG.md entry:

[enhancement] "properties" option for completed-docs rule now checks getter and setter accessors.

@ajafff
Copy link
Contributor

ajafff commented Nov 18, 2017

Added an "accessor" option to the completed-docs rule that causes GetAccessor and SetAccessor node types to be checked for JSDoc comments.

To the consumer it doesn't matter if it's an accessor or a "real" property. That's an implementation detail.
I think checking accessors should be enabled by enabling "properties".

@reduckted
Copy link
Contributor Author

Not sure why the "testNext" configuration is failing. The "trailing-comma" rule has failing tests, but my changes have nothing to do with that. 😕

@ajafff
Copy link
Contributor

ajafff commented Nov 19, 2017

@reduckted it's a bug in typescript@next, see #3494 (comment)

Copy link
Contributor

@ajafff ajafff left a comment

Choose a reason for hiding this comment

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

Code LGTM
Please add a test with accessors in an object literal. There should be no docs required?

@reduckted
Copy link
Contributor Author

@ajafff Ah, good catch. Accessors in object literals were expected to have documentation comments. I've made the rule skip over them now.

Properties on object literals didn't require documentation. I'm not too familiar with the entire TypeScript syntax tree, but I'm guessing properties in object literals and properties in classes don't use the same SyntaxKind.

@ajafff
Copy link
Contributor

ajafff commented Nov 19, 2017

@reduckted

I'm guessing properties in object literals and properties in classes don't use the same SyntaxKind.

That's correct. But MethodDeclaration is the same for both.

@ajafff ajafff merged commit fc4e6d3 into palantir:master Nov 30, 2017
@ajafff
Copy link
Contributor

ajafff commented Nov 30, 2017

thanks @reduckted

I'll open a follow-up PR to exclude method declarations in object literals

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.

2 participants