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.

Sign up for GitHub

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

Jump to bottom

Do not duplicate reported warnings/errors #3050

Merged
merged 4 commits into from Oct 20, 2016
Merged

Do not duplicate reported warnings/errors #3050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2a31c94
[[FIX]] Offset line no.s of errors from eval code
jugglinmike Oct 18, 2016
3b06d01
[[FIX]] Report error for offending token value
jugglinmike Oct 18, 2016
dc4a4fe
[[FIX]] Do not duplicate reported warnings/errors
jugglinmike Oct 18, 2016
6707c54
[[CHORE]] Rename internal function
jugglinmike Oct 20, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
69 changes: 39 additions & 30 deletions src/jshint.js
View file
Open in desktop
Expand Up @@ -408,15 +408,13 @@ var JSHINT = (function() {
}

// Tracking of "internal" scripts, like eval containing a static string
function addInternalSrc(elem, src) {
var i;
i = {
function addEvalCode(elem, token) {
JSHINT.internals.push({
id: "(internal)",
elem: elem,
value: src
};
JSHINT.internals.push(i);
return i;
token: token,
code: token.value.replace(/([^\\])(\\*)\2\\n/g, "$1\n")
});
}

function doOption() {
Expand Down Expand Up @@ -1348,13 +1346,7 @@ var JSHINT = (function() {
state.nameStack.set(state.tokens.prev);
return true;
} else if (left.id === "{" || left.id === "[") {
if (allowDestructuring && left.destructAssign) {
left.destructAssign.forEach(function(t) {
if (t.id) {
state.funct["(scope)"].block.modify(t.id, t.token);
}
});
} else {
if (!allowDestructuring || !left.destructAssign) {
if (left.id === "{" || !left.left) {
warning("E031", assignToken);
} else if (left.left.value === "arguments" && !state.isStrict()) {
Expand Down Expand Up @@ -1514,7 +1506,7 @@ var JSHINT = (function() {
}

if (!state.tokens.next.identifier) {
warning("E024", state.tokens.curr, "...");
warning("E024", state.tokens.curr, state.tokens.next.id);
return;
}

Expand Down Expand Up @@ -2473,13 +2465,13 @@ var JSHINT = (function() {
// to the fact that the behavior was never formally documented). This
// branch should be enabled as part of a major release.
//if (p[0] && p[0].id === "(string)") {
// addInternalSrc(left, p[0].value);
// addEvalCode(left, p[0]);
//}
} else if (p[0] && p[0].id === "(string)" &&
(left.value === "setTimeout" ||
left.value === "setInterval")) {
warning("W066", left);
addInternalSrc(left, p[0].value);
addEvalCode(left, p[0]);

// window.setTimeout/setInterval
} else if (p[0] && p[0].id === "(string)" &&
Expand All @@ -2488,7 +2480,7 @@ var JSHINT = (function() {
(left.right === "setTimeout" ||
left.right === "setInterval")) {
warning("W066", left);
addInternalSrc(left, p[0].value);
addEvalCode(left, p[0]);
}
}
if (!left.identifier && left.id !== "." && left.id !== "[" && left.id !== "=>" &&
Expand Down Expand Up @@ -4427,10 +4419,9 @@ var JSHINT = (function() {
stmt("continue", function() {
var v = state.tokens.next.value;

if (state.funct["(breakage)"] === 0)
warning("W052", state.tokens.next, this.value);
if (!state.funct["(loopage)"])
if (state.funct["(breakage)"] === 0 || !state.funct["(loopage)"]) {
warning("W052", state.tokens.next, this.value);
}

if (!state.option.asi)
nolinebreak(this);
Expand Down Expand Up @@ -5171,13 +5162,38 @@ var JSHINT = (function() {
}
}

/**
* Lint dynamically-evaluated code, appending any resulting errors/warnings
* into the global `errors` array.
*
* @param {array} internals - collection of "internals" objects describing
* string tokens that contain evaluated code
* @param {object} options - linting options to apply
* @param {object} globals - globally-defined bindings for the evaluated code
*/
function lintEvalCode(internals, options, globals) {
var priorErrorCount, idx, jdx, internal;

for (idx = 0; idx < internals.length; idx += 1) {
internal = internals[idx];
options.scope = internal.elem;
priorErrorCount = JSHINT.errors.length;

itself(internal.code, options, globals);

for (jdx = priorErrorCount; jdx < JSHINT.errors.length; jdx += 1) {
JSHINT.errors[jdx].line += internal.token.line - 1;
}
}
}

var escapeRegex = function(str) {
return str.replace(/[-\/\\^$*+?.()|[\]{}]/g, "\\$&");
};

// The actual JSHINT function itself.
var itself = function(s, o, g) {
var i, k, x, reIgnoreStr, reIgnore;
var x, reIgnoreStr, reIgnore;
var optionKeys;
var newOptionObj = {};
var newIgnoredObj = {};
Expand Down Expand Up @@ -5424,15 +5440,8 @@ var JSHINT = (function() {
}

// Loop over the listed "internals", and check them as well.

if (JSHINT.scope === "(main)") {
o = o || {};

for (i = 0; i < JSHINT.internals.length; i += 1) {
k = JSHINT.internals[i];
o.scope = k.elem;
itself(k.value, o, g);
}
lintEvalCode(JSHINT.internals, o || {}, g);
}

return JSHINT.errors.length === 0;
Expand Down
16 changes: 14 additions & 2 deletions tests/helpers/testhelper.js
View file
Open in desktop
Expand Up @@ -111,6 +111,12 @@ exports.setup.testRun = function (test, name) {
}).filter(function (er) {
return !!er;
});
var duplicateErrors = errors.filter(function (er) {
return errors.filter(function (other) {
return er.line === other.line && er.character === other.character &&
er.reason === other.reason;
}).length > 1;
});

// remove undefined errors, if there is a definition with wrong line number
undefinedErrors = undefinedErrors.filter(function (er) {
Expand All @@ -125,8 +131,10 @@ exports.setup.testRun = function (test, name) {
});

test.ok(
undefinedErrors.length === 0
&& unthrownErrors.length === 0 && wrongLineNumbers.length === 0,
undefinedErrors.length === 0 &&
unthrownErrors.length === 0 &&
wrongLineNumbers.length === 0 &&
duplicateErrors.length === 0,

(name === null ? "" : "\n TestRun: [bold]{" + name + "}") +
unthrownErrors.map(function (el, idx) {
Expand All @@ -140,6 +148,10 @@ exports.setup.testRun = function (test, name) {
wrongLineNumbers.map(function (el, idx) {
return (idx === 0 ? "\n [yellow]{Errors with wrong line number}\n" : "") +
" [bold]{Line " + el.line + "} " + el.message + " [red]{not in line(s)} [bold]{" + el.definedIn.join(", ") + "}";
}).join("\n") +
duplicateErrors.map(function (el, idx) {
return (idx === 0 ? "\n [yellow]{Duplicated errors}\n": "") +
" [bold]{Line " + el.line + ", Char " + el.character + "} " + el.reason;
}).join("\n") + "\n"
);
}
Expand Down
7 changes: 4 additions & 3 deletions tests/unit/core.js
View file
Open in desktop
Expand Up @@ -430,9 +430,10 @@ exports.insideEval = function (test) {

// The "TestRun" class (and these errors) probably needs some
// facility for checking the expected scope of the error
.addError(1, "Unexpected early end of program.")
.addError(1, "Unrecoverable syntax error. (100% scanned).")
.addError(1, "Unrecoverable syntax error. (100% scanned).")
.addError(13, "Unexpected early end of program.")
.addError(13, "Unrecoverable syntax error. (100% scanned).")
.addError(17, "Unexpected early end of program.")
.addError(17, "Unrecoverable syntax error. (100% scanned).")

.test(src, { es3: true, evil: false });

Expand Down
22 changes: 12 additions & 10 deletions tests/unit/parser.js
View file
Open in desktop
Expand Up @@ -7588,7 +7588,9 @@ exports.extraRestOperator = function (test) {
.addError(1, "Unexpected '...'.")
.addError(2, "Unexpected '...'.")
.addError(3, "Unexpected '...'.")
.addError(3, "Unexpected ')'.")
.addError(4, "Unexpected '...'.")
.addError(4, "Unexpected ')'.")
.addError(5, "Unexpected '...'.")
.addError(6, "Unexpected '...'.")
.addError(7, "Unexpected '...'.")
Expand Down Expand Up @@ -7624,16 +7626,16 @@ exports.restOperatorWithoutIdentifier = function (test) {
];

TestRun(test)
.addError(1, "Unexpected '...'.")
.addError(2, "Unexpected '...'.")
.addError(3, "Unexpected '...'.")
.addError(4, "Unexpected '...'.")
.addError(5, "Unexpected '...'.")
.addError(6, "Unexpected '...'.")
.addError(7, "Unexpected '...'.")
.addError(8, "Unexpected '...'.")
.addError(9, "Unexpected '...'.")
.addError(10, "Unexpected '...'.")
.addError(1, "Unexpected ']'.")
.addError(2, "Unexpected ']'.")
.addError(3, "Unexpected ')'.")
.addError(4, "Unexpected ')'.")
.addError(5, "Unexpected ']'.")
.addError(6, "Unexpected ']'.")
.addError(7, "Unexpected ')'.")
.addError(8, "Unexpected ')'.")
.addError(9, "Unexpected ']'.")
.addError(10, "Unexpected ']'.")
.test(code, { esnext: true });

test.done();
Expand Down