[explicit-member-accessibility]: add support for tslint member-access config options

[Hotell]

TSlint member-access supports following config:

• "no-public" forbids public accessibility to be specified, because this is the default.
• "check-accessor" enforces explicit visibility on get/set accessors
• "check-constructor" enforces explicit visibility on constructors
• "check-parameter-property" enforces explicit visibility on parameter properties

Currently this rule follows only all or nothing rule. It would be nice to implement at least no-public config to prevent devs to use public which is a default in TS.

Repro

{
  "rules": {
    "@typescript-eslint/explicit-member-accessibility": ["error", "no-public"]
  }
}

class Test {
 // Expect lint error
 public render(){
   return `<div>Hello</div>`
 }

 private handleClick = () => { /*...*/ }
}

[bradzacher]

I don't agree with the no-public option in the context of the rule as it is.

If I write out a class and do not annotate any member accessibility, then I will get no lint errors.
It turns it from a rule which forces you to annotate every method, and reminds you if you don't, to a rule which purely forbids the public keyword.

In order for us to implement that setting, we would have to rename the rule to member-accessibility, as it's no longer about being explicit.

The other settings definitely have merit in the context of the rule right now though.

[JamesHenry]

@bradzacher I agree it doesn't fit well with the current name of the rule (vs TSLint's name), but the use-case is valuable when you take the approach of not using public and want to enforce it consistently.

This:

class Foo {
  bar: string;
  baz: string;
  public qux: string;
}

Would be allowable currently in a codebase that didn't want to use public, but would be a desirable thing to catch

[bradzacher]

there's definitely value in the option; eslint is about enforcing coding consistency, so adding an option to enforce not using public make sense.

But I would block hard against adding it whilst the rule is called ***explicit***-member-accessibility for the reasons above.
We can add this to the list of rules in #203 that we're looking to breaking change rename.

[JamesHenry]

Sounds good!

[gavinbarron]

I've been taking a short look at this rule while you two have been chatting here and considering how this might wind up being implemented and the interplay between the options outlined above.

The no-public options is pretty easy to implement and I have a spike that handles that vs the current all implementation.

It seems to me that there is a degree of mutual exclusivity in the options. e.g. It don't makes sense to allow a user to specify both no-public and check-constructor. Does that gel with your thinking @JamesHenry?

[bradzacher]

That was my thinking - the config is a bit confusing as is (what does it mean to have no-public on and check-accessor on?).

Off the top of my head I'd expect the config to look something like this (we do this for a number of our rules)

type Override = boolean | { // false = don't check, true = check and use base config, object = override base config 
  noPublic?: boolean;
};
type Options = [
  {
    noPublic?: boolean;
    overrides?: {
      accessor?: Override;
      constructor?: Override;
      method?: Override;
      parameterProperty?: Override;
    }
  }
]

[gavinbarron]

Yeah, that's making sense to me.

TBH, I think that if there are overrides set then that will take precedence, so

{ 'no-public': true, 'overrides': { 'accessor': true }

Would result in the below being invalid and failing on line 3, character 3 but passing the constructor and the private property

class foo {
  get name(): string {
    return this.myName
  }
  constructor(name: string){
    this.myName = name;
  }
  private myName: string;
}

Are there any guidelines to how to contribute to this project?
I'll admit I was jazzed when the blog post about TypeScript adopting ESLint as the preferred tools came out and I'd like to help make this project as successful as possible for people migrating from TSLint, of which I am a huge fan.

[gavinbarron]

@Hotell I have a question for you on the parameter-properties.
Is this required to have a configuration option here given that the no-parameter-properties rule allows users to set the allowed accessibility options? If so can you give a use case and description of how the rule is supposed to work and interplay with no-parameter-properties?

Thanks

[kachkaev]

I guess this can be closed given that #322 is merged. Can't wait for this to be released 🙌 🚀