Disallow statements starting with continuation characters

[christianbundy]

What version of this package are you using? 14.1.0

What problem do you want to solve? I'd like to continue the semicolon purge by disallowing semicolons as a workaround for lines that start with [, (, `, +, *, /, -, ,, or .. In other words:

Note: If you're often writing code like this, you may be trying to be too clever.

Clever short-hands are discouraged, in favor of clear and readable expressions, whenever possible.

What do you think is the correct solution to this problem??

{ "beforeStatementContinuationChars": "never" }

Before:

;(function () {
  window.alert('ok')
  var x = 42
}())
 
;[1, 2, 3].forEach(bar) 

;`hello`.indexOf('o')

After:

{
  // ES6 curly jail
  // All the benefits of IIFE without the mess
  window.alert('ok')
  let x = 42
}

var nums = [1, 2, 3]
nums.forEach(bar)

var word = `hello`
word.indexOf('o')

Are you willing to submit a pull request to implement this change? Absolutely.

---

Pairs well with #633.

[feross]

This is a fascinating suggestion. The first step is to look into ecosystem impact of the change by temporarily adding the rule to eslintrc.json and running the test suite. We need to get a sense of how much code would need to change and if we think the changes are a net win.

@christianbundy Want to give that a shot?

[christianbundy]

Yeah! It looks like the beforeStatementContinuationChars rule works differently than I expected -- it seems to allow semicolons when it prevents an unexpected multi-line statement, whereas the behavior I'm looking for is throwing an error on both:

• an unexpected multi-line statement
• a semicolon used to circumvent an unexpected multi-line statement

I ran the test suite on the rule as-is, which only throws errors when it sees a leading semicolon that does not circumvent an unexpected multi-line statement, which it sees as an extra semicolon, but I'm sure there will be more problems found once I fix the rule in ESLint. I'll report back when I have some more info. 👍

[christianbundy]

I made a small change to ESLint to get the behavior we're discussing. To be explicit about the new behavior:

// ✔️ fine because there's no semi or unexpected multi-line statement
[1, 2, 3].forEach((x) => console.log(x))

// ❌ error because of the unexpected multi-line statement
foo()
[1, 2, 3].forEach((x) => console.log(x))

// ❌ error because of the semicolon
foo()
;[1, 2, 3].forEach((x) => console.log(x))

// ✔️ fine because there's no semi or unexpected multi-line
foo()
var nums = [1, 2, 3]
nums.forEach((x) => console.log(x))

I re-ran the tests and it looks like only four modules needed fixes, all were super simple:

simple-get/index.js:94:1: Extra semicolon. (semi)
binary-split/test.js:40:5: Extra semicolon. (semi)
reusify/benchmarks/createNoCodeFunction.js:14:3: Extra semicolon. (semi)
bitmidi.com/src/views/adsense.mjs:9:5: Extra semicolon. (semi)

1..188
# tests 188
# pass  184
# fail  4

Note: this transfers all responsibility for avoiding unexpected multi-line expressions to no-unexpected-multiline. In the future it might be nice to disallow any statement that starts with a continuation character, but I think this change is a step in the right direction.

I've included the solution diffs below. I've made the smallest change, using `--fix` when possible, to illustrate how this rule change will interact with these projects.

simple-get

diff --git a/index.js b/index.js
index ab88116..92aad0c 100644
--- a/index.js
+++ b/index.js
@@ -91,7 +91,7 @@ simpleGet.concat = (opts, cb) => {
   })
 }
 
-;['get', 'post', 'put', 'patch', 'head', 'delete'].forEach(method => {
+['get', 'post', 'put', 'patch', 'head', 'delete'].forEach(method => {
   simpleGet[method] = (opts, cb) => {
     if (typeof opts === 'string') opts = { url: opts }
     return simpleGet(Object.assign({ method: method.toUpperCase() }, opts), cb)

binary-split

diff --git a/test.js b/test.js
index f122e08..b7230fd 100644
--- a/test.js
+++ b/test.js
@@ -37,9 +37,11 @@ test('custom matcher', function (t) {
     t.equals(items.length, 5)
     t.equals(items.join(' '), 'hello yes this is dog')
     t.end()
-  });
+  })
+
+  var strings = ['hello yes ', 'this', ' is d', 'og']
 
-  ['hello yes ', 'this', ' is d', 'og'].map(function (chunk) {
+  strings.map(function (chunk) {
     return bufferFrom ? Buffer.from(chunk) : new Buffer(chunk) // eslint-disable-line
   }).forEach(function (chunk) {
     splitStream.write(chunk)

reusify

diff --git a/benchmarks/createNoCodeFunction.js b/benchmarks/createNoCodeFunction.js
index ce1aac7..2d19fff 100644
--- a/benchmarks/createNoCodeFunction.js
+++ b/benchmarks/createNoCodeFunction.js
@@ -11,13 +11,13 @@ function createNoCodeFunction () {
   /* eslint no-constant-condition: "off" */
   var num = 100
 
-  ;(function () {
+  {
     if (null) {
       // do nothing
     } else {
       fib(num)
     }
-  })()
+  }
 }
 
 for (var i = 0; i < max; i++) {

bitmidi

diff --git a/src/views/adsense.mjs b/src/views/adsense.mjs
index ada6982..d652f32 100644
--- a/src/views/adsense.mjs
+++ b/src/views/adsense.mjs
@@ -6,7 +6,7 @@ import { isBrowser, isProd, tokens } from '../config'
 export default class Adsense extends Component {
   componentDidMount () {
     if (!isBrowser || !isProd) return
-    ;(window.adsbygoogle = window.adsbygoogle || []).push({})
+    (window.adsbygoogle = window.adsbygoogle || []).push({})
   }
 
   shouldComponentUpdate () {

[mightyiam]

Thank you for this suggestion and work, @christianbundy.

Personally, I'd like to see more examples before I can feel about this.

[christianbundy]

Update: I wrote a new rule that resolves this by throwing errors on any statement that starts with a continuation character. Documentation is available here, but in summary this prevents statements from starting with [, (, +, /, -, /, and ` regardless of semicolons.

Code that passes this rule should be able to be safely used, copied and pasted, and even stripped of semicolons without changing the semantics. Here's the new pass/fail info for the code above:

// ❌ error because of the [
[1, 2, 3].forEach((x) => console.log(x))

// ❌ error because of the [
foo()
[1, 2, 3].forEach((x) => console.log(x))

// ❌ error because of the [
foo()
;[1, 2, 3].forEach((x) => console.log(x))

// ✔️ fine because no statements start with a continuation character
foo()
var nums = [1, 2, 3]
nums.forEach((x) => console.log(x))

This was what I was originally looking for when I opened the issue, I just didn't realize that it'd require writing a new rule. 😅 It passes on 177 / 188 (95%) modules in the test suite, and I'm a bit surprised that it only fails on 9 modules with 13 errors total. I'd love feedback either here or in my ESLint PR, I'm super open to questions and suggestions.

[mightyiam]

I have felt into this and I'm okay with it. What would make it clear that this is desirable is how much more concise the semicolons section in the rules docs would be, I imagine. If you're willing to make a PR for that change, I feel that it would make it clear that this is desirable.