[no-unnecessary-type-assertion] False negative if type is temporarily narrowed but narrowing is never relied on

[peterflynn]

This is probably more of an enhancement request...

• I have tried restarting my IDE and the issue persists.
• I have updated to the latest version of the packages.
• I have read the FAQ and my problem is not listed.

Repro

{
  "rules": {
    "@typescript-eslint/no-unnecessary-type-assertion": "error"
  }
}

let x: string | undefined = Math.random() > 0.5 ? "yes" : undefined;
let y: string | undefined = undefined;

if (Math.random() > 0.5) {
    y = x!;
}

if (y !== undefined) {
    console.log("Not undefined");
}

Expected Result

Expected:
"This assertion is unnecessary since the receiver accepts the original type of the expression."

By all appearances, the ! postfix is unnecessary since it's being assigned right back to a nullable variable again.

Actual Result

No error.

This happens because inside the scope of the if, the type of y is narrowed to exclude undefined. If there were more code inside that block, it could rely on y being non-null without TSC complaining.

However, in cases like this where there isn't any more code inside the block, there's nothing relying on that narrowing -- everything would compile just as successfully with the ! operator removed.

Additional Info

The rule correctly flags simpler examples where there is no temporary type narrowing -- like this one (from #1310):

let x: string | undefined = Math.random() > 0.5 ? "yes" : undefined;
let y: string | undefined = x!;

The more complex example above may be a harder scenario to detect, but it does still fit the bill as "unnecessary," IMHO.

Versions

package	version
@typescript-eslint/eslint-plugin	4.1.0
@typescript-eslint/parser	4.1.0
TypeScript	3.9.7
ESLint	7.12.1
node	12.18.3

[bradzacher]

typescript-eslint/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts

Line 163 in 55111af

const contextualType = util.getContextualType(checker, originalNode);

This line asks TS for the contextual type of the non-null expression - which means it is attempting to get the type that TS thinks it's assigning to.
For let z: string | undefined = x!, this returns the type string | undefined.
However, for y = x!, this returns the value undefined (i.e. it is telling us it doesn't know).

This is a gap in the handling of our function:

typescript-eslint/packages/eslint-plugin/src/util/types.ts

Lines 477 to 512 in e7528e6

	export function getContextualType(
	checker: ts.TypeChecker,
	node: ts.Expression,
	): ts.Type | undefined {
	const parent = node.parent;
	if (!parent) {
	return;
	}
	if (isCallExpression(parent) || isNewExpression(parent)) {
	if (node === parent.expression) {
	// is the callee, so has no contextual type
	return;
	}
	} else if (
	isVariableDeclaration(parent) ||
	isPropertyDeclaration(parent) ||
	isParameterDeclaration(parent)
	) {
	return parent.type ? checker.getTypeFromTypeNode(parent.type) : undefined;
	} else if (isJsxExpression(parent)) {
	return checker.getContextualType(parent);
	} else if (isPropertyAssignment(parent) && isIdentifier(node)) {
	return checker.getContextualType(node);
	} else if (
	![ts.SyntaxKind.TemplateSpan, ts.SyntaxKind.JsxExpression].includes(
	parent.kind,
	)
	) {
	// parent is not something we know we can get the contextual type of
	return;
	}
	// TODO - support return statement checking
	return checker.getContextualType(node);
	}

Should be simple enough to fix up.

[msquinn]

@peterflynn are you working on this? If not I'd love to give it a shot tonight, thanks 😄

[bradzacher]

We're going to have to rollback this change.
For simple cases like you presented - sure, it makes sense to validate this the non-null assertion is unnecessary.

However what you may not realise is that this actually changes the control-flow of the rest of the code.

Here is an example:

let x: number | undefined;
let y: number | undefined;
// typeof y === number | undefined

y = x;
// typeof y === number | undefined

// error: Object is possibly 'undefined'.(2532)
(y + 1);

y = x!;
// typeof y === number     !!!!!!!!

// no error below
(y + 1);