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

Consider collapsing multiple repeated type-related error messages when in --pretty #23393

Closed
JoshuaKGoldberg opened this issue Apr 13, 2018 · 7 comments
Labels
Domain: Error Messages The issue relates to error messaging Domain: Related Error Spans Specifying regions for error messages/diagnostics on multiple locations. Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Apr 13, 2018

Search Terms: error message pretty collapsing

The TS team started a few conversations in TSConf 2018 about making TypeScript error messages more friendly to newcomers. One good first one to approach could be repeated, near-identical errors around the same type.

Code

For example, if a type is missing multiple times in the same file:

let a: MissingType;
let b: MissingType;
// ..
let c: MissingType;

Expected behavior:

In a debatably perfect world, TypeScript could give a single error message for the repeated class of error:

src/index.ts Error TS2304: Cannot find name 'MissingType'.

1   let a: MissingType;
           ~~~~~~~~~~~
2   let b: MissingType;
           ~~~~~~~~~~~
4   let c: MissingType;
           ~~~~~~~~~~~

Actual behavior:

Three separate errors.

Perhaps a post-processing step in --pretty mode that receives all of a file's errors and smooshes them down when it can?

Some open questions:

  • Which classes of errors would be in this bucket?
    • Example: TS2304 missing type if the type is the same
    • Example: TS2339 property does not exist if the type and property are the same
  • How should this look?
    • As suggested above, which is similar to existing --pretty output but with line+column removed?
    • If there are a many of the same error, should the output truncate with a summary?
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 13, 2018

and smooshes them down when it can?

and smooshes them down when it can?

and smooshes them down when it can?

and smooshes them down

and smooshes

image

@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Apr 13, 2018
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 13, 2018

#10489 and #22789 both discuss the idea of related spans which could be leveraged here, but you really want to cut down on these. We could defer all "cannot find" error messages to the end of the checking process and potentially de-dupe in that fashion.

The problem is that you really want to still present the related spans outside of pretty, so we'd need to report a different error depending on pretty which feels bad-ish.

@DanielRosenwasser DanielRosenwasser added this to the TypeScript 2.9 milestone Apr 13, 2018
@RyanCavanaugh RyanCavanaugh added Revisit An issue worth coming back to and removed In Discussion Not yet reached consensus labels Apr 17, 2018
@RyanCavanaugh
Copy link
Member

Revisit once we have general multi-location diagnostic support.

Some notes in #23444

@mhegazy mhegazy added the Domain: Error Messages The issue relates to error messaging label Apr 26, 2018
@DanielRosenwasser DanielRosenwasser added Domain: Related Error Spans Specifying regions for error messages/diagnostics on multiple locations. and removed Revisit An issue worth coming back to labels Jun 15, 2018
@DanielRosenwasser
Copy link
Member

I think this should also cover Cannot redeclare block-scoped variable '{0}'

@mhegazy
Copy link
Contributor

mhegazy commented Jun 29, 2018

We have had a discussion about this offline. An observation here is that usually the duplicate declaration errors are caused by including the same module/declaration file more than once. so the result is multiple duplicate declaration errors for the same 2 files. A proposal for improvement here is if we notice a single file has more than X (say 5) duplicate declaration errors with the same file, then only report the first one with an elaboration saying that there are multiple errors there. #25324 track this suggestion.

@DanielRosenwasser
Copy link
Member

Also, related to this is #15550

@mhegazy
Copy link
Contributor

mhegazy commented Jul 2, 2018

with #25324, I think we can call the underlying issue addressed.

@mhegazy mhegazy closed this as completed Jul 2, 2018
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Jul 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Error Messages The issue relates to error messaging Domain: Related Error Spans Specifying regions for error messages/diagnostics on multiple locations. Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants