Merge pull request #5559 from MartyIX/issue-4045

Improve error messages for property declarations

Author: DanielRosenwasser

src/compiler/checker.ts

@@ -15492,6 +15492,11 @@ namespace ts {
             let flags = 0;
             for (const modifier of node.modifiers) {
                 switch (modifier.kind) {
+                    case SyntaxKind.ConstKeyword:
+                        if (node.kind !== SyntaxKind.EnumDeclaration && node.parent.kind === SyntaxKind.ClassDeclaration) {
+                            return grammarErrorOnNode(node, Diagnostics.A_class_member_cannot_have_the_0_keyword, tokenToString(SyntaxKind.ConstKeyword));
+                        }
+                        break;
                     case SyntaxKind.PublicKeyword:
                     case SyntaxKind.ProtectedKeyword:
                     case SyntaxKind.PrivateKeyword:

src/compiler/diagnosticMessages.json

@@ -791,7 +791,10 @@
         "category": "Error",
         "code": 1247
     },
-
+    "A class member cannot have the '{0}' keyword.": {
+        "category": "Error",
+        "code": 1248
+    },
     "'with' statements are not allowed in an async function block.": {
         "category": "Error",
         "code": 1300

src/compiler/parser.ts

@@ -1134,6 +1134,14 @@ namespace ts {
             return token === t && tryParse(nextTokenCanFollowModifier);
         }
 
+        function nextTokenIsOnSameLineAndCanFollowModifier() {
+            nextToken();
+            if (scanner.hasPrecedingLineBreak()) {
+                return false;
+            }
+            return canFollowModifier();
+        }
+
         function nextTokenCanFollowModifier() {
             if (token === SyntaxKind.ConstKeyword) {
                 // 'const' is only a modifier if followed by 'enum'.
@@ -1154,11 +1162,7 @@ namespace ts {
                 return canFollowModifier();
             }
 
-            nextToken();
-            if (scanner.hasPrecedingLineBreak()) {
-                return false;
-            }
-            return canFollowModifier();
+            return nextTokenIsOnSameLineAndCanFollowModifier();
         }
 
         function parseAnyContextualModifier(): boolean {
@@ -4923,15 +4927,31 @@ namespace ts {
             return decorators;
         }
 
-        function parseModifiers(): ModifiersArray {
+        /*
+         * There are situations in which a modifier like 'const' will appear unexpectedly, such as on a class member.
+         * In those situations, if we are entirely sure that 'const' is not valid on its own (such as when ASI takes effect
+         * and turns it into a standalone declaration), then it is better to parse it and report an error later.
+         *
+         * In such situations, 'permitInvalidConstAsModifier' should be set to true.
+         */
+        function parseModifiers(permitInvalidConstAsModifier?: boolean): ModifiersArray {
             let flags = 0;
             let modifiers: ModifiersArray;
             while (true) {
                 const modifierStart = scanner.getStartPos();
                 const modifierKind = token;
 
-                if (!parseAnyContextualModifier()) {
-                    break;
+                if (token === SyntaxKind.ConstKeyword && permitInvalidConstAsModifier) {
+                    // We need to ensure that any subsequent modifiers appear on the same line
+                    // so that when 'const' is a standalone declaration, we don't issue an error.                
+                    if (!tryParse(nextTokenIsOnSameLineAndCanFollowModifier)) {
+                        break;
+                    }
+                }
+                else {
+                    if (!parseAnyContextualModifier()) {
+                        break;
+                    }
                 }
 
                 if (!modifiers) {
@@ -4976,7 +4996,7 @@ namespace ts {
 
             const fullStart = getNodePos();
             const decorators = parseDecorators();
-            const modifiers = parseModifiers();
+            const modifiers = parseModifiers(/*permitInvalidConstAsModifier*/ true);
 
             const accessor = tryParseAccessorDeclaration(fullStart, decorators, modifiers);
             if (accessor) {

tests/baselines/reference/ClassDeclaration26.errors.txt

@@ -0,0 +1,23 @@
+tests/cases/compiler/ClassDeclaration26.ts(2,22): error TS1005: ';' expected.
+tests/cases/compiler/ClassDeclaration26.ts(4,5): error TS1068: Unexpected token. A constructor, method, accessor, or property was expected.
+tests/cases/compiler/ClassDeclaration26.ts(4,20): error TS1005: '=' expected.
+tests/cases/compiler/ClassDeclaration26.ts(4,23): error TS1005: '=>' expected.
+tests/cases/compiler/ClassDeclaration26.ts(5,1): error TS1128: Declaration or statement expected.
+
+
+==== tests/cases/compiler/ClassDeclaration26.ts (5 errors) ====
+    class C {
+        public const var export foo = 10;
+                         ~~~~~~
+!!! error TS1005: ';' expected.
+    
+        var constructor() { }
+        ~~~
+!!! error TS1068: Unexpected token. A constructor, method, accessor, or property was expected.
+                       ~
+!!! error TS1005: '=' expected.
+                          ~
+!!! error TS1005: '=>' expected.
+    }
+    ~
+!!! error TS1128: Declaration or statement expected.

tests/baselines/reference/ClassDeclaration26.js

@@ -0,0 +1,15 @@
+//// [ClassDeclaration26.ts]
+class C {
+    public const var export foo = 10;
+
+    var constructor() { }
+}
+
+//// [ClassDeclaration26.js]
+var C = (function () {
+    function C() {
+        this.foo = 10;
+    }
+    return C;
+})();
+var constructor = function () { };

tests/baselines/reference/ClassDeclarationWithInvalidConstOnPropertyDeclaration.errors.txt

@@ -0,0 +1,9 @@
+tests/cases/compiler/ClassDeclarationWithInvalidConstOnPropertyDeclaration.ts(2,3): error TS1248: A class member cannot have the 'const' keyword.
+
+
+==== tests/cases/compiler/ClassDeclarationWithInvalidConstOnPropertyDeclaration.ts (1 errors) ====
+    class AtomicNumbers {
+      static const H = 1;
+      ~~~~~~~~~~~~~~~~~~~
+!!! error TS1248: A class member cannot have the 'const' keyword.
+    }

tests/baselines/reference/ClassDeclarationWithInvalidConstOnPropertyDeclaration.js

@@ -0,0 +1,12 @@
+//// [ClassDeclarationWithInvalidConstOnPropertyDeclaration.ts]
+class AtomicNumbers {
+  static const H = 1;
+}
+
+//// [ClassDeclarationWithInvalidConstOnPropertyDeclaration.js]
+var AtomicNumbers = (function () {
+    function AtomicNumbers() {
+    }
+    AtomicNumbers.H = 1;
+    return AtomicNumbers;
+})();

tests/baselines/reference/ClassDeclarationWithInvalidConstOnPropertyDeclaration2.js

@@ -0,0 +1,13 @@
+//// [ClassDeclarationWithInvalidConstOnPropertyDeclaration2.ts]
+class C {
+    const
+    x = 10;
+}
+
+//// [ClassDeclarationWithInvalidConstOnPropertyDeclaration2.js]
+var C = (function () {
+    function C() {
+        this.x = 10;
+    }
+    return C;
+})();

tests/baselines/reference/ClassDeclarationWithInvalidConstOnPropertyDeclaration2.symbols

@@ -0,0 +1,10 @@
+=== tests/cases/compiler/ClassDeclarationWithInvalidConstOnPropertyDeclaration2.ts ===
+class C {
+>C : Symbol(C, Decl(ClassDeclarationWithInvalidConstOnPropertyDeclaration2.ts, 0, 0))
+
+    const
+>const : Symbol(const, Decl(ClassDeclarationWithInvalidConstOnPropertyDeclaration2.ts, 0, 9))
+
+    x = 10;
+>x : Symbol(x, Decl(ClassDeclarationWithInvalidConstOnPropertyDeclaration2.ts, 1, 9))
+}

tests/baselines/reference/ClassDeclarationWithInvalidConstOnPropertyDeclaration2.types

@@ -0,0 +1,11 @@
+=== tests/cases/compiler/ClassDeclarationWithInvalidConstOnPropertyDeclaration2.ts ===
+class C {
+>C : C
+
+    const
+>const : any
+
+    x = 10;
+>x : number
+>10 : number
+}

tests/cases/compiler/ClassDeclaration26.ts

@@ -0,0 +1,5 @@
+class C {
+    public const var export foo = 10;
+
+    var constructor() { }
+}

tests/cases/compiler/ClassDeclarationWithInvalidConstOnPropertyDeclaration.ts

@@ -0,0 +1,3 @@
+class AtomicNumbers {
+  static const H = 1;
+}

tests/cases/compiler/ClassDeclarationWithInvalidConstOnPropertyDeclaration2.ts

@@ -0,0 +1,4 @@
+class C {
+    const
+    x = 10;
+}

tests/cases/fourslash/getOccurrencesConst04.ts

@@ -1,12 +1,14 @@
 /// <reference path='fourslash.ts' />
 
 ////export const class C {
-////    private static c/*1*/onst foo;
-////    constructor(public con/*2*/st foo) {
+////    private static c/*1*/onst f/*2*/oo;
+////    constructor(public con/*3*/st foo) {
 ////    }
 ////}
 
 goTo.marker("1");
-verify.occurrencesAtPositionCount(1);
+verify.occurrencesAtPositionCount(0);
 goTo.marker("2");
+verify.occurrencesAtPositionCount(1);
+goTo.marker("3");
 verify.occurrencesAtPositionCount(0);