Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat trailing 'void' as optional for assignability #40231

Merged
merged 1 commit into from
Aug 28, 2020
Merged

Treat trailing 'void' as optional for assignability #40231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 42 additions & 13 deletions src/compiler/checker.ts
View file
Open in desktop
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 @@ -28024,21 +28034,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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What’s this about?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its likely due to the fact that we now compare a signature with a trailing void that we would have previously skipped because the minArgumentCount didn't match, and as a result we encounter ArrayLike during type checking earlier than we do Iterable. As a result, ArrayLike ends up with an earlier type id and thus its order in the union changes.

>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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading