[bug 1.4.1] semicolon insertion before ArrayExpression and others

[Andarist]

--no-semi affects this

[] produces ;[]
({}) produces ;({})
<div/> produces ;<div/>

Possibly other expressions are affected too.

The weird part is that by going back in git's history to the v1.4.0 or even before that still produces the same result. However those cases didnt parse like this on v1.4.0 nor on v1.3.1.

You can checkout what prettier have changed for our codebase when upgrading to v1.4.0 (ive ran prettier on all files) here. State before that upgrade is also full-prettier on version v1.3.1.

After an attempt to upgrade to v1.4.1 those 2 lines got affected by the 'bug' described, I didnt commit it yet though.

I could have some stale npm packages in my node_modules before (didnt rm -rf node_modules after upgrade or before), maybe that was the case? Dunno how to verify that though

Nevertheless the behaviour is rly weird and I guess it should be fixed. Can help with that if u give a green light to it ;)

[Andarist]

so I've diagnosed the 'issue' and it seems this is inserted intentionally to prevent issues with automatic semicolon insertion. Dunno why this was not acting like this before v1.4.1 for me.

However that being said - it shouldnt be inserted automatically, only when the problem really might occur. For example eslint recognized those different situations correctly, it wont have problem with allowing [], however it will warn when something like this is written:

someCall()
[a] = obj

and the possible fix for the problem is to put semi at the end of the previous line like this

someCall();
[a] = obj

I think this looks way more naturally and we should print like this, gonna dig into the code to see if I can make that happen.

[azz]

Having the semicolon at the start of the line is more common with semi-colon-less development. It makes it obvious that the semi-colon is there for a semantic reason.

https://standardjs.com/rules.html#semicolons

The reason we always print the semi-colon at the start, even if it isn't strictly required, is that it makes it easier to move lines of code around the file without accidentally introducing ASI issues.

Closing this as prettier is working as intended.

[Andarist]

@azz
Hm, i understand the reasoning, guess its ok to prefer semi at the start. However would prefer to add them only if strictly required. This looks really odd:

  if (is.array(fn)) {
    ;[context, fn] = fn
  } else if (fn.fn) {
    ;({ context, fn } = fn)
  }

it makes it easier to move lines of code around the file without accidentally introducing ASI issues.

this argument is OTOH not that much appealing as prettier would just prevent ASI when formatting, its an automated process after all

also this conflicts with no-extra-semi which is in the recommended rules

[lydell]

This has already been discussed numerous times.

• Semicolon option #736 (comment)
• Wrong semicolon on semi = false mode #1388
• Flow default case - extra semicolon #1566
• No-semi is too agressive with JSX #1433
• Extra semicolon in the begin of the line #1907