From 6afd94594eb3f9cdc22ecc3d2a7b05b5c273b295 Mon Sep 17 00:00:00 2001
From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Date: Thu, 19 Dec 2019 11:30:43 -0800
Subject: [PATCH 1/2] Ignore @private on constructor functions

This was incorrect in the best of circumstances and caused a crash when
the parent of the function had no symbol, because the accessibility
check assumed it was operating on a constructor and that the parent was
always the containing class.
---
 src/compiler/checker.ts                       |  4 +--
 .../reference/privateConstructorFunction.js   | 31 +++++++++++++++++++
 .../privateConstructorFunction.symbols        | 16 ++++++++++
 .../privateConstructorFunction.types          | 21 +++++++++++++
 .../salsa/privateConstructorFunction.ts       | 15 +++++++++
 5 files changed, 85 insertions(+), 2 deletions(-)
 create mode 100644 tests/baselines/reference/privateConstructorFunction.js
 create mode 100644 tests/baselines/reference/privateConstructorFunction.symbols
 create mode 100644 tests/baselines/reference/privateConstructorFunction.types
 create mode 100644 tests/cases/conformance/salsa/privateConstructorFunction.ts

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 4095b0b4b5c7a..da5a059036cdc 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -24865,8 +24865,8 @@ namespace ts {
             const declaration = signature.declaration;
             const modifiers = getSelectedModifierFlags(declaration, ModifierFlags.NonPublicAccessibilityModifier);
 
-            // Public constructor is accessible.
-            if (!modifiers) {
+            // (1) Public constructors and (2) constructor functions are always accessible.
+            if (!modifiers || isFunctionLikeDeclaration(declaration)) {
                 return true;
             }
 
diff --git a/tests/baselines/reference/privateConstructorFunction.js b/tests/baselines/reference/privateConstructorFunction.js
new file mode 100644
index 0000000000000..bc1b84479e779
--- /dev/null
+++ b/tests/baselines/reference/privateConstructorFunction.js
@@ -0,0 +1,31 @@
+//// [privateConstructorFunction.js]
+{
+    // make sure not to crash when parent's a block rather than a source file or some other
+    // symbol-having node.
+
+    /** @private */
+    function C() {
+        this.x = 1
+    }
+    new C()
+}
+
+
+//// [privateConstructorFunction.js]
+{
+    // make sure not to crash when parent's a block rather than a source file or some other
+    // symbol-having node.
+    /** @private */
+    function C() {
+        this.x = 1;
+    }
+    new C();
+}
+
+
+//// [privateConstructorFunction.d.ts]
+/** @private */
+declare function C(): void;
+declare class C {
+    x: number;
+}
diff --git a/tests/baselines/reference/privateConstructorFunction.symbols b/tests/baselines/reference/privateConstructorFunction.symbols
new file mode 100644
index 0000000000000..32e8406918dd6
--- /dev/null
+++ b/tests/baselines/reference/privateConstructorFunction.symbols
@@ -0,0 +1,16 @@
+=== tests/cases/conformance/salsa/privateConstructorFunction.js ===
+{
+    // make sure not to crash when parent's a block rather than a source file or some other
+    // symbol-having node.
+
+    /** @private */
+    function C() {
+>C : Symbol(C, Decl(privateConstructorFunction.js, 0, 1))
+
+        this.x = 1
+>x : Symbol(C.x, Decl(privateConstructorFunction.js, 5, 18))
+    }
+    new C()
+>C : Symbol(C, Decl(privateConstructorFunction.js, 0, 1))
+}
+
diff --git a/tests/baselines/reference/privateConstructorFunction.types b/tests/baselines/reference/privateConstructorFunction.types
new file mode 100644
index 0000000000000..19fae01d8eb84
--- /dev/null
+++ b/tests/baselines/reference/privateConstructorFunction.types
@@ -0,0 +1,21 @@
+=== tests/cases/conformance/salsa/privateConstructorFunction.js ===
+{
+    // make sure not to crash when parent's a block rather than a source file or some other
+    // symbol-having node.
+
+    /** @private */
+    function C() {
+>C : typeof C
+
+        this.x = 1
+>this.x = 1 : 1
+>this.x : any
+>this : any
+>x : any
+>1 : 1
+    }
+    new C()
+>new C() : C
+>C : typeof C
+}
+
diff --git a/tests/cases/conformance/salsa/privateConstructorFunction.ts b/tests/cases/conformance/salsa/privateConstructorFunction.ts
new file mode 100644
index 0000000000000..5b2d244e37de0
--- /dev/null
+++ b/tests/cases/conformance/salsa/privateConstructorFunction.ts
@@ -0,0 +1,15 @@
+// @allowjs: true
+// @checkjs: true
+// @outdir: salsa
+// @declaration: true
+// @filename: privateConstructorFunction.js
+{
+    // make sure not to crash when parent's a block rather than a source file or some other
+    // symbol-having node.
+
+    /** @private */
+    function C() {
+        this.x = 1
+    }
+    new C()
+}

From 89814ac382bf99917d1ba8a5285a082cefbb6b01 Mon Sep 17 00:00:00 2001
From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Date: Thu, 19 Dec 2019 14:34:55 -0800
Subject: [PATCH 2/2] Non-constructors are always accessible

Previously, all function-like kinds were accessible, which includes
constructors. This was wrong.
---
 src/compiler/checker.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index da5a059036cdc..80ad73a4b8d8a 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -24866,7 +24866,7 @@ namespace ts {
             const modifiers = getSelectedModifierFlags(declaration, ModifierFlags.NonPublicAccessibilityModifier);
 
             // (1) Public constructors and (2) constructor functions are always accessible.
-            if (!modifiers || isFunctionLikeDeclaration(declaration)) {
+            if (!modifiers || declaration.kind !== SyntaxKind.Constructor) {
                 return true;
             }