Add proper error messages to linter; fix bug in 'collinear-segments' rule (#5061)

* Implement better linter error handling; fix error in 'collinear-segments' lint rule

* Revert changes

* Fix error with resolution tips

* Fix lint error

* Fix minor errors

* Increase performance bumping svg-path-segments version

* Minor change in function name and bump svg-path-segments to 0.1.5

* Update function doc

* Remove 'segmentsSVGPath' function and update functions documentation

* Fix error in index number

* Fix automatic collinear segment

.svglintrc.js

@@ -2,8 +2,9 @@ const fs = require('fs');
 
 const data = require("./_data/simple-icons.json");
 const { htmlFriendlyToTitle } = require("./scripts/utils.js");
-const svgPath = require("svgpath");
+const svgpath = require("svgpath");
 const { svgPathBbox } = require("svg-path-bbox");
+const parsePath = require("svg-path-segments");
 
 const titleRegexp = /(.+) icon$/;
 const svgRegexp = /^<svg( [^\s]*=".*"){3}><title>.*<\/title><path d=".*"\/><\/svg>\r?\n?$/;
@@ -46,6 +47,29 @@ function collinear(x1, y1, x2, y2, x3, y3) {
     return (x1 * (y2 - y3) + x2 * (y3 - y1) +  x3 * (y1 - y2)) === 0;
 }
 
+/**
+ * Returns the number of digits after the decimal point.
+ * @param num The number of interest.
+ */
+function countDecimals(num) {
+  if (num && num % 1) {
+    let [base, op, trail] = num.toExponential().split(/e([+-])/);
+    let elen = parseInt(trail, 10);
+    let idx = base.indexOf('.');
+    return idx == -1 ? elen : base.length - idx - 1 + (op === '+' ? -elen : elen);
+  }
+  return 0;
+};
+
+/**
+ * Get the index at which the first path value of an SVG starts.
+ * @param svgFileContent The raw SVG as text.
+ */
+function getPathDIndex(svgFileContent) {
+  const pathDStart = '<path d="';
+  return svgFileContent.indexOf(pathDStart) + pathDStart.length;
+}
+
 if (updateIgnoreFile) {
   process.on('exit', () => {
     // ensure object output order is consistent due to async svglint processing
@@ -153,29 +177,27 @@ module.exports = {
               return;
             }
 
-            const { segments } = svgPath(iconPath);
+            const segments = parsePath(iconPath),
-            const segmentParts = segments.flat().filter((num) => (typeof num === 'number'));
+                  svgFileContent = $.html();
 
-            const countDecimals = (num) => {
+            segments.forEach((segment) => {
-              if (num && num % 1) {
+              const precisionMax = Math.max(...segment.params.slice(1).map(countDecimals));
-                let [base, op, trail] = num.toExponential().split(/e([+-])/);
+              if (precisionMax > iconMaxFloatPrecision) {
-                let elen = parseInt(trail, 10);
+                let errorMsg = `found ${precisionMax} decimals in segment "${iconPath.substring(segment.start, segment.end)}"`;
-                let idx = base.indexOf('.');
+                if (segment.chained) {
-                return idx == -1 ? elen : base.length - idx - 1 + (op === '+' ? -elen : elen);
+                  let readableChain = iconPath.substring(segment.chainStart, chainEnd);
+                  if (readableChain.length > 20) {
+                    readableChain = `${readableChain.substring(0, 20)}...`;
+                  }
+                  errorMsg += ` of chain "${readableChain}"`
+                }
+                errorMsg += ` at index ${segment.start + getPathDIndex(svgFileContent)}`;
+                reporter.error(`Maximum precision should not be greater than ${iconMaxFloatPrecision}; ${errorMsg}`);
+                if (updateIgnoreFile) {
+                  ignoreIcon(reporter.name, iconPath, $);
+                }
               }
-              return 0;
+            })
-            };
-            const precisionArray = segmentParts.map(countDecimals);
-            const precisionMax = precisionArray && precisionArray.length > 0 ?
-              Math.max(...precisionArray) :
-              0;
-
-            if (precisionMax > iconMaxFloatPrecision) {
-              reporter.error(`Maximum precision should not be greater than ${iconMaxFloatPrecision}; it is currently ${precisionMax}`);
-              if (updateIgnoreFile) {
-                ignoreIcon(reporter.name, iconPath, $);
-              }
-            }
           },
           function(reporter, $, ast) {
             reporter.name = "ineffective-segments";
@@ -185,8 +207,8 @@ module.exports = {
               return;
             }
 
-            const { segments } = svgPath(iconPath);
+            const segments = parsePath(iconPath);
-            const { segments: absSegments } = svgPath(iconPath).abs().unshort();
+            const absSegments = svgpath(iconPath).abs().unshort().segments;
 
             const lowerMovementCommands = ['m', 'l'];
             const lowerDirectionCommands = ['h', 'v'];
@@ -202,14 +224,14 @@ module.exports = {
             const upperCurveCommands = [upperCurveCommand, upperShorthandCurveCommand];
             const curveCommands = [...lowerCurveCommands, ...upperCurveCommands];
             const commands = [...lowerMovementCommands, ...lowerDirectionCommands, ...upperMovementCommands, ...upperDirectionCommands, ...curveCommands];
-            const getInvalidSegments = ([command, x1Coord, y1Coord, ...rest], index) => {
+            const isInvalidSegment = ([command, x1Coord, y1Coord, ...rest], index) => {
               if (commands.includes(command)) {
                 // Relative directions (h or v) having a length of 0
                 if (lowerDirectionCommands.includes(command) && x1Coord === 0) {
                   return true;
                 }
                 // Relative movement (m or l) having a distance of 0
-                if (lowerMovementCommands.includes(command) && x1Coord === 0 && y1Coord === 0) {
+                if (index > 0 && lowerMovementCommands.includes(command) && x1Coord === 0 && y1Coord === 0) {
                   return true;
                 }
                 if (lowerCurveCommands.includes(command) && x1Coord === 0 && y1Coord === 0) {
@@ -273,40 +295,49 @@ module.exports = {
                 }
               }
             };
-            const invalidSegments = segments.filter(getInvalidSegments);
 
-            if (invalidSegments.length) {
+            const svgFileContent = $.html();
-              invalidSegments.forEach(([command, x1Coord, y1Coord, ...rest]) => {
+
-                let readableSegment = `${command}${x1Coord}`,
+            segments.forEach((segment, index) => {
+              if (isInvalidSegment(segment.params, index)) {
+                const [command, x1, y1, ...rest] = segment.params;
+
+                let errorMsg = `Innefective segment "${iconPath.substring(segment.start, segment.end)}" found`,
                     resolutionTip = 'should be removed';
-                if (y1Coord !== undefined) {
+
-                  readableSegment += ` ${y1Coord}`;
-                }
                 if (curveCommands.includes(command)) {
-                  const [x2Coord, y2Coord, xCoord, yCoord] = rest;
+                  const [x2, y2, x, y] = rest;
-                  readableSegment += `, ${x2Coord} ${y2Coord}`;
+
-                  if (yCoord !== undefined) {
+                  if (command === lowerShorthandCurveCommand && (x2 !== 0 || y2 !== 0)) {
-                    readableSegment += `, ${xCoord} ${yCoord}`;
+                    resolutionTip = `should be "l${removeLeadingZeros(x2)} ${removeLeadingZeros(y2)}" or removed`;
-                  }
-                  if (command === lowerShorthandCurveCommand && (x2Coord !== 0 || y2Coord !== 0)) {
-                    resolutionTip = `should be "l${removeLeadingZeros(x2Coord)} ${removeLeadingZeros(y2Coord)}" or removed`;
                   }
                   if (command === upperShorthandCurveCommand) {
-                    resolutionTip = `should be "L${removeLeadingZeros(x2Coord)} ${removeLeadingZeros(y2Coord)}" or removed`;
+                    resolutionTip = `should be "L${removeLeadingZeros(x2)} ${removeLeadingZeros(y2)}" or removed`;
                   }
-                  if (command === lowerCurveCommand && (xCoord !== 0 || yCoord !== 0)) {
+                  if (command === lowerCurveCommand && (x !== 0 || y !== 0)) {
-                    resolutionTip = `should be "l${removeLeadingZeros(xCoord)} ${removeLeadingZeros(yCoord)}" or removed`;
+                    resolutionTip = `should be "l${removeLeadingZeros(x)} ${removeLeadingZeros(y)}" or removed`;
                   }
                   if (command === upperCurveCommand) {
-                    resolutionTip = `should be "L${removeLeadingZeros(xCoord)} ${removeLeadingZeros(yCoord)}" or removed`;
+                    resolutionTip = `should be "L${removeLeadingZeros(x)} ${removeLeadingZeros(y)}" or removed`;
                   }
+                };
+
+                if (segment.chained) {
+                  let readableChain = iconPath.substring(segment.chainStart, segment.chainEnd);
+                  if (readableChain.length > 20) {
+                    readableChain = `${chain.substring(0, 20)}...`
+                  }
+                  errorMsg += ` in chain "${readableChain}"`
+                }
+                errorMsg += ` at index ${segment.start + getPathDIndex(svgFileContent)}`;
+
+                reporter.error(`${errorMsg} (${resolutionTip})`);
+
+                if (updateIgnoreFile) {
+                  ignoreIcon(reporter.name, iconPath, $);
                 }
-                reporter.error(`Ineffective segment "${readableSegment}" in path (${resolutionTip}).`);
-              });
-              if (updateIgnoreFile) {
-                ignoreIcon(reporter.name, iconPath, $);
               }
-            }
+            })
           },
           function(reporter, $, ast) {
             reporter.name = "collinear-segments";
@@ -320,11 +351,12 @@ module.exports = {
              * Extracts collinear coordinates from SVG path straight lines
              *   (does not extracts collinear coordinates from curves).
              **/
-            const getCollinearSegments = (path) => {
+            const getCollinearSegments = (iconPath) => {
-              const { segments } = svgPath(path).unarc().unshort(),
+              const segments = parsePath(iconPath),
                     collinearSegments = [],
                     straightLineCommands = 'HhVvLlMm',
                     zCommands = 'Zz';
+
               let currLine = [],
                   currAbsCoord = [undefined, undefined],
                   startPoint,
@@ -333,7 +365,7 @@ module.exports = {
                   _resetStartPoint = false;
 
               for (let s = 0; s < segments.length; s++) {
-                let seg = segments[s],
+                let seg = segments[s].params,
                     cmd = seg[0],
                     nextCmd = s + 1 < segments.length ? segments[s + 1][0] : null;
 
@@ -354,6 +386,24 @@ module.exports = {
                 } else if (cmd === 'C') {
                   currAbsCoord[0] = seg[5];
                   currAbsCoord[1] = seg[6];
+                } else if (cmd === "a") {
+                  currAbsCoord[0] = (!currAbsCoord[0] ? 0 : currAbsCoord[0]) + seg[6];
+                  currAbsCoord[1] = (!currAbsCoord[1] ? 0 : currAbsCoord[1]) + seg[7];
+                } else if (cmd === "A") {
+                  currAbsCoord[0] = seg[6];
+                  currAbsCoord[1] = seg[7];
+                } else if (cmd === "s") {
+                  currAbsCoord[0] = (!currAbsCoord[0] ? 0 : currAbsCoord[0]) + seg[1];
+                  currAbsCoord[1] = (!currAbsCoord[1] ? 0 : currAbsCoord[1]) + seg[2];
+                } else if (cmd === "S") {
+                  currAbsCoord[0] = seg[1];
+                  currAbsCoord[1] = seg[2];
+                } else if (cmd === "t") {
+                  currAbsCoord[0] = (!currAbsCoord[0] ? 0 : currAbsCoord[0]) + seg[1];
+                  currAbsCoord[1] = (!currAbsCoord[1] ? 0 : currAbsCoord[1]) + seg[2];
+                } else if (cmd === "T") {
+                  currAbsCoord[0] = seg[1];
+                  currAbsCoord[1] = seg[2];
                 } else if (cmd === 'c') {
                   currAbsCoord[0] = (!currAbsCoord[0] ? 0 : currAbsCoord[0]) + seg[5];
                   currAbsCoord[1] = (!currAbsCoord[1] ? 0 : currAbsCoord[1]) + seg[6];
@@ -398,7 +448,9 @@ module.exports = {
                                                       currLine[p + 1][0],
                                                       currLine[p + 1][1]);
                       if (_collinearCoord) {
-                        collinearSegments.push(segments[s - currLine.length + p + 1]);
+                        collinearSegments.push(
+                          segments[s - currLine.length + p + 1]
+                        );
                       }
                     }
                   }
@@ -409,19 +461,31 @@ module.exports = {
               return collinearSegments;
             }
 
-            getCollinearSegments(iconPath).forEach((segment) => {
+            const collinearSegments = getCollinearSegments(iconPath),
-              let segmentString = `${segment[0]}${segment[1]}`;
+                  pathDIndex = getPathDIndex($.html());
-              if ('LlMm'.includes(segment[0])) {
+            collinearSegments.forEach((segment) => {
-                segmentString += ` ${segment[2]}`
+              let errorMsg = `Collinear segment "${iconPath.substring(segment.start, segment.end)}" found`
+              if (segment.chained) {
+                let readableChain = iconPath.substring(segment.chainStart, segment.chainEnd);
+                if (readableChain.length > 20) {
+                  readableChain = `${readableChain.substring(0, 20)}...`
+                }
+                errorMsg += ` in chain "${readableChain}"`;
               }
-              reporter.error(`Collinear segment "${segmentString}" in path (should be removed)`);
+              errorMsg += ` at index ${segment.start + pathDIndex} (should be removed)`;
+              reporter.error(errorMsg);
             });
+
+            if (collinearSegments.length) {
+              if (updateIgnoreFile) {
+                ignoreIcon(reporter.name, iconPath, $);
+              }
+            }
           },
           function(reporter, $, ast) {
             reporter.name = "extraneous";
 
-            const rawSVG = $.html();
+            if (!svgRegexp.test($.html())) {
-            if (!svgRegexp.test(rawSVG)) {
               reporter.error("Unexpected character(s), most likely extraneous whitespace, detected in SVG markup");
             }
           },
@@ -437,13 +501,12 @@ module.exports = {
             const negativeZeroMatches = Array.from(iconPath.matchAll(negativeZerosRegexp));
             if (negativeZeroMatches.length) {
               // Calculate the index for each match in the file
-              const pathDStart = '<path d="';
+              const svgFileContent = $.html();
-              const svgFileHtml = $.html();
+              const pathDIndex = getPathDIndex(svgFileContent);
-              const pathDIndex = svgFileHtml.indexOf(pathDStart) + pathDStart.length;
 
               negativeZeroMatches.forEach((match) => {
                 const negativeZeroFileIndex = match.index + pathDIndex;
-                const previousChar = svgFileHtml[negativeZeroFileIndex - 1];
+                const previousChar = svgFileContent[negativeZeroFileIndex - 1];
                 const replacement = "0123456789".includes(previousChar) ? " 0" : "0";
                 reporter.error(`Found "-0" at index ${negativeZeroFileIndex} (should be "${replacement}")`);
               })
@@ -491,8 +554,7 @@ module.exports = {
 
               const validPathCharacters = "MmZzLlHhVvCcSsQqTtAaEe0123456789-,. ",
                     invalidCharactersMsgs = [],
-                    pathDStart = '<path d="',
+                    pathDIndex = getPathDIndex($.html());
-                    pathDIndex = $.html().indexOf(pathDStart) + pathDStart.length;
 
               for (let [i, char] of Object.entries(iconPath)) {
                 if (validPathCharacters.indexOf(char) === -1) {

icons/automatic.svg

@@ -1 +1 @@
-<svg role="img" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><title>Automatic icon</title><path d="M12 6.768v.002-1.237c-.485.033-.754.293-.99.71L5.87 16.72h2.464l-.753-.96.654-1.363.005.007L12 6.774v-.006zM10.526 13.123h2.946L12 10.076M8.233 14.416h.017l-.01-.013M13.473 13.123v.002M21.496 5.066L13.26.308c-.693-.4-1.827-.4-2.52 0L2.504 5.066c-.693.398-1.26 1.38-1.26 2.182v9.507c0 .802.567 1.782 1.26 2.18l8.236 4.757c.693.4 1.826.4 2.52 0l8.235-4.768c.692-.39 1.26-1.38 1.26-2.174V7.246c0-.8-.567-1.78-1.26-2.18zm-6.066 12.05l-.687-1.384h-5.5l-.673 1.384H5.287l5.396-11.033c.305-.607.777-.9 1.317-.9s1.034.328 1.316.89l5.396 11.043H15.43zM12 6.77V9.244l2.518 5.173H8.25l.758.94h5.972l.674 1.35h2.474l-1.708-.99v.04L12 6.77"/></svg>
+<svg role="img" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><title>Automatic icon</title><path d="M12 6.768v-1.235c-.485.033-.754.293-.99.71L5.87 16.72h2.464l-.753-.96.654-1.363.005.007L12 6.774v-.006zM10.526 13.123h2.946L12 10.076M8.233 14.416h.017l-.01-.013M13.473 13.123v.002M21.496 5.066L13.26.308c-.693-.4-1.827-.4-2.52 0L2.504 5.066c-.693.398-1.26 1.38-1.26 2.182v9.507c0 .802.567 1.782 1.26 2.18l8.236 4.757c.693.4 1.826.4 2.52 0l8.235-4.768c.692-.39 1.26-1.38 1.26-2.174V7.246c0-.8-.567-1.78-1.26-2.18zm-6.066 12.05l-.687-1.384h-5.5l-.673 1.384H5.287l5.396-11.033c.305-.607.777-.9 1.317-.9s1.034.328 1.316.89l5.396 11.043H15.43zM12 6.77V9.244l2.518 5.173H8.25l.758.94h5.972l.674 1.35h2.474l-1.708-.99v.04L12 6.77"/></svg>

package-lock.json

@@ -5212,6 +5212,12 @@
         "svgpath": "^2.3.0"
       }
     },
+    "svg-path-segments": {
+      "version": "0.1.5",
+      "resolved": "https://registry.npmjs.org/svg-path-segments/-/svg-path-segments-0.1.5.tgz",
+      "integrity": "sha512-7USWOeNJEQxzj5KgyvyuWxqF/ULyI/iY1cMl7mGB7aI5ujtH6jcR9cwJKnPbNyaV2xDmHq4RkCHUM8HII6Al+w==",
+      "dev": true
+    },
     "svglint": {
       "version": "1.0.7",
       "resolved": "https://registry.npmjs.org/svglint/-/svglint-1.0.7.tgz",

package.json

@@ -24,6 +24,7 @@
     "jsonschema": "1.4.0",
     "npm-run-all": "4.1.5",
     "svg-path-bbox": "0.2.0",
+    "svg-path-segments": "0.1.5",
     "svglint": "1.0.7",
     "svgo": "2.2.0",
     "svgpath": "2.3.1",