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

Commit

Permalink
Errors: add ugly exception for validateQuoteMarks of position calc
Browse files Browse the repository at this point in the history
Fixes #2223
  • Loading branch information
markelog committed May 13, 2016
1 parent 819e75c commit 8441d6f
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 51 deletions.
16 changes: 11 additions & 5 deletions lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ Errors.prototype = {
*/
calculateErrorLocations: function(tokenIndex) {
this._errorList.forEach(function(error) {
var pos = Errors.getPosition(error.element, error.offset, tokenIndex);
var pos = Errors.getPosition(error, tokenIndex);
error.line = pos.line;
error.column = pos.column;
});
Expand Down Expand Up @@ -286,18 +286,24 @@ function renderPointer(column, colorize) {
/**
* Get position of the element
*
* @param {Object} [element]
* @param {Number} [offset]
* @param {Error} [error]
* @param {TokenIndex} [tokenIndex]
* @return {Object}
*/
Errors.getPosition = function(element, offset, tokenIndex) {
Errors.getPosition = function(error, tokenIndex) {
var element = error.element;
var offset = error.offset;
var rule = error.rule;

if (!element) {
return EMPTY_POS;
}

if (offset === undefined) {
if (element.getSourceCodeLength() === 1) {
// TODO: probably should be generalized
if (rule === 'validateQuoteMarks') {
offset = 0;
} else if (element.getSourceCodeLength() === 1) {
offset = 0;
} else {
offset = (element.getNewlineCount() === 0 && Math.ceil(element.getSourceCodeLength() / 2)) || 0;
Expand Down
101 changes: 83 additions & 18 deletions test/specs/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,89 @@ describe('errors', function() {
describe('getPosition', function() {
it('should get position', function() {
var position = Errors.getPosition({
getNewlineCount: function() {
return 0;
},
getSourceCodeLength: function() {
return 1;
},
getLoc: function() {
return {
start: {
line: 1,
column: 10
},

end: {
line: 1,
column: 11
}
};
element: {
getNewlineCount: function() {
return 0;
},
getSourceCodeLength: function() {
return 1;
},
getLoc: function() {
return {
start: {
line: 1,
column: 10
},

end: {
line: 1,
column: 20
}
};
}
}
});

expect(position).to.deep.equal({
line: 1,
column: 10
});
});

it('should get position for element with length > 1', function() {
var position = Errors.getPosition({
element: {
getNewlineCount: function() {
return 0;
},
getSourceCodeLength: function() {
return 10;
},
getLoc: function() {
return {
start: {
line: 1,
column: 10
},

end: {
line: 1,
column: 20
}
};
}
}
});

expect(position).to.deep.equal({
line: 1,
column: 15
});
});

it('should set position on the first char for `validateQuoteMarks` rule', function() {
var position = Errors.getPosition({
rule: 'validateQuoteMarks',
element: {
getNewlineCount: function() {
return 0;
},
getSourceCodeLength: function() {
return 10;
},
getLoc: function() {
return {
start: {
line: 1,
column: 10
},

end: {
line: 1,
column: 20
}
};
}
}
});

Expand Down
4 changes: 2 additions & 2 deletions test/specs/rules/disallow-curly-braces.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ describe('rules/disallow-curly-braces', function() {
'}'
].join('\n')).getErrorList()[ 0 ];

expect(getPosition(error.element).column).to.equal(5);
expect(getPosition(error.element).line).to.equal(4);
expect(getPosition(error).column).to.equal(5);
expect(getPosition(error).line).to.equal(4);
});

it('should report for a block with 1 statement', function() {
Expand Down
8 changes: 4 additions & 4 deletions test/specs/rules/disallow-trailing-comma.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ describe('rules/disallow-trailing-comma', function() {

it('should report right location for trailing comma in object (#1018)', function() {
var errs = checker.checkString('var obj = {\n foo: "foo",\n};').getErrorList();
expect(getPosition(errs[0].element).line).to.equal(2);
expect(getPosition(errs[0].element).column).to.equal(14);
expect(getPosition(errs[0]).line).to.equal(2);
expect(getPosition(errs[0]).column).to.equal(14);
});

it('should report right location for trailing comma in array (#1018)', function() {
var errs = checker.checkString('var arr = [\n \'foo\',\n];').getErrorList();
expect(getPosition(errs[0].element).line).to.equal(2);
expect(getPosition(errs[0].element).column).to.equal(9);
expect(getPosition(errs[0]).line).to.equal(2);
expect(getPosition(errs[0]).column).to.equal(9);
});

it('should not throw on literal', function() {
Expand Down
4 changes: 2 additions & 2 deletions test/specs/rules/require-curly-braces.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,8 @@ describe('rules/require-curly-braces', function() {
'}'
].join('\n')).getErrorList()[ 0 ];

expect(getPosition(error.element).column).to.equal(2);
expect(getPosition(error.element).line).to.equal(4);
expect(getPosition(error).column).to.equal(2);
expect(getPosition(error).line).to.equal(4);
});

it('should not report missing `else` braces for `else if`', function() {
Expand Down
4 changes: 2 additions & 2 deletions test/specs/rules/require-trailing-comma.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ describe('rules/require-trailing-comma', function() {
});

it('should report right location for no trailing comma in object (#1018)', function() {
var errs = checker.checkString('var obj = {\n foo: "foo"\n};').getErrorList()[0].element;
var errs = checker.checkString('var obj = {\n foo: "foo"\n};').getErrorList()[0];
expect(getPosition(errs).line + ':' + getPosition(errs).column).to.equal('2:12');
});

it('should report right location for no trailing comma in array (#1018)', function() {
var errs = checker.checkString('var arr = [\n \'foo\'\n];').getErrorList()[0].element;
var errs = checker.checkString('var arr = [\n \'foo\'\n];').getErrorList()[0];
expect(getPosition(errs).line + ':' + getPosition(errs).column).to.equal('2:7');
});
});
Expand Down
36 changes: 18 additions & 18 deletions test/specs/token-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ describe('token-assert', function() {

var error = onError.getCall(0).args[0];
expect(error.message).to.contain('Missing space between x and =');
expect(getPosition(error.element).line).to.equal(1);
expect(getPosition(error.element).column).to.equal(0);
expect(getPosition(error).line).to.equal(1);
expect(getPosition(error).column).to.equal(1);
});

it('should accept message for missing whitespace between tokens', function() {
Expand Down Expand Up @@ -116,8 +116,8 @@ describe('token-assert', function() {
var error = onError.getCall(0).args[0];

expect(error.message).to.contain('2 spaces required between x and =');
expect(getPosition(error.element).line).to.equal(1);
expect(getPosition(error.element).column).to.equal(0);
expect(getPosition(error).line).to.equal(1);
expect(getPosition(error).column).to.equal(1);
});

it('should not trigger error on newline between tokens', function() {
Expand Down Expand Up @@ -219,8 +219,8 @@ describe('token-assert', function() {

var error = onError.getCall(0).args[0];
expect(error.message).to.contain('at most 1 spaces required between x and =');
expect(getPosition(error.element).line).to.equal(1);
expect(getPosition(error.element).column).to.equal(0);
expect(getPosition(error).line).to.equal(1);
expect(getPosition(error).column).to.equal(1);
});

it('should not trigger error on valid space count between tokens', function() {
Expand Down Expand Up @@ -280,8 +280,8 @@ describe('token-assert', function() {

var error = onError.getCall(0).args[0];
expect(error.message).to.contain('at most 1 spaces required between x and =');
expect(getPosition(error.element).line).to.equal(1);
expect(getPosition(error.element).column).to.equal(0);
expect(getPosition(error).line).to.equal(1);
expect(getPosition(error).column).to.equal(1);
});

it('should trigger plural error on invalid maximum space count between tokens', function() {
Expand All @@ -303,8 +303,8 @@ describe('token-assert', function() {

var error = onError.getCall(0).args[0];
expect(error.message).to.contain('at most 2 spaces required between x and =');
expect(getPosition(error.element).line).to.equal(1);
expect(getPosition(error.element).column).to.equal(0);
expect(getPosition(error).line).to.equal(1);
expect(getPosition(error).column).to.equal(1);
});

it('should not trigger error on newline between tokens for maximum spaces', function() {
Expand Down Expand Up @@ -382,8 +382,8 @@ describe('token-assert', function() {

var error = onError.getCall(0).args[0];
expect(error.message).to.contain('Unexpected whitespace between x and =');
expect(getPosition(error.element).line).to.equal(1);
expect(getPosition(error.element).column).to.equal(0);
expect(getPosition(error).line).to.equal(1);
expect(getPosition(error).column).to.equal(1);
});

it('should not trigger error on newline between tokens', function() {
Expand Down Expand Up @@ -422,8 +422,8 @@ describe('token-assert', function() {

var error = onError.getCall(0).args[0];
expect(error.message).to.contain('Unexpected whitespace between x and =');
expect(getPosition(error.element).line).to.equal(1);
expect(getPosition(error.element).column).to.equal(0);
expect(getPosition(error).line).to.equal(1);
expect(getPosition(error).column).to.equal(1);
});

it('should not trigger error on missing whitespace between tokens', function() {
Expand Down Expand Up @@ -482,8 +482,8 @@ describe('token-assert', function() {
var error = onError.getCall(0).args[0];
expect(error.message).to.contain('x and = should be on the same line');

expect(getPosition(error.element).line).to.equal(1);
expect(getPosition(error.element).column).to.equal(0);
expect(getPosition(error).line).to.equal(1);
expect(getPosition(error).column).to.equal(1);
});

it('should not trigger error on missing newline between tokens', function() {
Expand Down Expand Up @@ -568,8 +568,8 @@ describe('token-assert', function() {

var error = onError.getCall(0).args[0];
expect(error.message).to.contain('x and = should be on different lines');
expect(getPosition(error.element).line).to.equal(1);
expect(getPosition(error.element).column).to.equal(0);
expect(getPosition(error).line).to.equal(1);
expect(getPosition(error).column).to.equal(1);
});

it('should not trigger error on existing newline between tokens', function() {
Expand Down

0 comments on commit 8441d6f

Please sign in to comment.