`typescript:S2699` raises false positives on `supertest` (with reproducer)

Rules and Languages Report False-positive / False-negative...
, ,
1

Hello friends at Sonar :waving_hand:

I came across a lot of FP with typescript:S2699 on tests that rely on the supertest library.

  • What language is this for?
    • TypeScript (probably also JavaScript)
  • Which rule?
    • typescript:S2699
  • Why do you believe it’s a false-positive/false-negative?
    • See comments in the reproducer below :innocent:
  • Are you using
    • SonarQube Cloud :white_check_mark:
    • SonarQube for IDE :white_check_mark: - WebStorm latest, VSCode latest
      • in connected mode with SonarQube Cloud (also reproduced in standalone mode)
  • How can we reproduce the problem? Give us a self-contained snippet of code (formatted text, no screenshots)
    • See details below

Thanks!

Reproducer 
import supertest from 'supertest';

const ROOT_URL = 'http://localhost:3000';

describe('Reproducer for FP with typescript:S2699', () => {

    function setupSupertest() {
        return supertest(ROOT_URL);
    }

    // Expected behavior, as expected by the rule implementation:
    // (see https://github.com/SonarSource/SonarJS/blob/3d68d220ef6689b3fc067e3cb68d21bc2d157246/packages/jsts/src/rules/helpers/supertest.ts#L58)
    // - `supertest` is called directly
    // - `expect` is exactly the third call in the chain
    it('should resolve supertest when used directly', async () => {
        await supertest(ROOT_URL)
            .get('/')
            .expect(200);
    });

    // FP(typescript:S2699) - the rule implementation does not resolve the return type of `setupSupertest()`
    it('should resolve supertest when used from builder function', async () => {
        /*
         * Using a builder function to create the supertest instance is a common pattern,
         * e.g. when the server base URL comes from a Web framework (e.g. NestJS).
         * The fix may not be trivial in the rule implementation (resolution of types/imports).
         */
        await setupSupertest()
            .get('/')
            .expect(200);
    });

    // FP(typescript:S2699) - expect is not the third call in the chain
    it('should resolve expect when used with a long chain', async () => {
        /*
         * Any "interesting" use of a Web API may require a long chain of calls to `supertest`,
         * e.g. to set headers, cookies, body, etc. so the `expect` call is often not exactly the third call in the chain.
         * Furthermore, several calls to `expect` can be chained to assert multiple conditions on the response.
         * (see https://www.npmjs.com/package/supertest)
         * This one may be easier to fix given the current implementation of the rule.
         */
        await supertest(ROOT_URL)
            .get('/')
            .set('Authorization', 'Bearer token')
            .expect(200)
            .expect('Content-Type', /html/);
    });
});
2

Hi @JB.L, thanks for your comprehensive report! You’re in luck: this was fixed very recently and will be part of the next release of the SonarJS analyzer.

3

Hey @andres, thanks for the good news!

The way I understand the new implementation of the rule, it should fix the behavior for long chains of calls (resolving expect at the end of the chain), but probably not the resolution of the call to supertest in case it is ‘hidden’ in a function.

Still, I’m looking forward to the availability of the update on SQC and I’ll report back in case we still see false-positives.