diff --git a/.eslintrc.js b/.eslintrc.js index 238ef3a..a8f9bcb 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -18,6 +18,7 @@ module.exports = { "semi": [ "error", "always" - ] + ], + "no-console": "warn", } }; diff --git a/src/config.js b/src/config.js deleted file mode 100644 index 5fa3fb3..0000000 --- a/src/config.js +++ /dev/null @@ -1,35 +0,0 @@ -const path = require("path"); -const fs = require("fs"); - -/** - * Gets the configuration file to use - * Throws if file isn't found - * @param {String} filename The filename to look for - * @param {String} folder The folder to look in - * @returns {Promise} The path to the configuration file, or false - */ -function getConfigurationFile(filename=".svglintrc.js", folder=__dirname) { - let resolved = path.resolve(folder, filename); - return new Promise((res,rej)=>{ - fs.exists(resolved, exists=>{ - if (exists) { - // if file exists, finalize - res(resolved); - } else { - const parent = path.resolve(folder, ".."); - if (parent === folder) { - return rej(new Error("Config file not found")); - } - // if not, get next folder - getConfigurationFile( - filename, - path.resolve(folder, "..") - ).then(res).catch(rej); - } - }); - }); -} - -module.exports = { - getConfigurationFile, -}; \ No newline at end of file diff --git a/src/lib/linting.js b/src/lib/linting.js new file mode 100644 index 0000000..aaf5d9b --- /dev/null +++ b/src/lib/linting.js @@ -0,0 +1,139 @@ +/** + * @fileoverview The main linting file. + * This is the object responsible for the actual linting of each file. + * Each instance represents a single file being linted, including results and + * current state. + * It receives the parsed AST and rules from ../svglint.js, and then runs each + * rule and gathers the results. + */ +const EventEmitter = require("events").EventEmitter; +const path = require("path"); +const Reporter = require("./reporter"); +const logger = require("./logger"); + +const STATES = Object.freeze({ + "ignored": -1, + "linting": undefined, + "success": "success", + "warn": "warning", + "error": "error", + + "_-1": "ignored", + "_undefined": "linting", + "_success": "success", + "_warning": "warn", + "_error": "error", +}); + +/** + * Represents a single file that is being linted. + * Contains the status and potential result of the linting. + * @event rule Emitted when a rule is finished + * @event done Emitted when the linting is done + */ +class Linting extends EventEmitter { + /** + * Creates and starts a new linting. + * @param {String} file The file to lint + * @param {AST} ast The AST of the file + * @param {NormalizedRules} rules The rules that represent + */ + constructor(file, ast, rules) { + super(); + this.ast = ast; + this.rules = rules; + this.path = file; + this.state = STATES.linting; + this.name = file + ? path.relative(process.cwd(), file) + : ""; + /** @type Object */ + this.results = {}; + + this.lint(); + // TODO: add reporter + } + + /** + * Starts the linting. + * Errors from rules are safely caught and logged as exceptions from the rule. + */ + lint() { + this.state = STATES.linting; + + // keep track of when every rule has finished + const rules = Object.keys(this.rules); + this.activeRules = rules.length; + + logger.debug("Started linting", (this.name || "API-provided file")); + logger.debug(" Rules:", rules); + + // start every rule + rules.forEach(ruleName => { + // gather results from the rule through a reporter + const reporter = this._generateReporter(ruleName); + const onDone = () => { + this._onRuleFinish(ruleName, reporter); + }; + + // execute the rule, potentially waiting for async rules + // also handles catching errors from the rule + Promise.resolve() + .then(() => this.rules[ruleName](reporter)) + .catch(e => reporter.exception(e)) + .then(onDone); + }); + } + + /** + * Handles a rule finishing. + * @param {String} ruleName The name of the rule that just finished + * @param {Reporter} reporter The reporter containing rule results + * @emits rule + * @private + */ + _onRuleFinish(ruleName, reporter) { + logger.debug("Rule finished", logger.colorize(ruleName)); + this.emit("rule", { + name: ruleName, + reporter, + }); + this.results[ruleName] = reporter; + + --this.activeRules; + if (this.activeRules === 0) { + this.state = this._calculateState(); + logger.debug("Linting finished", logger.colorize(STATES["_"+this.state])); + this.emit("done"); + } + } + + /** + * Calculates the current state from this.results. + * @returns One of the valid states + */ + _calculateState() { + let state = STATES.success; + for (let k in this.results) { + const result = this.results[k]; + if (result.errors.length) { return STATES.error; } + if (result.warns.length || state === STATES.warn) { + state = STATES.warn; + } + } + return state; + } + + /** + * Generates a Reporter for use with this file. + * Remember to call .done() on it. + * @param {String} ruleName The name of the rule that this reporter is used for + * @private + */ + _generateReporter(ruleName) { + return new Reporter(ruleName); + } +} +Linting.STATES = STATES; + +module.exports = Linting; diff --git a/src/lib/logger.js b/src/lib/logger.js new file mode 100644 index 0000000..13e815b --- /dev/null +++ b/src/lib/logger.js @@ -0,0 +1,41 @@ +/** + * @fileoverview Exposes the logger we should use for displaying info. + * If called using the JS API, this will be `console` with methods prefixed. + * If called using the CLI, this will be our own custom logger. + */ +const chalk = require("chalk"); +const inspect = require("util").inspect; + +const CONSOLE_COLORS = Object.freeze({ + debug: chalk.dim.gray, + log: chalk.blue, + warn: chalk.yellow, + error: chalk.red, +}); + +const wrappedConsole = Object.create(console); +const prefixRegexp = /^\[([^\s]+)\]$/; +["debug", "log", "warn", "error"].forEach(method => { + const color = CONSOLE_COLORS[method] + ? CONSOLE_COLORS[method] + : v => v; + + wrappedConsole[method] = function() { + let prefix = "[SVGLint"; + const args = [...arguments]; + // merge the two prefixes if given + if (typeof args[0] === "string") { + const prefixResult = prefixRegexp.exec(args[0]); + if (prefixResult) { + prefix = prefix + " " + prefixResult[1]; + args.shift(); + } + } + // eslint-disable-next-line no-console + console[method].apply(console, [color(prefix + "]"), ...args]); + }; +}); + + +module.exports = wrappedConsole; +module.exports.colorize = value => inspect(value, true, 2, true); diff --git a/src/lib/parse.js b/src/lib/parse.js new file mode 100644 index 0000000..79cf810 --- /dev/null +++ b/src/lib/parse.js @@ -0,0 +1,129 @@ +/** + * @fileoverview The SVG -> AST parser. + * This handles turning an SVG source into an AST representing it. + * It uses htmlparser2 to parse the source, which it gathers from either + * a string or a file. + */ +const Parser = require("htmlparser2"); +const fs = require("fs"); + +module.exports = { + /** + * Parses an SVG source into an AST + * @param {String} source The source to parse + * @returns {AST} The parsed AST + */ + parseSource(source) { + return normalizeAST( + sourceToAST(source), + source + ); + }, + + /** + * Parses the content of a file into an AST + * @param {String} file The path of the file in question + * @returns {Promise} The parsed AST + */ + parseFile(file) { + return new Promise((res, rej) => { + fs.readFile( + file, + "utf8", + (err, data) => { + if (err) { + return rej(err); + } + try { return res(module.exports.parseSource(data)); } + catch (e) { return rej(e); } + } + ); + }); + } +}; + +/** + * @typedef {Object} Attributes + */ +/** + * @typedef Node + * @property {String} type The type of node + * @property {Node} next The next sibling + * @property {Node} prev The previous sibling + * @property {Node} parent The parent of the node + * @property {Number} startIndex The string index at which the element starts + * @property {Number} endIndex The string index at which the element ends + * @property {Number} lineNum The line number at which the element starts + * @property {Number} lineIndex The index in the line at which the element starts + * + * @property {Attributes} [attribs] An object of attributes on the Node + * @property {AST} [children] The children of the Node + * @property {String} [data] If type==="text", the content of the Node + * @property {String} [name] If type!=="text", the tag name + */ +/** + * @typedef {Node[]} AST + * @property {String} source The source that generated the AST + * An AST representing an SVG document (or a list of children). + */ + +/** + * Parses an SVG source code into an AST. + * @param {String} source + * @returns {AST} The parsed AST + */ +function sourceToAST(source) { + // @ts-ignore + return Parser.parseDOM(source, { + withStartIndices: true, + withEndIndices: true, + xmlMode: true, + }); +} + +/** + * Normalizes a Node to the format we want. + * Currently translates the startIndex to a line number+index. + * == MODIFIES THE NODE IN-PLACE! == + * @param {Node} node The node to normalize + * @param {String} source The string the AST was generated from + */ +function normalizeNode(node, source) { + // calculate the distance from node start to line start + const lineStart = ( + source.lastIndexOf("\n", node.startIndex + + // make sure newline text nodes are set to start on the proper line + ((node.type === "text" && node.data.startsWith("\n")) ? -1 : 0)) + ) + 1; + node.lineIndex = node.startIndex - lineStart; + + // calculate the line number + let numLines = 0; + let lineIndex = lineStart; + while ((lineIndex = source.lastIndexOf("\n", lineIndex - 1)) !== -1) { + ++numLines; + } + node.lineNum = numLines; + return node; +} + +/** + * Normalizes the AST returned by htmlparser2 to the format we want. + * Currently translates the startIndex to a line number+index. + * == MODIFIES THE AST IN-PLACE! == + * @param {AST} ast The AST to normalize + * @param {String} source The source the AST was generated from + * @returns {AST} The normalized AST + */ +function normalizeAST(ast, source) { + const handleNode = node => { + normalizeNode(node, source); + if (node.children) { + node.children.forEach(handleNode); + } + }; + ast.forEach(handleNode); + // @ts-ignore + ast.source = source; + return ast; +} diff --git a/src/lib/reporter.js b/src/lib/reporter.js new file mode 100644 index 0000000..67b4c5d --- /dev/null +++ b/src/lib/reporter.js @@ -0,0 +1,99 @@ +/** + * @fileoverview The object that rules use to report errors, warnings and messages. + */ +const EventEmitter = require("events").EventEmitter; +const logger = require("./logger"); + +/** + * @typedef {Object} Result + * @property {String} message The message as a single string, suitable for human consumption + * @property {String} [stacktrace] If Result is related to a node, a human-suitable string showing the related part of the file + * @property {any[]} _message The original message, as given by the rule + * @property {Node} [_node] If Result is related to a node, the related node + * @property {AST} [_ast] If Result is related to a node, the related AST + */ + +/** + * Generates a Result from the arguments given to .error()/.warn()/.log(). + * Mostly involves formatting the message that should be shown when logged. + * @param {any[]|any} message The message of the result, in console.log format + * @param {Node} [node] If the error is related to a node, the related node + * @param {AST} [ast] If the error is related to a node, the related AST + * @returns {Result} + */ +function generateResult(message, node, ast) { + const _message = message instanceof Array ? message : [message]; + return { + message: "foo", + _message, + _node: node, + _ast: ast, + }; +} + +class Reporter extends EventEmitter { + /** + * @param {String} name The name of this reporter + */ + constructor(name) { + super(); + this.name = name; + /** @type {Error[]} */ + this.exceptions = []; + /** @type {Result[]} */ + this.errors = []; + /** @type {Result[]} */ + this.warns = []; + /** @type {Result[]} */ + this.logs = []; + } + + /** + * Reports that an exception occured during rule processing. + * This doesn't change the current linting result, but is important to show + * to users as it indicates that the linting result cannot be trusted. + * @param {Error} e The exception that occured. + */ + exception(e) { + logger.debug("["+this.name+"]", "Exception reported:", e); + this.emit("exception", e); + this.exceptions.push(e); + } + + /** + * Reports that an error was found during linting. + * @param {any[]|any} message The message of the result, in console.log format + * @param {Node} [node] If the error is related to a node, the related node + * @param {AST} [ast] If the error is related to a node, the AST of the file + */ + error(message, node, ast) { + logger.debug("["+this.name+"]", "Error reported:", JSON.stringify(message)); + const result = generateResult(message, node, ast); + this.errors.push(result); + } + + /** + * Reports that a warning was found during linting. + * @param {any[]|any} message The message of the result, in console.log format + * @param {Node} [node] If the warning is related to a node, the related node + * @param {AST} [ast] If the warning is related to a node, the AST of the file + */ + warn(message, node, ast) { + logger.debug("["+this.name+"]", "Warn reported:", JSON.stringify(message)); + const result = generateResult(message, node, ast); + this.warns.push(result); + } + + /** + * Shows a message to the user. + * @param {any[]|any} message The message of the result, in console.log format + * @param {Node} [node] If the message is related to a node, the related node + * @param {AST} [ast] If the message is related to a node, the AST of the file + */ + log(message, node, ast) { + logger.debug("["+this.name+"]", "Log reported:", JSON.stringify(message)); + const result = generateResult(message, node, ast); + this.logs.push(result); + } +} +module.exports = Reporter; diff --git a/src/lib/rule-loader.js b/src/lib/rule-loader.js new file mode 100644 index 0000000..a8c95f5 --- /dev/null +++ b/src/lib/rule-loader.js @@ -0,0 +1,33 @@ +/** + * @fileoverview Turns a rule name into a module import. + * Can be extended to use a cache if we have to do heavier processing when + * loading a rule. + * Currently NodeJS' import cache is just fine. + */ +const path = require("path"); + +/** + * @typedef RuleModule + * @property {Function} generate When given a config, returns a linting function + */ + +/** + * Finds and imports a rule from its name. + * If the rule is named in the format "a/b" then the rule will be loaded from + * the package "svglint-plugin-a/b". + * If the rule name does not contain a slash then it will be loaded from the + * built-in SVGLint rules. + * @param {String} ruleName The name of the rule + * @param {String} [dir] The dir to load the rules from + * @returns {RuleModule} The function exported by the rule if found. + */ +function ruleLoader(ruleName, dir="../rules") { + const fileName = ruleName.endsWith(".js") + ? ruleName + : ruleName + ".js"; + const isExternal = ruleName.includes("/"); + return require(isExternal + ? "svglint-plugin-" + ruleName + : path.join(dir, fileName)); +} +module.exports = ruleLoader; diff --git a/src/log.js b/src/log.js deleted file mode 100644 index 826a6ea..0000000 --- a/src/log.js +++ /dev/null @@ -1,285 +0,0 @@ -const util = require("util"); -const chalk = require("chalk"); -const logUpdate = require("log-update"); -const FileReporter = require("./reporter"); -const { chunkString } = require("./util"); -const STATES = FileReporter.STATES; - -// @ts-ignore -const columns = process.stdout.columns || 80; -const separator = (str="") => { - const halfLength = str ? - Math.floor((columns-str.length)/2) - : Math.floor(columns/2); - const half = " ".repeat(halfLength); - str = chalk.bold.underline(str); - return `${half}${str}${half}`; -}; -separator.toString = separator; - -const MSG_META = { - "unknown": { - symbol: "?", - color: chalk.gray, - }, - "success": { - symbol: "✓", - color: chalk.green, - prefix: "[Success]", - }, - "debug": { - symbol: "d", - color: chalk.gray, - prefix: "[Debug]" - }, - "info": { - symbol: "i", - color: chalk.blue, - prefix: "[Info]", - }, - "warning": { - symbol: "!", - color: chalk.yellow, - prefix: "[Warn]", - }, - "error": { - symbol: "x", - color: chalk.red, - prefix: "[Error]" - } -}; - -class Log { - constructor() { - this.state = { - logs: [], /** @type {Array} */ - fileReporters: [], - frame: 0, - }; - this._cachedRender = ""; - this.debugging = false; - this.PREFIX_LENGTH = 8; - this.SPINNER = [ - " ", - ". ", - ".. ", - "...", - " ..", - " .", - ]; - } - - /** - * Animates if we should animate, increasing frame count and rendering - * Recalls self - */ - tick() { - if (this.shouldAnimate()) { - this.setState({ frame: this.state.frame + 1 }); - - clearTimeout(this.tickTimeout); - this.tickTimeout = setTimeout(this.tick.bind(this), 120); - } - } - - shouldAnimate() { - return this.state.fileReporters.some( - reporter => reporter.getStatus() === STATES.unknown - ); - } - - /** Updates the actually displayed string in terminal */ - output(string) { - logUpdate(string); - } - - /** @returns {String} */ - render() { - return [ - this.state.logs.length ? separator("LOG") : "", - ...this.state.logs, - this.state.fileReporters.length ? separator("FILES") : "", - ...this.renderFiles(this.state.fileReporters) - ].filter(Boolean).join("\n"); - } - - /** Forces an update */ - forceUpdate() { - const rendered = this.render(); - this._cachedRender = rendered; - this.output(rendered); - } - - /** @param {Object} newState The new state to set - is shallow merged with current state */ - setState(newState) { - this.state = Object.assign(this.state, newState); - const rendered = this.render(); - if (rendered !== this._cachedRender) { - this._cachedRender = rendered; - this.output(rendered); - } - } - - /** - * Formats some files for logging - * @param {Object[]} fileReporters - * @returns {Array} - */ - renderFiles(fileReporters) { - return fileReporters.map(reporter => { - const { status, messages } = reporter.getState(); - let meta; - switch (status) { - case STATES.success: - meta = MSG_META.success; - break; - case STATES.warning: - meta = MSG_META.warning; - break; - case STATES.error: - meta = MSG_META.error; - break; - default: - meta = MSG_META.unknown; - break; - } - - let outp = meta.color(`[${meta.symbol}] ${reporter.name}`); - if (status === STATES.unknown) { - outp += " " + this.getSpinner(); - } else if (status !== STATES.success) { - const messages = reporter.getMessages(); - const padding = "\n "; - - outp += padding+chunkString( - messages.map( - this.stringifyMessage.bind(this) - ).filter(Boolean).join("\n"), - columns-(padding.length-1) - ).join(padding); - } - return outp; - }); - } - - /** - * Stringifies a message from a reporter - */ - stringifyMessage(msg) { - if (!msg || (msg.type === STATES.success - && (!msg.args || !msg.args.length))) { - return null; - } - - let meta; - switch (msg.type) { - case STATES.success: - meta = MSG_META.success; - break; - case STATES.warning: - meta = MSG_META.warning; - break; - case STATES.error: - meta = MSG_META.error; - break; - default: - meta = MSG_META.unknown; - } - - return `${meta.color(msg.rule)} ${this.stringifyArgs(msg.args)}`; - } - - /** - * Stringifies a list of data into a colorized single line - */ - stringifyArgs(args) { - return args.map( - v => ( - typeof v === "string" - ? v - : util.inspect(v, { colors: true, depth: 3 }) - ).replace(/^Error: /, "") - ).join(" "); - } - - /** - * Gets an object which handles logging for a single file - * @param {String} filePath The path to log for - */ - getFileReporter(filePath) { - const reporter = new FileReporter(filePath); - this.setState({ - fileReporters: [...this.state.fileReporters, reporter] - }); - return reporter; - } - - /** - * Formats some arguments for logging - * @returns {String} - */ - formatLog(...args) { - const stringified = this.stringifyArgs(args); - - // split string into column sized chunks - // then indent them to the prefix length - return chunkString(stringified, columns-this.PREFIX_LENGTH) - .map((v,i) => " ".repeat(i?this.PREFIX_LENGTH-1:0)+v) - .join("\n"); - } - - /** Gets a colored and padded prefix */ - getPrefix(metaObj) { - return metaObj.color( - metaObj.prefix.padEnd(this.PREFIX_LENGTH, " ") - ); - } - - /** Gets the current frame of the spinner */ - getSpinner() { - return this.SPINNER[this.state.frame % this.SPINNER.length]; - } - - /** Logs a debug message */ - debug(...args) { - if (this.debugging) { - this.setState({ - logs: [ - ...this.state.logs, - this.getPrefix(MSG_META.debug)+this.formatLog.apply(this, args) - ] - }); - } - } - /** Logs an informational message */ - log(...args) { - this.setState({ - logs: [ - ...this.state.logs, - this.getPrefix(MSG_META.info)+this.formatLog.apply(this, args) - ] - }); - } - /** Logs a warning message */ - warn(...args) { - this.setState({ - logs: [ - ...this.state.logs, - this.getPrefix(MSG_META.warning)+this.formatLog.apply(this, args) - ] - }); - } - /** Logs an error message */ - error(...args) { - this.setState({ - logs: [ - ...this.state.logs, - this.getPrefix(MSG_META.error)+this.formatLog.apply(this, args) - ] - }); - } -} - - - -module.exports = new Log(); diff --git a/src/reporter.js b/src/reporter.js deleted file mode 100644 index a131f60..0000000 --- a/src/reporter.js +++ /dev/null @@ -1,132 +0,0 @@ -const EventEmitter = require("events").EventEmitter; -const path = require("path"); -const STATES = { - unknown: undefined, - success: true, - warning: "warn", - error: "error" -}; - -/** - * A reporter for a single file's linting - * Emits messages of the format { type: , args: any[] } - */ -class FileReporter extends EventEmitter { - constructor(filePath) { - super(); - this.name = path.relative(process.cwd(), filePath); - this.rules = []; - this.messages = []; - } - - getState() { - const STATES_ORDER = [ - STATES.success, - STATES.unknown, - STATES.warning, - STATES.error - ]; - let messages = []; - let status = STATES.success; - this.rules.forEach(rule => { - messages = messages.concat( - rule.getMessages() - ); - const ruleStatus = rule.status; - if (STATES_ORDER.indexOf(status) < STATES_ORDER.indexOf(ruleStatus)) { - status = ruleStatus; - } - }); - - return { - status, - messages - }; - } - - getMessages() { - return this.messages; - } - - emitMsg(msg) { - this.messages.push(msg); - this.emit("msg", msg); - switch (msg.type) { - case STATES.success: - this.emit("msg--success", msg); - break; - case STATES.warning: - this.emit("msg--warning", msg); - break; - case STATES.error: - this.emit("msg--error", msg); - break; - } - } - - getRuleReporter(name) { - const reporter = new RuleReporter(name); - reporter.on("msg", this.emitMsg.bind(this)); - this.rules.push(reporter); - return reporter; - } -} - -class RuleReporter extends EventEmitter { - constructor(name) { - super(); - this.name = name; - this.messages = []; - this.status = STATES.unknown; - } - - succeed(...args) { - if (this.status === STATES.unknown) { - this.status = STATES.success; - } - let msg = undefined; - if (args.length) { - msg = { - type: STATES.success, - rule: this.name, - args - }; - this.messages.push(msg); - } - this.emit("msg", msg); - this.emit("msg--success", msg); - } - - warn(...args) { - if (this.status !== STATES.error) { - this.status = STATES.warning; - } - const msg = { - type: STATES.warning, - rule: this.name, - args - }; - this.messages.push(msg); - this.emit("msg", msg); - this.emit("msg--warning", msg); - } - - error(...args) { - this.status = STATES.error; - const msg = { - type: STATES.error, - rule: this.name, - args - }; - this.messages.push(msg); - this.emit("msg", msg); - this.emit("msg--error", msg); - } - - getMessages() { - return this.messages; - } -} - -FileReporter.STATES = STATES; -module.exports = FileReporter; diff --git a/src/rules/CONTRIBUTING.md b/src/rules/CONTRIBUTING.md deleted file mode 100644 index f1ae201..0000000 --- a/src/rules/CONTRIBUTING.md +++ /dev/null @@ -1,26 +0,0 @@ -# Writing rules - -## Rule naming - -Rules are generally named after the format `[/]`. -Meta-rules (rules that change how the rule functions) are named after the format -`rule::/`. The rule name should never contain `:`. - -For instance, a rule that checks for the presence of a11y attributes -could be called `a11y/attr`, while a generic rule that checks for attributes -could be called `attr`. - -## File naming - -Rules should be defined in their own file, named the same as the rule name -with `"/"` replaced by `"_"`. For instance, `a11y/attr` should be defined in -`a11y_attr.js`. - -## Rule behavior - -The rule should consist of a generator function that receives the rule value from the config. -The generator function should then return a new function, which receives [a cheerio document](https://www.npmjs.com/package/cheerio) and a reporter, on which it should call `.warn()`, `.error()`, or `.succeed()`. - -### Rule results - -The rule should report its result through the passed RuleReporter - an object with three methods, `.warn()`, `.error()`, and `.succeed()`. Succeed should only be called once, but the other methods can be called once for each warning/error you wish to report. diff --git a/src/rules/README.md b/src/rules/README.md deleted file mode 100644 index ebbaecf..0000000 --- a/src/rules/README.md +++ /dev/null @@ -1,66 +0,0 @@ -# Rules - -Rules are specified in the configuration under the `rules` key, as a map: - -```javascript -{ - rules: { - "attr/root": { viewBox: "0 0 16 16" }, - title: true - } -} -``` - -## Available rules - -### `attr` - -Specifies the attributes on the elements that match the selector. -Specified as a map with keys mapping to the wanted values. Supported value types are `Array|String|Boolean`. -The selector is given in key `rule::selector`. It defaults to `"*"`. - -Default functionality acts as a blacklist. If the key `rule::whitelist` is set to `true`, it will instead act as a whitelist. - -```javascript -[{ - role: ["img", "progressbar"], // role must be one of ["img","progressbar"] - viewBox: "0 0 24 24", // viewBox must be "0 0 24 24" - xmlns: true, // xmlns must be set - width: false, // width must not be set - "rule::whitelist": true, // no other attributes can be set - "rule::selector": "svg", // check attributes on the root svg object -}, { - "rule::whitelist": true, // ban all attributes - "rule::selector": "title", // on all title elements -}, { - stroke: false, // ban strokes on all elements -}] -``` - -### `elm` - -Specifies the elements that must/must not exist. -Specified as a map with keys mapping to a value. Supported value types are `Array|Number|Boolean`. -The key is used as a selector. -If the value is a boolean, it indicates whether the selector must be matched (`true`) or must not be matched (`false`). -If the value is a number, it indicates that exactly that number of matches must be found. -If the number is an array, it must have a length of 2, and indicates the range between which the number of matches must be. - -If an element is permitted by one rule and rejected by another, it is overall permitted. - -```javascript -{ - "svg": true, // the root svg element must exist - "svg > title": true, // the title element must exist inside the root element - "g": 2, // exactly 2 groups must exist - "g > path": [0,2], // up to and including two paths can be in each group - "*": false, // nothing else can exist -} -``` - -### `custom` - -A custom function. -Is given the cheerio object representing the SVG, and must return either `true` for success, or `false` for error. -If a string is returned it is interpretted as an error, and the string used as error message. -If an array of strings is returned it is interpretted as an array of errors, and the strings used as error messages. diff --git a/src/rules/attr.js b/src/rules/attr.js deleted file mode 100644 index 466fbb7..0000000 --- a/src/rules/attr.js +++ /dev/null @@ -1,111 +0,0 @@ -const { str_expected, flatten } = require("../util"); -const log = require("../log"); - -function testAttr(value, expected) { - // handle arrays (must be one of) - if (expected instanceof Array) { - // at least one value in the expected array must match - return expected.map(exp => testAttr(value, exp)) - .some(Boolean); - } - - // handle booleans (must be set/must not be set) - if (expected instanceof Boolean) { - // if value is set, fail if expected === false - if (value !== undefined) { - return !!expected; - // if value is not set, fail if expected === true - } else { - return !expected; - } - } - - // handle custom regexs - if (expected instanceof RegExp) { - return expected.test(value); - } - - // handle custom functions - if (expected instanceof Function) { - return expected(value); - } - - // handle booleans - if (expected === true) { - return !!value; - } - - // handle other matches - return value === expected; -} - -module.exports = function attrGenerator(config={}) { - const allowUndefined = !config["rule::whitelist"]; - const selector = config["rule::selector"] || "*"; - - config = Object.assign({}, config); - delete config["rule::whitelist"]; - delete config["rule::selector"]; - log.debug("[attr] Executing", config); - - // the actual linting function - return function($, reporter) { - const nodes = $(selector).toArray(); - const nodeResults = nodes.map( - node => { - const attrResults = Object.keys(node.attribs || {}) - .map(attr => { - log.debug("[attr] Checking", attr, { actual: node.attribs[attr], expected: config[attr] }); - if (config[attr] === undefined) { - if (!allowUndefined) { - reporter.error( - `Unexpected attribute "${attr}" - ${$.html($(node).empty())}`); - } - return; - } - - const result = testAttr(node.attribs[attr], config[attr]); - if (result === true) { - return true; - } - - reporter.error( - `Failed on attr "${attr}"; ${str_expected(config[attr])}`, config[attr], ` - ${$.html($(node).empty())}` - ); - }); - - // also check for missing attributes - const missing = Object.keys(config) - .filter(k => config[k] === true) - .map(k => { - log.debug("[attr] Checking for missing", k); - if (node.attribs && node.attribs[k] === undefined) { - reporter.error( - `Missing attribute "${k}" - ${$.html($(node).empty())}` - ); - return; - } - return true; - }) - .filter(Boolean); - - const results = attrResults.concat(missing); - if (results.every(v => v === true)) { - return true; - } else { - return results.filter(v => v !== true); - } - } - ); - - if (nodeResults.every(v => v === true)) { - reporter.succeed(); - return; - } else { - return flatten(nodeResults.filter(v => v !== true)); - } - }; -}; diff --git a/src/rules/elm.js b/src/rules/elm.js deleted file mode 100644 index 394167c..0000000 --- a/src/rules/elm.js +++ /dev/null @@ -1,75 +0,0 @@ -const cheerio = require("cheerio"); - -function checkMatch(nodes, value, config, selector, $, reporter) { - if (value === true) { - if (nodes.length !== 0) { return true; } - reporter.error( - "Element required for selector", `"${selector}"` - ); - return; - } - if (typeof value === "number") { - if (nodes.length === value) { return true; } - reporter.error( - "Expected", value, "elements, found", nodes.length, "for selector", `"${selector}"` - ); - return; - } - if (value instanceof Array) { - if (nodes.length >= value[0] && nodes.length <= value[1]) { return true; } - reporter.error( - "Expected between", value[0], "and", value[0], "elements, found", nodes.length, "for selector", `"${selector}"` - ); - return; - } - if (value === false) { - // check that at least one node isn't allowed by other rules - const nonAllowedNodes = nodes.filter( - (i, node) => { - let allowed = false; - for (let selector in config) { - if (!config.hasOwnProperty(selector)) { continue; } - const val = config[selector]; - if (val === false) { continue; } - if ($(selector).is(node)) { - allowed = true; - break; - } - } - return !allowed; - } - ); - - if (!nonAllowedNodes.length) { return true; } - reporter.error( - "Expected no elements, found", nonAllowedNodes.length, "for selector", `"${selector}"`, ` - ${nonAllowedNodes.map( - (i, node) => cheerio.html(cheerio(node).empty()) - ).toArray().join("\n ")}` - ); - return; - } -} - -module.exports = function elmGenerator(config={}) { - // the actual rule function - return function($, reporter) { - const outp = []; - Object.keys(config).forEach( - selector => { - const nodes = $(selector); - const val = config[selector]; - - const match = checkMatch(nodes, val, config, selector, $, reporter); - if (match !== true) { - outp.push(match); - } - } - ); - - if (!outp.length) { - return true; - } - return outp; - }; -}; diff --git a/src/rules/identity.js b/src/rules/identity.js new file mode 100644 index 0000000..143f4f5 --- /dev/null +++ b/src/rules/identity.js @@ -0,0 +1,23 @@ +const logger = require("../lib/logger"); + +/** + * @typedef IdentityConfig + * @property {"error"|"warn"|null} method The method to call on reporter + * @property {String} message The message to warn/error with + */ + +module.exports = { + /** + * Generates a linting function from a config + * @param {IdentityConfig} config + */ + generate(config) { + return function IdentityRule(reporter) { + logger.log("[rule:identity]", "Called", config); + // Report the message if type !== succeed + if (config.method) { + reporter[config.method](config.message); + } + }; + } +}; diff --git a/src/svglint.js b/src/svglint.js index 8f93e48..ee26a7b 100644 --- a/src/svglint.js +++ b/src/svglint.js @@ -1,103 +1,124 @@ -const fs = require("fs"); -const path = require("path"); -const cheerio = require("cheerio"); -const flatten = require("./util").flatten; -const transformRuleName = require("./util").transformRuleName; -const log = require("./log"); +/** + * @fileoverview The SVGLint entry file. + * This is the publicly exposed JS API, which the CLI also uses. + * It exposes .lintSource() and .lintFile(). + * Main responsibility is handling the consumer<->Linting communication, + * and converting the user-provided config into an object of rules. + */ +const Linting = require("./lib/linting"); +const parse = require("./lib/parse"); +const loadRule = require("./lib/rule-loader"); +const logger = require("./lib/logger.js"); -class SVGLint { - constructor(config) { - this.config = config; - } +/** + * @typedef {Object|false>} RulesConfig + * An object with each key representing a rule name, and each value representing + * a rule config. + * If the rule config is set to `false`, then the rule is disabled (useful for + * e.g. overwriting presets). + */ +/** + * @typedef {Object} NormalizedRules + * The RulesConfig after being normalized - each function is a rule. + */ +/** + * @typedef {String[]} IgnoreList + * An array of strings, each of which is a blob that represents files to ignore. + * If any blob matches a file, the file is not linted. + */ +/** + * @typedef Config + * @property {Boolean} [useSvglintRc=true] Whether to merge config with the one defined in .svglintrc + * @property {RulesConfig} [rules={}] The rules to lint by + * @property {IgnoreList} [ignore=[]] The blobs representing which files to ignore + */ +/** + * @typedef NormalizedConfig + * @property {NormalizedRules} rules The rules to lint by + * @property {IgnoreList} ignore The blobs representing which files to ignore + */ - /** - * Gets the function responsible for executing a rule, given a rule object - * @param {String} ruleName The name of the rule to get executor for - * @returns {Function} The function that executes the rule - */ - getRuleExecutor(ruleName) { - const rulePath = path.join("rules/", transformRuleName(ruleName)); - try { - return require(`./${rulePath}`); - } catch (e) { - e.message = `Couldn't load rule ${ruleName}: ${e.message}`; - throw e; - } - } +/** @type Config */ +const DEFAULT_CONFIG = Object.freeze({ + useSvglintRc: true, + rules: {}, + ignore: [], +}); - /** - * Lints a file - * @param {String} fileName Either the path to a file, or a string to lint - * @param {Object} logger The logger to use for logging, or undefined if no log - * @returns {Promise} Resolves to `true` or a {String[]} of errors - */ - async lint(fileName, logger=undefined) { - log.debug("Linting", fileName); - const fileReporter = logger.getFileReporter(fileName); - const file = await this.getFile(fileName); - let ast; - try { - ast = cheerio.load(file, { xmlMode: true }); - } catch (e) { - e.message = `SVG parsing error: ${e.message}`; - throw e; - } +/** + * Normalizes a user-provided RulesConfig into a NormalizedRules. + * Figures out which rules should be kept, and calls their generator with the + * user-provided config. The returned function is the actual linting func. + * @param {RulesConfig} rulesConfig The user-provided config + * @returns {NormalizedRules} The normalized rules + */ +function normalizeRules(rulesConfig) { + /** @type {NormalizedRules} */ + const outp = {}; + Object.keys(rulesConfig) + // make sure no disabled rules are allowed in + .filter(k => rulesConfig[k] !== false) + // then convert each rule config into a rule func + .forEach( + ruleName => { + // TODO: error handling when invalid rule given + try { + outp[ruleName] = loadRule(ruleName) + .generate(rulesConfig[ruleName]); + } catch (e) { + logger.warn(`Unknown rule "${ruleName}".`); + } + } + ); + return outp; +} - // execute each rule - const promises = Object.keys(this.config.rules) - .map( - async name => { - log.debug("Executing rule", name); - const executor = this.getRuleExecutor(name); - let configs = this.config.rules[name]; - // normalize our config to an array, even if there is only 1 config - if (!(configs instanceof Array)) { configs = [configs]; } +/** + * Normalizes a user-provided config to make sure it has every property we need. + * Also handles merging with defaults and .svglintrc. + * @param {Config} config The user-provided config + * @returns {NormalizedConfig} The normalized config + */ +function normalizeConfig(config) { + // TODO: load and merge .svglintrc if { useSvglintRc: true } is set + const defaulted = Object.assign({}, + DEFAULT_CONFIG, + config, + ); + /** @type NormalizedConfig */ + const outp = { + rules: normalizeRules(defaulted.rules), + ignore: defaulted.ignore, + }; + return outp; +} - const reporter = fileReporter.getRuleReporter(name); - const result = Promise.all( - configs.map( - async config => { - return await executor(config)(ast, reporter); - } - ) - ); - return result; - } - ); - - // wait for each result to come in, and then return - Promise.all(promises) - .then(results => { - const errors = flatten(results).filter(v => v !== true); - if (errors.length) { - return errors; - } - return true; - }); - } +module.exports = { + /** + * Lints a single SVG string. + * The function returns before the Linting is finished. + * You should listen to Linting.on("done") to wait for the result. + * @param {String} source The SVG to lint + * @param {Config} config The config to lint by + * @return {Linting} The Linting that represents the result + */ + lintSource(source, config) { + const ast = parse.parseSource(source); + const conf = normalizeConfig(config); + return new Linting(null, ast, conf.rules); + }, /** - * Calls a callback with the content of a file - * Can also handle being given the content of the file + * Lints a single file. + * The returned Promise resolves before the Linting is finished. + * You should listen to Linting.on("done") to wait for the result. + * @param {String} file The file path to lint + * @param {Config} config The config to lint by + * @returns {Promise} Resolves to the Linting that represents the result */ - getFile(file) { - return new Promise(res => { - log.debug("Loading file", file); - try { - fs.readFile( - file, - "utf8", - (err, data) => { - if (err) { - return res(file); - } - return res(data); - } - ); - } catch (e) { - res(file); - } - }); + async lintFile(file, config) { + const ast = await parse.parseFile(file); + const conf = normalizeConfig(config); + return new Linting(file, ast, conf.rules); } -} -module.exports = SVGLint; +}; diff --git a/src/test.js b/src/test.js new file mode 100644 index 0000000..493e989 --- /dev/null +++ b/src/test.js @@ -0,0 +1,29 @@ +const svglint = require("./svglint"); +const linting = svglint.lintSource(` + + + + + + +`, + { + rules: { + fails: {}, + doesntFailBecauseRemoved: {}, + identity: { + method: "error", + message: "This fails spectacularly", + } + } + } +); + +console.log(linting); +linting.on("rule", function(){ + console.log("Linting emitted event", [...arguments]); +}); +linting.once("done", () => { + console.log(`Linting done + State: ${linting.state}`); +}); diff --git a/src/util.js b/src/util.js deleted file mode 100644 index 2478b9c..0000000 --- a/src/util.js +++ /dev/null @@ -1,95 +0,0 @@ -const ansiRegex = require("ansi-regex"); - -/** - * Splits a string into N sized chunks. - * Newlines also indicates a chunk end. - * Handles ANSI color codes correctly (does not count them towards length) - * @param {String} str The string to chunk - * @param {Number} N The length to chunk into - * @returns {Array} - */ -function chunkString(str, N) { - const outp = []; - const blacklist = [ansiRegex()]; - let tmp = ""; let tmpLen = 0; - for (let i = 0, l = str.length; i < l; ++i) { - if (str[i] === "\n" // split at newlines - || tmpLen === N) { // and at length - outp.push(tmp); - tmp = ""; - tmpLen = 0; - } - if (str[i] === "\n") { // don't add newlines to our outp str - continue; - } - let blacklisted = false; - for (let regex of blacklist) { // skip blacklist matches - const tester = new RegExp("^"+regex.source); - const match = tester.exec(str.slice(i)); - if (match) { - i += match[0].length - 1; - tmp += match[0]; - blacklisted = true; - break; - } - } - if (blacklisted) { continue; } - - tmp += str[i]; - ++tmpLen; - } - if (tmp) { outp.push(tmp); } - return outp; -} - -/** - * Turns a user-friendly rule name into a file-friendly rule name - * @param {String} rule The rule name to transform - * @return {String} The transformed rule name - */ -function transformRuleName(rule) { - return rule.replace(/\//g, "_"); -} - -/** - * Returns the relevant version of "expected" for a value - * Arrays: expected one of - * RegExp: expected to match - * Others: expected - */ -function str_expected(value) { - if (value instanceof Array) { - return "expected one of"; - } - if (value instanceof RegExp) { - return "expected to match"; - } - if (value === true) { - return "must be set"; - } - return "expected"; -} - -/** - * Flattens an array such that it consists solely of non-array elements - * @param {Array} arr The array to flatten - * @returns {Array} The flattened array - */ -function flatten(arr) { - let outp = []; - for (let i = 0; i < arr.length; ++i) { - if (arr[i] instanceof Array) { - outp = outp.concat(flatten(arr[i])); - } else { - outp.push(arr[i]); - } - } - return outp; -} - -module.exports = { - chunkString, - transformRuleName, - str_expected, - flatten, -};