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

member-access: Clean up and add "no-public" option. #2247

Merged
merged 2 commits into from
Mar 20, 2017

Conversation

andy-hanson
Copy link
Contributor

@andy-hanson andy-hanson commented Feb 26, 2017

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

Added a no-public option to member-access.
Also removed documentation about constructor and get/set only being public, because this appears to not be true any more.

CHANGELOG.md entry:

[new-rule-option] member-access adds no-public option

let checkConstructor = options.indexOf(OPTION_CHECK_CONSTRUCTOR) !== -1;
if (noPublic) {
if (checkAccessor || checkConstructor) {
throw new Error("If 'no-public' is present, it should be the only option.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Config error should state rule name like:

"member-access rule: the no-public option cannot be combined with other options"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I disagree; rules shouldn't have to state their name. It should just be the default to include the name of the failing rule in the output. See #2228.

let checkAccessor = options.indexOf(OPTION_CHECK_ACCESSOR) !== -1;
let checkConstructor = options.indexOf(OPTION_CHECK_CONSTRUCTOR) !== -1;
if (noPublic) {
if (checkAccessor || checkConstructor) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I can compile a private constructor and private getter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This checks that when no-public is set, check-accessor and check-constructor aren't set, because someone who wants to ban the public keyword presumably wants to ban it everywhere, so we always check everything, so other options aren't necessary.

@nchen63 nchen63 merged commit 2929ccc into palantir:master Mar 20, 2017
@nchen63
Copy link
Contributor

nchen63 commented Mar 20, 2017

@andy-hanson thanks!

@andy-hanson andy-hanson deleted the member_access_no_public branch March 20, 2017 02:57
AJamesPhillips pushed a commit to AJamesPhillips/tslint that referenced this pull request Mar 23, 2017
@aluanhaddad
Copy link

So happy to have this rule. Thank you!

@CKGrafico
Copy link

Probably we didn't understand well the rule but for me this is to don't use 'public' because is default value in TS.
When we use "member-access": [true, "no-public"] we have this error:

The class method 'kendoDatePicker' must be marked either 'private', 'public', or 'protected'

@ajafff
Copy link
Contributor

ajafff commented Jul 4, 2017

@CKGrafico do you use tslint 5.0.0 or greater?

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

Successfully merging this pull request may close these issues.

6 participants