New Rule: requireNewlineBeforeBlockStatements

[cipiripper]

Solves this issue: #65

This rule gets the oppening brace of a BlockStatement, and comapres is vertical location to the previous token vertical location. If those two are equal, that is an error.

[mikesherov]

Hi @cipiripper , thanks for contributing! In order for us to land this pull request, please do the following:

1. Run the tests before submitting a PR: npm test so there are no travis failures.
2. Add new unit tests for the rule you're providing.
3. Add an entry to the README.md file describing your rule.
4. Provide the opposite rule as well: disallowNewlineBeforeBlockStatements (and tests and docs for it).

Thanks again for your help!

[cipiripper]

Oooh, didn't know about that... Sorry!

I'll follow your guide now... :)

[mikesherov]

No worries. Thanks again!

[mikesherov]

@cowwoc can you please comment on the actual PR instead of the commit so they appear inline?

[mikesherov]

round bracket -> curly brace

[cipiripper]

Okay, thanks, will fix those shortly. :)

[cowwoc]

I think the documentation should indicate that rule is applied where possible and explicitly mention that return statements are unaffected (link to http://stackoverflow.com/a/3641525/14731); otherwise people might file bugs against this.

[cipiripper]

@cowwoc Hm true.

[cowwoc]

The latest commit looks good to me.

[mikesherov]

The check for return here is not needed. Please remove.

[mikesherov]

@cipiripper one last change to make and then I think this is good to land!

[cowwoc]

@mikesherov You're going to hate me but I think we need to address the following issues as well:

1. We need to cover the closing brace.
2. We need to cover the case where the block is empty.
3. Add a parameter that adds an exception for empty blocks: they should not require newlines before each brace (by default this exception would be true).

We can add two additional rules to cover the closing brace, or rename this rule to requireNewlineBeforeCurlyBraces and have it apply to both the opening and close braces.

[mikesherov]

1. The closing brace is a separate set of rules in practice and spirit. That can be saved for another PR.
2. Yes, agreed. In that we always ignore it.
3. I'd like to see someone complain that they want

function whatever()
{}

before implementing the parameter.

[cowwoc]

Sorry for any confusion I have caused. I'm going to provide a concrete example of what I'm expecting and we can then break it down into one or more rules:

var goodObject1 =
{
  key: value
};

var goodObject2 =
{
};

var goodObject3 = {};

function goodReturn1()
{
  return {
    key: value
  };
}

function goodReturn2()
{
  return {
  };
}

function goodReturn3()
{
  return {};
}

var badObject1 = {
  key: value
};

var badObject2 = {
};

var badObject3 =
{};

function badReturn1()
{
  return {
    key: value; };
}

function badReturn2()
{
  return {
    key: value; };
}

Open questions for @mikesherov:

1. We're talking about blocks, and object declarations. Should we have separate rules for each?
2. We have rule requireSpacesInsideObjectBrackets (notice the terminology is "brackets") but the description talks about curly braces. Should this rule be renamed to requireSpacesInsideObjectBraces? This impacts the previous question in case we decide to add a separate rule for object braces.
3. We didn't discuss badObject3 before. If the object braces are empty, I'd like the open/close brace to show up on the same line as the variable declaration.
4. We need revisit requireBlocksOnNewline. It only enforces a newline before the closing brace, but not the opening brace so the name is a bit misleading. Perhaps this rule should be renamed to requireNewlineBeforeClosingBlock?

I'm trying to make sure the terminology is consistent across all rules.

One option is to break this down to 8 separate rules:

1. requireNewlineBeforeOpeningBlock
2. requireNewlineBeforeClosingBlock
3. requireNewlineBeforeOpeningObjectBraces
4. requireNewlineBeforeClosingObjectBraces

and the disallow equivalents. Let me know what you prefer.

[mikesherov]

@cowwoc can we move these discussions to new issues. I'd really only like to focus on the opening curly brace of a block statement (not objects) for this rule as they are fundamentally different.

As far as what these rules should do:

1. only operate on blocks, not objects.
2. don't error out with for same line empty blocks when paired requireNewLineBeforeBlockStatements.
3. not provide the ability to override the empty block behavior of requireNewLineBeforeBlockStatements until someone complains.

[cowwoc]

@mikesherov Okay, I'll open separate issues. Thanks.

[cowwoc]

I've filed the following new issues:

#538: requireSpacesInsideObjectBrackets using inconsistent terminology
#539: Braces should be opened on newline for objects
#540: Require newline after object declaration/assignment

[cipiripper]

👍

[mikesherov]

Thanks! Landed in 6dc228a