By analyzing the blame information on this pull request, we identified @kaicataldo to be a potential reviewer

Thanks for the pull request, @diki! I took a look to make sure it's ready for merging and found some changes are needed:

• Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

Thanks, @diki! Can you add some tests to this to make sure we don't inadvertently reintroduce this bug later?

@btmills 👍

I'm not at my computer right now, but it looks like blocks that only contain comments aren't getting calculated correctly. Do you know what exactly is happening?

I'd love to see a few more tests for other scenarios (if statements, etc.).

Would you mind writing the actual cause in the comment rather than just linking to the issue? Makes it easier for the next person :)

Thanks for fixing this!

I'd also love to see the inverse invalid cases tested as well, if possible :)

@kaicataldo

https://github.com/eslint/eslint/blob/master/lib/rules/newline-before-return.js#L70
sourceCode.getComments(node).leading miscalculates comment lines for return if there are no other expressions between return and block upside of return. That is; method counts for the comment lines inside block while it should only count for comment lines between return and block statement,

fail

function main() {
  var test = {
    //
  };

  //
  return 1;
}

calcCommentLines = 2

also fail

function main() {
  var test = {
    //
   /*

   */
  };

  //
  return 1;
}

calcCommentLines = 5

fail

function main() {
  var a = 5;
  if(true) {
    //
  }

  return 1;
}

fail if it is if statement

function main() {
  if(true) {
    //
  }

  return 1;
}

last fail :)

function main() {
  var a = 5;
  if(true) {
    var foo = 6;
    //
  }

  return 1;
}

pass

function main() {
  var test = {
    //
  };
  var foo = 5;

  //
  return 1;
}
calcCommentLines = 1

Fixing getCommentsmethod on parser (espree?) would be much nicer yet this commit quickly fixes this issue.

Ah, got it - thanks for the thorough explanation! I've asked in the issue of this is intended behavior or a bug, so hopefully we can understand why that's the case. Great work here!

Looks good to me, Thank you!

👍

Thanks for doing this!