newline-before-return: false positive

[ghost]

What version of ESLint are you using?
v2.9.0

What parser (default, Babel-ESLint, etc.) are you using?
babel-eslint

Please show your full configuration:

module.exports = {
  "extends": "eslint:recommended",
  "parser": "babel-eslint",
  "parserOptions": {
    "sourceType": "module"
  },
  "env": {
    "browser": false,
    "es6": true,
    "node": true,
    "mocha": true
  },
  "globals": {
  },
  "rules": {
    "accessor-pairs": "error",
    "array-bracket-spacing": [
      "error",
      "never"
    ],
    "array-callback-return": "error",
    "arrow-body-style": "error",
    "arrow-parens": "error",
    "arrow-spacing": "error",
    "block-scoped-var": "error",
    "block-spacing": [
      "error",
      "never"
    ],
    "brace-style": [
      "error",
      "1tbs",
      {
        "allowSingleLine": true
      }
    ],
    "callback-return": "error",
    "comma-spacing": [
      "error",
      {
        "after": true,
        "before": false
      }
    ],
    "comma-style": [
      "error",
      "last"
    ],
    "computed-property-spacing": [
      "error",
      "never"
    ],
    "consistent-this": [
      "error",
      "self"
    ],
    "curly": "error",
    "dot-location": [
      "error",
      "property"
    ],
    "eol-last": "error",
    "eqeqeq": "error",
    "generator-star-spacing": [
      "error",
      "both"
    ],
    "global-require": "off",
    "handle-callback-err": [
      "error",
      "^(err|error|anySpecificError)$"
    ],
    "indent": "error",
    "jsx-quotes": "error",
    "key-spacing": "error",
    "keyword-spacing": "error",
    "linebreak-style": [
      "error",
      "unix"
    ],
    "new-cap": [
      "error",
      {
        "newIsCap": true,
        "capIsNew": false
      }
    ],
    "new-parens": "error",
    "newline-after-var": "error",
    "newline-before-return": "error",
    "no-array-constructor": "error",
    "no-caller": "error",
    "no-catch-shadow": "error",
    "no-cond-assign": [
      "error",
      "always"
    ],
    "no-duplicate-imports": "error",
    "no-eq-null": "error",
    "no-eval": "error",
    "no-extend-native": "error",
    "no-extra-bind": "error",
    "no-extra-label": "error",
    "no-extra-parens": [
      "error",
      "functions"
    ],
    "no-floating-decimal": "error",
    "no-implicit-coercion": "error",
    "no-implicit-globals": "error",
    "no-implied-eval": "error",
    "no-iterator": "error",
    "no-label-var": "error",
    "no-labels": "error",
    "no-lone-blocks": "error",
    "no-lonely-if": "error",
    "no-mixed-requires": "error",
    "no-multi-spaces": "error",
    "no-multi-str": "error",
    "no-multiple-empty-lines": [
      "error",
      {
        "max": 1,
        "maxEOF": 0,
        "maxBOF": 0
      }
    ],
    "no-native-reassign": "error",
    "no-nested-ternary": "error",
    "no-new": "error",
    "no-new-func": "error",
    "no-new-object": "error",
    "no-new-require": "error",
    "no-new-wrappers": "error",
    "no-octal-escape": "error",
    "no-path-concat": "error",
    "no-proto": "error",
    "no-return-assign": "error",
    "no-self-compare": "error",
    "no-sequences": "error",
    "no-shadow-restricted-names": "error",
    "no-spaced-func": "error",
    "no-sync": "error",
    "no-throw-literal": "error",
    "no-trailing-spaces": "error",
    "no-undef-init": "error",
    "no-unmodified-loop-condition": "error",
    "no-unneeded-ternary": "error",
    "no-unused-expressions": "error",
    "no-unused-vars": [
      "error",
      {
        "vars": "all",
        "args": "none"
      }
    ],
    "no-useless-call": "error",
    "no-useless-concat": "error",
    "no-useless-constructor": "error",
    "no-void": "error",
    "no-whitespace-before-property": "error",
    "no-with": "error",
    "object-curly-spacing": "error",
    "object-shorthand": "off",
    "one-var": [
      "error",
      "never"
    ],
    "one-var-declaration-per-line": "error",
    "operator-linebreak": [
        "error",
        "after"
    ],
    "padded-blocks": [
      "error",
      {
        "blocks": "never",
        "switches": "never",
        "classes": "never"
      }
    ],
    "prefer-arrow-callback": "off",
    "prefer-const": "error",
    "prefer-reflect": "off",
    "prefer-rest-params": "off",
    "prefer-spread": "off",
    "prefer-template": "off",
    "quotes": [
      "error",
      "single",
      {
        "avoidEscape": true
      }
    ],
    "radix": "error",
    "require-jsdoc": "error",
    "require-yield": "error",
    "semi": "error",
    "semi-spacing": "error",
    "space-before-blocks": "error",
    "space-before-function-paren": "error",
    "space-in-parens": "error",
    "space-infix-ops": "error",
    "space-unary-ops": [
      "error",
      {
        "words": true,
        "nonwords": false
      }
    ],
    "spaced-comment": "error",
    "template-curly-spacing": "error",
    "vars-on-top": "error",
    "wrap-iife": "error",
    "yield-star-spacing": "error",
    "yoda": "error"
  }
};

Plus .eslintrc:

{
    "extends": "isi",
    "globals": {
        "Ext": true,
        "Ext3": true,
        "Q": true,
        "Isilon": true,
        "i18n": true,
        "expect": true,
        "sinon": true,
        "breeze": true
    },
    "env": {
        "browser": true
    },
    "rules": {
        "consistent-this": [
            "error",
            "me"
        ],
        "fix-spaced-comment": [
        "error",
            "always"
        ],
    "newline-after-var": "error",
        "newline-before-return": "error",
        "no-lonely-if": "off"
    }
}

What did you do? Please include the actual source code causing the issue.

    getFolderHierarchy: function () {
        var folderHierarchy = [];
        var comboPath = this.currentLocation;

        while (comboPath.length) {
            folderHierarchy.unshift(comboPath);
            comboPath = comboPath.split('/');
            comboPath = comboPath.splice(0, comboPath.length - 1).join('/');   // remove last token
        }

        return folderHierarchy;     // <-- It errors on this line.
    }

also, here:

    determineValidity: function () {
        var validity = true;
        var andWidget;
        var j;
        var mainWidget = this.down('container[andWidgets]');
        var andWidgetLen = mainWidget.andWidgets.length;

        for (j = 0; j < andWidgetLen; j++) {
            andWidget = mainWidget.andWidgets[j];
            validity = andWidget.determineValidity();
            if (validity === false) {
                break;  // j loop
            }
        }

        return validity;  // <--- errors here
    },

What did you expect to happen?
I would expect that newline-before-return would think I had a newline before the return, as the code above shows.

What actually happened? Please include the actual, raw output from ESLint.

/static/v2/scripts/uilib/form/field/FiltersField.js
  118:9  error  Expected newline before return statement  newline-before-return

/static/v2/scripts/uilib/window/FileBrowser.js
  406:9  error  Expected newline before return statement  newline-before-return

✖ 2 problems (2 errors, 0 warnings)

[qfox]

Here is the minimal code sample reporting this error:

a=function() {
{// dummy
}

return}

Going to fix it.

[qfox]

It looks like a bug in espree. For some reason there is leading comment for return statement. Didn't find why there it is atm.

[platinumazure]

Pinging @kaicataldo: Didn't you fix something similar to this in espree recently?

[kaicataldo]

Yes - thought I had finally found a solution that covered all cases in Espree. Will take a look at this ASAP - thanks for investigating, @zxqfox!

[qfox]

Thank you for finishing this ;-)

[kaicataldo]

@zxqfox Actually, did you confirm that this issue occurs with Espree? I'm noticing the issue creator is using babel-eslint, which uses Babel's Babylon parser. Babylon uses essentially the same algorithm as Espree for comment attachment, and the changes I made to Espree that fix this haven't been ported over yet.

It's a known issue: babel/babel-eslint#289
And there's already an open PR to Babylon to fix it: babel/babylon#23

[qfox]

@kaicataldo My bad. I should run npm i before investigating. With the last version of espree all is fine.

What you think about tests? Do we need them?

[kaicataldo]

I added tests when I bumped the Espree version in ESLint, I'm always for more test coverage if there are cases we aren't testing already :D I don't remember there being tests for the cases in the original post - i.e. comments inside and at the end of 'while' and 'for' loops. The bug in question would happen when a block would end with a comment inside it - it would then get attached as a leading comment to the next node.

Sorry, not at my computer right now or I'd check!

[kaicataldo]

@justineakehurst Would you mind checking if this happens with the default parser? If not, this is unfortunately out of our hands, but it does look like the babel-eslint is aware of the issue (see comment above)

[ghost]

@kaicataldo I confirmed that when I switch the parser to 'espree' from 'babel-eslint', the problem goes away.

[kaicataldo]

@justineakehurst Thanks! I'm going to close this now for the reasons above.