[explicit-member-accessibility] False positive when using private fields

[doberkofler]

Repro

The rule @typescript-eslint/explicit-member-accessibility reports a false positive when using private fields. At the same time, TypeScript reports the error An accessibility modifier cannot be used with a private identifier if an accessibility modifier would be used.

{
  "rules": {
    "@typescript-eslint/explicit-member-accessibility": ["warn"]
  }
}

class Person {
    #name: string

    constructor(name: string) {
        this.#name = name;
    }

    greet() {
        console.log(`Hello, my name is ${this.#name}!`);
    }
}

Expected Result

No false positive when using private fields.

Actual Result

Additional Info

Versions

package	version
@typescript-eslint/eslint-plugin	2.22.0
@typescript-eslint/parser	2.22.0
TypeScript	3.8.3
ESLint	6.8.0
node	12.16.1
npm	6.14.1

[bradzacher]

We do not yet support 3.8 syntax, including private fields.
See #1436 and associated PRs.

[jens-duttke]

@bradzacher Wouldn't it be possible to simply ignore properties starting with # in this rule, just like the ignoredMethodNames option does it for method names?

[bradzacher]

No. The AST for private names is different, and thus the # is not included in the name.

The AST is not yet finalised either, so we cannot build a rule on top of it.

[jens-duttke]

the # is not included in the name.

This statement seems to be incorrect. The util.getNameFromMember method returns the name with the leading #.

I've just checked that and this small change fixed the problem:
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts
Method checkPropertyAccessibilityModifier.
Replace:

const propertyName = util.getNameFromMember(classProperty, sourceCode);

by

const propertyName = util.getNameFromMember(classProperty, sourceCode);
if (propertyName.startsWith('#')) { return; }

[bradzacher]

To clarify - this will work right now due to "dumb luck" of sorts.
The getNameFromMember function just says "if I don't know how to handle the node, just get the source code string for the node".
Hence the function returns "#foo".

However - this isn't entirely correct. The name of the member is foo, not #foo.
Why does this distinction matter?

class Test {
  #foo: string = 'a';
  '#foo': string = 'b';

  test() {
    console.log(this.#foo, this['#foo']);
  }
}

#foo is a private class property whose name is foo.
'#foo' is a public class property whose name is #foo.

For both of these cases, however, getNameFromMember will currently print #foo.
So we cannot currently rely on this value to drive the lint rule as it is not correct.

[colxi]

Do we have any updates on this issue?
Unless we have a solution for this, this rule becomes useless for most projects as it will throw continuously false positives.

[LukasMachetanz]

Any updates?

[LukasMachetanz]

Is there a rough estimation when this could be fixed?

[stalniy]

To clarify - this will work right now due to "dumb luck" of sorts.
The getNameFromMember function just says "if I don't know how to handle the node, just get the source code string for the node".
Hence the function returns "#foo".

However - this isn't entirely correct. The name of the member is foo, not #foo.
Why does this distinction matter?

class Test {
  #foo: string = 'a';
  '#foo': string = 'b';

  test() {
    console.log(this.#foo, this['#foo']);
  }
}

#foo is a private class property whose name is foo.
'#foo' is a public class property whose name is #foo.

For both of these cases, however, getNameFromMember will currently print #foo.
So we cannot currently rely on this value to drive the lint rule as it is not correct.

I believe majority of projects don't use leading dash and quotes around class properties and even they do, now they have a problem because leading dash means a private class property in js. Even if js doesn't throw an error for this syntax, it will be confusing for developers.

So, I don't understand why all the rest should struggle to use new private class properties just for the sake of rule to be 100% complete (even though it's not 100% complete now, if we take into consideration private properties)

[HolgerJeromin]

Adding exact search term here because I stumbled over this issue, too:

Missing accessibility modifier on class property

[JoshuaKGoldberg]

Aha, I believe this was fixed by #3808. Please post back if that's incorrect and I can reopen! 🚀

[OldStarchy]

I've just updated to eslint@^8.2.0 and I'm still getting this error

package	version
eslint	8.2.0
@typescript-eslint/parser	5.3.0
@typescript-eslint/eslint-plugin	5.3.0
typescript	4.3.5
node	16.4.0

   26:2  error  Missing accessibility modifier on class property #length                     @typescript-eslint/explicit-member-accessibility

Do I need to do something to make this work?

[JoshuaKGoldberg]

@OldStarchy could you please post a full reproduction so that I can take a look? With just your versions and a single error there's nothing I can help with.

[jramstedt]

I'm getting Missing accessibility modifier on class property #doesItWork. error also.

Test case:
export class PrivateTest { #doesItWork?: number }
My rule:
'@typescript-eslint/explicit-member-accessibility': ['error', { overrides: { constructors: 'no-public' } }]

[JoshuaKGoldberg]

@jramstedt, friend, fellow, internet person, please post a full reproduction including your package versions. We can't help you if you just post a couple of snippets.

[jramstedt]

Here are the versions.

package	version
@typescript-eslint/eslint-plugin	5.3.1
@typescript-eslint/parser	5.3.1
TypeScript	4.4.4
ESLint	8.2.0
node	16.13.0

Do you have all the needed info to reproduce it?

[bradzacher]

looks like it was implemented for methods

typescript-eslint/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts

Lines 113 to 118 in 4875cd6

	function checkMethodAccessibilityModifier(
	methodDefinition: TSESTree.MethodDefinition,
	): void {
	if (methodDefinition.key.type === AST_NODE_TYPES.PrivateIdentifier) {
	return;
	}

But not for properties

typescript-eslint/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts

Lines 212 to 223 in 4875cd6

	function checkPropertyAccessibilityModifier(
	propertyDefinition:
	| TSESTree.PropertyDefinition
	| TSESTree.TSAbstractPropertyDefinition,
	): void {
	const nodeType = 'class property';
	const { name: propertyName } = util.getNameFromMember(
	propertyDefinition,
	sourceCode,
	);
	if (