Skip to content

Commit

Permalink
Fix for gh-3013 (#3016)
Browse files Browse the repository at this point in the history
* [[CHORE]] Remove `lex.start` method

This method simply proxies `lex.nextLine` to scan a single line of the
program. It is currently only used to scan the very first line of input,
which is problematic for two reasons:

1. Linting options have not yet been applied via the `assume` function,
   so linting options that effect the parser's behavior are not honored
   for this first line (see the unit test corrected in this patch for an
   example)
2. So-called "checks" for asynchronously-reported warnings on the first
   line cannot be associated with a token. This makes it impossible to
   issue warnings asynchronously for the first line.

Because the `lex.token` method itself invokes `lex.nextLine`, these
problems may be avoided by removing `lex.start` and deferring the
invocation of `nextLine` until the first token is requested.

* [[FIX]] Allow W100 to be ignored during lookahead

Defer the reporting of warning W100 until parse time (using
`triggerAsync`) so that it may be disabled via in-line directives even
in contexts where a "lookahead" is taking place.

* fixup! [[FIX]] Allow W100 to be ignored during lookahead
  • Loading branch information
jugglinmike authored and rwaldron committed Sep 22, 2016
1 parent 6aee2fd commit a2b3881
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 65 deletions.
2 changes: 0 additions & 2 deletions src/jshint.js
Expand Up @@ -5329,8 +5329,6 @@ var JSHINT = (function() {
emitter.emit("Number", ev);
});

lex.start();

// Check options
for (var name in o) {
if (_.has(o, name)) {
Expand Down
174 changes: 111 additions & 63 deletions src/lex.js
Expand Up @@ -377,7 +377,7 @@ Lexer.prototype = {
* also recognizes JSHint- and JSLint-specific comments such as
* /*jshint, /*jslint, /*globals and so on.
*/
scanComments: function() {
scanComments: function(checks) {
var ch1 = this.peek();
var ch2 = this.peek(1);
var rest = this.input.substr(2);
Expand Down Expand Up @@ -514,7 +514,7 @@ Lexer.prototype = {

// If we hit EOF and our comment is still unclosed,
// trigger an error and end the comment implicitly.
if (!this.nextLine()) {
if (!this.nextLine(checks)) {
this.trigger("error", {
code: "E017",
line: startLine,
Expand Down Expand Up @@ -720,7 +720,7 @@ Lexer.prototype = {
* This method's implementation was heavily influenced by the
* scanNumericLiteral function in the Esprima parser's source code.
*/
scanNumericLiteral: function() {
scanNumericLiteral: function(checks) {
var index = 0;
var value = "";
var length = this.input.length;
Expand Down Expand Up @@ -778,12 +778,17 @@ Lexer.prototype = {
base = 8;

if (!state.inES6(true)) {
this.trigger("warning", {
code: "W119",
line: this.line,
character: this.char,
data: [ "Octal integer literal", "6" ]
});
this.triggerAsync(
"warning",
{
code: "W119",
line: this.line,
character: this.char,
data: [ "Octal integer literal", "6" ]
},
checks,
function() { return true; }
);
}

index += 1;
Expand All @@ -796,12 +801,17 @@ Lexer.prototype = {
base = 2;

if (!state.inES6(true)) {
this.trigger("warning", {
code: "W119",
line: this.line,
character: this.char,
data: [ "Binary integer literal", "6" ]
});
this.triggerAsync(
"warning",
{
code: "W119",
line: this.line,
character: this.char,
data: [ "Binary integer literal", "6" ]
},
checks,
function() { return true; }
);
}

index += 1;
Expand Down Expand Up @@ -978,6 +988,8 @@ Lexer.prototype = {
var hexCode = this.input.substr(1, 4);
var code = parseInt(hexCode, 16);
if (isNaN(code)) {
// This condition unequivocally describes a syntax error.
// TODO: Re-factor as an "error" (not a "warning").
this.trigger("warning", {
code: "W052",
line: this.line,
Expand Down Expand Up @@ -1044,12 +1056,17 @@ Lexer.prototype = {

if (this.peek() === "`") {
if (!state.inES6(true)) {
this.trigger("warning", {
code: "W119",
line: this.line,
character: this.char,
data: ["template literal syntax", "6"]
});
this.triggerAsync(
"warning",
{
code: "W119",
line: this.line,
character: this.char,
data: ["template literal syntax", "6"]
},
checks,
function() { return true; }
);
}
// Template must start with a backtick.
tokenType = Token.TemplateHead;
Expand All @@ -1068,7 +1085,7 @@ Lexer.prototype = {
while (this.peek() !== "`") {
while ((ch = this.peek()) === "") {
value += "\n";
if (!this.nextLine()) {
if (!this.nextLine(checks)) {
// Unclosed template literal --- point to the starting "`"
var startPos = this.templateStarts.pop();
this.trigger("error", {
Expand Down Expand Up @@ -1172,6 +1189,8 @@ Lexer.prototype = {
// but it generates too many false positives.

if (!allowNewLine) {
// This condition unequivocally describes a syntax error.
// TODO: Re-factor as an "error" (not a "warning").
this.trigger("warning", {
code: "W112",
line: this.line,
Expand Down Expand Up @@ -1199,7 +1218,7 @@ Lexer.prototype = {
// If we get an EOF inside of an unclosed string, show an
// error and implicitly close it at the EOF point.

if (!this.nextLine()) {
if (!this.nextLine(checks)) {
this.trigger("error", {
code: "E029",
line: startLine,
Expand All @@ -1225,12 +1244,17 @@ Lexer.prototype = {

if (char < " ") {
// Warn about a control character in a string.
this.trigger("warning", {
code: "W113",
line: this.line,
character: this.char,
data: [ "<non-printable>" ]
});
this.triggerAsync(
"warning",
{
code: "W113",
line: this.line,
character: this.char,
data: [ "<non-printable>" ]
},
checks,
function() { return true; }
);
}

// Special treatment for some escaped characters.
Expand Down Expand Up @@ -1267,7 +1291,7 @@ Lexer.prototype = {
* rare edge cases where one JavaScript engine complains about
* your regular expression while others don't.
*/
scanRegExp: function() {
scanRegExp: function(checks) {
var index = 0;
var length = this.input.length;
var char = this.peek();
Expand All @@ -1282,22 +1306,32 @@ Lexer.prototype = {
// Unexpected control character
if (char < " ") {
malformed = true;
this.trigger("warning", {
code: "W048",
line: this.line,
character: this.char
});
this.triggerAsync(
"warning",
{
code: "W048",
line: this.line,
character: this.char
},
checks,
function() { return true; }
);
}

// Unexpected escaped character
if (char === "<") {
malformed = true;
this.trigger("warning", {
code: "W049",
line: this.line,
character: this.char,
data: [ char ]
});
this.triggerAsync(
"warning",
{
code: "W049",
line: this.line,
character: this.char,
data: [ char ]
},
checks,
function() { return true; }
);
}
}.bind(this);

Expand Down Expand Up @@ -1399,12 +1433,17 @@ Lexer.prototype = {
}
if (char === "y") {
if (!state.inES6(true)) {
this.trigger("warning", {
code: "W119",
line: this.line,
character: this.char,
data: [ "Sticky RegExp flag", "6" ]
});
this.triggerAsync(
"warning",
{
code: "W119",
line: this.line,
character: this.char,
data: [ "Sticky RegExp flag", "6" ]
},
checks,
function() { return true; }
);
}
if (value.indexOf("y") > -1) {
malformedDesc = "Duplicate RegExp flag";
Expand Down Expand Up @@ -1480,7 +1519,7 @@ Lexer.prototype = {
// Methods that work with multi-line structures and move the
// character pointer.

var match = this.scanComments() ||
var match = this.scanComments(checks) ||
this.scanStringLiteral(checks) ||
this.scanTemplateLiteral(checks);

Expand All @@ -1491,11 +1530,11 @@ Lexer.prototype = {
// Methods that don't move the character pointer.

match =
this.scanRegExp() ||
this.scanRegExp(checks) ||
this.scanPunctuator() ||
this.scanKeyword() ||
this.scanIdentifier() ||
this.scanNumericLiteral();
this.scanNumericLiteral(checks);

if (match) {
this.skip(match.tokenLength || match.value.length);
Expand All @@ -1511,7 +1550,7 @@ Lexer.prototype = {
* Switch to the next line and reset all char pointers. Once
* switched, this method also checks for other minor warnings.
*/
nextLine: function() {
nextLine: function(checks) {
var char;

if (this.line >= this.getLines().length) {
Expand Down Expand Up @@ -1547,14 +1586,24 @@ Lexer.prototype = {

char = this.scanNonBreakingSpaces();
if (char >= 0) {
this.trigger("warning", { code: "W125", line: this.line, character: char + 1 });
this.triggerAsync(
"warning",
{ code: "W125", line: this.line, character: char + 1 },
checks,
function() { return true; }
);
}

this.input = this.input.replace(/\t/g, state.tab);
char = this.scanUnsafeChars();

if (char >= 0) {
this.trigger("warning", { code: "W100", line: this.line, character: char });
this.triggerAsync(
"warning",
{ code: "W100", line: this.line, character: char },
checks,
function() { return true; }
);
}

// If there is a limit on line length, warn when lines get too
Expand All @@ -1569,21 +1618,18 @@ Lexer.prototype = {
var shouldTriggerError = !inComment || !reg.maxlenException.test(inputTrimmed);

if (shouldTriggerError) {
this.trigger("warning", { code: "W101", line: this.line, character: this.input.length });
this.triggerAsync(
"warning",
{ code: "W101", line: this.line, character: this.input.length },
checks,
function() { return true; }
);
}
}

return true;
},

/*
* This is simply a synonym for nextLine() method with a friendlier
* public name.
*/
start: function() {
this.nextLine();
},

/*
* Produce the next token. This function is called by advance() to get
* the next token. It returns a token in a JSLint-compatible format.
Expand Down Expand Up @@ -1707,7 +1753,7 @@ Lexer.prototype = {

for (;;) {
if (!this.input.length) {
if (this.nextLine()) {
if (this.nextLine(checks)) {
return create("(endline)", "");
}

Expand Down Expand Up @@ -1813,6 +1859,8 @@ Lexer.prototype = {

case Token.NumericLiteral:
if (token.isMalformed) {
// This condition unequivocally describes a syntax error.
// TODO: Re-factor as an "error" (not a "warning").
this.trigger("warning", {
code: "W045",
line: this.line,
Expand Down
Binary file added tests/unit/fixtures/peek-over-directives.js
Binary file not shown.
1 change: 1 addition & 0 deletions tests/unit/options.js
Expand Up @@ -2613,6 +2613,7 @@ exports.enforceall = function (test) {

// Throws errors not normally on be default
TestRun(test)
.addError(1, "This line contains non-breaking spaces: http://jshint.com/docs/options/#nonbsp")
.addError(1, "['key'] is better written in dot notation.")
.addError(1, "'obj' is not defined.")
.addError(1, "Missing semicolon.")
Expand Down

0 comments on commit a2b3881

Please sign in to comment.