[CLI DX] Improve positioning of compiler error messaging info #45717

orta · 2021-09-03T20:14:31Z

Note: This is still up for debate, but we think this is a pretty strong contender to ship. We may still change our minds on details. So, if you give it a shot, expect some of us to come in and maybe make changes at the end of the PR's lifecycle.

Suggestion

As of 4.4, a run of tsc looks like:

$ /Users/ortatherox/dev/typescript/repros/compilerSamples/node_modules/.bin/tsc
index.ts:2:1 - error TS2588: Cannot assign to 'a' because it is a constant.

2 a = "123"
  ~

index.ts:4:1 - error TS2588: Cannot assign to 'a' because it is a constant.

4 a = 543
  ~

I propose instead we make the output terminal length aware, and by using that information we change the whitespace and positioning of the different pieces of information.

Example

$ /Users/ortatherox/dev/typescript/repros/compilerSamples/node_modules/.bin/tsc
● index.ts:2:1                                                                       TS2720
  | a = "123"                                                                              
    ▔                                                      
  Cannot assign to 'a' because it is a constant.                                           
                                                                                           
● index.ts:4:1                                                                       TS2720
  | a = 543                                                                                
    ▔                                                                                        
  Cannot assign to 'a' because it is a constant.

This change does not add any new lines to the output. There's a reasonable argument that we might want an extra new-line between the location and source code though, might have to get a feel for this in prod. We also switch from ~ to ▔ to "fake" a full linebreak.

Details

We take into account the knowledge about the width of the current terminal in order to provide more space between the key elements of the TS output.

● is a sigil per-compiler message, and lives on its own column. This means you can easily see when compiler messages start/end
The TS1234 error message is de-empathized and moved to the right, these messages are useful for searching the internet for similar problems but are less valuable for understanding the errors. I dropped 'error' from that message because aside from #45714 we always show errors here.
Code is moved above the error message. There's two ideas in play here:
1. We don't need to repeat the location information by placing the location and source code next to each other. That also means that the code samples are in a consistent location because the length of the line number does not affect how far right we show the source code.
2. The line of code is one of the best ways to get context before you read the error message, so IMO having it first makes a bit more sense than after. I have a sense of the cases when it does not, and I have ideas to address that in another issue later.

Notes

We had a lot of design experiments ranging from rust-y, eslint-y, errata-y, miette-y but we've got quite a lot of backwards compatibility guarantees to keep and this proposal is about the wrapping to the error messaging - and it leaves individual error messages to be able to do a better job of giving context.

We want these to be safe to copy & paste (it should be better now the message is on its own lines) and parsable via regexes for things like problem matchers (though they now need to be multi-line to grab the message)

Exceptions

If we do not know the width of the terminal or it is too thin ( < 60 cols) then we drop all fancy re-ordering and leave it to your terminal to handle. This means:

The TS#### is moved to be a space after the location
We don't indent the code/message

$ /Users/ortatherox/dev/typescript/repros/compil
erSamples/node_modules/.bin/tsc
● index.ts:2:1 TS2720
| a = "123"                                                                              
  ▔                                                      
Cannot assign to 'a' because it is a constant.                                           
                                                                                           
● index.ts:4:1 TS2720
| a = 543                                                                                
  ▔                                                                                        
Cannot assign to 'a' because it is a constant.

The text was updated successfully, but these errors were encountered:

sandersn · 2021-09-03T22:10:34Z

One question, one suggestion:

What about related spans? Related span errors are not distinct enough from top-level errors in the current output, and are the worst single offender in making long error output hard to read. (There are lots of offenders, they're just the worst.)
I like the switch to real underlines instead of tildes -- most tildes are too sparse to imitate squiggly underlines in my opinion -- but I hope that they stay red.

Edit: Adding bullets will help (1) a lot, I think, so I hope not much additional adaption will be needed.

orta · 2021-09-04T12:08:13Z

4.4

src/compiler/commandLineParser.ts:1169:13 - error TS2322: Type '{ name: string; type: "boolean";                           
affectsSemanticDiagnostics: true; affectsEmit: true; category: DiagnosticMessage; description: DiagnosticMessage;          
defaultValueDesciption: DiagnosticMessage; }' is not assignable to type 'CommandLineOption'.                               
  Object literal may only specify known properties, but 'defaultValueDesciption' does not exist in type                    
'CommandLineOptionOfPrimitiveType'. Did you mean to write 'defaultValueDescription'?                                       
                                                                                                                           
1169             defaultValueDesciption: Diagnostics.true_for_ES2022_and_above_including_ESNext                            
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

After

$ /Users/ortatherox/dev/typescript/repros/compilerSamples/node_modules/.bin/tsc             
                                                                                            
● src/compiler/commandLineParser.ts:1169:13                                          TS2720 
  | defaultValueDesciption: Diagnostics.true_for_ES2022_and_above_including_ESNext          
    ▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔          
  Type '{ name: string; type: "boolean"; affectsSemanticDiagnostics: true; affectsEmit: true;  
  category: DiagnosticMessage; description: DiagnosticMessage; defaultValueDesciption:        
  DiagnosticMessage; }' is not assignable to type 'CommandLineOption'.                        
                                                                                            
  Object literal may only specify known properties, but 'defaultValueDesciption' does not     
  exist in type 'CommandLineOptionOfPrimitiveType'. Did you mean to write                     
  'defaultValueDescription'?

So I think the first related span should have a newline with a separation. Will trigger something with a deeper set of relations to look at next.

orta · 2021-09-04T12:32:35Z

Before

index.ts:4:1 - error TS2322: Type '{ f(x: string): void; }' is not assignable to type '{ f(x: number): void; }'.
  Types of property 'f' are incompatible.                                                                       
    Type '(x: string) => void' is not assignable to type '(x: number) => void'.                                 
      Types of parameters 'x' and 'x' are incompatible.                                                         
        Type 'number' is not assignable to type 'string'.                                                       
                                                                                                                
4 b1 = b2                                                                                                       
  ~~

After

$ /Users/ortatherox/dev/typescript/repros/compilerSamples/node_modules/.bin/tsc             
                                                                                            
● index.ts:4:1                                                                        TS2322
  | b1 = b2                                                                                 
    ▔▔▔▔▔▔▔                                                                                 
  Type '{ f(x: string): void; }' is not assignable to type '{ f(x: number): void; }'.       
                                                                                            
  Types of property 'f' are incompatible.                                                   
    Type '(x: string) => void' is not assignable to type '(x: number) => void'.             
      Types of parameters 'x' and 'x' are incompatible.                                     
        Type 'number' is not assignable to type 'string'.

I'm not certain it's the most elegant answer, because I think the existing indentation is paying for itself in these reports. Separating with a newline does help in the readability a bunch though. The largest chains come from TS2322, which I have separate ideas to address outside of this issue.

orta · 2021-09-04T12:58:51Z

Another related example, these related have a source code location to display:

Before

$ /Users/ortatherox/dev/typescript/resolve-ts/node_modules/.bin/tsc              
index.ts:6:22 - error TS2451: Cannot redeclare block-scoped variable 'conflict'. 
                                                                                 
6         export const conflict = 0;                                             
                       ~~~~~~~~                                                  
                                                                                 
  a.d.ts:3:14                                                                    
    3 export const conflict = 0;                                                 
                   ~~~~~~~~                                                      
    'conflict' was also declared here.                                           
                                                                                 
index.ts:12:18 - error TS2451: Cannot redeclare block-scoped variable 'conflict'.
                                                                                 
12     export const conflict = 0;                                                
                    ~~~~~~~~                                                     
                                                                                 
  a.d.ts:3:14                                                                    
    3 export const conflict = 0;                                                 
                   ~~~~~~~~                                                      
    'conflict' was also declared here.

After

$ /Users/ortatherox/dev/typescript/repros/compilerSamples/node_modules/.bin/tsc             
                                                                                            
● index.ts:6:22                                                                       TS2451
  | export const conflict = 0;                                                              
                 ▔▔▔▔▔▔▔▔                                                                   
  Cannot redeclare block-scoped variable 'conflict'.                                        
                                                                                            
  'conflict' was also declared here:                                                                                                                                                 
  a.d.ts:3:14

  | export const conflict = 0;                                                            
                 ▔▔▔▔▔▔▔▔                                                                 
● index.ts:12:18                                                                      TS2451
  | export const conflict = 0;                                                              
                                                                                            
  Cannot redeclare block-scoped variable 'conflict'.                                        
                                                                                            
  'conflict' was also declared here:                                                        
  a.d.ts:3:14                                                                             

  | export const conflict = 0;                                                            
                 ▔▔▔▔▔▔▔▔

mdaj06 · 2021-09-08T07:50:20Z

@orta can I give it a shot?

orta · 2021-09-08T10:13:34Z

Sure, give it a shot - until there's a solid WIP PR it's open for anyone to give it a go.

mdaj06 · 2021-09-08T11:29:40Z

thanks @orta ! Fair enough will start work on it, what would be a good starting point?

orta · 2021-09-08T13:34:49Z

I'd start at trying to just scope the output like this:

index.ts:6:22 - error TS2451: Cannot redeclare block-scoped variable 'conflict'. 
                                                                                 
6         export const conflict = 0;

to

● index.ts:6:22                                                                       TS2720
  | export const conflict = 0;

  Cannot redeclare block-scoped variable 'conflict

Then work on more complex bits once you're comfy

prabhatmishra33 · 2021-10-05T16:46:55Z

Hello @mdaj06 ! Hope you are working on it, please let me know if there is any change in plan 😉

mdaj06 · 2021-10-07T13:27:31Z

Hey @prabhatmishra33 i was side tracked with another set of tasks, will be picking up where i left off!

sidharthv96 · 2021-10-15T10:38:07Z

index.ts:2:1 - error TS2588: Cannot assign to 'a' because it is a constant.

2 a = "123"
  ~

● index.ts:2:1                                                                       TS2720
  | a = "123"                                                                              
    ▔                                                      
  Cannot assign to 'a' because it is a constant.

One thing to note is that in the earlier error message, error TS2588: Cannot assign to 'a' because it is a constant. is a single line, so select -> copy -> google is seamless.

With the new layout, the error code and error message cannot be selected easily in case someone want's to google it.

Simply searching the error code does reveal answers to common problems, but there could be cases where the message would be necessary.

Apart from this, new one looks awesome!

orta added Help Wanted Discussion Good First Issue Experience Enhancement Effort: Casual labels Sep 3, 2021

Oct	NOV	Dec
	23
2020	2021	2022

microsoft / TypeScript Public

[CLI DX] Improve positioning of compiler error messaging info #45717

[CLI DX] Improve positioning of compiler error messaging info #45717

orta commented Sep 3, 2021

sandersn commented Sep 3, 2021 •

edited

Loading

orta commented Sep 4, 2021

orta commented Sep 4, 2021

orta commented Sep 4, 2021 •

edited

Loading

mdaj06 commented Sep 8, 2021

orta commented Sep 8, 2021

mdaj06 commented Sep 8, 2021 •

edited

Loading

orta commented Sep 8, 2021 •

edited

Loading

prabhatmishra33 commented Oct 5, 2021

mdaj06 commented Oct 7, 2021

sidharthv96 commented Oct 15, 2021

microsoft / TypeScript Public

[CLI DX] Improve positioning of compiler error messaging info #45717

[CLI DX] Improve positioning of compiler error messaging info #45717

Comments

orta commented Sep 3, 2021

Suggestion

Example

Details

Notes

Exceptions

sandersn commented Sep 3, 2021 • edited Loading

orta commented Sep 4, 2021

4.4

After

orta commented Sep 4, 2021

Before

After

orta commented Sep 4, 2021 • edited Loading

Before

After

mdaj06 commented Sep 8, 2021

orta commented Sep 8, 2021

mdaj06 commented Sep 8, 2021 • edited Loading

orta commented Sep 8, 2021 • edited Loading

prabhatmishra33 commented Oct 5, 2021

mdaj06 commented Oct 7, 2021

sidharthv96 commented Oct 15, 2021

sandersn commented Sep 3, 2021 •

edited

Loading

orta commented Sep 4, 2021 •

edited

Loading

mdaj06 commented Sep 8, 2021 •

edited

Loading

orta commented Sep 8, 2021 •

edited

Loading