Improved error message for calling/constructing types

[dragomirtitian]

Fixes #32013

[DanielRosenwasser]

What's with the annotation?

[dragomirtitian]

Mistake, will delete

[DanielRosenwasser]

I really like this improvement, but I'm starting to get uneasy about the wording.

• "This expression" really refers to the target of the call, but we're highlighting the call expression range.
• We can change the range to focus on the invoked expression, but I think we still want the current range.
• The wording is so nice and simple! I don't want to change that!

[DanielRosenwasser]

In other words, can we find better wording that refers to the invoked expression without sounding terrible?

[dragomirtitian]

@DanielRosenwasser Personally I think the range of this error would be better on the target. I think it would make the most sense, the problem is with the target of the call, the arguments have no bearing on the error and are just extra noise. Unlike for example when overload selection fails when it makes sense to highlight the whole call expression since the problem is with the combination of call target and arguments.

I think changing the target might cause some problems with some quick fixes (fixInvalidImportSyntax uses the node for this specific error), but nothing that can't be fixed 😊

[DanielRosenwasser]

Yeah, I think you have a good point. I would try to change the range to the target if possible. Maybe see #31414 for the better range.

If you'd be up for fixing the ranges on overload errors to include arguments (which I think technically regressed in #31414), that'd be awesome too.

[dragomirtitian]

@DanielRosenwasser Sure I will look into it. Not sure about a time line, this week is a bit busy though.

[dragomirtitian]

@DanielRosenwasser

What do programmers do Friday night ? They code for fun 😁

I had a chance to finish the changes we discussed above to error spans. The behavior will be as follows:

1. If the error is related to the call target (the call target is not callable), the span only includes the target (not the arguments).

declare const test: string;
test();
~~~~

1. a. If the call target is part of a property access expression, only the last name is part of the span.

declare const o: { test: string }
o.test();
  ~~~~

2. If the error is related to overload resolution, the whole call expression (including arguments) is part of the error span

declare const test: (o:string) => void
test();
~~~~~~

a. If the call target is part of a property access expression, only the last name is part of the span (together with the arguments).

declare const o: { test(o:string): void }
o.test();
  ~~~~~~

I think this is a nice improvement over #31414 and also improves error spans on long invocations chains where the offending call has a non-callable target (something #31414 did not do).

I know @sandersn is also working in this area (#32092) hope this will not conflict with that too much.

[DanielRosenwasser]

@typescript-bot pack this

[typescript-bot]

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at e4bca96. You can monitor the build here. It should now contribute to this PR's status checks.

[typescript-bot]

Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/34870/artifacts?artifactName=tgz&fileId=10754D00A75967808B74148E48416DD562143561FFC54740CCD6E2892E22050602&fileName=/typescript-3.6.0-insiders.20190628.tgz"
    }
}

and then running npm install.