ESLint is missing rules for blank lines after blocks (section 19.7)

[jfrej]

Section 19.7 requires a blank line after blocks and before the next statement. This uses JSCS requirePaddingNewLinesAfterBlocks but the equivalent padding-line-between-statements is missing from the base ESLint config.

I believe ESLint base config should include something like this:

    "padding-line-between-statements": [
        "error",
        { "blankLine": "always", "prev": ["const", "let", "var", "block", "block-like"], "next": "*" }
    ]

[ljharb]

I tried out this config, and it's a bit too aggressive in some ways, and too relaxed in others.

We'd want variables to be able to be grouped; and we'd want a line after class, and module.exports =. The jscs config drifted pretty frequently from the actual guide, sadly :-/

Do you have a suggestion how to fine-tune this so that:

const foo = {};
let bar = {};

did not warn, for example?

[jfrej]

Ah, fair point about variables. How about this:

    "padding-line-between-statements": [
        "error",
        {
            "blankLine": "always",
            "prev": "*",
            "next": [
                "block",
                "block-like",
                "cjs-export",
                "class",
                "const",
                "export",
                "import",
                "let",
                "var"
            ]
        },
        {
            "blankLine": "always",
            "prev": [
                "block",
                "block-like",
                "cjs-export",
                "class",
                "const",
                "export",
                "import",
                "let",
                "var"
            ],
            "next": "*"
        },
        {
            "blankLine": "never",
            "prev": ["const", "let", "var"],
            "next": ["const", "let", "var"]
        },
        { "blankLine": "any", "prev": ["export", "import"], "next": ["export", "import"] }
    ]

I wasn't sure if blank line between variable declarations is forbidden or optional. If optional, we should have this instead:

       {
            "blankLine": "any",
            "prev": ["const", "let", "var"],
            "next": ["const", "let", "var"]
        },

I added the module.export = ("cjs-export") you asked for although the guide actually forbids this style in 10.1.

Should I write tests for this?

[jfrej]

Actually, I've just searched the guide and it's not clear whether blank lines after variable declarations are required or not. 19.7 only talks about variables using blocks.

Do you have an opinion about blank lines after simple variable declarations? So for example, should there be a blank line between these:

const foo = 'Foo';
bar();

Also, 10.5 gives this example with const and export, should there be a blank line between these:

// good
const foo = 3;
export { foo };

If not then the rules in my previous comment should be adjusted to allow that.

Generally it doesn't help that some examples in the guide don't follow the blank line rules. For instance, I believe there should be a blank line after the block in the first "good" example in 11.1:

// good
let sum = 0;
numbers.forEach((num) => {
  sum += num;
});
sum === 15;

But what about before the block?

UPDATE: I checked and I can't find a way to enforce a blank line here without enforcing blank lines after all variable declarations:

// good
const obj = {
  foo() {
  },

  bar() {
  },
};

return obj;

This might be a bug in eslint, though. "block-like" should match objects.

UPDATE2: In the example above, blank line above bar() { is not enforceable at the moment, this will be addressed in https://github.com/eslint/eslint/issues/9048

On the other hand there is lines-between-class-members that you might want to add:

"lines-between-class-members": ["error", "always"],

[ljharb]

Blank lines between variable declarations are allowed but not required.

I'll give your suggestion a shot, with the "any" modification.

[ljharb]

OK, that dropped our codebase down to 6 thousand errors (－‸ლ)

Here's some more patterns that should be allowed:

      case 1:
        foo = ['some string'];
        if (this.props.inquiryDatesAvailable) { // it wants a blank line before this line
          buttons.push('PREAPPROVE_NEW');
        }

        break;
      case 2:

I kind of think that maybe array literals shouldn't be included?

[jfrej]

That code was breaking because my rules required a blank line both before and after blocks.

I believe the following matches the rules as described in the guide:

    'padding-line-between-statements': [
      'error',
      {
        blankLine: 'always',
        prev: [
          'block',
          'block-like',
          'cjs-export',
          'class',
          'export',
          'import'
        ],
        next: '*'
      },
      { blankLine: 'any', prev: ['export', 'import'], next: ['export', 'import'] }
    ],

I added the rule above and some tests here if you want to have a look: https://github.com/jfrej/javascript/tree/padding-line-between-statements/packages/eslint-config-airbnb-base Tests: https://github.com/jfrej/javascript/blob/padding-line-between-statements/packages/eslint-config-airbnb-base/test/test-padding-line-between-statements.js

I can add more test cases if there are other examples you'd like covered.

I removed const, let and var from the rule because the guide doesn't specify line spacing rules for variable declaration. But as I mentioned before, if you want to enforce a blank line after object declarations, you'd have to also enforce blank lines after all variable declarations (apart from when followed by another variable declaration, of course).

[ljharb]

Thanks, your latest suggestion dropped it down to 2500 errors, much more reasonable. I'll review them more in depth.

[jfrej]

Any progress on this?

[JAMES025]

On?

On Jan 15, 2018 12:41 PM, "jfrej" [email protected] wrote:

Any progress on this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/airbnb/javascript/issues/1660#issuecomment-357659253, or mute the thread https://github.com/notifications/unsubscribe-auth/AhwZll28QdNzu0A00XaJUb4hjBbYEj-kks5tKzlogaJpZM4RB_r8 .

[ljharb]

No, not quite yet. Thanks for being patient :-)

[rdsedmundo]

I really I'd like to see that being covered by Airbnb's standards.

I really find annoying seeing code like that:

const a = 'airbnb';
let c = '';
if (b(a) === 'string') {
  c = '-standards';
}
return a + c;

I didn't know that ESLint had a rule that could cover that since Airbnb package never complained about that. It's difficult to enforce a consistent pattern for all developers regarding that, so I'm looking forward to having this introduced into the package.

[DKavaliou-by]

I saw, that this issue have solution under related ticket https://github.com/eslint/eslint/issues/10731#issuecomment-429710128

Could someone confirm that it works and close this one issue? CC: @ljharb @jfrej ?

[jfrej]

@DKavaliou-by this issue is about modifying the Airbnb ESLint config to match the rules described in the Airbnb JavaScript code style guide (section 19.7). The issue you linked to describes a way to enforce the rules but does not modify the Airbnb ESLint config and therefore does not solve my issue.

I already suggested a solution that enforces the rules here: https://github.com/airbnb/javascript/issues/1660#issuecomment-353691340 But, if I understand correctly, the problem is that Airbnb's own code does not follow the rules around the blank lines and applying my solution would cause many linting errors.

This issue should remain open until either my solution is added to the config or the JS guide is changed to no longer require the blank lines.

[DKavaliou-by]

Thanks for explanation

[jabacchetta]

Looking forward to seeing this implemented as well.

Curious, what's the logic on allowing no blank lines before the block, if one is required after the block?

How about blank lines before the block as well, unless that block is the first statement inside another block (no padded blocks).

[ljharb]

Often, there’s statements right before the block that directly relate to it, setting up variables etc.