Enforce consistent line breaks inside braces (object-curly-newline)

[feross]

I propose that we enforce the following:

// ok
var one = { a: 1 }

// ok
var two = { a: 1, b: 2 }

// not ok, should be split onto multiple lines
var three = { a: 1, b: 2, c: 3 }

// ok, preemptively splitting onto multiple lines is always ok
var oneM = {
  a: 1
}

// ok
var twoM = {
  a: 1,
  b: 2
}

// ok
var threeM = {
  a: 1,
  b: 2,
  c: 3
}

// not ok
var wat = { a: 1
}

// not ok
var wat2 = {
a: 1 }

// not ok
var wat3 = { a: 1,
  b: 2,
  c: 3 }

The "consistent" option in this rule is not part of ESLint yet, but when it is, we should enable it:

Rule:

"object-curly-newline": ["error", { "multiline": true, "minProperties": 2, "consistent": true }]

For context, see:

• eslint/eslint#6488 (comment)
• eslint/eslint#7720

[feross]

Thanks for the feedback on this rule, everyone.

In re-reading what I proposed here, I think @mafintosh's downvote is actually the right call. We have a policy of never accepting rules which involve "magic numbers". The requirement to count the number of properties and change style based on the difference between 2 vs. 3 properties is not in the spirit of standard. These types of rules are not obvious nor easy to remember and we always want standard style code to be easy to write without requiring the use of a tool to format code.

That said, if we just remove the "minProperties": 2 part of the rule, the remaining rule is actually quite reasonable! Here's what this modified rule does:

// ok
var one = { a: 1 }

// ok
var two = { a: 1, b: 2 }

// ok
var three = { a: 1, b: 2, c: 3 }

// ok
var oneM = {
  a: 1
}

// ok
var twoM = {
  a: 1,
  b: 2
}

// ok
var threeM = {
  a: 1,
  b: 2,
  c: 3
}

// not ok
var wat = { a: 1
}

// not ok
var wat2 = {
a: 1 }

// not ok
var wat3 = { a: 1,
  b: 2,
  c: 3 }

This seems totally uncontroversial and very useful to enforce. All 4 ecosystem repos which fail after this rule is enabled actually have bad style. Here are some examples:

const options = { native: false, // not ok
    skipCovers: true,
    fileSize: file.length,
    observer: event => {
      ipc.send('wt-audio-metadata', infoHash, index, event.metadata)
    } } // not ok

  const bestScore = ['audio', 'video', 'image'].map(mediaType => {
    return {
      type: mediaType,
      size: calculateDataLengthByExtension(torrent, mediaExtensions[mediaType]) } // not ok
  })

      model.Course
        .find({ $or: [ // not ok
          { name: regexForQuery(query) },
          { searchName: regexForQuery(query) }
        ] })

  var opts = {
    pkg: { binary: { // not ok
      module_name: 'module_name',
      module_path: 'module_path'
    } }
  }

I am going to enable this in standard 14. Hope this makes sense to everyone, including you too @mafintosh!

[mightyiam]

Beautiful. Right call.