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.

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

Confusing error messages: expected N arguments #42891

Closed
amcasey opened this issue Feb 19, 2021 · 4 comments Β· Fixed by #43855
Closed

Confusing error messages: expected N arguments #42891

amcasey opened this issue Feb 19, 2021 · 4 comments Β· Fixed by #43855
Assignees
Labels
Bug A bug in TypeScript Domain: Error Messages The issue relates to error messaging Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fix Available A PR has been opened for this issue Help Wanted You can do this

Comments

@amcasey
Copy link
Member

amcasey commented Feb 19, 2021

Bug Report

πŸ”Ž Search Terms

error, arguments, overloads

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

Reduced from https://github.com/elibarzilay/process-tracing/blob/59a790c839258ee737481edad0a614f094a1d68c/index.js

import fs = require("fs");
import stream = require("stream");
import util = require("util");
import zlib = require("zlib");

declare function split(matcher: RegExp, mapper: (line: string) => any): stream.Transform;

declare function test(x: any, y: () => string): string | undefined;
declare function jsonTransform(t: typeof test): stream.Transform;
declare function disp(x: any): string;

declare const input: string;
declare const output: string;

const pipeline = util.promisify(stream.pipeline);

async function processFile(fastMode: boolean) {
    await pipeline(
        fs.createReadStream(input),
        ...(/\.gz$/.test(input) ? [zlib.createGunzip()]
            : /\.br$/.test(input) ? [zlib.createBrotliDecompress()]
                : []),
        fastMode
            ? split(/,?\r?\n/, x => x.length > 1 ? test(JSON.parse(x), () => x) : undefined)
            : jsonTransform(test),
        async function* (inp) {
            for await (const x of inp) yield disp(x);
            yield "\n]\n";
        },
        ...(/\.gz$/.test(output) ? [zlib.createGzip()]
            : /\.br$/.test(output) ? [zlib.createBrotliCompress()]
                : []),
        fs.createWriteStream(output)
    );
}

There are several weird things going on here:

  1. fs.createWriteStream(output) is squiggled and the error produced is Expected at least 1 arguments, but got 6 or more. There are no elaborations. I have no idea what's actually wrong.
  2. As @minestarks points out, it's not clear how any numbers could be substituted into this error template to make the statement sensible.
  3. In 4.1.5 or 4.2.0-dev, if you comment out the async function * argument, the new error is No overload expects 4 arguments, but overloads do exist that expect either 4 or Infinity arguments. The phrasing "4 or Infinity" is strange, the value 4 is both expected and not expected, and it's not clear where 4 actually comes from (maybe it assumes each spread is empty?).
  4. In 4.2.0-dev.20210219, the squiggle range is reduced to only some of the arguments and the error is Expected at least 1 arguments, but got 4 or more.
@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Domain: Error Messages The issue relates to error messaging Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this labels Feb 20, 2021
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 4.3.1 milestone Feb 20, 2021
@sandersn
Copy link
Member

sandersn commented Apr 10, 2021

Here's a standalone repro:

type R = { a: number }
type W = { b: number }
type RW = { a: number, b: number }
declare const pli: {
    (s1: R, s2: RW, s3: RW, s4: RW, s5: W): Promise<void>;
    (streams: ReadonlyArray<R | W | RW>): Promise<void>;
    (s1: R, s2: RW | W, ...streams: Array<W | RW>): Promise<void>;
}

declare var writes: W
declare var reads: R
declare var tr: number
declare var gun: number[]
declare var gz: number[]
declare var fun: (inp: any) => AsyncGenerator<string, void, unknown>
pli(
    reads,
    ...gun,
    tr,
    fun,
    ...gz,
    writes
);

@sandersn
Copy link
Member

The basic problem is that there are plenty of arguments to satisfy the last signature. Confusingly (like the rest of this bug), if you keep only the last signature, you still get an arity error on fun:

"Expected at least 2 arguments, but got 6 or more."

@sandersn
Copy link
Member

Actually, spread arguments that are not tuples (like number[] in the reduced example), have to appear after a rest parameter in the signature. So the arity error is intended, though unexpected to me. It's still a matter of fixing the error text, then.

@sandersn
Copy link
Member

sandersn commented Apr 13, 2021

The arity error code has two problems: (1) it doesn't understand the newer rules for matching spreads to rests (2) it's so complicated that it's hard to tell whether it was even correct at the time it was written.

Three problems. It has 3 problems: (3) the spread/rest mismatch error message was never particularly understandable.

This really needs a complete overhaul. I'll do that after looking at the other 4.3 bugs. I put my early scribbles and tests at the branch reform-arity-errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Error Messages The issue relates to error messaging Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fix Available A PR has been opened for this issue Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants