diff --git a/lib/svgo.js b/lib/svgo.js index fe5a2252..d29213fb 100644 --- a/lib/svgo.js +++ b/lib/svgo.js @@ -29,7 +29,12 @@ const optimize = (input, config) => { } for (let i = 0; i < maxPassCount; i += 1) { info.multipassCount = i; - svgjs = svg2js(input); + // TODO throw this error in v3 + try { + svgjs = svg2js(input, config.path); + } catch (error) { + return { error: error.toString(), modernError: error }; + } if (svgjs.error != null) { if (config.path != null) { svgjs.path = config.path; diff --git a/lib/svgo.test.js b/lib/svgo.test.js index 71102b33..e23d2925 100644 --- a/lib/svgo.test.js +++ b/lib/svgo.test.js @@ -12,12 +12,11 @@ test('allow to setup default preset', () => { `; - expect( - optimize(svg, { - plugins: ['preset-default'], - js2svg: { pretty: true, indent: 2 }, - }).data - ).toMatchInlineSnapshot(` + const { data } = optimize(svg, { + plugins: ['preset-default'], + js2svg: { pretty: true, indent: 2 }, + }); + expect(data).toMatchInlineSnapshot(` " @@ -35,24 +34,23 @@ test('allow to disable and customize plugins in preset', () => { `; - expect( - optimize(svg, { - plugins: [ - { - name: 'preset-default', - params: { - overrides: { - removeXMLProcInst: false, - removeDesc: { - removeAny: false, - }, + const { data } = optimize(svg, { + plugins: [ + { + name: 'preset-default', + params: { + overrides: { + removeXMLProcInst: false, + removeDesc: { + removeAny: false, }, }, }, - ], - js2svg: { pretty: true, indent: 2 }, - }).data - ).toMatchInlineSnapshot(` + }, + ], + js2svg: { pretty: true, indent: 2 }, + }); + expect(data).toMatchInlineSnapshot(` " @@ -186,19 +184,18 @@ test('allow to customize precision for preset', () => { `; - expect( - optimize(svg, { - plugins: [ - { - name: 'preset-default', - params: { - floatPrecision: 4, - }, + const { data } = optimize(svg, { + plugins: [ + { + name: 'preset-default', + params: { + floatPrecision: 4, }, - ], - js2svg: { pretty: true, indent: 2 }, - }).data - ).toMatchInlineSnapshot(` + }, + ], + js2svg: { pretty: true, indent: 2 }, + }); + expect(data).toMatchInlineSnapshot(` " @@ -212,27 +209,118 @@ test('plugin precision should override preset precision', () => { `; - expect( - optimize(svg, { - plugins: [ - { - name: 'preset-default', - params: { - floatPrecision: 4, - overrides: { - cleanupNumericValues: { - floatPrecision: 5, - }, + const { data } = optimize(svg, { + plugins: [ + { + name: 'preset-default', + params: { + floatPrecision: 4, + overrides: { + cleanupNumericValues: { + floatPrecision: 5, }, }, }, - ], - js2svg: { pretty: true, indent: 2 }, - }).data - ).toMatchInlineSnapshot(` + }, + ], + js2svg: { pretty: true, indent: 2 }, + }); + expect(data).toMatchInlineSnapshot(` " " `); }); + +test('provides informative error in result', () => { + const svg = ` + + + `; + const { modernError: error } = optimize(svg, { path: 'test.svg' }); + expect(error.name).toEqual('SvgoParserError'); + expect(error.message).toEqual('test.svg:2:33: Unquoted attribute value'); + expect(error.reason).toEqual('Unquoted attribute value'); + expect(error.line).toEqual(2); + expect(error.column).toEqual(33); + expect(error.source).toEqual(svg); +}); + +test('provides code snippet in rendered error', () => { + const svg = ` + + +`; + const { modernError: error } = optimize(svg, { path: 'test.svg' }); + expect(error.toString()) + .toEqual(`SvgoParserError: test.svg:2:29: Unquoted attribute value + + 1 | +> 2 | + | ^ + 3 | + 4 | +`); +}); + +test('supports errors without path', () => { + const svg = ` + + + + + + + + + + + +`; + const { modernError: error } = optimize(svg); + expect(error.toString()) + .toEqual(`SvgoParserError: :11:29: Unquoted attribute value + + 9 | + 10 | +> 11 | + | ^ + 12 | + 13 | +`); +}); + +test('slices long line in error code snippet', () => { + const svg = ` + + +`; + const { modernError: error } = optimize(svg); + expect(error.toString()) + .toEqual(`SvgoParserError: :2:211: Invalid attribute name + + 1 | …-0.dtd" viewBox="0 0 230 120"> +> 2 | …7.334 250.955 222.534t579.555 155.292z stroke-width="1.5" fill="red" strok… + | ^ + 3 | + 4 | +`); +}); + +test('provides legacy error message', () => { + const svg = ` + + +`; + const { error } = optimize(svg, { path: 'test.svg' }); + expect(error) + .toEqual(`SvgoParserError: test.svg:2:29: Unquoted attribute value + + 1 | +> 2 | + | ^ + 3 | + 4 | +`); +}); diff --git a/lib/svgo/coa.js b/lib/svgo/coa.js index 93cda652..fbf31af7 100644 --- a/lib/svgo/coa.js +++ b/lib/svgo/coa.js @@ -2,7 +2,7 @@ const FS = require('fs'); const PATH = require('path'); -const { green } = require('colorette'); +const { green, red } = require('colorette'); const { loadConfig, optimize } = require('../svgo-node.js'); const pluginsMap = require('../../plugins/plugins.js'); const PKG = require('../../package.json'); @@ -383,12 +383,9 @@ function processSVGData(config, info, data, output, input) { prevFileSize = Buffer.byteLength(data, 'utf8'); const result = optimize(data, { ...config, ...info }); - if (result.error) { - let message = result.error; - if (result.path != null) { - message += `\nFile: ${result.path}`; - } - throw Error(message); + if (result.modernError) { + console.error(red(result.modernError.toString())); + process.exit(1); } if (config.datauri) { result.data = encodeSVGDatauri(result.data, config.datauri); diff --git a/lib/svgo/svg2js.js b/lib/svgo/svg2js.js index b150a5ca..3d6e8862 100644 --- a/lib/svgo/svg2js.js +++ b/lib/svgo/svg2js.js @@ -4,6 +4,55 @@ const SAX = require('@trysound/sax'); const JSAPI = require('./jsAPI.js'); const { textElems } = require('../../plugins/_collections.js'); +class SvgoParserError extends Error { + constructor(message, line, column, source, file) { + super(message); + this.name = 'SvgoParserError'; + this.message = `${file || ''}:${line}:${column}: ${message}`; + this.reason = message; + this.line = line; + this.column = column; + this.source = source; + if (Error.captureStackTrace) { + Error.captureStackTrace(this, SvgoParserError); + } + } + toString() { + const lines = this.source.split(/\r?\n/); + const startLine = Math.max(this.line - 3, 0); + const endLine = Math.min(this.line + 2, lines.length); + const lineNumberWidth = String(endLine).length; + const startColumn = Math.max(this.column - 54, 0); + const endColumn = Math.max(this.column + 20, 80); + const code = lines + .slice(startLine, endLine) + .map((line, index) => { + const lineSlice = line.slice(startColumn, endColumn); + let ellipsisPrefix = ''; + let ellipsisSuffix = ''; + if (startColumn !== 0) { + ellipsisPrefix = startColumn > line.length - 1 ? ' ' : '…'; + } + if (endColumn < line.length - 1) { + ellipsisSuffix = '…'; + } + const number = startLine + 1 + index; + const gutter = ` ${number.toString().padStart(lineNumberWidth)} | `; + if (number === this.line) { + const gutterSpacing = gutter.replace(/[^|]/g, ' '); + const lineSpacing = ( + ellipsisPrefix + line.slice(startColumn, this.column - 1) + ).replace(/[^\t]/g, ' '); + const spacing = gutterSpacing + lineSpacing; + return `>${gutter}${ellipsisPrefix}${lineSlice}${ellipsisSuffix}\n ${spacing}^`; + } + return ` ${gutter}${ellipsisPrefix}${lineSlice}${ellipsisSuffix}`; + }) + .join('\n'); + return `${this.name}: ${this.message}\n\n${code}\n`; + } +} + const entityDeclaration = //g; const config = { @@ -20,7 +69,7 @@ const config = { * * @param {String} data input data */ -module.exports = function (data) { +module.exports = function (data, from) { const sax = SAX.parser(config.strict, config); const root = new JSAPI({ type: 'root', children: [] }); let current = root; @@ -115,16 +164,18 @@ module.exports = function (data) { }; sax.onerror = function (e) { - e.message = 'Error in parsing SVG: ' + e.message; - if (e.message.indexOf('Unexpected end') < 0) { - throw e; + const error = new SvgoParserError( + e.reason, + e.line + 1, + e.column, + data, + from + ); + if (e.message.indexOf('Unexpected end') === -1) { + throw error; } }; - try { - sax.write(data).close(); - return root; - } catch (e) { - return { error: e.message }; - } + sax.write(data).close(); + return root; }; diff --git a/package-lock.json b/package-lock.json index 8feff928..8623c344 100644 --- a/package-lock.json +++ b/package-lock.json @@ -617,6 +617,17 @@ "rimraf": "^3.0.0", "slash": "^3.0.0", "strip-ansi": "^6.0.0" + }, + "dependencies": { + "strip-ansi": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.0.tgz", + "integrity": "sha512-AuvKTrTfQNYNIctbR1K/YGTR1756GycPsg7b9bdV9Duqur4gv6aKqHXah67Z8ImS7WEz5QVcOtlfW2rZEugt6w==", + "dev": true, + "requires": { + "ansi-regex": "^5.0.0" + } + } } }, "@jest/environment": { @@ -867,9 +878,9 @@ "dev": true }, "@trysound/sax": { - "version": "0.1.1", - "resolved": "https://registry.npmjs.org/@trysound/sax/-/sax-0.1.1.tgz", - "integrity": "sha512-Z6DoceYb/1xSg5+e+ZlPZ9v0N16ZvZ+wYMraFue4HYrE4ttONKtsvruIRf6t9TBR0YvSOfi1hUU0fJfBLCDYow==" + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/@trysound/sax/-/sax-0.2.0.tgz", + "integrity": "sha512-L7z9BgrNEcYyUYtF+HaEfiS5ebkh9jXqbszz7pC0hRBPaatV0XjSD3+eHrpqFemQfgwiFF0QPIarnIihIDn7OA==" }, "@types/babel__core": { "version": "7.1.15", @@ -1403,6 +1414,17 @@ "string-width": "^4.2.0", "strip-ansi": "^6.0.0", "wrap-ansi": "^7.0.0" + }, + "dependencies": { + "strip-ansi": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.0.tgz", + "integrity": "sha512-AuvKTrTfQNYNIctbR1K/YGTR1756GycPsg7b9bdV9Duqur4gv6aKqHXah67Z8ImS7WEz5QVcOtlfW2rZEugt6w==", + "dev": true, + "requires": { + "ansi-regex": "^5.0.0" + } + } } }, "co": { @@ -1852,6 +1874,15 @@ "resolved": "https://registry.npmjs.org/ignore/-/ignore-4.0.6.tgz", "integrity": "sha512-cyFDKrqc/YdcWFniJhzI42+AzS+gNwmUzOSFcRCQYwySuBBBy/KjuxWLZ/FHEH6Moq1NizMOBWyTcv8O4OZIMg==", "dev": true + }, + "strip-ansi": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.0.tgz", + "integrity": "sha512-AuvKTrTfQNYNIctbR1K/YGTR1756GycPsg7b9bdV9Duqur4gv6aKqHXah67Z8ImS7WEz5QVcOtlfW2rZEugt6w==", + "dev": true, + "requires": { + "ansi-regex": "^5.0.0" + } } } }, @@ -3946,6 +3977,17 @@ "requires": { "char-regex": "^1.0.2", "strip-ansi": "^6.0.0" + }, + "dependencies": { + "strip-ansi": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.0.tgz", + "integrity": "sha512-AuvKTrTfQNYNIctbR1K/YGTR1756GycPsg7b9bdV9Duqur4gv6aKqHXah67Z8ImS7WEz5QVcOtlfW2rZEugt6w==", + "dev": true, + "requires": { + "ansi-regex": "^5.0.0" + } + } } }, "string-width": { @@ -3957,6 +3999,17 @@ "emoji-regex": "^8.0.0", "is-fullwidth-code-point": "^3.0.0", "strip-ansi": "^6.0.0" + }, + "dependencies": { + "strip-ansi": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.0.tgz", + "integrity": "sha512-AuvKTrTfQNYNIctbR1K/YGTR1756GycPsg7b9bdV9Duqur4gv6aKqHXah67Z8ImS7WEz5QVcOtlfW2rZEugt6w==", + "dev": true, + "requires": { + "ansi-regex": "^5.0.0" + } + } } }, "string_decoder": { @@ -4059,6 +4112,15 @@ "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-1.0.0.tgz", "integrity": "sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==", "dev": true + }, + "strip-ansi": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.0.tgz", + "integrity": "sha512-AuvKTrTfQNYNIctbR1K/YGTR1756GycPsg7b9bdV9Duqur4gv6aKqHXah67Z8ImS7WEz5QVcOtlfW2rZEugt6w==", + "dev": true, + "requires": { + "ansi-regex": "^5.0.0" + } } } }, @@ -4339,6 +4401,17 @@ "ansi-styles": "^4.0.0", "string-width": "^4.1.0", "strip-ansi": "^6.0.0" + }, + "dependencies": { + "strip-ansi": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.0.tgz", + "integrity": "sha512-AuvKTrTfQNYNIctbR1K/YGTR1756GycPsg7b9bdV9Duqur4gv6aKqHXah67Z8ImS7WEz5QVcOtlfW2rZEugt6w==", + "dev": true, + "requires": { + "ansi-regex": "^5.0.0" + } + } } }, "wrappy": { diff --git a/package.json b/package.json index 6007968f..dd1ba13d 100644 --- a/package.json +++ b/package.json @@ -92,7 +92,7 @@ ] }, "dependencies": { - "@trysound/sax": "0.1.1", + "@trysound/sax": "0.2.0", "colorette": "^1.3.0", "commander": "^7.2.0", "css-select": "^4.1.3", @@ -118,6 +118,7 @@ "prettier": "^2.3.2", "rollup": "^2.56.3", "rollup-plugin-terser": "^7.0.2", + "strip-ansi": "^6.0.0", "tar-stream": "^2.2.0", "typescript": "^4.4.2" }, diff --git a/test/cli/cli.test.js b/test/cli/cli.test.js new file mode 100644 index 00000000..e15f73b9 --- /dev/null +++ b/test/cli/cli.test.js @@ -0,0 +1,33 @@ +'use strict'; + +const { spawn } = require('child_process'); +const stripAnsi = require('strip-ansi'); + +test('should exit with 1 code on syntax error', async () => { + const proc = spawn('node', ['../../bin/svgo', 'invalid.svg'], { + cwd: __dirname, + }); + const [code, stderr] = await Promise.all([ + new Promise((resolve) => { + proc.on('close', (code) => { + resolve(code); + }); + }), + new Promise((resolve) => { + proc.stderr.on('data', (error) => { + resolve(error.toString()); + }); + }), + ]); + expect(code).toEqual(1); + expect(stripAnsi(stderr)) + .toEqual(`SvgoParserError: invalid.svg:2:27: Unquoted attribute value + + 1 | +> 2 | + | ^ + 3 | + 4 | + +`); +}); diff --git a/test/cli/invalid.svg b/test/cli/invalid.svg new file mode 100644 index 00000000..c0a0e7e3 --- /dev/null +++ b/test/cli/invalid.svg @@ -0,0 +1,3 @@ + + + diff --git a/test/svg2js/_index.test.js b/test/svg2js/_index.test.js index 84c1a596..dabd7efa 100644 --- a/test/svg2js/_index.test.js +++ b/test/svg2js/_index.test.js @@ -341,46 +341,4 @@ describe('svg2js', function () { }); }); }); - - describe('malformed svg', function () { - var filepath = PATH.resolve(__dirname, './test.bad.svg'), - root, - error; - - beforeAll(function (done) { - FS.readFile(filepath, 'utf8', function (err, data) { - if (err) { - throw err; - } - - try { - root = SVG2JS(data); - } catch (e) { - error = e; - } - - done(); - }); - }); - - describe('root', function () { - it('should have property "error"', function () { - expect(root).toHaveProperty('error'); - }); - }); - - describe('root.error', function () { - it('should be "Error in parsing SVG: Unexpected close tag"', function () { - expect(root.error).toEqual( - 'Error in parsing SVG: Unexpected close tag\nLine: 10\nColumn: 15\nChar: >' - ); - }); - }); - - describe('error', function () { - it('should not be thrown', function () { - expect(error).not.toEqual(expect.anything()); - }); - }); - }); }); diff --git a/test/svg2js/test.bad.svg b/test/svg2js/test.bad.svg deleted file mode 100644 index a02dcd9a..00000000 --- a/test/svg2js/test.bad.svg +++ /dev/null @@ -1,18 +0,0 @@ - - - - - style type="text/css"> - svg { fill: red; } - - - - - test - - - diff --git a/test/svgo/_index.test.js b/test/svgo/_index.test.js index 67b76f00..5b2b87e4 100644 --- a/test/svgo/_index.test.js +++ b/test/svgo/_index.test.js @@ -46,14 +46,6 @@ describe('svgo', () => { const result = optimize(original, { input: 'file', path: 'input.svg' }); expect(normalize(result.data)).toEqual(expected); }); - it('should handle parse error', async () => { - const fixture = await fs.promises.readFile( - path.resolve(__dirname, 'invalid.svg') - ); - const result = optimize(fixture, { input: 'file', path: 'input.svg' }); - expect(result.error).toMatch(/Error in parsing SVG/); - expect(result.path).toEqual('input.svg'); - }); it('should handle empty svg tag', async () => { const result = optimize('', { input: 'file', path: 'input.svg' }); expect(result.data).toEqual(''); diff --git a/tsconfig.json b/tsconfig.json index 383037e8..5d2560ac 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -10,7 +10,12 @@ "resolveJsonModule": true, "noImplicitAny": true }, - "include": ["plugins/**/*", "lib/xast.test.js", "lib/path.test.js"], + "include": [ + "plugins/**/*", + "lib/xast.test.js", + "lib/path.test.js", + "test/cli/**/*" + ], "exclude": [ "plugins/_applyTransforms.js", "plugins/collapseGroups.js",