Idea: extend capability of `extract-sequence-expressions` or applicable rule

[epreston]

One class of sequence expressions missed by the extract-sequence-expressions rule (or some other rule for this purpose) is when the sequence statements are tucked into an "if expression'. For example:

let a = true;

if(console.log('one'), console.log('two'), a) {
  console.log('three')
}

This is equivalent and more clearly written as:

let a = true;

console.log('one');
console.log('two');

if (a) {
  console.log('three')
}

Javascript returns the last expression in the sequence for evaluation in the "if expression".

[epreston]

The issue with leaving these undetected is that it causes another rule, remove-unused-variables, to incorrectly determine that a variable is unused. For example, "a" will incorrectly be matched as unused but it is used as the condition of the first "if statement".

let a = true;
let b = false;

if ((console.log('one'), console.log('two'), a)) {
  console.log('three');

  b = true;

  if (b) {
    console.log('four');
  }
}

[coderaiser]

Just landed support of SequenceExpressions to @putout/plugin-remove-unused-variables v4.2.0. Is it works for you?

About

let a = true;

if (console.log('one'), console.log('two'), a) {
  console.log('three')
}

What if if would be a part of while for example

let a = true;

for (const b of c)
    if (console.log('one'), console.log('two'), a) {
        console.log('three')
    }

In this case we should check that parent of IfStatement is BodyBlock, and if it's not we should create it. Also SequenceExpression can be part of WhileStatement and all kinds of For-loops etc.

I don't think that this is very popular case, but sounds interesting. How did it get to your codebase?

[epreston]

Can confirm that plugin-remove-unused-variables now correctly detects the usage of a variable when:

• it is the evaluated as the condition of an if statement at the end of a sequence expression.

This is the edge case where some control statements, "if statements" in this case, will evaluate the last expression in a sequence as its condition. Little known but valid javascript.

To answer your question: Older Vue 2 templates promoted a style of javascript that required a factory pattern that leads to these hard to spot issues. It looks like an "if statement" with a complicated expression, but, its actually one or two simple statements, comma separated in a sequence, with trailing expression for the "if statement" to evaluate. Over a a large project this created 80+ false flags for unused variables.

[epreston]

You are right about "left" associating single line control statements where the BodyBlock is optional. If there is no block, one will have to be made. "for, while, if" maybe others will use the following statement as its body without a BodyBlock.

I'd be excited to see if we can hoist out these sequence expressions. Helpful if someone puts a comma in the wrong place creating a sequence expression when they mean to combine a condition into a larger expression for the control block. It would be neat to hit save and see the statement get ejected above on its own line.

[coderaiser]

OK, it can be something like this, but I think about more cases that can be counted (while, for, for-of, for-in, switch... did I miss something?).

You can make a 🐊Putout plugin from this draft, and install it separately. There is a yoman generator for this purpose that simplifies things a lot :).

[coderaiser]

Well, about loops, it changes behavior of code, so can be skipped, what statements besides if do you think should be supported?

[coderaiser]

I'd be excited to see if we can hoist out these sequence expressions. Helpful if someone puts a comma in the wrong place creating a sequence expression when they mean to combine a condition into a larger expression for the control block. It would be neat to hit save and see the statement get ejected above on its own line.

Landed in @putout/plugin-extract-sequence-expressions@3.5.0 🎉. Is it works for you?

[epreston]

It works well.

I'm going to write out some detailed bits so we have a good record of test overage, environment, versions.

Tested Versions

module	version
putout	"28.3.0"
@putout/plugin-extract-sequence-expressions	"3.5.0"
@putout/plugin-logical-expressions	"1.2.0"

Test Project Metrics

language	files	code	comment	blank	total
JavaScript	283	50,763	2,617	6,566	59,946

Test Detection

94 errors in 50 files

Test Detail

Test 1:

• It hoists the sequence expressions found in 'if statements" correctly
• this is the simple case found above

Test 2:

• When it is proceeded by a single line, left associating control statement with an optional BodyBlock
• It creates a BodyBlock if required, then hoists the sequence statements above leaving the condition
• preserves execution order and practical outcomes
• exposes the actual execution of the code by the runtime while improving readability

"For Statement" example for clarity:

for (let n = 0; n < t.length; n++)
  if (((i = '(' + e + ': ' + t[n] + ')'), window.matchMedia(i).matches)) return t[n];

Is transformed into the more readable equivalent:

for (let n = 0; n < t.length; n++) {
  i = '(' + e + ': ' + t[n] + ')';
  if (window.matchMedia(i).matches) return t[n];
}