Skip to content
This repository has been archived by the owner on Mar 23, 2024. It is now read-only.

disallowTrailingWhitespace: Fix autofixing whitespace on last line #1978

Closed
wants to merge 1 commit into from
Closed

disallowTrailingWhitespace: Fix autofixing whitespace on last line #1978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/token-assert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,14 @@ TokenAssert.prototype.noTrailingSpaces = function(options) {
fixed = true;
}

if (!fixed && precendingToken && precendingToken.type === 'EOF') {
precendingToken = this._file.getPrevToken(precendingToken, { includeWhitespace: true });
if (precendingToken && precendingToken.isWhitespace) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have 3 tests in outer if and 2 in inner, are you sure you need all of them?
Can you cover it? That's why coveralls reporting about coverage decrease.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no I'm not sure I need them. are you sure I don't? Its the classic push-pull between defensive coding and code coverage. If you know better tell me what to change or a test that checks it. Or alternatively, maybe you prefer my more radical approach in #1981?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you don't need test precendingToken here if there is no purpose ;-).

I can't get how to break it if you remove precendingToken.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm waiting on making this change to see if the better approach in #1981 will be considered.

precendingToken.value = precendingToken.value.replace(/[^\S\n\r]$/g, '');
fixed = true;
}
}

precendingToken = null;

this.emit('error', {
Expand Down
24 changes: 24 additions & 0 deletions test/specs/rules/disallow-trailing-whitespace.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,30 @@ describe('rules/disallow-trailing-whitespace', function() {
output: '/*\nbla\n\t\n*/'
});

reportAndFix({
name: 'fixes spaces on the last line',
rules: rules,
errors: 1,
input: 'var a;\n ',
output: 'var a;\n'
});

reportAndFix({
name: 'fixes space and comment on the last line',
rules: rules,
errors: 1,
input: 'var a;\n/**/ ',
output: 'var a;\n/**/'
});

reportAndFix({
name: 'fixes spaces on the last lines',
rules: rules,
errors: 3,
input: 'var a;\n \n \n ',
output: 'var a;\n\n\n'
});

describe('option value true', function() {
beforeEach(function() {
checker.configure({ disallowTrailingWhitespace: true });
Expand Down