From 7aa5a80e5eff3f043024793bb386e027e0d4c4ba Mon Sep 17 00:00:00 2001
From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Date: Thu, 5 Dec 2019 15:58:20 -0800
Subject: [PATCH 1/3] Require all parameters in JS

This is an experiment in fixing #31888. This is the simplest experiment,
which requires all parameters to be provided in JS, same as TS. If this
doesn't break the user tests that are written in JS too badly, then I
won't proceed to the more complex experiments, which are to require all
paremeters for contextually typed functions, or to require all
parameters when `"noImplicitAny": true`.
---
 src/compiler/checker.ts                       |  9 +----
 ...argumentsObjectCreatesRestForJs.errors.txt | 24 +++++++++++++
 .../argumentsObjectCreatesRestForJs.symbols   |  2 +-
 .../argumentsObjectCreatesRestForJs.types     |  2 +-
 ...ileFunctionParametersAsOptional.errors.txt | 24 +++++++++++++
 .../typeFromPropertyAssignment17.errors.txt   | 35 +++++++++++++++++++
 .../typeFromPropertyAssignment7.errors.txt    | 13 +++++++
 .../argumentsObjectCreatesRestForJs.ts        |  2 +-
 .../signatureHelpCallExpressionJs.ts          | 16 ++++-----
 9 files changed, 108 insertions(+), 19 deletions(-)
 create mode 100644 tests/baselines/reference/argumentsObjectCreatesRestForJs.errors.txt
 create mode 100644 tests/baselines/reference/jsFileFunctionParametersAsOptional.errors.txt
 create mode 100644 tests/baselines/reference/typeFromPropertyAssignment17.errors.txt
 create mode 100644 tests/baselines/reference/typeFromPropertyAssignment7.errors.txt

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 1a5559794be9e..99a970d696531 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -10094,17 +10094,11 @@ namespace ts {
                 let thisParameter: Symbol | undefined;
                 let hasThisParameter = false;
                 const iife = getImmediatelyInvokedFunctionExpression(declaration);
-                const isJSConstructSignature = isJSDocConstructSignature(declaration);
-                const isUntypedSignatureInJSFile = !iife &&
-                    isInJSFile(declaration) &&
-                    isValueSignatureDeclaration(declaration) &&
-                    !hasJSDocParameterTags(declaration) &&
-                    !getJSDocType(declaration);
 
                 // If this is a JSDoc construct signature, then skip the first parameter in the
                 // parameter list.  The first parameter represents the return type of the construct
                 // signature.
-                for (let i = isJSConstructSignature ? 1 : 0; i < declaration.parameters.length; i++) {
+                for (let i = isJSDocConstructSignature(declaration) ? 1 : 0; i < declaration.parameters.length; i++) {
                     const param = declaration.parameters[i];
 
                     let paramSymbol = param.symbol;
@@ -10130,7 +10124,6 @@ namespace ts {
                     const isOptionalParameter = isOptionalJSDocParameterTag(param) ||
                         param.initializer || param.questionToken || param.dotDotDotToken ||
                         iife && parameters.length > iife.arguments.length && !type ||
-                        isUntypedSignatureInJSFile ||
                         isJSDocOptionalParameter(param);
                     if (!isOptionalParameter) {
                         minArgumentCount = parameters.length;
diff --git a/tests/baselines/reference/argumentsObjectCreatesRestForJs.errors.txt b/tests/baselines/reference/argumentsObjectCreatesRestForJs.errors.txt
new file mode 100644
index 0000000000000..a48b7248ba69d
--- /dev/null
+++ b/tests/baselines/reference/argumentsObjectCreatesRestForJs.errors.txt
@@ -0,0 +1,24 @@
+tests/cases/compiler/main.js(5,1): error TS2555: Expected at least 2 arguments, but got 0.
+
+
+==== tests/cases/compiler/main.js (1 errors) ====
+    function allRest() { arguments; }
+    allRest();
+    allRest(1, 2, 3);
+    function someRest(x, y) { arguments; }
+    someRest(); // x and y are still required even though they're in a JS file
+    ~~~~~~~~~~
+!!! error TS2555: Expected at least 2 arguments, but got 0.
+!!! related TS6210 tests/cases/compiler/main.js:4:19: An argument for 'x' was not provided.
+    someRest(1, 2, 3);
+    
+    /**
+     * @param {number} x - a thing
+     */
+    function jsdocced(x) { arguments; }
+    jsdocced(1);
+    
+    function dontDoubleRest(x, ...y) { arguments; }
+    dontDoubleRest(1, 2, 3);
+    
+    
\ No newline at end of file
diff --git a/tests/baselines/reference/argumentsObjectCreatesRestForJs.symbols b/tests/baselines/reference/argumentsObjectCreatesRestForJs.symbols
index 6a95dbc86d82e..661ee3bc7dce5 100644
--- a/tests/baselines/reference/argumentsObjectCreatesRestForJs.symbols
+++ b/tests/baselines/reference/argumentsObjectCreatesRestForJs.symbols
@@ -15,7 +15,7 @@ function someRest(x, y) { arguments; }
 >y : Symbol(y, Decl(main.js, 3, 20))
 >arguments : Symbol(arguments)
 
-someRest(); // x and y are still optional because they are in a JS file
+someRest(); // x and y are still required even though they're in a JS file
 >someRest : Symbol(someRest, Decl(main.js, 2, 17))
 
 someRest(1, 2, 3);
diff --git a/tests/baselines/reference/argumentsObjectCreatesRestForJs.types b/tests/baselines/reference/argumentsObjectCreatesRestForJs.types
index 28cdfcda4132c..e01ee8af8c148 100644
--- a/tests/baselines/reference/argumentsObjectCreatesRestForJs.types
+++ b/tests/baselines/reference/argumentsObjectCreatesRestForJs.types
@@ -20,7 +20,7 @@ function someRest(x, y) { arguments; }
 >y : any
 >arguments : IArguments
 
-someRest(); // x and y are still optional because they are in a JS file
+someRest(); // x and y are still required even though they're in a JS file
 >someRest() : void
 >someRest : (x: any, y: any, ...args: any[]) => void
 
diff --git a/tests/baselines/reference/jsFileFunctionParametersAsOptional.errors.txt b/tests/baselines/reference/jsFileFunctionParametersAsOptional.errors.txt
new file mode 100644
index 0000000000000..966bdbe0e11ff
--- /dev/null
+++ b/tests/baselines/reference/jsFileFunctionParametersAsOptional.errors.txt
@@ -0,0 +1,24 @@
+tests/cases/compiler/bar.ts(1,1): error TS2554: Expected 3 arguments, but got 0.
+tests/cases/compiler/bar.ts(2,1): error TS2554: Expected 3 arguments, but got 1.
+tests/cases/compiler/bar.ts(3,1): error TS2554: Expected 3 arguments, but got 2.
+
+
+==== tests/cases/compiler/foo.js (0 errors) ====
+    function f(a, b, c) { }
+    
+    
+==== tests/cases/compiler/bar.ts (3 errors) ====
+    f();
+    ~~~
+!!! error TS2554: Expected 3 arguments, but got 0.
+!!! related TS6210 tests/cases/compiler/foo.js:1:12: An argument for 'a' was not provided.
+    f(1);
+    ~~~~
+!!! error TS2554: Expected 3 arguments, but got 1.
+!!! related TS6210 tests/cases/compiler/foo.js:1:15: An argument for 'b' was not provided.
+    f(1, 2);
+    ~~~~~~~
+!!! error TS2554: Expected 3 arguments, but got 2.
+!!! related TS6210 tests/cases/compiler/foo.js:1:18: An argument for 'c' was not provided.
+    f(1, 2, 3);
+    
\ No newline at end of file
diff --git a/tests/baselines/reference/typeFromPropertyAssignment17.errors.txt b/tests/baselines/reference/typeFromPropertyAssignment17.errors.txt
new file mode 100644
index 0000000000000..4ad82b4ee077e
--- /dev/null
+++ b/tests/baselines/reference/typeFromPropertyAssignment17.errors.txt
@@ -0,0 +1,35 @@
+tests/cases/conformance/salsa/use.js(3,8): error TS2554: Expected 1 arguments, but got 0.
+
+
+==== tests/cases/conformance/salsa/use.js (1 errors) ====
+    /// <reference path='./types.d.ts'/>
+    var mini = require('./minimatch')
+    mini.M.defaults()
+           ~~~~~~~~~~
+!!! error TS2554: Expected 1 arguments, but got 0.
+!!! related TS6210 /.src/tests/cases/conformance/salsa/minimatch.js:10:24: An argument for 'def' was not provided.
+    var m = new mini.M()
+    m.m()
+    mini.filter()
+    
+==== tests/cases/conformance/salsa/types.d.ts (0 errors) ====
+    declare var require: any;
+    declare var module: any;
+==== tests/cases/conformance/salsa/minimatch.js (0 errors) ====
+    /// <reference path='./types.d.ts'/>
+    module.exports = minimatch
+    minimatch.M = M
+    minimatch.filter = filter
+    function filter() {
+        return minimatch()
+    }
+    function minimatch() {
+    }
+    M.defaults = function (def) {
+        return def
+    }
+    M.prototype.m = function () {
+    }
+    function M() {
+    }
+    
\ No newline at end of file
diff --git a/tests/baselines/reference/typeFromPropertyAssignment7.errors.txt b/tests/baselines/reference/typeFromPropertyAssignment7.errors.txt
new file mode 100644
index 0000000000000..e848fdf8c0dc5
--- /dev/null
+++ b/tests/baselines/reference/typeFromPropertyAssignment7.errors.txt
@@ -0,0 +1,13 @@
+tests/cases/conformance/salsa/a.js(5,13): error TS2554: Expected 1 arguments, but got 0.
+
+
+==== tests/cases/conformance/salsa/a.js (1 errors) ====
+    var obj = {};
+    obj.method = function (hunch) {
+        return true;
+    }
+    var b = obj.method();
+                ~~~~~~~~
+!!! error TS2554: Expected 1 arguments, but got 0.
+!!! related TS6210 tests/cases/conformance/salsa/a.js:2:24: An argument for 'hunch' was not provided.
+    
\ No newline at end of file
diff --git a/tests/cases/compiler/argumentsObjectCreatesRestForJs.ts b/tests/cases/compiler/argumentsObjectCreatesRestForJs.ts
index 4c3ca335d1df1..a49ebaa1a0177 100644
--- a/tests/cases/compiler/argumentsObjectCreatesRestForJs.ts
+++ b/tests/cases/compiler/argumentsObjectCreatesRestForJs.ts
@@ -6,7 +6,7 @@ function allRest() { arguments; }
 allRest();
 allRest(1, 2, 3);
 function someRest(x, y) { arguments; }
-someRest(); // x and y are still optional because they are in a JS file
+someRest(); // x and y are still required even though they're in a JS file
 someRest(1, 2, 3);
 
 /**
diff --git a/tests/cases/fourslash/signatureHelpCallExpressionJs.ts b/tests/cases/fourslash/signatureHelpCallExpressionJs.ts
index 3b19ddb393ac5..ef606e2c0cca7 100644
--- a/tests/cases/fourslash/signatureHelpCallExpressionJs.ts
+++ b/tests/cases/fourslash/signatureHelpCallExpressionJs.ts
@@ -4,15 +4,15 @@
 // @allowJs: true
 
 // @Filename: main.js
-////function allOptional() { arguments; }
-////allOptional(/*1*/);
-////allOptional(1, 2, 3);
-////function someOptional(x, y) { arguments; }
-////someOptional(/*2*/);
-////someOptional(1, 2, 3);
-////someOptional(); // no error here; x and y are optional in JS
+//// function allOptional() { arguments; }
+//// allOptional(/*1*/);
+//// allOptional(1, 2, 3);
+//// function someOptional(x, y) { arguments; }
+//// someOptional(/*2*/);
+//// someOptional(1, 2, 3);
+//// /*missing*/someOptional(); // no error here; x and y are optional in JS
 
-verify.noErrors();
+verify.errorExistsAfterMarker("missing");
 verify.signatureHelp(
     {
         marker: "1",

From 0e0bff880aa1e2e176ba0765fd2986c08fc3f0f3 Mon Sep 17 00:00:00 2001
From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Date: Wed, 18 Dec 2019 10:20:07 -0800
Subject: [PATCH 2/3] Require all parameters for contextually-typed functions
 only

---
 src/compiler/checker.ts                       | 10 +++++-
 ...argumentsObjectCreatesRestForJs.errors.txt | 24 -------------
 ...ileFunctionParametersAsOptional.errors.txt | 24 -------------
 .../typeFromPropertyAssignment17.errors.txt   | 35 -------------------
 .../typeFromPropertyAssignment7.errors.txt    | 13 -------
 5 files changed, 9 insertions(+), 97 deletions(-)
 delete mode 100644 tests/baselines/reference/argumentsObjectCreatesRestForJs.errors.txt
 delete mode 100644 tests/baselines/reference/jsFileFunctionParametersAsOptional.errors.txt
 delete mode 100644 tests/baselines/reference/typeFromPropertyAssignment17.errors.txt
 delete mode 100644 tests/baselines/reference/typeFromPropertyAssignment7.errors.txt

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index eee0bff34d8d2..336d4de7b6e67 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -10099,11 +10099,18 @@ namespace ts {
                 let thisParameter: Symbol | undefined;
                 let hasThisParameter = false;
                 const iife = getImmediatelyInvokedFunctionExpression(declaration);
+                const isJSConstructSignature = isJSDocConstructSignature(declaration);
+                const isUntypedSignatureInJSFile = !iife &&
+                    isInJSFile(declaration) &&
+                    isValueSignatureDeclaration(declaration) &&
+                    !hasJSDocParameterTags(declaration) &&
+                    !getJSDocType(declaration) &&
+                    (!isFunctionExpressionOrArrowFunction(declaration) || !isContextSensitiveFunctionLikeDeclaration(declaration) || !getContextualType(declaration));
 
                 // If this is a JSDoc construct signature, then skip the first parameter in the
                 // parameter list.  The first parameter represents the return type of the construct
                 // signature.
-                for (let i = isJSDocConstructSignature(declaration) ? 1 : 0; i < declaration.parameters.length; i++) {
+                for (let i = isJSConstructSignature ? 1 : 0; i < declaration.parameters.length; i++) {
                     const param = declaration.parameters[i];
 
                     let paramSymbol = param.symbol;
@@ -10129,6 +10136,7 @@ namespace ts {
                     const isOptionalParameter = isOptionalJSDocParameterTag(param) ||
                         param.initializer || param.questionToken || param.dotDotDotToken ||
                         iife && parameters.length > iife.arguments.length && !type ||
+                        isUntypedSignatureInJSFile ||
                         isJSDocOptionalParameter(param);
                     if (!isOptionalParameter) {
                         minArgumentCount = parameters.length;
diff --git a/tests/baselines/reference/argumentsObjectCreatesRestForJs.errors.txt b/tests/baselines/reference/argumentsObjectCreatesRestForJs.errors.txt
deleted file mode 100644
index a48b7248ba69d..0000000000000
--- a/tests/baselines/reference/argumentsObjectCreatesRestForJs.errors.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-tests/cases/compiler/main.js(5,1): error TS2555: Expected at least 2 arguments, but got 0.
-
-
-==== tests/cases/compiler/main.js (1 errors) ====
-    function allRest() { arguments; }
-    allRest();
-    allRest(1, 2, 3);
-    function someRest(x, y) { arguments; }
-    someRest(); // x and y are still required even though they're in a JS file
-    ~~~~~~~~~~
-!!! error TS2555: Expected at least 2 arguments, but got 0.
-!!! related TS6210 tests/cases/compiler/main.js:4:19: An argument for 'x' was not provided.
-    someRest(1, 2, 3);
-    
-    /**
-     * @param {number} x - a thing
-     */
-    function jsdocced(x) { arguments; }
-    jsdocced(1);
-    
-    function dontDoubleRest(x, ...y) { arguments; }
-    dontDoubleRest(1, 2, 3);
-    
-    
\ No newline at end of file
diff --git a/tests/baselines/reference/jsFileFunctionParametersAsOptional.errors.txt b/tests/baselines/reference/jsFileFunctionParametersAsOptional.errors.txt
deleted file mode 100644
index 966bdbe0e11ff..0000000000000
--- a/tests/baselines/reference/jsFileFunctionParametersAsOptional.errors.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-tests/cases/compiler/bar.ts(1,1): error TS2554: Expected 3 arguments, but got 0.
-tests/cases/compiler/bar.ts(2,1): error TS2554: Expected 3 arguments, but got 1.
-tests/cases/compiler/bar.ts(3,1): error TS2554: Expected 3 arguments, but got 2.
-
-
-==== tests/cases/compiler/foo.js (0 errors) ====
-    function f(a, b, c) { }
-    
-    
-==== tests/cases/compiler/bar.ts (3 errors) ====
-    f();
-    ~~~
-!!! error TS2554: Expected 3 arguments, but got 0.
-!!! related TS6210 tests/cases/compiler/foo.js:1:12: An argument for 'a' was not provided.
-    f(1);
-    ~~~~
-!!! error TS2554: Expected 3 arguments, but got 1.
-!!! related TS6210 tests/cases/compiler/foo.js:1:15: An argument for 'b' was not provided.
-    f(1, 2);
-    ~~~~~~~
-!!! error TS2554: Expected 3 arguments, but got 2.
-!!! related TS6210 tests/cases/compiler/foo.js:1:18: An argument for 'c' was not provided.
-    f(1, 2, 3);
-    
\ No newline at end of file
diff --git a/tests/baselines/reference/typeFromPropertyAssignment17.errors.txt b/tests/baselines/reference/typeFromPropertyAssignment17.errors.txt
deleted file mode 100644
index 4ad82b4ee077e..0000000000000
--- a/tests/baselines/reference/typeFromPropertyAssignment17.errors.txt
+++ /dev/null
@@ -1,35 +0,0 @@
-tests/cases/conformance/salsa/use.js(3,8): error TS2554: Expected 1 arguments, but got 0.
-
-
-==== tests/cases/conformance/salsa/use.js (1 errors) ====
-    /// <reference path='./types.d.ts'/>
-    var mini = require('./minimatch')
-    mini.M.defaults()
-           ~~~~~~~~~~
-!!! error TS2554: Expected 1 arguments, but got 0.
-!!! related TS6210 /.src/tests/cases/conformance/salsa/minimatch.js:10:24: An argument for 'def' was not provided.
-    var m = new mini.M()
-    m.m()
-    mini.filter()
-    
-==== tests/cases/conformance/salsa/types.d.ts (0 errors) ====
-    declare var require: any;
-    declare var module: any;
-==== tests/cases/conformance/salsa/minimatch.js (0 errors) ====
-    /// <reference path='./types.d.ts'/>
-    module.exports = minimatch
-    minimatch.M = M
-    minimatch.filter = filter
-    function filter() {
-        return minimatch()
-    }
-    function minimatch() {
-    }
-    M.defaults = function (def) {
-        return def
-    }
-    M.prototype.m = function () {
-    }
-    function M() {
-    }
-    
\ No newline at end of file
diff --git a/tests/baselines/reference/typeFromPropertyAssignment7.errors.txt b/tests/baselines/reference/typeFromPropertyAssignment7.errors.txt
deleted file mode 100644
index e848fdf8c0dc5..0000000000000
--- a/tests/baselines/reference/typeFromPropertyAssignment7.errors.txt
+++ /dev/null
@@ -1,13 +0,0 @@
-tests/cases/conformance/salsa/a.js(5,13): error TS2554: Expected 1 arguments, but got 0.
-
-
-==== tests/cases/conformance/salsa/a.js (1 errors) ====
-    var obj = {};
-    obj.method = function (hunch) {
-        return true;
-    }
-    var b = obj.method();
-                ~~~~~~~~
-!!! error TS2554: Expected 1 arguments, but got 0.
-!!! related TS6210 tests/cases/conformance/salsa/a.js:2:24: An argument for 'hunch' was not provided.
-    
\ No newline at end of file

From a0fc7a7a1f90f8a0d1c0a93bedd15c6aa77888c2 Mon Sep 17 00:00:00 2001
From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Date: Wed, 18 Dec 2019 13:13:04 -0800
Subject: [PATCH 3/3] Don't require all JS params when ctx type is any

---
 src/compiler/checker.ts                               | 11 ++++++++---
 .../cases/fourslash/signatureHelpCallExpressionJs.ts  |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 336d4de7b6e67..92a581f0f22aa 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -10100,12 +10100,17 @@ namespace ts {
                 let hasThisParameter = false;
                 const iife = getImmediatelyInvokedFunctionExpression(declaration);
                 const isJSConstructSignature = isJSDocConstructSignature(declaration);
-                const isUntypedSignatureInJSFile = !iife &&
+                let isUntypedSignatureInJSFile = !iife &&
                     isInJSFile(declaration) &&
                     isValueSignatureDeclaration(declaration) &&
                     !hasJSDocParameterTags(declaration) &&
-                    !getJSDocType(declaration) &&
-                    (!isFunctionExpressionOrArrowFunction(declaration) || !isContextSensitiveFunctionLikeDeclaration(declaration) || !getContextualType(declaration));
+                    !getJSDocType(declaration);
+                if (isUntypedSignatureInJSFile && isFunctionExpressionOrArrowFunction(declaration) && isContextSensitiveFunctionLikeDeclaration(declaration)) {
+                    const t = getContextualType(declaration);
+                    if (t) {
+                        isUntypedSignatureInJSFile = t !== anyType;
+                    }
+                }
 
                 // If this is a JSDoc construct signature, then skip the first parameter in the
                 // parameter list.  The first parameter represents the return type of the construct
diff --git a/tests/cases/fourslash/signatureHelpCallExpressionJs.ts b/tests/cases/fourslash/signatureHelpCallExpressionJs.ts
index ef606e2c0cca7..7329d8d7c053b 100644
--- a/tests/cases/fourslash/signatureHelpCallExpressionJs.ts
+++ b/tests/cases/fourslash/signatureHelpCallExpressionJs.ts
@@ -12,7 +12,7 @@
 //// someOptional(1, 2, 3);
 //// /*missing*/someOptional(); // no error here; x and y are optional in JS
 
-verify.errorExistsAfterMarker("missing");
+verify.not.errorExistsAfterMarker("missing");
 verify.signatureHelp(
     {
         marker: "1",