Why is assignment in while statement discouraged in Javascript?

Question

I just coded the following loop which works just fine:

let position = 0;
// tslint:disable-next-line:no-conditional-assignment
while ((position = source.indexOf('something interesting', position)) > 0) {
    // do something interesting with something interesting
  position += 1;
}

But note I was obliged to add the tslint exception. Apparently somebody has decided that it is undesirable to allow assignment statements inside the while conditional. Yet this is the most direct code pattern I know for this particular situation.

If this is not "allowed" (or maybe I should say "discouraged") then what do the people driving this think the correct code pattern is?

Answer 1

It makes the intent of the code harder to determine. Using assignment as an expression is unusual - if you see it inside a condition, how sure are you that the writer of the code intended to use the assignment, or might it be a typo? What if the writer of the code intended to compare the value? EG

while ((position = source.indexOf('something interesting', position)) > 0) {

vs

while ((position == source.indexOf('something interesting', position)) > 0) {

Those two are very different. (In this particular case, Typescript will warn you that the comparison against the boolean and number is probably a mistake, but that won't happen with Javascript, nor in some other situations.)

Rather than assigning inside the conditional, assign outside of it, or even better, for the code above, you could probably use a regular expression (though it's impossible to say for sure without knowing the contents of // do something interesting with something interesting).

It looks repetitive, unfortunately, but an alternative is

let position = 0;
const getPos = () => {
  position = source.indexOf('something interesting', position);
};
getPos();
while (position > 0) {
  // do something interesting with something interesting
  getPos();
}

Answer 2

As in Adam's example in the comments, an alternative with unconditional assignment could look like this:

let position = 0;
while (true) {
  position = source.indexOf('something interesting', position);
  if (position < 0) {
    break;
  }
  // do something interesting with something interesting
  position += 1;
}

The syntax could become even more compact if you disable curly, which enforces braces on if/for/do/while statements even though K&R Ch 3 does not mandate those braces at all*. This invites a good comparison regarding the motivation for these rules in general. TSLint defines its purpose on its homepage:

TSLint is an extensible static analysis tool that checks TypeScript code for readability, maintainability, and functionality errors.

Is it entirely possible to use a brace-free if statement correctly, particularly if you are diligent about indentation and formatting? Of course. However, it is also entirely possible to misuse that feature in a subtle way, like in the docs for curly:

if (foo === bar)
    foo++;
    bar++;

Your question supposes "apparently somebody has decided that it is undesirable", but the more-accurate statement is that this pattern is a significant source of bugs across codebases, enough that the authors of tslint have offered built-in detection and feedback for that particular pattern. Rather than "undesirable", "not 'allowed'", or "discouraged", the pattern is simply rare-enough and misused-enough to have its detection on by default.

As with all rules in tslint, you can configure the default rule set using tslint.json or tslint.yaml, and you and your team may choose to disable rules like curly and no-conditional-assignment. This may be especially useful if this cases is often a false positive in your coding style (as it seems to be), or if you are applying tslint to an existing codebase where that pattern is common and correctly-used. However, I think it is entirely reasonable for a team to claim (as others have done on this question) that the pattern is prone to =/== typos and other errors and that the correct uses are insufficient to justify allowing the pattern in their codebase.

---

*Note: Though K&R doesn't mandate braces, it does note on p56 that "The indentation shows unequivocally what you want, but the compiler doesn't get the message, and associates the else with the inner if. This kind of bug can be hard to find; it's a good idea to use braces when there are nested ifs." This suggests that even the original K&R text is supportive of reasonable style rules or limits beyond what their syntax supports.