Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--fix mangles lines with trailing whitespace #6933

Closed
eviljoe opened this issue Aug 18, 2016 · 8 comments
Closed

--fix mangles lines with trailing whitespace #6933

eviljoe opened this issue Aug 18, 2016 · 8 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules

Comments

@eviljoe
Copy link

eviljoe commented Aug 18, 2016

What version of ESLint are you using?
3.3.0

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

Please show your full configuration:

{
    "root": true,

    "parserOptions": {
        "ecmaVersion": 5,
        "sourceType": "script"
    },

    "env": {
        "browser": true,
        "node": true
    },

    "rules": {
        # Possible Errors
        "no-cond-assign": ["error", "except-parens"],
        "no-console": ["warn"],
        "no-constant-condition": ["warn"],
        "no-control-regex": ["error"],
        "no-debugger": ["warn"],
        "no-dupe-args": ["error"],
        "no-dupe-keys": ["error"],
        "no-duplicate-case": ["error"],
        "no-empty": ["warn"],
        "no-empty-character-class": ["error"],
        "no-ex-assign": ["error"],
        "no-extra-boolean-cast": ["error"],
        "no-extra-parens": ["off"],
        "no-extra-semi": ["warn"],
        "no-func-assign": ["error"],
        "no-inner-declarations": ["error"],
        "no-invalid-regexp": ["error"],
        "no-irregular-whitespace": ["error"],
        "no-negated-in-lhs": ["error"],
        "no-obj-calls": ["error"],
        "no-regex-spaces": ["warn"],
        "no-sparse-arrays": ["error"],
        "no-unexpected-multiline": ["error"],
        "no-unreachable": ["warn"],
        "use-isnan": ["error"],
        "valid-jsdoc": ["warn"],
        "valid-typeof": ["error"],
        "no-unsafe-finally": ["error"],
        "no-prototype-builtins": ["off"],

        # Best Practices
        "accessor-pairs": ["error"],
        "array-callback-return": ["warn"],
        "block-scoped-var": ["error"],
        "complexity": ["warn", 10],
        "consistent-return": ["warn"],
        "curly": ["error", "all"],
        "default-case": ["warn"],
        "dot-location": ["warn", "property"],
        "dot-notation": ["warn"],
        "eqeqeq": ["error"],
        "guard-for-in": ["error"],
        "no-alert": ["warn"],
        "no-caller": ["error"],
        "no-case-declarations": ["error"],
        "no-div-regex": ["error"],
        "no-else-return": ["warn"],
        "no-empty-function": ["warn"],
        "no-empty-pattern": ["warn"],
        "no-eq-null": ["error"],
        "no-eval": ["error"],
        "no-extend-native": ["error"],
        "no-extra-bind": ["error"],
        "no-extra-label": ["error"],
        "no-fallthrough": ["warn"],
        "no-floating-decimal": ["warn"],
        "no-implicit-coercion": ["off"],
        "no-implicit-globals": ["error"],
        "no-implied-eval": ["error"],
        "no-invalid-this": ["error"],
        "no-iterator": ["error"],
        "no-labels": ["error"],
        "no-lone-blocks": ["error"],
        "no-loop-func": ["warn"],
        "no-magic-numbers": ["warn", {ignore: [-1, 0, 1, 2]}],
        "no-multi-spaces": ["warn"],
        "no-multi-str": ["error"],
        "no-native-reassign": ["error"],
        "no-new": ["error"],
        "no-new-func": ["error"],
        "no-new-wrappers": ["error"],
        "no-octal": ["error"],
        "no-octal-escape": ["error"],
        "no-param-reassign": ["warn"],
        "no-proto": ["error"],
        "no-redeclare": ["error"],
        "no-return-assign": ["error"],
        "no-script-url": ["error"],
        "no-self-assign": ["error"],
        "no-self-compare": ["error"],
        "no-sequences": ["error"],
        "no-throw-literal": ["error"],
        "no-unmodified-loop-condition": ["error"],
        "no-unused-expressions": ["warn"],
        "no-unused-labels": ["error"],
        "no-useless-call": ["error"],
        "no-useless-concat": ["warn"],
        "no-void": ["error"],
        "no-warning-comments": ["off"],
        "no-with": ["error"],
        "radix": ["warn"],
        "vars-on-top": ["off"],
        "wrap-iife": ["error"],
        "yoda": ["warn"],

        # Strict Mode
        "strict": ["error"],

        # Variables
        "init-declarations": ["off"],
        "no-catch-shadow": ["error"],
        "no-delete-var": ["error"],
        "no-label-var": ["error"],
        "no-restricted-globals": ["warn", "document", "window"],
        "no-shadow": ["warn"],
        "no-shadow-restricted-names": ["error"],
        "no-undef-init": ["warn"],
        "no-undef": ["error"],
        "no-undefined": ["error"],
        "no-unused-vars": ["warn", {"vars": "all", "args": "none"}],
        "no-use-before-define": ["error", {"functions": false, "classes": false}],

        # Stylistic Issues
        "array-bracket-spacing": ["warn"],
        "block-spacing": ["warn", "never"],
        "brace-style": ["warn"],
        "camelcase": ["warn"],
        "comma-dangle": ["warn", "only-multiline"],
        "comma-spacing": ["warn"],
        "comma-style": ["warn"],
        "computed-property-spacing": ["warn"],
        "consistent-this": ["warn", "ctrl", "fltr", "page", "srv"],
        "eol-last": ["warn", "windows"],
        "func-names": ["off"],
        "func-style": ["off"],
        "id-blacklist": ["off"],
        "id-length": ["off"],
        "id-match": ["warn", "^_?([0-9A-Za-z_$]+)*$"],
        "indent": ["warn", 4, {"SwitchCase": 1}],
        "jsx-quotes": ["warn"],
        "key-spacing": ["off"],
        "keyword-spacing": ["warn", {
            "before": false,
            "after": true,
            "overrides": {
                "catch": {"before": true, "after": true},
                "default": {"before": true},
                "else": {"before": true},
                "finally": {"before": true},
                "for": {"after": true},
                "from": {"before": true},
                "if": {"after": true},
                "super": {"after": false},
                "switch": {"after": true},
                "this": {"before": true},
                "while": {"after": true}
            }
        }],
        "linebreak-style": ["warn", "windows"],
        "lines-around-comment": ["off"],
        "max-depth": ["warn", 4],
        "max-len": ["warn", {"code": 120}],
        "max-lines": ["off"],
        "max-nested-callbacks": ["warn"],
        "max-params": ["off"],
        "max-statements-per-line": ["warn", {"max": 1}],
        "max-statements": ["warn", 10, {"ignoreTopLevelFunctions": true}],
        "multiline-ternary": ["off"],
        "new-cap": ["warn"],
        "new-parens": ["warn"],
        "newline-after-var": ["off"],
        "newline-before-return": ["warn"],
        "newline-per-chained-call": ["off"],
        "no-array-constructor": ["error"],
        "no-bitwise": ["off"],
        "no-continue": ["warn"],
        "no-inline-comments": ["off"],
        "no-lonely-if": ["warn"],
        "no-mixed-operators": ["off"],
        "no-mixed-spaces-and-tabs": ["warn"],
        "no-multiple-empty-lines": ["warn", {"max": 1, "maxBOF": 0, "maxEOF": 1}],
        "no-negated-condition": ["off"],
        "no-nested-ternary": ["warn"],
        "no-new-object": ["warn"],
        "no-plusplus": ["off"],
        "no-restricted-syntax": ["error", "WithStatement"],
        "no-spaced-func": ["warn"],
        "no-ternary": ["off"],
        "no-trailing-spaces": ["warn", {"skipBlankLines": true}],
        "no-underscore-dangle": ["off"],
        "no-unneeded-ternary": ["warn"],
        "no-whitespace-before-property": ["warn"],
        "object-curly-newline": ["off"],
        "object-curly-spacing": ["warn", "never"],
        "object-property-newline": ["warn", {"allowMultiplePropertiesPerLine": true}],
        "one-var-declaration-per-line": ["warn", "always"],
        "one-var": ["warn", "never"],
        "operator-assignment": ["warn"],
        "operator-linebreak": ["warn"],
        "padded-blocks": ["warn", "never"],
        "quote-props": ["warn", "as-needed"],
        "quotes": ["warn", "single", {"avoidEscape": true, "allowTemplateLiterals": false}],
        "require-jsdoc": ["off"],
        "semi-spacing": ["warn"],
        "semi": ["warn"],
        "sort-vars": ["off"],
        "space-before-blocks": ["warn", "always"],
        "space-before-function-paren": ["warn", "never"],
        "space-in-parens": ["warn"],
        "space-infix-ops": ["warn", {"int32Hint": true}],
        "space-unary-ops": ["warn", {"words": true, "nonwords": false}],
        "spaced-comment": ["warn", "always"],
        "unicode-bom": ["warn"],
        "wrap-regex": ["off"]
    }
}

What did you do? Please include the actual source code causing the issue.
When attempting to use the --fix option on the file below, ESLint mangles the source file by incorrectly removing characters from lines with trailing whitespace.

This is the command that I ran:

eslint --fix test.js

This is the original file. The lines that end with .$dirty || have a single space at the end of the line.

var _ = require('lodash');

function Controller($scope){

    var ctrl = this;

    function save(){
        if ($scope.foo.bar.$dirty){
            ctrl.foo.bars.forEach(function(bar,index) {
                bar.dirty = $scope.foo.bar['first'+index].$dirty || 
                            $scope.foo.bar['add'+index].$dirty || 
                            $scope.foo.bar['replace'+index].$dirty;
            });
        }
    }
}

What did you expect to happen?
I ran this command on the file:

eslint --fix test.js

I expected ESLint to remove the whitespace character from the end of the line.

What actually happened? Please include the actual, raw output from ESLint.
This is the file after the command eslint --fix test.js is run:

var _ = require('lodash');

function Controller($scope) {
    var ctrl = this;

    function save() {
        if ($scope.foo.bar.$dirty) {
            ctrl.foo.bars.forEach(function(bar, index) {
                bar.dirty = $scope.foo.bar['first' + index]ty || 
                            $scope.foo.bar['add' + index]ty || 
                            $scope.foo.bar['replace' + index].$dirty;
            });
        }
    }
}

The lines that previously ended in .$dirty || now end in ty ||. ESLint removed .dir from those lines.

This is the output from ESLint:

C:\Users\joe\Desktop\eslint-bug\test.js
  9:60  error  Parsing error: Unexpected token ty

✖ 1 problem (1 error, 0 warnings)
@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Aug 18, 2016
@gyandeeps
Copy link
Member

gyandeeps commented Aug 18, 2016

I cant reproduce this issue, here is what i did:

test.js

/* eslint "no-trailing-spaces": ["warn", {"skipBlankLines": true}] */
var _ = require('lodash');

function Controller($scope){

    var ctrl = this;

    function save(){
        if ($scope.foo.bar.$dirty){
            ctrl.foo.bars.forEach(function(bar,index) {
                bar.dirty = $scope.foo.bar['first'+index].$dirty || 
                            $scope.foo.bar['add'+index].$dirty || 
                            $scope.foo.bar['replace'+index].$dirty;
            });
        }
    }
}

Run:

GS025879@WIN1551863 MINGW64 ~/Documents/webstrom/test
$ ./node_modules/.bin/eslint -v
v3.3.1

GS025879@WIN1551863 MINGW64 ~/Documents/webstrom/test
$ ./node_modules/.bin/eslint test.js --no-eslintrc --color

C:\Users\gs025879\Documents\webstrom\test\test.js
  11:68  warning  Trailing spaces not allowed  no-trailing-spaces
  12:66  warning  Trailing spaces not allowed  no-trailing-spaces

✖ 2 problems (0 errors, 2 warnings)


GS025879@WIN1551863 MINGW64 ~/Documents/webstrom/test
$ ./node_modules/.bin/eslint test.js --no-eslintrc --color --fix

GS025879@WIN1551863 MINGW64 ~/Documents/webstrom/test
$

Final file:

/* eslint "no-trailing-spaces": ["warn", {"skipBlankLines": true}] */
var _ = require('lodash');

function Controller($scope){

    var ctrl = this;

    function save(){
        if ($scope.foo.bar.$dirty){
            ctrl.foo.bars.forEach(function(bar,index) {
                bar.dirty = $scope.foo.bar['first'+index].$dirty ||
                            $scope.foo.bar['add'+index].$dirty ||
                            $scope.foo.bar['replace'+index].$dirty;
            });
        }
    }
}

UPDATE:
I even tried the whole config from the issue, still no issues. Everything worked as expected.

@platinumazure
Copy link
Member

I've seen this sort of thing happen at my work, but I could never reliably reproduce it.

Can you please reduce the test case? For example, if we're fairly sure that no-trailing-spaces is the rule which is linting/fixing and causing this problem, see what happens when your config only has no-trailing-spaces in its rules list. Does that still reproduce the error?

@eviljoe
Copy link
Author

eviljoe commented Aug 18, 2016

When I turned that rule off, the problem was still happening so maybe it is not that rule. I was also not able to recreate the problem when I copied the source code from this bug listing and tried it again. Here is a ZIP of the exact file and .eslintrc that I am using. I was able to unzip this and recreate the problem.

eslint-bug-recreate.zip

@mysticatea mysticatea added bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Aug 18, 2016
@mysticatea
Copy link
Member

I could reproduce with the zip.
I will investigate it.

@mysticatea mysticatea added rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Aug 18, 2016
mysticatea added a commit that referenced this issue Aug 18, 2016
If `skipBlankLines` option is enabled, counting total length for blank
lines was lacking.
@gyandeeps
Copy link
Member

@mysticatea Just curious, what was the difference between the code in zip file vs the one on the issue?

@eviljoe
Copy link
Author

eviljoe commented Aug 19, 2016

The one in the zip has four spaces on the line below var ctrl = this; (it is a whitespace only line). I guess when I originally copied and pasted to the ticket that whitespace somehow got lost.

@gyandeeps
Copy link
Member

ah, i see. Thanks @eviljoe for explaining.

@mysticatea
Copy link
Member

Yup. The different is 4 whitespaces in a blank line.

var _ = require('lodash');

function Controller($scope){

    var ctrl = this;
    // ← THERE WERE 4 SPACES.
    function save(){
        if ($scope.foo.bar.$dirty){
            ctrl.foo.bars.forEach(function(bar,index) {
                bar.dirty = $scope.foo.bar['first'+index].$dirty ||  // ← When it removes this trailing space, it removed "r" at the 6 chars (4 spaces and \r\n) left side from the trailing space because the blank line was ignored.
                            $scope.foo.bar['add'+index].$dirty || 
                            $scope.foo.bar['replace'+index].$dirty;
            });
        }
    }
}

mysticatea added a commit that referenced this issue Aug 19, 2016
If `skipBlankLines` option is enabled, counting total length for blank
lines was lacking.
nzakas pushed a commit that referenced this issue Aug 21, 2016
If `skipBlankLines` option is enabled, counting total length for blank
lines was lacking.
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

5 participants