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

New rule: classMemberNoDefaultAccess. #404

Closed
wants to merge 0 commits into from
Closed

Conversation

meros
Copy link
Contributor

@meros meros commented May 22, 2015

Disallows public access modifiers that are not explicit in the class.

Review on Reviewable

@leeavital
Copy link
Contributor

Can you add this rule to the README and sample.tslint.json?

@meros
Copy link
Contributor Author

meros commented Jun 15, 2015

Sure thing.

@adidahiya
Copy link
Contributor

Sorry this has taken so long to get reviewed. Can you please rebase these changes on the latest master branch? There are a couple changes I'd like you to pick up and incorporate into this code:

  1. We now use tsconfig.json project files in this project. So you don't have to use reference paths anywhere. You do, however, have to keep the relevant config files up to date when you add new source files to the project. See src/tsconfig.json, src/rules/tsconfig.json, test/tsconfig.json
  2. utils.ts now provides a function called Lint.hasModifier() which should be able to replace much of the code in this new rule walker.


private checkModifiers(node: ts.Node, current: IModifiers): void {
if (!this.followsRules(current)) {
var message = "Default access modifier on member/method not allowed";
Copy link
Contributor

Choose a reason for hiding this comment

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

change this to be a public static FAILURE_STRING property on the Rule class in this file -- similar to how other rule walkers are structured.

@adidahiya
Copy link
Contributor

@meros I like this rule, and I want it in tslint :)

Will you have time to address the PR comments soon? Or can I take the code and make a new PR?

@ashwinr
Copy link
Contributor

ashwinr commented Jul 22, 2015

@adidahiya let's take this up when you get time and merge it in. also let's rename the rule to member-no-default-access to be consistent with member-ordering.

@adidahiya
Copy link
Contributor

I think member-access is slightly better

@ashwinr
Copy link
Contributor

ashwinr commented Jul 22, 2015

What would the value be? Just a Boolean?

Sent from a phone

On Jul 22, 2015, at 09:20, Adi Dahiya <notifications@github.commailto:notifications@github.com> wrote:

I think member-access is slightly better


Reply to this email directly or view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_palantir_tslint_pull_404-23issuecomment-2D123717353&d=BQMCaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=jafYnmwAIy7b95Z05TKJr1hFAxZozsv6_JDDlWyuJk8&m=aFijXHPQLx_9jUqLjHpnGVa_67H_iEEi_GdHL_OYbQg&s=MJCcjU_NTcxeAspPC0dET1GNkGMfjZ9a9sfztvQAnoI&e=.

@meros
Copy link
Contributor Author

meros commented Jul 29, 2015

Hi guys, sorry I have been a little under the radar a while. I have been in vacation mode, and in sweden that is a non-negligible part of the year :) I will look into this again soon, probably next week.

I see no problem in the comments so far. About the name, member-access doesn't really give a hint on what setting the flag would give. I like member-no-default-access the best, it's longer but the behaviour is spot on with what the module enforces.

@meros
Copy link
Contributor Author

meros commented Aug 6, 2015

Please see this pull request instead: #552

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.

4 participants