Typescript: "If this was intentional, convert the expression to 'unknown' first" diagnostic should come with a codefix

Created on 23 Oct 2018  ·  8Comments  ·  Source: microsoft/TypeScript

TypeScript Version: 3.2.0-dev.20181020

Code

0 as string

Expected behavior:

Codefix to convert to 0 as unknown as string.

Actual behavior:

No codefix.

Quick Fixes Fixed Suggestion good first issue help wanted

Most helpful comment

Thanks for the response. I hope your team will reconsider this approach.

  1. The syntax is ugly and would turn me away from typescript if I was a beginner
  2. Violates the idea the programmer knows what they are doing over the compiler.

This seems to be the equivalent to having a double confirmation dialog in a UI. In this analogy I imagine a pop up box asking a user if they accept terms and conditions, and when they hit accept they are prompted again if they are sure they accept the terms and conditions.

Just as that double confirmation is not a good user experience in a UI, having to double assert what you want is not a good user experience when programming.

All 8 comments

I'm interested in taking this issue. Any pointers you think would be helpful for a first-time Typescript contributor?

Look at some existing simple codefixes like addMissingInvocationForDecorator.
The code fix should be registered corresponding the the error code for "If this was intentional, convert the expression to 'unknown' first" (search for the error text in diagnosticMessages.json). Then to make the change all you have to do is replace the type assertion's in expression with another node from createTypeAssertion or createAsExpression (depending on what kind the original type assertion is). You can use changeTracker.replaceNode for that.
For an example of a test, see tests/cases/fourslash/codeFixAddMissingInvocationForDecorator01.ts.

My fix for this issue is coming along nicely. In fact, I had finished and tested it before I noticed you wanted this to apply to TypeAssertions as well as AsExpressions. So I'm working on adding that functionality. Just to verify, <string>0 should codefix to <string><unknown>0?

<string> <unknown> 0

@andy-ms

I'm hoping you can clear up some confusion I'm having with this solution. I don't find the double assertion first to unknown and then to the type we want to be very elegant or even consistent with documentation.

The basic basic types docs mentions in the type assertion section

Type assertions are a way to tell the compiler “trust me, I know what I’m doing.” A type assertion is like a type cast in other languages, but performs no special checking or restructuring of data. It has no runtime impact, and is used purely by the compiler. TypeScript assumes that you, the programmer, have performed any special checks that you need.

Doing a double cast seems to go against the whole "trust me, I know what I'm doing" theory behind type assertions. Is the reason for this a technical limitation with the typescript compiler?

It's "trust me I know what I'm doing" within means. We try to ensure there's some overlap, and if not you can do jump up and down the type hierarchy.

Thanks for the response. I hope your team will reconsider this approach.

  1. The syntax is ugly and would turn me away from typescript if I was a beginner
  2. Violates the idea the programmer knows what they are doing over the compiler.

This seems to be the equivalent to having a double confirmation dialog in a UI. In this analogy I imagine a pop up box asking a user if they accept terms and conditions, and when they hit accept they are prompted again if they are sure they accept the terms and conditions.

Just as that double confirmation is not a good user experience in a UI, having to double assert what you want is not a good user experience when programming.

I hope we can switch this off in tsconfig.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Roam-Cooper picture Roam-Cooper  ·  3Comments

CyrusNajmabadi picture CyrusNajmabadi  ·  3Comments

kyasbal-1994 picture kyasbal-1994  ·  3Comments

uber5001 picture uber5001  ·  3Comments

jbondc picture jbondc  ·  3Comments