Skip to content

Commit

Permalink
fix: don't serialize the fileName as undefined: when not present
Browse files Browse the repository at this point in the history
BREAKING CHANGE: when ``fileName`` is undefined in the parser options saxes does
not show a file name in error messages. Previously it was showing the name
``undefined``. To get the previous behavior, in all cases where you'd leave
``fileName`` undefined, you must set it to the string ``"undefined"`` instead.
  • Loading branch information
papandreou authored and lddubeau committed Jul 18, 2019
1 parent 5266d0d commit 4ff2365
Show file tree
Hide file tree
Showing 19 changed files with 95 additions and 36 deletions.
17 changes: 12 additions & 5 deletions lib/saxes.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,7 @@ const FORBIDDEN_BRACKET_BRACKET = 2;
* @property {boolean} [position] Whether to track positions. Unset means
* ``true``.
*
* @property {string} [fileName] A file name to use for error reporting. Leaving
* this unset will report a file name of "undefined". "File name" is a loose
* @property {string} [fileName] A file name to use for error reporting. "File name" is a loose
* concept. You could use a URL to some resource, or any descriptive name you
* like.
*/
Expand Down Expand Up @@ -493,9 +492,17 @@ class SaxesParser {
* @returns this
*/
fail(er) {
const message = (this.trackPosition) ?
`${this.fileName}:${this.line}:${this.column}: ${er}` : er;

let message = this.fileName || "";
if (this.trackPosition) {
if (message.length > 0) {
message += ":";
}
message += `${this.line}:${this.column}`;
}
if (message.length > 0) {
message += ": ";
}
message += er;
this.onerror(new Error(message));
return this;
}
Expand Down
2 changes: 1 addition & 1 deletion test/attribute-no-space.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require(".").test({
xml: "<root attr1=\"first\"attr2=\"second\"/>",
expect: [
["opentagstart", { name: "root", attributes: {} }],
["error", "undefined:1:20: no whitespace between attributes."],
["error", "1:20: no whitespace between attributes."],
["opentag", { name: "root", attributes: { attr1: "first", attr2: "second" }, isSelfClosing: true }],
["closetag", { name: "root", attributes: { attr1: "first", attr2: "second" }, isSelfClosing: true }],
],
Expand Down
2 changes: 1 addition & 1 deletion test/attribute-unquoted.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ require(".").test({
name: "attribute unquoted",
expect: [
["opentagstart", { name: "root", attributes: {}, ns: {} }],
["error", "undefined:1:14: unquoted attribute value."],
["error", "1:14: unquoted attribute value."],
["opentag", {
name: "root",
attributes: {
Expand Down
6 changes: 3 additions & 3 deletions test/bad-entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require(".").test({
expect: [
["opentagstart", { name: "r", attributes: {} }],
["opentag", { name: "r", attributes: {}, isSelfClosing: false }],
["error", "undefined:1:5: empty entity name."],
["error", "1:5: empty entity name."],
["text", "&;"],
["closetag", { name: "r", attributes: {}, isSelfClosing: false }],
],
Expand All @@ -18,7 +18,7 @@ require(".").test({
expect: [
["opentagstart", { name: "r", attributes: {} }],
["opentag", { name: "r", attributes: {}, isSelfClosing: false }],
["error", "undefined:1:6: malformed character entity."],
["error", "1:6: malformed character entity."],
["text", "&#;"],
["closetag", { name: "r", attributes: {}, isSelfClosing: false }],
],
Expand All @@ -30,7 +30,7 @@ require(".").test({
expect: [
["opentagstart", { name: "r", attributes: {} }],
["opentag", { name: "r", attributes: {}, isSelfClosing: false }],
["error", "undefined:1:7: malformed character entity."],
["error", "1:7: malformed character entity."],
["text", "&#x;"],
["closetag", { name: "r", attributes: {}, isSelfClosing: false }],
],
Expand Down
4 changes: 2 additions & 2 deletions test/bom.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ require(".").test({
name: "BOM outside of root, but not initial",
xml: " \uFEFF<P></P>",
expect: [
["error", "undefined:1:2: text data outside of root node."],
["error", "1:2: text data outside of root node."],
["text", "\uFEFF"],
["opentagstart", { name: "P", attributes: {} }],
["opentag", { name: "P", attributes: {}, isSelfClosing: false }],
Expand All @@ -41,7 +41,7 @@ require(".").test({
name: "multiple BOMs",
xml: "\uFEFF\uFEFF<P></P>",
expect: [
["error", "undefined:1:2: text data outside of root node."],
["error", "1:2: text data outside of root node."],
["text", "\uFEFF"],
["opentagstart", { name: "P", attributes: {} }],
["opentag", { name: "P", attributes: {}, isSelfClosing: false }],
Expand Down
4 changes: 2 additions & 2 deletions test/close-without-open.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ test({
name: "close a tag that was not opened",
xml: "</root>",
expect: [
["error", "undefined:1:7: unmatched closing tag: root."],
["error", "undefined:1:7: document must contain a root element."],
["error", "1:7: unmatched closing tag: root."],
["error", "1:7: document must contain a root element."],
["text", "</root>"],
],
opt: {
Expand Down
2 changes: 1 addition & 1 deletion test/duplicate-attribute.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require(".").test({
name: "span",
attributes: {},
}],
["error", "undefined:1:28: duplicate attribute: id."],
["error", "1:28: duplicate attribute: id."],
["opentag", {
name: "span",
attributes: { id: "there" },
Expand Down
2 changes: 1 addition & 1 deletion test/entity-nan.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require(".").test({
expect: [
["opentagstart", { name: "r", attributes: {} }],
["opentag", { name: "r", attributes: {}, isSelfClosing: false }],
["error", "undefined:1:9: malformed character entity."],
["error", "1:9: malformed character entity."],
["text", "&#NaN;"],
["closetag", { name: "r", attributes: {}, isSelfClosing: false }],
],
Expand Down
52 changes: 52 additions & 0 deletions test/errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
"use strict";

require(".").test({
name: "without fileName",
xml: "<doc>",
expect: [
["opentagstart", { name: "doc", attributes: {} }],
["opentag", { name: "doc", isSelfClosing: false, attributes: {} }],
["error", "1:5: unclosed tag: doc"],
],
opt: {},
});

require(".").test({
name: "without fileName, when not tracking position position",
xml: "<doc>",
expect: [
["opentagstart", { name: "doc", attributes: {} }],
["opentag", { name: "doc", isSelfClosing: false, attributes: {} }],
["error", "unclosed tag: doc"],
],
opt: {
position: false,
},
});

require(".").test({
name: "with fileName",
xml: "<doc>",
expect: [
["opentagstart", { name: "doc", attributes: {} }],
["opentag", { name: "doc", isSelfClosing: false, attributes: {} }],
["error", "foobar.xml:1:5: unclosed tag: doc"],
],
opt: {
fileName: "foobar.xml",
},
});

require(".").test({
name: "with fileName, when not tracking position position",
xml: "<doc>",
expect: [
["opentagstart", { name: "doc", attributes: {} }],
["opentag", { name: "doc", isSelfClosing: false, attributes: {} }],
["error", "foobar.xml: unclosed tag: doc"],
],
opt: {
fileName: "foobar.xml",
position: false,
},
});
2 changes: 1 addition & 1 deletion test/fragments.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ describe("fragments", () => {
ns: {},
isSelfClosing: false,
}],
["error", "undefined:1:31: unclosed tag: more"],
["error", "1:31: unclosed tag: more"],
["text", "2"],
],
opt: {
Expand Down
4 changes: 2 additions & 2 deletions test/issue-86.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ require(".").test({
],
[
"error",
"undefined:1:19: text data outside of root node.",
"1:19: text data outside of root node.",
],
[
"error",
"undefined:1:20: unexpected end.",
"1:20: unexpected end.",
],
[
"text",
Expand Down
2 changes: 1 addition & 1 deletion test/trailing-attribute-no-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ require(".").test({
xml: "<root attrib></root>",
expect: [
["opentagstart", { name: "root", attributes: {} }],
["error", "undefined:1:13: attribute without value."],
["error", "1:13: attribute without value."],
["opentag", { name: "root", attributes: { attrib: "attrib" }, isSelfClosing: false }],
["closetag", { name: "root", attributes: { attrib: "attrib" }, isSelfClosing: false }],
],
Expand Down
2 changes: 1 addition & 1 deletion test/trailing-non-whitespace.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ require(".").test({
attributes: {},
isSelfClosing: false,
}],
["error", "undefined:1:36: text data outside of root node."],
["error", "1:36: text data outside of root node."],
["text", " to monkey land"],
["end", undefined],
["ready", undefined],
Expand Down
2 changes: 1 addition & 1 deletion test/unclosed-root.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ require(".").test({
],
[
"error",
"undefined:1:6: unclosed tag: root",
"1:6: unclosed tag: root",
],
],
opt: {},
Expand Down
6 changes: 3 additions & 3 deletions test/wrong-cdata-closure.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe("wrong cdata closure", () => {
isSelfClosing: false,
}],
["error",
"undefined:1:19: the string \"]]>\" is disallowed in char data."],
"1:19: the string \"]]>\" is disallowed in char data."],
["text", "somethingx]]>moo"],
["closetag", {
name: "span",
Expand Down Expand Up @@ -51,7 +51,7 @@ describe("wrong cdata closure", () => {
isSelfClosing: false,
}],
["error",
"undefined:1:19: the string \"]]>\" is disallowed in char data."],
"1:19: the string \"]]>\" is disallowed in char data."],
["text", "somethingx]]>moo"],
["closetag", {
name: "span",
Expand Down Expand Up @@ -83,7 +83,7 @@ describe("wrong cdata closure", () => {
isSelfClosing: false,
}],
["error",
"undefined:1:19: the string \"]]>\" is disallowed in char data."],
"1:19: the string \"]]>\" is disallowed in char data."],
["text", "somethingx]]>moo"],
["closetag", {
name: "span",
Expand Down
14 changes: 7 additions & 7 deletions test/xml-declaration.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe("xml declaration", () => {
name: "empty declaration",
xml: "<?xml?><root/>",
expect: [
["error", "undefined:1:7: XML declaration must contain a version."],
["error", "1:7: XML declaration must contain a version."],
["opentagstart", { name: "root", attributes: {}, ns: {} }],
[
"opentag",
Expand Down Expand Up @@ -45,7 +45,7 @@ describe("xml declaration", () => {
name: "version without value",
xml: "<?xml version?><root/>",
expect: [
["error", "undefined:1:15: XML declaration is incomplete."],
["error", "1:15: XML declaration is incomplete."],
["opentagstart", { name: "root", attributes: {}, ns: {} }],
[
"opentag",
Expand Down Expand Up @@ -81,7 +81,7 @@ describe("xml declaration", () => {
name: "version without value",
xml: "<?xml version=?><root/>",
expect: [
["error", "undefined:1:16: XML declaration is incomplete."],
["error", "1:16: XML declaration is incomplete."],
["opentagstart", { name: "root", attributes: {}, ns: {} }],
[
"opentag",
Expand Down Expand Up @@ -117,8 +117,8 @@ describe("xml declaration", () => {
name: "unquoted value",
xml: "<?xml version=a?><root/>",
expect: [
["error", "undefined:1:15: value must be quoted."],
["error", "undefined:1:17: XML declaration is incomplete."],
["error", "1:15: value must be quoted."],
["error", "1:17: XML declaration is incomplete."],
["opentagstart", { name: "root", attributes: {}, ns: {} }],
[
"opentag",
Expand Down Expand Up @@ -154,7 +154,7 @@ describe("xml declaration", () => {
name: "unterminated value",
xml: "<?xml version=\"a?><root/>",
expect: [
["error", "undefined:1:18: XML declaration is incomplete."],
["error", "1:18: XML declaration is incomplete."],
["opentagstart", { name: "root", attributes: {}, ns: {} }],
[
"opentag",
Expand Down Expand Up @@ -190,7 +190,7 @@ describe("xml declaration", () => {
name: "bad version",
xml: "<?xml version=\"a\"?><root/>",
expect: [
["error", "undefined:1:17: version number must match /^1\\.[0-9]+$/."],
["error", "1:17: version number must match /^1\\.[0-9]+$/."],
["opentagstart", { name: "root", attributes: {}, ns: {} }],
[
"opentag",
Expand Down
2 changes: 1 addition & 1 deletion test/xml-internal-entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ for (const entity in entitiesToTest) {

attributeErrors.push([
"error",
`undefined:1:${pos}: ${entity[0] === "#" ? "malformed character entity." :
`1:${pos}: ${entity[0] === "#" ? "malformed character entity." :
"disallowed character in entity name."}`,
]);
myAttributes[attribName] = `&${entity};`;
Expand Down
4 changes: 2 additions & 2 deletions test/xmlns-unbound.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe("xmlns unbound prefixes", () => {
],
[
"error",
"undefined:1:15: unbound namespace prefix: \"unbound\".",
"1:15: unbound namespace prefix: \"unbound\".",
],
[
"opentag",
Expand Down Expand Up @@ -127,7 +127,7 @@ describe("xmlns unbound prefixes", () => {
],
[
"error",
"undefined:1:28: unbound namespace prefix: \"unbound\".",
"1:28: unbound namespace prefix: \"unbound\".",
],
[
"opentag",
Expand Down
2 changes: 1 addition & 1 deletion test/xmlns-xml-default-prefix.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ describe("xml default prefix", () => {
],
[
"error",
"undefined:1:27: xml prefix must be bound to \
"1:27: xml prefix must be bound to \
http://www.w3.org/XML/1998/namespace.",
],
[
Expand Down

0 comments on commit 4ff2365

Please sign in to comment.