Skip to content

Commit

Permalink
Treat trailing 'void' as optional for assignability (#40231)
Browse files Browse the repository at this point in the history
  • Loading branch information
rbuckton committed Aug 28, 2020
1 parent b969b58 commit 10fb9c9
Show file tree
Hide file tree
Showing 32 changed files with 364 additions and 216 deletions.
55 changes: 42 additions & 13 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,12 @@ namespace ts {
ExportNamespace = 1 << 2,
}

const enum MinArgumentCountFlags {
None = 0,
StrongArityForUntypedJS = 1 << 0,
VoidIsNonOptional = 1 << 1,
}

function SymbolLinks(this: SymbolLinks) {
}

Expand Down Expand Up @@ -9859,6 +9865,7 @@ namespace ts {
sig.resolvedReturnType = resolvedReturnType;
sig.resolvedTypePredicate = resolvedTypePredicate;
sig.minArgumentCount = minArgumentCount;
sig.resolvedMinArgumentCount = undefined;
sig.target = undefined;
sig.mapper = undefined;
sig.unionSignatures = undefined;
Expand Down Expand Up @@ -11318,7 +11325,10 @@ namespace ts {
const signature = getSignatureFromDeclaration(node.parent);
const parameterIndex = node.parent.parameters.indexOf(node);
Debug.assert(parameterIndex >= 0);
return parameterIndex >= getMinArgumentCount(signature, /*strongArityForUntypedJS*/ true);
// Only consider syntactic or instantiated parameters as optional, not `void` parameters as this function is used
// in grammar checks and checking for `void` too early results in parameter types widening too early
// and causes some noImplicitAny errors to be lost.
return parameterIndex >= getMinArgumentCount(signature, MinArgumentCountFlags.StrongArityForUntypedJS | MinArgumentCountFlags.VoidIsNonOptional);
}
const iife = getImmediatelyInvokedFunctionExpression(node.parent);
if (iife) {
Expand Down Expand Up @@ -28028,21 +28038,40 @@ namespace ts {
return length;
}

function getMinArgumentCount(signature: Signature, strongArityForUntypedJS?: boolean) {
if (signatureHasRestParameter(signature)) {
const restType = getTypeOfSymbol(signature.parameters[signature.parameters.length - 1]);
if (isTupleType(restType)) {
const firstOptionalIndex = findIndex(restType.target.elementFlags, f => !(f & ElementFlags.Required));
const requiredCount = firstOptionalIndex < 0 ? restType.target.fixedLength : firstOptionalIndex;
if (requiredCount > 0) {
return signature.parameters.length - 1 + requiredCount;
function getMinArgumentCount(signature: Signature, flags?: MinArgumentCountFlags) {
const strongArityForUntypedJS = flags! & MinArgumentCountFlags.StrongArityForUntypedJS;
const voidIsNonOptional = flags! & MinArgumentCountFlags.VoidIsNonOptional;
if (voidIsNonOptional || signature.resolvedMinArgumentCount === undefined) {
let minArgumentCount: number | undefined;
if (signatureHasRestParameter(signature)) {
const restType = getTypeOfSymbol(signature.parameters[signature.parameters.length - 1]);
if (isTupleType(restType)) {
const firstOptionalIndex = findIndex(restType.target.elementFlags, f => !(f & ElementFlags.Required));
const requiredCount = firstOptionalIndex < 0 ? restType.target.fixedLength : firstOptionalIndex;
if (requiredCount > 0) {
minArgumentCount = signature.parameters.length - 1 + requiredCount;
}
}
}
if (minArgumentCount === undefined) {
if (!strongArityForUntypedJS && signature.flags & SignatureFlags.IsUntypedSignatureInJSFile) {
return 0;
}
minArgumentCount = signature.minArgumentCount;
}
if (voidIsNonOptional) {
return minArgumentCount;
}
for (let i = minArgumentCount - 1; i >= 0; i--) {
const type = getTypeAtPosition(signature, i);
if (filterType(type, acceptsVoid).flags & TypeFlags.Never) {
break;
}
minArgumentCount = i;
}
signature.resolvedMinArgumentCount = minArgumentCount;
}
if (!strongArityForUntypedJS && signature.flags & SignatureFlags.IsUntypedSignatureInJSFile) {
return 0;
}
return signature.minArgumentCount;
return signature.resolvedMinArgumentCount;
}

function hasEffectiveRestParameter(signature: Signature) {
Expand Down
45 changes: 41 additions & 4 deletions src/compiler/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,25 @@ namespace ts {
export function enableDebugInfo() {
if (isDebugInfoEnabled) return;

// avoid recomputing
let weakTypeTextMap: WeakMap<Type, string> | undefined;
let weakNodeTextMap: WeakMap<Node, string> | undefined;

function getWeakTypeTextMap() {
if (weakTypeTextMap === undefined) {
if (typeof WeakMap === "function") weakTypeTextMap = new WeakMap();
}
return weakTypeTextMap;
}

function getWeakNodeTextMap() {
if (weakNodeTextMap === undefined) {
if (typeof WeakMap === "function") weakNodeTextMap = new WeakMap();
}
return weakNodeTextMap;
}


// Add additional properties in debug mode to assist with debugging.
Object.defineProperties(objectAllocator.getSymbolConstructor().prototype, {
__debugFlags: { get(this: Symbol) { return formatSymbolFlags(this.flags); } }
Expand All @@ -421,7 +440,18 @@ namespace ts {
Object.defineProperties(objectAllocator.getTypeConstructor().prototype, {
__debugFlags: { get(this: Type) { return formatTypeFlags(this.flags); } },
__debugObjectFlags: { get(this: Type) { return this.flags & TypeFlags.Object ? formatObjectFlags((<ObjectType>this).objectFlags) : ""; } },
__debugTypeToString: { value(this: Type) { return this.checker.typeToString(this); } },
__debugTypeToString: {
value(this: Type) {
// avoid recomputing
const map = getWeakTypeTextMap();
let text = map?.get(this);
if (text === undefined) {
text = this.checker.typeToString(this);
map?.set(this, text);
}
return text;
}
},
});

const nodeConstructors = [
Expand All @@ -443,9 +473,16 @@ namespace ts {
__debugGetText: {
value(this: Node, includeTrivia?: boolean) {
if (nodeIsSynthesized(this)) return "";
const parseNode = getParseTreeNode(this);
const sourceFile = parseNode && getSourceFileOfNode(parseNode);
return sourceFile ? getSourceTextOfNodeFromSourceFile(sourceFile, parseNode!, includeTrivia) : "";
// avoid recomputing
const map = getWeakNodeTextMap();
let text = map?.get(this);
if (text === undefined) {
const parseNode = getParseTreeNode(this);
const sourceFile = parseNode && getSourceFileOfNode(parseNode);
text = sourceFile ? getSourceTextOfNodeFromSourceFile(sourceFile, parseNode!, includeTrivia) : "";
map?.set(this, text);
}
return text;
}
}
});
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5375,6 +5375,8 @@ namespace ts {
/* @internal */
minArgumentCount: number; // Number of non-optional parameters
/* @internal */
resolvedMinArgumentCount?: number; // Number of non-optional parameters (excluding trailing `void`)
/* @internal */
target?: Signature; // Instantiation target
/* @internal */
mapper?: TypeMapper; // Instantiation mapper
Expand Down
12 changes: 6 additions & 6 deletions src/lib/es2015.promise.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,18 +113,18 @@ interface PromiseConstructor {
*/
reject<T = never>(reason?: any): Promise<T>;

/**
* Creates a new resolved promise.
* @returns A resolved promise.
*/
resolve(): Promise<void>;

/**
* Creates a new resolved promise for the provided value.
* @param value A promise.
* @returns A promise whose internal state matches the provided promise.
*/
resolve<T>(value: T | PromiseLike<T>): Promise<T>;

/**
* Creates a new resolved promise .
* @returns A resolved promise.
*/
resolve(): Promise<void>;
}

declare var Promise: PromiseConstructor;
6 changes: 3 additions & 3 deletions tests/baselines/reference/arrayFrom.types
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const inputALike: ArrayLike<A> = { length: 0 };
const inputARand = getEither(inputA, inputALike);
>inputARand : ArrayLike<A> | Iterable<A>
>getEither(inputA, inputALike) : ArrayLike<A> | Iterable<A>
>getEither : <T>(in1: Iterable<T>, in2: ArrayLike<T>) => Iterable<T> | ArrayLike<T>
>getEither : <T>(in1: Iterable<T>, in2: ArrayLike<T>) => ArrayLike<T> | Iterable<T>
>inputA : A[]
>inputALike : ArrayLike<A>

Expand Down Expand Up @@ -161,12 +161,12 @@ const result11: B[] = Array.from(inputASet, ({ a }): B => ({ b: a }));
// the ?: as always taking the false branch, narrowing to ArrayLike<T>,
// even when the type is written as : Iterable<T>|ArrayLike<T>
function getEither<T> (in1: Iterable<T>, in2: ArrayLike<T>) {
>getEither : <T>(in1: Iterable<T>, in2: ArrayLike<T>) => Iterable<T> | ArrayLike<T>
>getEither : <T>(in1: Iterable<T>, in2: ArrayLike<T>) => ArrayLike<T> | Iterable<T>
>in1 : Iterable<T>
>in2 : ArrayLike<T>

return Math.random() > 0.5 ? in1 : in2;
>Math.random() > 0.5 ? in1 : in2 : Iterable<T> | ArrayLike<T>
>Math.random() > 0.5 ? in1 : in2 : ArrayLike<T> | Iterable<T>
>Math.random() > 0.5 : boolean
>Math.random() : number
>Math.random : () => number
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/asyncArrowFunction11_es5.types
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ class A {
await Promise.resolve();
>await Promise.resolve() : void
>Promise.resolve() : Promise<void>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }

const obj = { ["a"]: () => this }; // computed property name after `await` triggers case
>obj : { a: () => this; }
Expand Down
40 changes: 20 additions & 20 deletions tests/baselines/reference/asyncFunctionReturnType.types
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ async function fIndexedTypeForPromiseOfStringProp(obj: Obj): Promise<Obj["string

return Promise.resolve(obj.stringProp);
>Promise.resolve(obj.stringProp) : Promise<string>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>obj.stringProp : string
>obj : Obj
>stringProp : string
Expand All @@ -58,9 +58,9 @@ async function fIndexedTypeForExplicitPromiseOfStringProp(obj: Obj): Promise<Obj

return Promise.resolve<Obj["stringProp"]>(obj.stringProp);
>Promise.resolve<Obj["stringProp"]>(obj.stringProp) : Promise<string>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>obj.stringProp : string
>obj : Obj
>stringProp : string
Expand All @@ -82,9 +82,9 @@ async function fIndexedTypeForPromiseOfAnyProp(obj: Obj): Promise<Obj["anyProp"]

return Promise.resolve(obj.anyProp);
>Promise.resolve(obj.anyProp) : Promise<any>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>obj.anyProp : any
>obj : Obj
>anyProp : any
Expand All @@ -96,9 +96,9 @@ async function fIndexedTypeForExplicitPromiseOfAnyProp(obj: Obj): Promise<Obj["a

return Promise.resolve<Obj["anyProp"]>(obj.anyProp);
>Promise.resolve<Obj["anyProp"]>(obj.anyProp) : Promise<any>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>obj.anyProp : any
>obj : Obj
>anyProp : any
Expand All @@ -120,9 +120,9 @@ async function fGenericIndexedTypeForPromiseOfStringProp<TObj extends Obj>(obj:

return Promise.resolve(obj.stringProp);
>Promise.resolve(obj.stringProp) : Promise<string>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>obj.stringProp : string
>obj : TObj
>stringProp : string
Expand All @@ -134,9 +134,9 @@ async function fGenericIndexedTypeForExplicitPromiseOfStringProp<TObj extends Ob

return Promise.resolve<TObj["stringProp"]>(obj.stringProp);
>Promise.resolve<TObj["stringProp"]>(obj.stringProp) : Promise<TObj["stringProp"]>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>obj.stringProp : string
>obj : TObj
>stringProp : string
Expand All @@ -158,9 +158,9 @@ async function fGenericIndexedTypeForPromiseOfAnyProp<TObj extends Obj>(obj: TOb

return Promise.resolve(obj.anyProp);
>Promise.resolve(obj.anyProp) : Promise<any>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>obj.anyProp : any
>obj : TObj
>anyProp : any
Expand All @@ -172,9 +172,9 @@ async function fGenericIndexedTypeForExplicitPromiseOfAnyProp<TObj extends Obj>(

return Promise.resolve<TObj["anyProp"]>(obj.anyProp);
>Promise.resolve<TObj["anyProp"]>(obj.anyProp) : Promise<TObj["anyProp"]>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>obj.anyProp : any
>obj : TObj
>anyProp : any
Expand All @@ -198,9 +198,9 @@ async function fGenericIndexedTypeForPromiseOfKProp<TObj extends Obj, K extends

return Promise.resolve(obj[key]);
>Promise.resolve(obj[key]) : Promise<TObj[K]>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>obj[key] : TObj[K]
>obj : TObj
>key : K
Expand All @@ -213,9 +213,9 @@ async function fGenericIndexedTypeForExplicitPromiseOfKProp<TObj extends Obj, K

return Promise.resolve<TObj[K]>(obj[key]);
>Promise.resolve<TObj[K]>(obj[key]) : Promise<TObj[K]>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>obj[key] : TObj[K]
>obj : TObj
>key : K
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/callWithMissingVoid.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ tests/cases/conformance/expressions/functionCalls/callWithMissingVoid.ts(22,8):
tests/cases/conformance/expressions/functionCalls/callWithMissingVoid.ts(35,31): error TS2554: Expected 1 arguments, but got 0.
tests/cases/conformance/expressions/functionCalls/callWithMissingVoid.ts(36,35): error TS2554: Expected 1 arguments, but got 0.
tests/cases/conformance/expressions/functionCalls/callWithMissingVoid.ts(37,33): error TS2554: Expected 1 arguments, but got 0.
tests/cases/conformance/expressions/functionCalls/callWithMissingVoid.ts(48,1): error TS2554: Expected 3 arguments, but got 1.
tests/cases/conformance/expressions/functionCalls/callWithMissingVoid.ts(48,1): error TS2554: Expected 2-3 arguments, but got 1.
tests/cases/conformance/expressions/functionCalls/callWithMissingVoid.ts(55,1): error TS2554: Expected 4 arguments, but got 2.
tests/cases/conformance/expressions/functionCalls/callWithMissingVoid.ts(56,1): error TS2554: Expected 4 arguments, but got 3.
tests/cases/conformance/expressions/functionCalls/callWithMissingVoid.ts(57,1): error TS2554: Expected 4 arguments, but got 1.
Expand Down Expand Up @@ -79,7 +79,7 @@ tests/cases/conformance/expressions/functionCalls/callWithMissingVoid.ts(75,1):
a(4, "hello", void 0); // ok
a(4); // not ok
~~~~
!!! error TS2554: Expected 3 arguments, but got 1.
!!! error TS2554: Expected 2-3 arguments, but got 1.
!!! related TS6210 tests/cases/conformance/expressions/functionCalls/callWithMissingVoid.ts:42:23: An argument for 'y' was not provided.

function b(x: number, y: string, z: void, what: number): void {
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/exportDefaultAsyncFunction2.types
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ export default async(() => await(Promise.resolve(1)));
>await(Promise.resolve(1)) : any
>await : (...args: any[]) => any
>Promise.resolve(1) : Promise<number>
>Promise.resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>Promise.resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>Promise : PromiseConstructor
>resolve : { <T>(value: T | PromiseLike<T>): Promise<T>; (): Promise<void>; }
>resolve : { (): Promise<void>; <T>(value: T | PromiseLike<T>): Promise<T>; }
>1 : 1

=== tests/cases/compiler/b.ts ===
Expand Down

0 comments on commit 10fb9c9

Please sign in to comment.