raise syntax error on declaration of private method, and add syntax tests for private fields

Author: erik-krogh

javascript/extractor/src/com/semmle/jcorn/Parser.java

@@ -3238,6 +3238,9 @@ protected MemberDefinition<?> parseClassPropertyBody(
       if (pi.kind.equals("set") && node.getValue().hasRest())
         this.raiseRecoverable(params.get(params.size() - 1), "Setter cannot use rest params");
     }
+    if (pi.key instanceof Identifier && ((Identifier)pi.key).getName().startsWith("#")) {
+      raiseRecoverable(pi.key, "Only fields, not methods, can be declared private.");
+    }
     return node;
   }
 

javascript/ql/test/library-tests/Classes/privateFields.js

@@ -19,4 +19,9 @@ class Foo {
 	#privSecond; // is a PropNode, not a PropRef. Doesn't matter.
 
 	["#publicField"] = 6;
+	
+	calls() {
+		this.#privDecl();
+		new this.#privDecl();
+	}
 }

javascript/ql/test/library-tests/Classes/tests.expected

@@ -23,7 +23,7 @@ test_ClassDefinitions
 | fields.js:1:1:4:1 | class C ...  = 42\\n} |
 | points.js:1:1:18:1 | class P ... ;\\n  }\\n} |
 | points.js:20:1:33:1 | class C ... ;\\n  }\\n} |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} |
 | staticConstructor.js:1:1:3:1 | class M ... r"; }\\n} |
 | tst.js:1:9:4:1 | class { ...  */ }\\n} |
 | tst.js:6:1:8:1 | class B ... t); }\\n} |
@@ -43,7 +43,7 @@ test_ClassDefinition_getName
 | fields.js:1:1:4:1 | class C ...  = 42\\n} | C |
 | points.js:1:1:18:1 | class P ... ;\\n  }\\n} | Point |
 | points.js:20:1:33:1 | class C ... ;\\n  }\\n} | ColouredPoint |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | Foo |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | Foo |
 | staticConstructor.js:1:1:3:1 | class M ... r"; }\\n} | MyClass |
 | tst.js:1:9:4:1 | class { ...  */ }\\n} | A |
 | tst.js:6:1:8:1 | class B ... t); }\\n} | B |
@@ -60,10 +60,11 @@ test_MethodDefinitions
 | points.js:21:3:24:3 | constru ...  c;\\n  } | points.js:21:3:21:13 | constructor | points.js:21:14:24:3 | (x, y,  ...  c;\\n  } | points.js:20:1:33:1 | class C ... ;\\n  }\\n} |
 | points.js:26:3:28:3 | toStrin ... ur;\\n  } | points.js:26:3:26:10 | toString | points.js:26:11:28:3 | () {\\n   ... ur;\\n  } | points.js:20:1:33:1 | class C ... ;\\n  }\\n} |
 | points.js:30:3:32:3 | static  ... t";\\n  } | points.js:30:10:30:18 | className | points.js:30:19:32:3 | () {\\n   ... t";\\n  } | points.js:20:1:33:1 | class C ... ;\\n  }\\n} |
-| privateFields.js:1:11:1:10 | constructor() {} | privateFields.js:1:11:1:10 | constructor | privateFields.js:1:11:1:10 | () {} | privateFields.js:1:1:22:1 | class F ...  = 6;\\n} |
-| privateFields.js:4:2:8:2 | reads() ... #if;\\n\\t} | privateFields.js:4:2:4:6 | reads | privateFields.js:4:7:8:2 | () {\\n\\t\\t ... #if;\\n\\t} | privateFields.js:1:1:22:1 | class F ...  = 6;\\n} |
-| privateFields.js:10:2:12:2 | equals( ... ecl;\\n\\t} | privateFields.js:10:2:10:7 | equals | privateFields.js:10:8:12:2 | (o) {\\n\\t ... ecl;\\n\\t} | privateFields.js:1:1:22:1 | class F ...  = 6;\\n} |
-| privateFields.js:14:2:17:2 | writes( ... = 5;\\n\\t} | privateFields.js:14:2:14:7 | writes | privateFields.js:14:8:17:2 | () {\\n\\t\\t ... = 5;\\n\\t} | privateFields.js:1:1:22:1 | class F ...  = 6;\\n} |
+| privateFields.js:1:11:1:10 | constructor() {} | privateFields.js:1:11:1:10 | constructor | privateFields.js:1:11:1:10 | () {} | privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} |
+| privateFields.js:4:2:8:2 | reads() ... #if;\\n\\t} | privateFields.js:4:2:4:6 | reads | privateFields.js:4:7:8:2 | () {\\n\\t\\t ... #if;\\n\\t} | privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} |
+| privateFields.js:10:2:12:2 | equals( ... ecl;\\n\\t} | privateFields.js:10:2:10:7 | equals | privateFields.js:10:8:12:2 | (o) {\\n\\t ... ecl;\\n\\t} | privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} |
+| privateFields.js:14:2:17:2 | writes( ... = 5;\\n\\t} | privateFields.js:14:2:14:7 | writes | privateFields.js:14:8:17:2 | () {\\n\\t\\t ... = 5;\\n\\t} | privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} |
+| privateFields.js:23:2:26:2 | calls() ... l();\\n\\t} | privateFields.js:23:2:23:6 | calls | privateFields.js:23:7:26:2 | () {\\n\\t\\t ... l();\\n\\t} | privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} |
 | staticConstructor.js:1:15:1:14 | constructor() {} | staticConstructor.js:1:15:1:14 | constructor | staticConstructor.js:1:15:1:14 | () {} | staticConstructor.js:1:1:3:1 | class M ... r"; }\\n} |
 | staticConstructor.js:2:3:2:59 | static  ... tor"; } | staticConstructor.js:2:10:2:20 | constructor | staticConstructor.js:2:21:2:59 | () { re ... tor"; } | staticConstructor.js:1:1:3:1 | class M ... r"; }\\n} |
 | tst.js:2:3:2:50 | "constr ... r. */ } | tst.js:2:3:2:15 | "constructor" | tst.js:2:16:2:50 | () { /* ... r. */ } | tst.js:1:9:4:1 | class { ...  */ }\\n} |
@@ -87,14 +88,15 @@ test_getAMember
 | points.js:20:1:33:1 | class C ... ;\\n  }\\n} | points.js:21:3:24:3 | constru ...  c;\\n  } |
 | points.js:20:1:33:1 | class C ... ;\\n  }\\n} | points.js:26:3:28:3 | toStrin ... ur;\\n  } |
 | points.js:20:1:33:1 | class C ... ;\\n  }\\n} | points.js:30:3:32:3 | static  ... t";\\n  } |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | privateFields.js:1:11:1:10 | constructor() {} |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | privateFields.js:2:2:2:15 | #privDecl = 3; |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | privateFields.js:3:2:3:12 | #if = "if"; |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | privateFields.js:4:2:8:2 | reads() ... #if;\\n\\t} |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | privateFields.js:10:2:12:2 | equals( ... ecl;\\n\\t} |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | privateFields.js:14:2:17:2 | writes( ... = 5;\\n\\t} |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | privateFields.js:19:2:19:13 | #privSecond; |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | privateFields.js:21:2:21:22 | ["#publ ... "] = 6; |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | privateFields.js:1:11:1:10 | constructor() {} |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | privateFields.js:2:2:2:15 | #privDecl = 3; |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | privateFields.js:3:2:3:12 | #if = "if"; |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | privateFields.js:4:2:8:2 | reads() ... #if;\\n\\t} |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | privateFields.js:10:2:12:2 | equals( ... ecl;\\n\\t} |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | privateFields.js:14:2:17:2 | writes( ... = 5;\\n\\t} |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | privateFields.js:19:2:19:13 | #privSecond; |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | privateFields.js:21:2:21:22 | ["#publ ... "] = 6; |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | privateFields.js:23:2:26:2 | calls() ... l();\\n\\t} |
 | staticConstructor.js:1:1:3:1 | class M ... r"; }\\n} | staticConstructor.js:1:15:1:14 | constructor() {} |
 | staticConstructor.js:1:1:3:1 | class M ... r"; }\\n} | staticConstructor.js:2:3:2:59 | static  ... tor"; } |
 | tst.js:1:9:4:1 | class { ...  */ }\\n} | tst.js:2:3:2:50 | "constr ... r. */ } |
@@ -119,6 +121,7 @@ test_MethodNames
 | privateFields.js:4:2:8:2 | reads() ... #if;\\n\\t} | reads |
 | privateFields.js:10:2:12:2 | equals( ... ecl;\\n\\t} | equals |
 | privateFields.js:14:2:17:2 | writes( ... = 5;\\n\\t} | writes |
+| privateFields.js:23:2:26:2 | calls() ... l();\\n\\t} | calls |
 | staticConstructor.js:1:15:1:14 | constructor() {} | constructor |
 | staticConstructor.js:2:3:2:59 | static  ... tor"; } | constructor |
 | tst.js:2:3:2:50 | "constr ... r. */ } | constructor |
@@ -153,7 +156,7 @@ test_ClassNodeConstructor
 | fields.js:1:1:4:1 | class C ...  = 42\\n} | fields.js:1:9:1:8 | () {} |
 | points.js:1:1:18:1 | class P ... ;\\n  }\\n} | points.js:2:14:5:3 | (x, y)  ...  y;\\n  } |
 | points.js:20:1:33:1 | class C ... ;\\n  }\\n} | points.js:21:14:24:3 | (x, y,  ...  c;\\n  } |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | privateFields.js:1:11:1:10 | () {} |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | privateFields.js:1:11:1:10 | () {} |
 | staticConstructor.js:1:1:3:1 | class M ... r"; }\\n} | staticConstructor.js:1:15:1:14 | () {} |
 | tst.js:1:9:4:1 | class { ...  */ }\\n} | tst.js:2:16:2:50 | () { /* ... r. */ } |
 | tst.js:6:1:8:1 | class B ... t); }\\n} | tst.js:7:14:7:38 | () { su ... get); } |
@@ -163,9 +166,10 @@ test_ClassNodeInstanceMethod
 | dataflow.js:4:2:13:2 | class F ... \\n\\t\\t}\\n\\t} | getPriv | dataflow.js:6:10:8:3 | () {\\n\\t\\t ... iv;\\n\\t\\t} |
 | points.js:1:1:18:1 | class P ... ;\\n  }\\n} | toString | points.js:11:11:13:3 | () {\\n   ... )";\\n  } |
 | points.js:20:1:33:1 | class C ... ;\\n  }\\n} | toString | points.js:26:11:28:3 | () {\\n   ... ur;\\n  } |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | equals | privateFields.js:10:8:12:2 | (o) {\\n\\t ... ecl;\\n\\t} |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | reads | privateFields.js:4:7:8:2 | () {\\n\\t\\t ... #if;\\n\\t} |
-| privateFields.js:1:1:22:1 | class F ...  = 6;\\n} | writes | privateFields.js:14:8:17:2 | () {\\n\\t\\t ... = 5;\\n\\t} |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | calls | privateFields.js:23:7:26:2 | () {\\n\\t\\t ... l();\\n\\t} |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | equals | privateFields.js:10:8:12:2 | (o) {\\n\\t ... ecl;\\n\\t} |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | reads | privateFields.js:4:7:8:2 | () {\\n\\t\\t ... #if;\\n\\t} |
+| privateFields.js:1:1:27:1 | class F ... );\\n\\t}\\n} | writes | privateFields.js:14:8:17:2 | () {\\n\\t\\t ... = 5;\\n\\t} |
 | tst.js:1:9:4:1 | class { ...  */ }\\n} | constructor | tst.js:3:18:3:56 | () { /* ... r. */ } |
 | tst.js:11:1:14:1 | class C ... () {}\\n} | m | tst.js:12:4:12:8 | () {} |
 getAccessModifier
@@ -212,6 +216,9 @@ getAccessModifier
 | privateFields.js:15:3:15:16 | this.#privDecl | privateFields.js:15:8:15:16 | #privDecl | Private |
 | privateFields.js:16:3:16:17 | this["#public"] | privateFields.js:16:8:16:16 | "#public" | Public |
 | privateFields.js:21:2:21:22 | ["#publ ... "] = 6; | privateFields.js:21:3:21:16 | "#publicField" | Public |
+| privateFields.js:23:2:26:2 | calls() ... l();\\n\\t} | privateFields.js:23:2:23:6 | calls | Public |
+| privateFields.js:24:3:24:16 | this.#privDecl | privateFields.js:24:8:24:16 | #privDecl | Private |
+| privateFields.js:25:7:25:20 | this.#privDecl | privateFields.js:25:12:25:20 | #privDecl | Private |
 | staticConstructor.js:1:15:1:14 | constructor() {} | staticConstructor.js:1:15:1:14 | constructor | Public |
 | staticConstructor.js:2:3:2:59 | static  ... tor"; } | staticConstructor.js:2:10:2:20 | constructor | Public |
 | staticConstructor.js:4:1:4:11 | console.log | staticConstructor.js:4:9:4:11 | log | Public |

javascript/ql/test/query-tests/LanguageFeatures/SyntaxError/SyntaxError.expected

@@ -1,2 +1,4 @@
 | arrows.js:1:5:1:5 | Error: Argument name clash | Error: Argument name clash |
+| destructingPrivate.js:3:6:3:6 | Error: Unexpected token | Error: Unexpected token |
+| privateMethod.js:2:3:2:3 | Error: Only fields, not methods, can be declared private. | Error: Only fields, not methods, can be declared private. |
 | tst.js:2:12:2:12 | Error: Unterminated string constant | Error: Unterminated string constant |

javascript/ql/test/query-tests/LanguageFeatures/SyntaxError/destructingPrivate.js

@@ -0,0 +1,5 @@
+class C {
+  bar() {
+    {#privDecl} = this;
+  }
+}

javascript/ql/test/query-tests/LanguageFeatures/SyntaxError/privateMethod.js

@@ -0,0 +1,3 @@
+class C {
+  #privateMethod() {}
+}