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

New rule: memberNoDefaultAccess. Disallows public access modifiers th… #552

Merged
merged 3 commits into from
Aug 20, 2015

Conversation

meros
Copy link
Contributor

@meros meros commented Aug 5, 2015

…at are not explicit in the class

This pull request replaces a previous one since I had to rebase it onto current master. Sorry about that.

@meros
Copy link
Contributor Author

meros commented Aug 5, 2015

This is the old pull request: #404

@@ -5,6 +5,7 @@
"arguments",
"statements"],
"ban": false,
"member-no-default-access": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

should be alphabetical by prop (rule) name

@meros
Copy link
Contributor Author

meros commented Aug 19, 2015

I think I got all of them. By that rule should be refactored, did you mean the hasModifiers utility? Because I changed to the snippet of code you suggested. The tests pass, but take a look so that it looks sane (I haven't looked into that utility very much to be honest :)

}
}

interface IModifiers {
Copy link
Contributor

Choose a reason for hiding this comment

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

this interface is no longer being used and can be removed

@adidahiya
Copy link
Contributor

thanks for the revisions @meros! just a few more small comments.

I'd still like this rule to be named member-access. The idea here is that all rules with "no-" in the name are explicitly banning the existence of some syntax. "member-no-default-access" would be trying to ban the non-existence of access modifiers, which is slightly confusing.

Overall, I think the rule reads better as "member-access ensure that all class members have access modifiers" rather than "member-no-default-access bans the default access modifier (a.k.a. the lack of an access modifier)"

@meros
Copy link
Contributor Author

meros commented Aug 20, 2015

@adidahiya Here you go :)

adidahiya added a commit that referenced this pull request Aug 20, 2015
New rule: memberNoDefaultAccess. Disallows public access modifiers th…
@adidahiya adidahiya merged commit 9ddfc76 into palantir:master Aug 20, 2015
@adidahiya
Copy link
Contributor

great, thanks @meros!

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