Non-null assertions being reported as redundant

Clean Code Report False-positive / False-negative...
1

SonarQube version: Version 8.9 (build 43852)
Affected rule: typescript:S4325

Given code like this:

import { useState } from 'react';

interface UserData {
    name: string;
}

interface User {
    data: UserData | null;
}

const Component = () => {
    const [user, setUser] = useState<User | null>(null);

    const doSomethingWithUser = () => {
        console.log(user.data.name);
    };

    return user ? (
        <button onClick={doSomethingWithUser}>Do something</button>
    ) : null;
};

The TypeScript compiler will error on

console.log(user.data.name);

as user is possibly null. As it is, doSomethingWithUser cannot be called unless user is set due to the condition further down in the component, so changing to

console.log(user!.data.name);

makes sense, as we can at this point safely assert that user is not null.

However, SonarQube reports this as a violation of rule typescript:S4325, claiming that “Redundant casts and non-null assertions should be avoided”.

IMO, this assertion is not redundant - the only other way would be to add another condition whether user is not null within doSomethingWithUser, but that would in turn be redundant again, as it would result in a dead branch that could never be executed.

Note that this is a very condensed example, which could be solved differently (by returning null early before defining doSomethingWithUser), but this is not always possible.

2

Hello,

What is strict value in your tsconfig.json compilerOptions?

I could reproduce “typescript:S4325” issue only with "strict":false, but then compiler is not complaining on console.log(user.data.name);.

3

strict is set to true. This is the full config:

{
    "compilerOptions": {
        "target": "esnext",
        "lib": ["dom", "dom.iterable", "esnext"],
        "types": ["vite/client"],
        "allowJs": false,
        "skipLibCheck": false,
        "esModuleInterop": false,
        "allowSyntheticDefaultImports": true,
        "strict": true,
        "forceConsistentCasingInFileNames": true,
        "noFallthroughCasesInSwitch": true,
        "module": "esnext",
        "moduleResolution": "node",
        "resolveJsonModule": true,
        "isolatedModules": true,
        "noEmit": true,
        "jsx": "react-jsx"
    },
    "include": ["src"]
}
4

Are you sure about reproducer? I can’t reproduce it with your sample:

image
image2210×1410 236 KB

5

Fairly sure - but I can setup a repo, no problem, give me a few.

Any settings in Sonarqube that could cause this? We’re just using the default profile.

6

Here’s the repo: GitHub - lauraseidler/sonarqube-redundant-non-null-assertions

However, I’m also failing to reproduce this with my local sonarqube + sonar scanner.

This code smell shows up when I run the build through our Jenkins, connected to our actual sonarqube instance (running the developer edition of sonarqube):

image
image1015×565 47.5 KB

This code smell does not show up in either of these cases:

  • running with a locally installed sonar-scanner against a local sonarqube instance (community edition in docker)
  • running with a locally installed sonar-scanner against our actual sonarqube instance
  • running sonar-scanner in a docker container using the same image as Jenkins uses to spin up the container to run in CI (sonarsource/sonar-scanner-cli:4.6)

So the only differences seem to be:

  • Jenkins uses the SonarQube Scanner plugin and withSonarQubeEnv around the step that runs the scan, while locally I am passing sonar.host.url and sonar.login parameters to sonar-scanner
  • Jenkins might differ slightly on git things (sparse checkout, setting some environment variables)

The issue might well be with out setup, though I’m failing to see where this could be. I’m happy to provide debug logs or any further information.

7

So I took the debug logs from both runs, removed the timestamps and diffed the files, here’s some of the things I found:

186,187c187,189
< INFO: Load quality profiles (done) | time=129ms
---
> INFO: Load quality profiles (done) | time=86ms
> INFO: Auto-configuring with CI 'Jenkins'

Jenkins is detected, and “auto-configured” (not sure what exactly that means)

Here is the part in the Jenkins log* where the TS files are analysed and the issue is detected (“Saving issue for rule no-unnecessary-type-assertion on line 15”):

INFO: Found 1 tsconfig.json file(s): [/app/tsconfig.json]
DEBUG: /app/src/App.tsx matched /app/tsconfig.json
DEBUG: /app/src/main.tsx matched /app/tsconfig.json
INFO: Analyzing 2 files using tsconfig: /app/tsconfig.json
INFO: 2 source files to be analyzed
INFO: Load project repositories
DEBUG: GET 200 https://URL/batch/project.protobuf?key=sonarqube-redundant-non-null-assertions&branch=main | time=15ms
INFO: Load project repositories (done) | time=23ms
DEBUG: 'src/App.tsx' generated metadata with charset 'UTF-8'
DEBUG: Resolved 'react' as 'undefined'
DEBUG: ... 'react' was imported in '/app/src/App.tsx'
DEBUG: Saving issue for rule no-unnecessary-type-assertion on line 15
DEBUG: 'src/main.tsx' generated metadata with charset 'UTF-8'
DEBUG: Resolved 'react' as 'undefined'
DEBUG: ... 'react' was imported in '/app/src/main.tsx'
DEBUG: Resolved 'react-dom' as 'undefined'
DEBUG: ... 'react-dom' was imported in '/app/src/main.tsx'
DEBUG: Resolved './App' as '/app/src/App.tsx'
DEBUG: ... './App' was imported in '/app/src/main.tsx'
DEBUG: Not enough content in 'src/main.tsx' to have CPD blocks, it will not be part of the duplication detection
INFO: 2/2 source files have been analyzed
INFO: Sensor TypeScript analysis [javascript] (done) | time=7042ms

In the local log, this looks like this, with some kind of error, and no issue being detected:

INFO: Found 1 tsconfig.json file(s): [/app/tsconfig.json]
DEBUG: /app/src/App.tsx matched /app/tsconfig.json
DEBUG: /app/src/main.tsx matched /app/tsconfig.json
INFO: 2 source files to be analyzed
INFO: Analyzing 2 files using tsconfig: /app/tsconfig.json
INFO: Load project repositories
DEBUG: GET 200 https://URL/batch/project.protobuf?key=sonarqube-redundant-non-null-assertions&branch=main | time=28ms
INFO: Load project repositories (done) | time=36ms
DEBUG: 'src/App.tsx' generated metadata with charset 'UTF-8'
DEBUG: delta [ns] since modification FileSnapshot failed to detect
count, failures, racy limit [ns], delta min [ns], delta max [ns], delta avg [ns], delta stddev [ns]
20066, 521, 5000, 2513000.0, 6378000.0, 4463015.355086373, 1112940.7941585348
DEBUG: FileStoreAttributes[fsTimestampResolution=2 µs, minimalRacyInterval=6,378 µs]
DEBUG: loading config FileBasedConfig[/tmp/jgit/config]
DEBUG: Resolved 'react' as 'undefined'
DEBUG: ... 'react' was imported in '/app/src/App.tsx'
DEBUG: 'src/main.tsx' generated metadata with charset 'UTF-8'
DEBUG: Resolved 'react' as 'undefined'
DEBUG: ... 'react' was imported in '/app/src/main.tsx'
DEBUG: Resolved 'react-dom' as 'undefined'
DEBUG: ... 'react-dom' was imported in '/app/src/main.tsx'
DEBUG: Resolved './App' as '/app/src/App.tsx'
DEBUG: ... './App' was imported in '/app/src/main.tsx'
DEBUG: Not enough content in 'src/main.tsx' to have CPD blocks, it will not be part of the duplication detection
INFO: 2/2 source files have been analyzed

*In the Jenkins logs I replaced the path to the original workspace with /app to get better diffs