no-unnecessary-type-assertion false positive

[buu700]

Bug Report

• TSLint version: 5.8.0
• TypeScript version: 2.4.2
• Running TSLint via: CLI

TypeScript code being linted

const elements = <HTMLElement[]> Array.from(document.querySelectorAll('balls'));
elements[0].style.display = 'none';

with tslint.json configuration:

{
	"rules": {
		"no-unnecessary-type-assertion": true
	}
}

Actual behavior

ERROR: ... This assertion is unnecessary since it does not change the type of the expression.

This looks like #3505, but is actually a separate issue since it was reproduced without any other rules enabled.

Expected behavior

No error. The type of elements without the cast would have actually been Element[], breaking the following line that relies on a property of HTMLElement.

[ajafff]

can you also provide the compilerOptions used in your tsconfig.json?

[buu700]

Sure, here's my entire tsconfig:

{
	"angularCompilerOptions": {
		"preserveWhitespaces": false
	},
	"compileOnSave": true,
	"compilerOptions": {
		"emitDecoratorMetadata": true,
		"experimentalDecorators": true,
		"forceConsistentCasingInFileNames": true,
		"lib": ["dom", "dom.iterable", "es2016", "scripthost"],
		"module": "es2015",
		"moduleResolution": "node",
		"newLine": "LF",
		"noFallthroughCasesInSwitch": true,
		"noImplicitReturns": true,
		"noUnusedLocals": true,
		/* Pending Angular AOT fix: "noUnusedParameters": true, */
		"paths": {
			"_stream_duplex": ["js/externals/_stream_duplex"],
			"_stream_writable": ["js/externals/_stream_writable"],
			"faye-websocket": ["js/externals/faye-websocket"],
			"libsodium": ["js/externals/libsodium"],
			"request": ["js/externals/request"],
			"rsvp": ["js/externals/rsvp"]
		},
		"skipLibCheck": true,
		"sourceMap": false,
		"strict": true,
		"target": "es5"
	},
	"include": [
		"**/*.ts"
	]
}

[ajafff]

I can confirm that this bug is still present on master with typescript 2.6.1
It could be fixed by #3465 (if it lands)

Some explanation what's happening:
The assertion provides a contextual type to the expression it asserts. This causes TypeParameters that are not inferred (default to {} or use their declared default) to use this contextual type for inference.

You use this method:

interface ArrayConstructor {
  from<T, U = T>(arrayLike: ArrayLike<T>, mapfn?: (v: T, k: number) => U, thisArg?: any): U[];
}

If you don't provide an argument for mapfn, U cannot be inferred and defaults to T. At least that's the case if there is no contextual type.
If there is a contextual type, U will use this type instead.

const elements = <HTMLElement[]> Array.from(document.querySelectorAll('balls')); // U is HTMLElement

You get the same result (and no error from this rule) if you just use a type annotation:

const elements: HTMLElement[] = Array.from(document.querySelectorAll('balls')); // U is HTMLElement

You can even do other crazy stuff that is probably not intended:

const elements: string[] = Array.from(document.querySelectorAll('balls')); // U is string - there's no compile error

To summarize: I think this should be fixed upstream in TypeScript. There's no good solution to fix this in TSLint.

[buu700]

Interesting, thanks for the explanation. The type annotation form is actually what I tried first (before even running into this bug), but changed to a cast because vscode showed a compile error. I'm not 100% sure whether I had vscode configured to use 2.6.1 or 2.4.2, but does that compile for you with 2.6.1?

[ajafff]

I was about to open an issue in the TypeScript repo when I tried typescript@next.
The bug no longer exists in the nightly build.

Note that it only fixes this function by adding another overload. The underlying inference logic that caused the bug is still there.

[buu700]

Awesome, sounds good.

[unional]

Also:

declare type RGB = [number, number, number]

function rgbHex(rgb: RGB) { ... }

let rgb = [1, 2,3]
rgbHex(rgb.map(x => x++) as any)

The casting is needed above because number[] is not assignable to [number, number, number]

On the other hand, rgbHex(rgb.map(x => x++) as RGB) fix this issue.

[thejohnfreeman]

I have a similar problem, but the workaround does not work for me.

I am using a Reactive Form in Angular, with a getter that returns a control in that form:

get email (): AbstractControl { // 1
  return this.form.get('email') // 2
}

Line 1 declares a non-null return type, but the method called on line 2 returns a nullable type. As shown, tsc reports:

file.ts(2,3): error TS2322: Type 'AbstractControl | null' is not assignable to type 'AbstractControl'.
  Type 'null' is not assignable to type 'AbstractControl'.

The fix is to annotate with ! or as AbstractControl:

get email (): AbstractControl {
  return this.form.get('email')!
}

Now, tsc no longer complains, but with no-unnecessary-type-assertion, tslint does:

ERROR: file.ts[2, 10]: This assertion is unnecessary since it does not change the type of the expression.

Trying to assign to a temporary variable with a type declaration before returning does not help:

get email (): AbstractControl {
  const tmp: AbstractControl = this.form.get('email')!
  return tmp
}

Will this be fixed? I'll sadly have to disable no-unnecessary-type-assertion for now.

[IlCallo]

I'm experiencing a problem really similar to the one of @thejohnfreeman, where a variable is highlighted by VSCode as possibly undefined (I know it's not, by design) but when i put a ! after it (event.params!.alarm) TSLint complains that's not necessary

[JoshuaKGoldberg]

☠️ TSLint's time has come! ☠️

TSLint is no longer accepting most feature requests per #4534. See typescript-eslint.io for the new, shiny way to lint your TypeScript code with ESLint. ✨

It was a pleasure open sourcing with you all!

[JoshuaKGoldberg]

🤖 Beep boop! 👉 TSLint is deprecated 👈 (#4534) and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋