From 4ff2365ce63eea44a5226f8bdb69deb0505e279e Mon Sep 17 00:00:00 2001 From: Andreas Lind Date: Wed, 10 Jul 2019 14:07:59 +0200 Subject: [PATCH] fix: don't serialize the fileName as undefined: when not present 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. --- lib/saxes.js | 17 +++++++--- test/attribute-no-space.js | 2 +- test/attribute-unquoted.js | 2 +- test/bad-entities.js | 6 ++-- test/bom.js | 4 +-- test/close-without-open.js | 4 +-- test/duplicate-attribute.js | 2 +- test/entity-nan.js | 2 +- test/errors.js | 52 +++++++++++++++++++++++++++++ test/fragments.js | 2 +- test/issue-86.js | 4 +-- test/trailing-attribute-no-value.js | 2 +- test/trailing-non-whitespace.js | 2 +- test/unclosed-root.js | 2 +- test/wrong-cdata-closure.js | 6 ++-- test/xml-declaration.js | 14 ++++---- test/xml-internal-entities.js | 2 +- test/xmlns-unbound.js | 4 +-- test/xmlns-xml-default-prefix.js | 2 +- 19 files changed, 95 insertions(+), 36 deletions(-) create mode 100644 test/errors.js diff --git a/lib/saxes.js b/lib/saxes.js index 4cd82d59..6ed45327 100644 --- a/lib/saxes.js +++ b/lib/saxes.js @@ -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. */ @@ -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; } diff --git a/test/attribute-no-space.js b/test/attribute-no-space.js index 58ca7142..6ce79acf 100644 --- a/test/attribute-no-space.js +++ b/test/attribute-no-space.js @@ -6,7 +6,7 @@ require(".").test({ xml: "", 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 }], ], diff --git a/test/attribute-unquoted.js b/test/attribute-unquoted.js index aa698c94..7a3f6786 100644 --- a/test/attribute-unquoted.js +++ b/test/attribute-unquoted.js @@ -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: { diff --git a/test/bad-entities.js b/test/bad-entities.js index 59625ea6..eec63f13 100644 --- a/test/bad-entities.js +++ b/test/bad-entities.js @@ -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 }], ], @@ -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 }], ], @@ -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 }], ], diff --git a/test/bom.js b/test/bom.js index fb0abdb9..f4bb816d 100644 --- a/test/bom.js +++ b/test/bom.js @@ -28,7 +28,7 @@ require(".").test({ name: "BOM outside of root, but not initial", xml: " \uFEFF

", 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 }], @@ -41,7 +41,7 @@ require(".").test({ name: "multiple BOMs", xml: "\uFEFF\uFEFF

", 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 }], diff --git a/test/close-without-open.js b/test/close-without-open.js index dd863883..e3ade98e 100644 --- a/test/close-without-open.js +++ b/test/close-without-open.js @@ -6,8 +6,8 @@ test({ name: "close a tag that was not opened", xml: "", 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", ""], ], opt: { diff --git a/test/duplicate-attribute.js b/test/duplicate-attribute.js index bb66363d..2594ca24 100644 --- a/test/duplicate-attribute.js +++ b/test/duplicate-attribute.js @@ -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" }, diff --git a/test/entity-nan.js b/test/entity-nan.js index ca4baa51..84f953bb 100644 --- a/test/entity-nan.js +++ b/test/entity-nan.js @@ -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 }], ], diff --git a/test/errors.js b/test/errors.js new file mode 100644 index 00000000..3d7217f5 --- /dev/null +++ b/test/errors.js @@ -0,0 +1,52 @@ +"use strict"; + +require(".").test({ + name: "without fileName", + xml: "", + 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: "", + expect: [ + ["opentagstart", { name: "doc", attributes: {} }], + ["opentag", { name: "doc", isSelfClosing: false, attributes: {} }], + ["error", "unclosed tag: doc"], + ], + opt: { + position: false, + }, +}); + +require(".").test({ + name: "with fileName", + xml: "", + 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: "", + expect: [ + ["opentagstart", { name: "doc", attributes: {} }], + ["opentag", { name: "doc", isSelfClosing: false, attributes: {} }], + ["error", "foobar.xml: unclosed tag: doc"], + ], + opt: { + fileName: "foobar.xml", + position: false, + }, +}); diff --git a/test/fragments.js b/test/fragments.js index 7005a464..cc262b8a 100644 --- a/test/fragments.js +++ b/test/fragments.js @@ -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: { diff --git a/test/issue-86.js b/test/issue-86.js index 595b094e..051150a5 100644 --- a/test/issue-86.js +++ b/test/issue-86.js @@ -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", diff --git a/test/trailing-attribute-no-value.js b/test/trailing-attribute-no-value.js index d3d6d74b..0b477ef7 100644 --- a/test/trailing-attribute-no-value.js +++ b/test/trailing-attribute-no-value.js @@ -5,7 +5,7 @@ require(".").test({ xml: "", 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 }], ], diff --git a/test/trailing-non-whitespace.js b/test/trailing-non-whitespace.js index 0d463352..f9a8d5ec 100644 --- a/test/trailing-non-whitespace.js +++ b/test/trailing-non-whitespace.js @@ -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], diff --git a/test/unclosed-root.js b/test/unclosed-root.js index 9085247d..8271af9e 100644 --- a/test/unclosed-root.js +++ b/test/unclosed-root.js @@ -21,7 +21,7 @@ require(".").test({ ], [ "error", - "undefined:1:6: unclosed tag: root", + "1:6: unclosed tag: root", ], ], opt: {}, diff --git a/test/wrong-cdata-closure.js b/test/wrong-cdata-closure.js index d6813542..de9f4b7b 100644 --- a/test/wrong-cdata-closure.js +++ b/test/wrong-cdata-closure.js @@ -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", @@ -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", @@ -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", diff --git a/test/xml-declaration.js b/test/xml-declaration.js index 53e73ce3..30e2af36 100644 --- a/test/xml-declaration.js +++ b/test/xml-declaration.js @@ -9,7 +9,7 @@ describe("xml declaration", () => { name: "empty declaration", xml: "", 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", @@ -45,7 +45,7 @@ describe("xml declaration", () => { name: "version without value", xml: "", expect: [ - ["error", "undefined:1:15: XML declaration is incomplete."], + ["error", "1:15: XML declaration is incomplete."], ["opentagstart", { name: "root", attributes: {}, ns: {} }], [ "opentag", @@ -81,7 +81,7 @@ describe("xml declaration", () => { name: "version without value", xml: "", expect: [ - ["error", "undefined:1:16: XML declaration is incomplete."], + ["error", "1:16: XML declaration is incomplete."], ["opentagstart", { name: "root", attributes: {}, ns: {} }], [ "opentag", @@ -117,8 +117,8 @@ describe("xml declaration", () => { name: "unquoted value", xml: "", 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", @@ -154,7 +154,7 @@ describe("xml declaration", () => { name: "unterminated value", xml: "", expect: [ - ["error", "undefined:1:18: XML declaration is incomplete."], + ["error", "1:18: XML declaration is incomplete."], ["opentagstart", { name: "root", attributes: {}, ns: {} }], [ "opentag", @@ -190,7 +190,7 @@ describe("xml declaration", () => { name: "bad version", xml: "", 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", diff --git a/test/xml-internal-entities.js b/test/xml-internal-entities.js index 1966b0f3..3b4bd4b9 100644 --- a/test/xml-internal-entities.js +++ b/test/xml-internal-entities.js @@ -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};`; diff --git a/test/xmlns-unbound.js b/test/xmlns-unbound.js index 84a7c55e..035cf163 100644 --- a/test/xmlns-unbound.js +++ b/test/xmlns-unbound.js @@ -17,7 +17,7 @@ describe("xmlns unbound prefixes", () => { ], [ "error", - "undefined:1:15: unbound namespace prefix: \"unbound\".", + "1:15: unbound namespace prefix: \"unbound\".", ], [ "opentag", @@ -127,7 +127,7 @@ describe("xmlns unbound prefixes", () => { ], [ "error", - "undefined:1:28: unbound namespace prefix: \"unbound\".", + "1:28: unbound namespace prefix: \"unbound\".", ], [ "opentag", diff --git a/test/xmlns-xml-default-prefix.js b/test/xmlns-xml-default-prefix.js index dd0a3e0b..2a965313 100644 --- a/test/xmlns-xml-default-prefix.js +++ b/test/xmlns-xml-default-prefix.js @@ -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.", ], [