false negative of semi rule with never option

[mysticatea]

Tell us about your environment

• ESLint Version: 4.9.0
• Node Version: 8.6.0
• npm Version: 5.5.1

What parser (default, Babel-ESLint, etc.) are you using?
Please show your full configuration:
What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

• Open the online demo.

What did you expect to happen?

I expected semi rule to warn those semicolons because I'm using "never" option.

If a semicolon exists the end of the following statements, I can remove semicolons safely even if the next line starts with ( or [:

• DoWhileStatement
• ReturnStatement[argument=null]
• BreakStatement
• ContinueStatement
• DebuggerStatement
• ImportDeclaration
• ExportNamedDeclaration[declaration=null]
• ExportAllDeclaration
• Statements that the previous token of the semicolon is the right brace of an arrow function body.

What actually happened? Please include the actual, raw output from ESLint.

The semi rule does not warn those semicolons.

---

EDIT: After discussion, I changed this bug report to an enhancement proposal: #9521 (comment) and #9521 (comment)

[not-an-aardvark]

I think some users might prefer to always have a semicolon before ( and [ for consistency, even if there is no ASI hazard in those cases (e.g. after do-while.

cc @feross

[mysticatea]

I see. In that case, I'd like to add an option to control (require/disallow) the semicolons before (/[. Currently, semi rule merely ignores such semicolons.

For example:

/*eslint semi: [error, never, { beforeUnreliableLineButSafe: "any" }] */
import f from "f" //← OK either way. This is the current behavior. This is default.
[1,2,3].forEach(f)

/*eslint semi: [error, never, { beforeUnreliableLineButSafe: "always" }] */
import f from "f" //← Error: require semi. (note: `semi-style` rule will move the semicolon to the beginning of the next line in auto-fixing.)
[1,2,3].forEach(f)

/*eslint semi: [error, never, { beforeUnreliableLineButSafe: "never" }] */
import f from "f"; //← Error: disallow semi.
[1,2,3].forEach(f)

But I'm not sure better option name.

---

A side note, indent rule seems wrong after import declarations.

/*eslint indent: error */
import f from "f"
;[1,2,3].forEach(f) //← Error: expected indentation of 4 spaces but found 0. (indent)

[mysticatea]

I'd like to champion this new option.

I think the option is helpful for consistency of semicolons.

• { beforeUnreliableLineButSafe: "always" } requires semicolons before [ or ( always.
• { beforeUnreliableLineButSafe: "never" } disallow extra semicolons.
• { beforeUnreliableLineButSafe: "any" } would keep the current bahavior. This is default.

I'll implement this if this is accepted.

@eslint/eslint-team What do you think?

[aladdin-add]

hmm..always have a semi before ( and [ is more straightforward. so very few people would like to use the new option. But I'm fine to add it as long as it's not the default. 😄

[mysticatea]

@aladdin-add

always have a semi before ( and [ is more straightforward

Yeah, the { beforeUnreliableLineButSafe: "always" } will support it. The current semi rule allows missing semicolon before ( and [.

---

08b654c is a reference implementation.