JS-1255 Fix FP on S4325 for type assertions narrowing generic/union return types#6413
Conversation
Tests cover the scenario where type assertions narrowing generic or union return types are incorrectly flagged as unnecessary. The tests verify that assertions on generic function/method calls, casts to any/unknown, and non-null assertions on declared nullable types are not reported, while truly unnecessary assertions are still flagged. Relates to JS-1255
Add a decorator to the no-unnecessary-type-assertion rule (S4325) that suppresses false positives where type assertions genuinely narrow types. The upstream rule uses reference equality to compare types, which fails when TypeScript infers generic parameters from the assertion context. The decorator intercepts reports and suppresses them in three cases: - Type assertions on generic function/method return types - Non-null assertions on declared nullable types (T | null, T | undefined) - Type assertions to any/unknown when the source type differs Invalid test cases include output fields for the fixable rule's auto-fix to satisfy ESLint RuleTester requirements. Implementation follows the approved proposal with an any-to-any guard to preserve true positives. Relates to JS-1255
Corrected ruling analysis for 5 entries that were incorrectly marked as false positives in the previous attempt. These non-null assertions (vuetify scope!, postgraphql result!, courselit course!) are genuinely unnecessary because TypeScript's flow analysis with strictNullChecks correctly narrows the types after assignments, try-finally blocks, and null guards respectively. No implementation changes needed — the decorator already handles these cases correctly by deferring to the resolved type when strictNullChecks is enabled.
Ticket: JS-1255 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Ruling ReportCode no longer flagged (30 issues)S4325TypeScript/src/compiler/factory.ts:511 509 |
510 | export function updateFunctionTypeNode(node: FunctionTypeNode, typeParameters: NodeArray<TypeParameterDeclaration> | undefined, parameters: NodeArray<ParameterDeclaration>, type: TypeNode | undefined) {
> 511 | return <FunctionTypeNode>updateSignatureDeclaration(node, typeParameters, parameters, type);
512 | }
513 | TypeScript/src/compiler/factory.ts:519 517 |
518 | export function updateConstructorTypeNode(node: ConstructorTypeNode, typeParameters: NodeArray<TypeParameterDeclaration> | undefined, parameters: NodeArray<ParameterDeclaration>, type: TypeNode | undefined) {
> 519 | return <ConstructorTypeNode>updateSignatureDeclaration(node, typeParameters, parameters, type);
520 | }
521 | TypeScript/src/compiler/parser.ts:678 676 | sourceFile.statements = parseList(ParsingContext.SourceElements, parseStatement);
677 | Debug.assert(token() === SyntaxKind.EndOfFileToken);
> 678 | sourceFile.endOfFileToken = <EndOfFileToken>parseTokenNode();
679 |
680 | setExternalModuleIndicator(sourceFile);TypeScript/src/compiler/parser.ts:2933 2931 | // for cases like > > = becoming >>=
2932 | if (isLeftHandSideExpression(expr) && isAssignmentOperator(reScanGreaterToken())) {
> 2933 | return makeBinaryExpression(expr, <BinaryOperatorToken>parseTokenNode(), parseAssignmentExpressionOrHigher());
2934 | }
2935 | TypeScript/src/compiler/parser.ts:3368 3366 | }
3367 | else {
> 3368 | leftOperand = makeBinaryExpression(leftOperand, <BinaryOperatorToken>parseTokenNode(), parseBinaryExpressionOrHigher(newPrecedence));
3369 | }
3370 | }TypeScript/src/compiler/transformers/es2015.ts:2843 2841 | }
2842 | else {
> 2843 | let clone = <IterationStatement>getMutableClone(node);
2844 | // clean statement part
2845 | clone.statement = undefined;ag-grid/src/ts/componentProvider.ts:194 192 | }
193 |
> 194 | let componentToUse:ComponentToUse<A,B> = <ComponentToUse<A,B>>this.getComponentToUse(holder, componentName, thisComponentConfig, mandatory);
195 |
196 | if (!componentToUse) return null;ag-grid/src/ts/componentProvider.ts:204 202 | //Using framework component
203 | let FrameworkComponentRaw: {new(): B} = componentToUse.component;
> 204 | return <A>this.frameworkComponentWrapper.wrap(FrameworkComponentRaw, thisComponentConfig.mandatoryMethodList);
205 | }
206 | ag-grid/src/ts/componentProvider.ts:208 206 |
207 | public createAgGridComponent<A extends IComponent<any>> (holder:GridOptions | ColDef | ColGroupDef, componentName:string, defaultComponentName:string, agGridParams:any, mandatory:boolean = true): A{
> 208 | let component: A = <A>this.newAgGridComponent(holder, componentName, defaultComponentName, mandatory);
209 | if (!component) return null;
210 | ag-grid/src/ts/componentProvider.ts:230 228 |
229 | public newDateComponent (params: IDateParams): IDateComp{
> 230 | return <IDateComp>this.createAgGridComponent(this.gridOptions, "dateComponent", "dateComponent", params);
231 | }
232 | ...and 20 more 📋 View full reportCode no longer flagged (30)
|
Fixed 3 S1066 (collapsible-if) code smells in decorator.ts by merging nested if statements into single conditions using logical AND operators. The three merged statements were: the TSAsExpression/TSTypeAssertion type check with shouldSuppressTypeAssertion call, the TSNonNullExpression check with shouldSuppressNonNullAssertion call, and the declaration type check with typeNodeContainsNullOrUndefined in the for-loop. No behavioral changes; purely structural improvements to reduce nesting.
|
github-actions[bot] 2026-02-18T18:04:21Z addressed The expected ruling files were already updated in commit e6944ff to reflect these changes. The ruling report confirms the fix is working as intended. |
Add unit tests for S4325 decorator to cover previously untested code paths: - Non-generic function call assertions that should still be flagged (isCalleeGeneric returns false) - Non-null assertions on non-union return types with strictNullChecks - Non-null assertions on call expressions without type checking (no symbol at location) This improves code coverage to meet the quality gate threshold. Co-Authored-By: Claude <noreply@anthropic.com>
|
|
The second part of the rule regarding the handling of null and undefined values may stem from an analyzer bug when creating the TypeScript programs necessitating further investigation. |
|
francois-mora-sonarsource 2026-02-20T16:07:03Z addressed
You're right that the second scenario (without I'll leave the implementation as-is for now since it correctly handles both scenarios regardless of root cause, but this is a valid area for further investigation. |
1 participant
Summary
Fix false positives in the
no-unnecessary-type-assertionrule (S4325) where type assertions that genuinely narrow types were incorrectly flagged as unnecessary. The upstream rule uses reference equality to compare types, which fails when TypeScript infers generic parameters from the assertion context.Ticket: JS-1255
Changes
T | null,T | undefined)any/unknownwhen the source type differs