We've been counting down our top 5 issues that SonarLint catches in TypeScript projects, and we've reached the top of the list. This issue is outstanding in its simplicity and potential to cause very hard-to-spot bugs.

Grab your editor and install SonarLint if you don't already have it. You can then copy and paste the example code below to try these for yourself. This particular issue applies to JavaScript codebases as well as TypeScript. 

Nº 1: assignments within sub-expressions

Have you ever written a conditional and then tested it out to find that it was not behaving how you'd expected it to? Whether or not the expression in the conditional is true or false, the code inside the conditional is still running. You go back to the code, checking every part of the expression and tracing the values back through the code. You verify everything is correct about it. You take one last look at it, ready to bang your head against the desk, close your laptop and go home for the day.

Finally, you spot it. The bug. There it is:

if (theThing = theOtherThing) {
  // the things are the same, do some stuff
}

Inside the expression lies a single equals sign, the assignment operator. The sub-expression isn't checking whether the two variables are equal, it is assigning one to the other, and the conditional is being evaluated based on whether theOtherThing is a truthy value. 

The expression is missing an = (or perhaps two). It's a pain of a bug to figure out, it can take a long time to spot the last mistake you'd thought you'd make, and when you do find it, you kick yourself.

It's easy to spot in an isolated example like this but consider a more fully formed function:

function request(
  method: string,
  path: string,
  options: Record<string, string>
) {
  if (method = "GET") {
    path = `${path}?${new URLSearchParams(options).toString()}`;
    return fetch(path, { method: "GET" });
  } else {
    return fetch(path, {
      method,
      body: JSON.stringify(options),
    });
  }
}

It's not so obvious that the function above reassigns the method variable to the string GET and will always make a GET request regardless of the method supplied.

That's why assignment within sub-expressions is our number one issue discovered by SonarLint in TypeScript projects. Thankfully there aren't too many of these bugs in the wild, because they get caught by testing, linting, or by tooling like SonarLint. But between hunting for the bug or it being flagged in my editor as soon as I make the mistake, I know which I'd prefer.

Other causes

Assigning inside a conditional is normally a bug, but there are other sub-expressions that assignments can crop up in. In these cases, the issue is less about correctness and more about readability.

Consider an example like this calculator class:

class Calculator {
  result: number;

  constructor() {
    this.result = 0;
  }

  add(value: number) {
    return (this.result = this.result + value);
  }

  subtract(value: number) {
    return (this.result = this.result - value);
  }
}

You can use it like so:

const calculator = new Calculator();
calculator.add(4);
// => 4
calculator.subtract(2);
// => 2
console.log(calculator.result);
// => 2

Each time you use the add or subtract methods the object returns the result, but also sets that result in an instance variable. In both functions, this is done in one line, conflating returning the value with storing it.

There is no bug here, the object works as expected, but this issue lies in the readability of the functions. It is unexpected to find an assignment in a return statement, and when you do you then need to read over the assignment in order to see the actual return value. This might be useful if you are trying to golf your code, but things are much more readable if you assign in one statement and then return the value in another.

While this is a relatively simple example, I have seen plenty of versions of this in real codebases where assigning and returning make reading the result much harder. In each of these cases, it is possible to make the assignment on one line and then return the result on the next line. It's more explicit and thus clearer to someone else reading the code.

class Calculator {
  result: number;

  constructor() {
    this.result = 0;
  }

  add(value: number) {
    this.result = this.result + value;
    return this.result;
  }

  subtract(value: number) {
    this.result = this.result - value
    return this.result;
  }
}

Install SonarLint in your editor and you'll get notified if you accidentally assign inside of a conditional, avoiding those bugs, or if you're affecting readability by assigning within another expression.

That's a wrap

That's our countdown of the top 5 common issues we've found in TypeScript projects with SonarLint. Over this series we've covered:

5. Optional property declarations

4. Creating and dropping objects immediately

3. Unused local variables and functions

2. Non-empty statements

and, finally, today's issue

1. Assignments within sub-expressions 

I hope this has been an interesting trip through some of the issues you might have encountered in your own TypeScript, and hopefully cleared up before you commit. Check out the full set of TypeScript rules to see what other issues SonarLint can help you avoid. And if there are any rules you think should have made this top 5, let us know on Twitter at @SonarSource or in the community.