Be more tolerant with private identifier parsing, issue more targeted errors, and support private identifiers in forgotten 'this' codefix (microsoft#36188)

* Support private identifiers in forgotten this codefix

* Parse invalid private identifiers as identifiers and issue targeted errors

* Update codefix

* Remove accidentally deleted newline

Author: andrewbranch

src/compiler/parser.ts

@@ -1403,7 +1403,7 @@ namespace ts {
         // An identifier that starts with two underscores has an extra underscore character prepended to it to avoid issues
         // with magic property names like '__proto__'. The 'identifiers' object is used to share a single string instance for
         // each identifier in order to reduce memory consumption.
-        function createIdentifier(isIdentifier: boolean, diagnosticMessage?: DiagnosticMessage): Identifier {
+        function createIdentifier(isIdentifier: boolean, diagnosticMessage?: DiagnosticMessage, privateIdentifierDiagnosticMessage?: DiagnosticMessage): Identifier {
             identifierCount++;
             if (isIdentifier) {
                 const node = <Identifier>createNode(SyntaxKind.Identifier);
@@ -1417,6 +1417,11 @@ namespace ts {
                 return finishNode(node);
             }
 
+            if (token() === SyntaxKind.PrivateIdentifier) {
+                parseErrorAtCurrentToken(privateIdentifierDiagnosticMessage || Diagnostics.Private_identifiers_are_not_allowed_outside_class_bodies);
+                return createIdentifier(/*isIdentifier*/ true);
+            }
+
             // Only for end of file because the error gets reported incorrectly on embedded script tags.
             const reportAtCurrentPosition = token() === SyntaxKind.EndOfFileToken;
 
@@ -1430,8 +1435,8 @@ namespace ts {
             return createMissingNode<Identifier>(SyntaxKind.Identifier, reportAtCurrentPosition, diagnosticMessage || defaultMessage, msgArg);
         }
 
-        function parseIdentifier(diagnosticMessage?: DiagnosticMessage): Identifier {
-            return createIdentifier(isIdentifier(), diagnosticMessage);
+        function parseIdentifier(diagnosticMessage?: DiagnosticMessage, privateIdentifierDiagnosticMessage?: DiagnosticMessage): Identifier {
+            return createIdentifier(isIdentifier(), diagnosticMessage, privateIdentifierDiagnosticMessage);
         }
 
         function parseIdentifierName(diagnosticMessage?: DiagnosticMessage): Identifier {
@@ -1627,9 +1632,9 @@ namespace ts {
                         return isIdentifier() && !isHeritageClauseExtendsOrImplementsKeyword();
                     }
                 case ParsingContext.VariableDeclarations:
-                    return isIdentifierOrPattern();
+                    return isIdentifierOrPrivateIdentifierOrPattern();
                 case ParsingContext.ArrayBindingElements:
-                    return token() === SyntaxKind.CommaToken || token() === SyntaxKind.DotDotDotToken || isIdentifierOrPattern();
+                    return token() === SyntaxKind.CommaToken || token() === SyntaxKind.DotDotDotToken || isIdentifierOrPrivateIdentifierOrPattern();
                 case ParsingContext.TypeParameters:
                     return isIdentifier();
                 case ParsingContext.ArrayLiteralMembers:
@@ -2613,7 +2618,7 @@ namespace ts {
 
         function isStartOfParameter(isJSDocParameter: boolean): boolean {
             return token() === SyntaxKind.DotDotDotToken ||
-                isIdentifierOrPattern() ||
+                isIdentifierOrPrivateIdentifierOrPattern() ||
                 isModifierKind(token()) ||
                 token() === SyntaxKind.AtToken ||
                 isStartOfType(/*inStartOfParameter*/ !isJSDocParameter);
@@ -2633,7 +2638,7 @@ namespace ts {
 
             // FormalParameter [Yield,Await]:
             //      BindingElement[?Yield,?Await]
-            node.name = parseIdentifierOrPattern();
+            node.name = parseIdentifierOrPattern(Diagnostics.Private_identifiers_cannot_be_used_as_parameters);
             if (getFullWidth(node.name) === 0 && !node.modifiers && isModifierKind(token())) {
                 // in cases like
                 // 'use strict'
@@ -3436,6 +3441,7 @@ namespace ts {
                 case SyntaxKind.LessThanToken:
                 case SyntaxKind.AwaitKeyword:
                 case SyntaxKind.YieldKeyword:
+                case SyntaxKind.PrivateIdentifier:
                     // Yield/await always starts an expression.  Either it is an identifier (in which case
                     // it is definitely an expression).  Or it's a keyword (either because we're in
                     // a generator or async function, or in strict mode (or both)) and it started a yield or await expression.
@@ -5801,18 +5807,21 @@ namespace ts {
             return finishNode(node);
         }
 
-        function isIdentifierOrPattern() {
-            return token() === SyntaxKind.OpenBraceToken || token() === SyntaxKind.OpenBracketToken || isIdentifier();
+        function isIdentifierOrPrivateIdentifierOrPattern() {
+            return token() === SyntaxKind.OpenBraceToken
+                || token() === SyntaxKind.OpenBracketToken
+                || token() === SyntaxKind.PrivateIdentifier
+                || isIdentifier();
         }
 
-        function parseIdentifierOrPattern(): Identifier | BindingPattern {
+        function parseIdentifierOrPattern(privateIdentifierDiagnosticMessage?: DiagnosticMessage): Identifier | BindingPattern {
             if (token() === SyntaxKind.OpenBracketToken) {
                 return parseArrayBindingPattern();
             }
             if (token() === SyntaxKind.OpenBraceToken) {
                 return parseObjectBindingPattern();
             }
-            return parseIdentifier();
+            return parseIdentifier(/*diagnosticMessage*/ undefined, privateIdentifierDiagnosticMessage);
         }
 
         function parseVariableDeclarationAllowExclamation() {
@@ -5821,7 +5830,7 @@ namespace ts {
 
         function parseVariableDeclaration(allowExclamation?: boolean): VariableDeclaration {
             const node = <VariableDeclaration>createNode(SyntaxKind.VariableDeclaration);
-            node.name = parseIdentifierOrPattern();
+            node.name = parseIdentifierOrPattern(Diagnostics.Private_identifiers_are_not_allowed_in_variable_declarations);
             if (allowExclamation && node.name.kind === SyntaxKind.Identifier &&
                 token() === SyntaxKind.ExclamationToken && !scanner.hasPrecedingLineBreak()) {
                 node.exclamationToken = parseTokenNode<Token<SyntaxKind.ExclamationToken>>();

src/services/codefixes/fixForgottenThisPropertyAccess.ts

@@ -4,6 +4,7 @@ namespace ts.codefix {
     const didYouMeanStaticMemberCode = Diagnostics.Cannot_find_name_0_Did_you_mean_the_static_member_1_0.code;
     const errorCodes = [
         Diagnostics.Cannot_find_name_0_Did_you_mean_the_instance_member_this_0.code,
+        Diagnostics.Private_identifiers_are_not_allowed_outside_class_bodies.code,
         didYouMeanStaticMemberCode,
     ];
     registerCodeFix({
@@ -24,11 +25,16 @@ namespace ts.codefix {
         }),
     });
 
-    interface Info { readonly node: Identifier; readonly className: string | undefined; }
+    interface Info {
+        readonly node: Identifier | PrivateIdentifier;
+        readonly className: string | undefined;
+    }
+
     function getInfo(sourceFile: SourceFile, pos: number, diagCode: number): Info | undefined {
         const node = getTokenAtPosition(sourceFile, pos);
-        if (!isIdentifier(node)) return undefined;
-        return { node, className: diagCode === didYouMeanStaticMemberCode ? getContainingClass(node)!.name!.text : undefined };
+        if (isIdentifier(node)) {
+            return { node, className: diagCode === didYouMeanStaticMemberCode ? getContainingClass(node)!.name!.text : undefined };
+        }
     }
 
     function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, { node, className }: Info): void {

tests/baselines/reference/privateNameNotAllowedOutsideClass.errors.txt

@@ -1,14 +1,8 @@
-tests/cases/conformance/classes/members/privateNames/privateNameNotAllowedOutsideClass.ts(1,7): error TS1134: Variable declaration expected.
-tests/cases/conformance/classes/members/privateNames/privateNameNotAllowedOutsideClass.ts(1,12): error TS1134: Variable declaration expected.
-tests/cases/conformance/classes/members/privateNames/privateNameNotAllowedOutsideClass.ts(1,14): error TS1134: Variable declaration expected.
+tests/cases/conformance/classes/members/privateNames/privateNameNotAllowedOutsideClass.ts(1,7): error TS18029: Private identifiers are not allowed in variable declarations.
 
 
-==== tests/cases/conformance/classes/members/privateNames/privateNameNotAllowedOutsideClass.ts (3 errors) ====
+==== tests/cases/conformance/classes/members/privateNames/privateNameNotAllowedOutsideClass.ts (1 errors) ====
     const #foo = 3;
           ~~~~
-!!! error TS1134: Variable declaration expected.
-               ~
-!!! error TS1134: Variable declaration expected.
-                 ~
-!!! error TS1134: Variable declaration expected.
+!!! error TS18029: Private identifiers are not allowed in variable declarations.
 

tests/baselines/reference/privateNameNotAllowedOutsideClass.js

@@ -4,5 +4,4 @@ const #foo = 3;
 
 //// [privateNameNotAllowedOutsideClass.js]
 "use strict";
-const ;
-3;
+const #foo = 3;

tests/baselines/reference/privateNameNotAllowedOutsideClass.symbols

@@ -1,4 +1,4 @@
 === tests/cases/conformance/classes/members/privateNames/privateNameNotAllowedOutsideClass.ts ===
 const #foo = 3;
-No type information for this code.
-No type information for this code.
+>#foo : Symbol(#foo, Decl(privateNameNotAllowedOutsideClass.ts, 0, 5))
+

tests/baselines/reference/privateNameNotAllowedOutsideClass.types

@@ -1,4 +1,5 @@
 === tests/cases/conformance/classes/members/privateNames/privateNameNotAllowedOutsideClass.ts ===
 const #foo = 3;
+>#foo : 3
 >3 : 3
 

tests/baselines/reference/privateNamesAndIndexedAccess.errors.txt

@@ -1,14 +1,12 @@
 tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(7,28): error TS1005: ']' expected.
-tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(7,28): error TS2300: Duplicate identifier '#bar'.
-tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(7,28): error TS2717: Subsequent property declarations must have the same type.  Property '#bar' must be of type 'number', but here has type 'any'.
-tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(7,32): error TS1005: ';' expected.
-tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(7,34): error TS1068: Unexpected token. A constructor, method, accessor, or property was expected.
-tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(7,36): error TS7008: Member '3' implicitly has an 'any' type.
+tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(7,28): error TS7005: Variable '#bar' implicitly has an 'any' type.
+tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(7,32): error TS1005: ',' expected.
+tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(7,34): error TS1134: Variable declaration expected.
+tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(7,36): error TS1134: Variable declaration expected.
 tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(9,28): error TS2339: Property '#bar' does not exist on type 'C'.
-tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts(11,1): error TS1128: Declaration or statement expected.
 
 
-==== tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts (8 errors) ====
+==== tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts (6 errors) ====
     class C {
         foo = 3;
         #bar = 3;
@@ -18,24 +16,18 @@ tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAcces
             const badForNow: C[#bar] = 3;   // Error
                                ~~~~
 !!! error TS1005: ']' expected.
-!!! related TS1007 tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts:4:20: The parser expected to find a '}' to match the '{' token here.
                                ~~~~
-!!! error TS2300: Duplicate identifier '#bar'.
-                               ~~~~
-!!! error TS2717: Subsequent property declarations must have the same type.  Property '#bar' must be of type 'number', but here has type 'any'.
-!!! related TS6203 tests/cases/conformance/classes/members/privateNames/privateNamesAndIndexedAccess.ts:3:5: '#bar' was also declared here.
+!!! error TS7005: Variable '#bar' implicitly has an 'any' type.
                                    ~
-!!! error TS1005: ';' expected.
+!!! error TS1005: ',' expected.
                                      ~
-!!! error TS1068: Unexpected token. A constructor, method, accessor, or property was expected.
+!!! error TS1134: Variable declaration expected.
                                        ~
-!!! error TS7008: Member '3' implicitly has an 'any' type.
+!!! error TS1134: Variable declaration expected.
             // will never use this syntax, already taken:
             const badAlways: C["#bar"] = 3; // Error
                                ~~~~~~
 !!! error TS2339: Property '#bar' does not exist on type 'C'.
         }
     }
-    ~
-!!! error TS1128: Declaration or statement expected.
 

tests/baselines/reference/privateNamesAndIndexedAccess.js

@@ -14,17 +14,17 @@ class C {
 
 //// [privateNamesAndIndexedAccess.js]
 "use strict";
-var _bar, _bar_1;
+var _bar;
 class C {
     constructor() {
         this.foo = 3;
-        _bar_1.set(this, 3);
-        _bar_1.set(this, void 0);
-        // will never use this syntax, already taken:
-        this.badAlways = 3; // Error
+        _bar.set(this, 3);
         const ok = 3;
         // not supported yet, could support in future:
-        const badForNow;
+        const badForNow, #bar;
+        3; // Error
+        // will never use this syntax, already taken:
+        const badAlways = 3; // Error
     }
 }
-_bar = new WeakMap(), _bar_1 = new WeakMap();
+_bar = new WeakMap();

tests/baselines/reference/privateNamesAndIndexedAccess.symbols

@@ -6,7 +6,7 @@ class C {
 >foo : Symbol(C.foo, Decl(privateNamesAndIndexedAccess.ts, 0, 9))
 
     #bar = 3;
->#bar : Symbol(C.#bar, Decl(privateNamesAndIndexedAccess.ts, 1, 12), Decl(privateNamesAndIndexedAccess.ts, 6, 27))
+>#bar : Symbol(C.#bar, Decl(privateNamesAndIndexedAccess.ts, 1, 12))
 
     constructor () {
         const ok: C["foo"] = 3;
@@ -17,12 +17,11 @@ class C {
         const badForNow: C[#bar] = 3;   // Error
 >badForNow : Symbol(badForNow, Decl(privateNamesAndIndexedAccess.ts, 6, 13))
 >C : Symbol(C, Decl(privateNamesAndIndexedAccess.ts, 0, 0))
->#bar : Symbol(C.#bar, Decl(privateNamesAndIndexedAccess.ts, 1, 12), Decl(privateNamesAndIndexedAccess.ts, 6, 27))
->3 : Symbol(C[3], Decl(privateNamesAndIndexedAccess.ts, 6, 34))
+>#bar : Symbol(#bar, Decl(privateNamesAndIndexedAccess.ts, 6, 27))
 
         // will never use this syntax, already taken:
         const badAlways: C["#bar"] = 3; // Error
->badAlways : Symbol(C.badAlways, Decl(privateNamesAndIndexedAccess.ts, 6, 37))
+>badAlways : Symbol(badAlways, Decl(privateNamesAndIndexedAccess.ts, 8, 13))
 >C : Symbol(C, Decl(privateNamesAndIndexedAccess.ts, 0, 0))
     }
 }

tests/baselines/reference/privateNamesAndIndexedAccess.types

@@ -18,8 +18,8 @@ class C {
         // not supported yet, could support in future:
         const badForNow: C[#bar] = 3;   // Error
 >badForNow : C[]
->#bar : number
->3 : any
+>#bar : any
+>3 : 3
 
         // will never use this syntax, already taken:
         const badAlways: C["#bar"] = 3; // Error