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

Error elaborations should not be de-duped #33143

Open
AnyhowStep opened this issue Aug 29, 2019 · 8 comments
Open

Error elaborations should not be de-duped #33143

AnyhowStep opened this issue Aug 29, 2019 · 8 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Aug 29, 2019

TypeScript Version: 3.5.1

Search Terms:

  • Error message different

Code

interface ExprData {
  mapper : () => unknown,
  blah : string,
}
interface Expr<DataT extends ExprData> {
  mapper : DataT["mapper"],
  blah : DataT["blah"],

  extra : number,
}

declare function where (
  callback : () => Expr<{
    mapper : () => boolean,
    blah : string,
  }>
) : void;

declare function getInvalidWhere () : Expr<{
  mapper : () => (boolean|null),
  blah : string,
}>;

/*
Type 'Expr<{ mapper: () => boolean | null; blah: string; }>' is not assignable to type 'Expr<{ mapper: () => boolean; blah: string; }>'.
  Type '{ mapper: () => boolean | null; blah: string; }' is not assignable to type '{ mapper: () => boolean; blah: string; }'.
    Types of property 'mapper' are incompatible.
      Type '() => boolean | null' is not assignable to type '() => boolean'.
        Type 'boolean | null' is not assignable to type 'boolean'.
          Type 'null' is not assignable to type 'boolean'.
*/
where(() => getInvalidWhere())
/*
  Expected: Same error as above,
  Actual:
    Type 'Expr<{ mapper: () => boolean | null; blah: string; }>' is not assignable to type
    'Expr<{ mapper: () => boolean; blah: string; }>'.
*/
where(() => getInvalidWhere())

Expected behavior:

  • Both errors should have the same error message, if they're in the same file.
  • Both errors should have the same error message, if they're in different files.

Actual behavior:

  • Both errors have different error messages in the same file.
  • Both errors have different error messages in different files.

Playground Link:

Playground

Related Issues:

@AnyhowStep
Copy link
Contributor Author

AnyhowStep commented Aug 29, 2019

In my particular case,

I write compile time tests and it just so happens I was testing two different files.

One was the HAVING clause. The other was the WHERE clause.

Both clauses expect a boolean expression. Not a boolean|null expression.

However, I noticed that adding the HAVING clause test changed the WHERE clause test results...


This is like the union-order-during-compile-time problem
The union order can change depending on which line/file is compiled first.
This affects my compile-time tests.


In this case, the error message can change depending on which line/file is encountered first.
This also affects my compile-time tests.

@AnyhowStep
Copy link
Contributor Author

The union-order-during-compile-time problem
#32224

AnyhowStep added a commit to AnyhowStep/tsql-mysql-5.7 that referenced this issue Aug 29, 2019
@sandersn
Copy link
Member

sandersn commented Sep 3, 2019

So, duplicate of #32224 then?

@sandersn sandersn added the Duplicate An existing issue was already created label Sep 3, 2019
@AnyhowStep
Copy link
Contributor Author

AnyhowStep commented Sep 3, 2019

Not a duplicate!

This second instance of the error omits lines that the first instance would have.

It isn't reordering types, it's completely omitting lines from the message.

The first instance of an error could have 5 lines and the second instance of the same error, produced in a different way, in a different file, could have just 2 lines.

Because the second instance is omitting the last 3 lines that should be there.

They should be there because the extra lines can help with figuring out errors. It makes no sense to omit lines from a completely unrelated repro of the same error.


It's just that it reminds me of the union order problem a little. (Output changes based on what line is encountered first) They're completely different, though.

@sandersn
Copy link
Member

sandersn commented Sep 4, 2019

Oh, that's just the fact that we only issue elaborations once per file. This is an intentional choice for batch compilation. I'm not sure it makes sense for the language service, but it sounds like you are getting these errors from batch compilation.

Without elaboration deduplication, batch compiler output would be huge. Are you proposing issuing all elaborations for every error in batch compilation, or in the language service?

@sandersn sandersn added Suggestion An idea for TypeScript Needs More Info The issue still hasn't been fully clarified and removed Duplicate An existing issue was already created labels Sep 4, 2019
@sandersn sandersn changed the title Different instances of the same error give different error messages Error elaborations should not be de-duped Sep 4, 2019
@AnyhowStep
Copy link
Contributor Author

AnyhowStep commented Sep 4, 2019

Not the language service, I suppose. I'm not actually familiar with the TS API.

I think I might be using the batch compilation.
I pass in every single file I'm compiling to rootNames of the createProgram function.

https://github.com/AnyhowStep/tsql/blob/master/test/compile-time/util.ts#L129

So, I guess I'm asking for a way to turn off error deduplication for batch compilation. I'm comparing each file's actual output (.d.ts and errors) with some expected output.

So, if errors are getting deduplicated, it'll cause my actual output to change randomly as I add more tests.

I added comments to a commit on my project, it shows which lines were removed because of a test I added,

AnyhowStep/tsql-mysql-5.7@67374c1

@sandersn sandersn added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature and removed Needs More Info The issue still hasn't been fully clarified labels Sep 4, 2019
@AnyhowStep
Copy link
Contributor Author

I tried to create one program per compile-time test file. It's super slow =(

Before, I had,

const program = createProgram({
  rootNames,
  options,
});
program.emit(/*snip*/);

It took 30s to run tests on 674 files.

Now, I have,

for (const rootName of rootNames) {
  const program = createProgram({
    rootNames : [rootName],
    options,
  });
  program.emit(/*snip*/);
}

And I've been waiting for minutes and it still hasn't completed.

Is there any way I can speed things up? =(

@AnyhowStep
Copy link
Contributor Author

It took 2319.367 to use createProgram().emit() 674 times.

That's about 40 minutes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

2 participants