Immediately-returned variable rule conflicts with Typescript excess-property checking

Versions: SQ 8.3, scanner 4.3.0, TS plugin 2.1, JS plugin 6.2.1
Rule typescript:S1488

The subject rule flags cases where a variable is declared on one line and returned on the next, which makes sense in the rule examples, where the variable being assigned is the result of a function call that returns a typed object, or primitive operators that return a specific primitive.

There is a common pattern in Typescript involving generics where the rule is actively harmful:

interface MyInterface {
    num: number;
    str?: string;
}

declare function wrap<T>(t: T): T;

function bad(nums: number[]): MyInterface[] {
    return wrap(nums.map(num => {
        if (num > 0) {
            return {
                num,
                str_wrong: `Number ${num}`,
            };
        } else {
            return { num: -1 };
        }
    }));
}

function good(nums: number[]): MyInterface[] {
    return wrap(nums.map(num => {
        if (num > 0) {
            const ret: MyInterface = {
                num,
                str_wrong: `Number ${num}`, // Compiler error: type is not assignable to "MyInterface"
            };
            return ret;
        } else {
            return { num: -1 };
        }
    }));
}

ETA: the above example in Typescript Playground

Note that in bad(), the returned object used the wrong key name for the string property, but the compiler didn’t notice. That’s because the generic type argument is inferred for wrap<T> as {num: number, str_wrong: string}, which is technically assignable to MyInterface. Note that even if I changed this to return {...} as MyInterface, the compiler will not complain about the misspelled property name.

Now, look at good(). Explicitly typing the returned object literal forces the compiler to notice that you’re including a property (str_wrong) that is not part of the interface.

I’m not sure exactly how to improve the rule. It might involve noticing that the returned object is being passed to a generic method, or it might be that interface-typed (versus primitive-typed or class-typed) values shouldn’t be flagged. It just doesn’t make sense to apply the rule as it stands to this particular pattern.

For my own reference as much as anybody else: the thing I’m trying to enforce here is called excess property checking. Literals returned from a generic method are not subject to excess property checks, even if the type argument for the generic method is explicitly provided, i.e. wrap<MyInterface>({num: 1, str_wrong: "Hello"}) does not get flagged.

Hi,

I know it’s been a while, still I think it’s an interesting finding, I created a ticket (Improve 'prefer-immediate-return': ignore when returned variable has a declared type · Issue #274 · SonarSource/eslint-plugin-sonarjs · GitHub) to not report if type is available in the variable declaration. I believe it’s a nice pattern to exclude from the rule as when type is available refactoring we suggest is not equivalent.

Let me know if you confirm such approach

Looks good to me! I think TS is also working on being smarter about excess-property checks, but it’s a good idea for Sonar to catch this as well.

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.