Chore: replace is-my-json-valid with Ajv (#8852)

* Upgrade: replace is-my-json-valid with Ajv This PR replaces the validation functionality. epoberezkin needs to have a look into how to handle the error message formatting. * Fix: error message code and messages in tests * Chore: use one instance of ajv, JSON-schema draft-04 compatibility * Chore: use meta-schema in ajv, disable default meta-schema validation * Chore: validate config-schema against meta-schema * Chore: compile config-schema only once * Fix: RuleTester error message for invalid schema * Fix: ignore missing $ref in schemas for backward compatibility
eslint · Jul 7, 2017 · 72f22eb · 72f22eb
1 parent 7c8de92
commit 72f22eb
Show file tree

Hide file tree

Showing 8 changed files with 95 additions and 184 deletions.
diff --git a/conf/json-schema-schema.json b/conf/json-schema-schema.json
diff --git a/lib/config/config-validator.js b/lib/config/config-validator.js
@@ -9,7 +9,7 @@
 // Requirements
 //------------------------------------------------------------------------------
 
-const schemaValidator = require("is-my-json-valid"),
+const ajv = require("../util/ajv"),
     configSchema = require("../../conf/config-schema.js"),
     util = require("util");
 
@@ -20,6 +20,7 @@ const validators = {
 //------------------------------------------------------------------------------
 // Private
 //------------------------------------------------------------------------------
+let validateSchema;
 
 /**
  * Gets a complete options schema for a rule.
@@ -79,15 +80,15 @@ function validateRuleSchema(id, localOptions, rulesContext) {
     const schema = getRuleOptionsSchema(id, rulesContext);
 
     if (!validators.rules[id] && schema) {
-        validators.rules[id] = schemaValidator(schema, { verbose: true });
+        validators.rules[id] = ajv.compile(schema);
     }
 
     const validateRule = validators.rules[id];
 
     if (validateRule) {
         validateRule(localOptions);
         if (validateRule.errors) {
-            throw new Error(validateRule.errors.map(error => `\tValue "${error.value}" ${error.message}.\n`).join(""));
+            throw new Error(validateRule.errors.map(error => `\tValue "${error.data}" ${error.message}.\n`).join(""));
         }
     }
 }
@@ -158,19 +159,23 @@ function validateRules(rulesConfig, source, rulesContext) {
  * @returns {string} Formatted error message
  */
 function formatErrors(errors) {
-
     return errors.map(error => {
-        if (error.message === "has additional properties") {
-            return `Unexpected top-level property "${error.value.replace(/^data\./, "")}"`;
+        if (error.keyword === "additionalProperties") {
+            const formattedPropertyPath = error.dataPath.length ? `${error.dataPath.slice(1)}.${error.params.additionalProperty}` : error.params.additionalProperty;
+
+            return `Unexpected top-level property "${formattedPropertyPath}"`;
         }
-        if (error.message === "is the wrong type") {
-            const formattedField = error.field.replace(/^data\./, "");
-            const formattedExpectedType = typeof error.type === "string" ? error.type : error.type.join("/");
-            const formattedValue = JSON.stringify(error.value);
+        if (error.keyword === "type") {
+            const formattedField = error.dataPath.slice(1);
+            const formattedExpectedType = Array.isArray(error.schema) ? error.schema.join("/") : error.schema;
+            const formattedValue = JSON.stringify(error.data);
 
             return `Property "${formattedField}" is the wrong type (expected ${formattedExpectedType} but got \`${formattedValue}\`)`;
         }
-        return `"${error.field.replace(/^(data\.)/, "")}" ${error.message}. Value: ${JSON.stringify(error.value)}`;
+
+        const field = error.dataPath[0] === "." ? error.dataPath.slice(1) : error.dataPath;
+
+        return `"${field}" ${error.message}. Value: ${JSON.stringify(error.data)}`;
     }).map(message => `\t- ${message}.\n`).join("");
 }
 
@@ -181,10 +186,10 @@ function formatErrors(errors) {
  * @returns {void}
  */
 function validateConfigSchema(config, source) {
-    const validator = schemaValidator(configSchema, { verbose: true });
+    validateSchema = validateSchema || ajv.compile(configSchema);
 
-    if (!validator(config)) {
-        throw new Error(`${source}:\n\tESLint configuration is invalid:\n${formatErrors(validator.errors)}`);
+    if (!validateSchema(config)) {
+        throw new Error(`${source}:\n\tESLint configuration is invalid:\n${formatErrors(validateSchema.errors)}`);
     }
 }
 

diff --git a/lib/testers/rule-tester.js b/lib/testers/rule-tester.js
@@ -44,10 +44,9 @@ const lodash = require("lodash"),
     assert = require("assert"),
     util = require("util"),
     validator = require("../config/config-validator"),
-    validate = require("is-my-json-valid"),
+    ajv = require("../util/ajv"),
     Linter = require("../linter"),
     Environments = require("../config/environments"),
-    metaSchema = require("../../conf/json-schema-schema.json"),
     SourceCodeFixer = require("../util/source-code-fixer");
 
 //------------------------------------------------------------------------------
@@ -73,8 +72,6 @@ const RuleTesterParameters = [
     "output"
 ];
 
-const validateSchema = validate(metaSchema, { verbose: true });
-
 const hasOwnProperty = Function.call.bind(Object.hasOwnProperty);
 
 /**
@@ -318,12 +315,16 @@ class RuleTester {
             const schema = validator.getRuleOptionsSchema(ruleName, linter.rules);
 
             if (schema) {
-                validateSchema(schema);
+                ajv.validateSchema(schema);
+
+                if (ajv.errors) {
+                    const errors = ajv.errors.map(error => {
+                        const field = error.dataPath[0] === "." ? error.dataPath.slice(1) : error.dataPath;
+
+                        return `\t${field}: ${error.message}`;
+                    }).join("\n");
 
-                if (validateSchema.errors) {
-                    throw new Error([
-                        `Schema for rule ${ruleName} is invalid:`
-                    ].concat(validateSchema.errors.map(error => `\t${error.field}: ${error.message}`)).join("\n"));
+                    throw new Error([`Schema for rule ${ruleName} is invalid:`, errors]);
                 }
             }
 

diff --git a/lib/util/ajv.js b/lib/util/ajv.js
@@ -0,0 +1,29 @@
+/**
+ * @fileoverview The instance of Ajv validator.
+ * @author Evgeny Poberezkin
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const Ajv = require("ajv"),
+    metaSchema = require("ajv/lib/refs/json-schema-draft-04.json");
+
+//------------------------------------------------------------------------------
+// Public Interface
+//------------------------------------------------------------------------------
+
+const ajv = new Ajv({
+    meta: false,
+    validateSchema: false,
+    missingRefs: "ignore",
+    verbose: true
+});
+
+ajv.addMetaSchema(metaSchema);
+// eslint-disable-next-line no-underscore-dangle
+ajv._opts.defaultMeta = metaSchema.id;
+
+module.exports = ajv;
diff --git a/package.json b/package.json
@@ -34,6 +34,7 @@
   "homepage": "http://eslint.org",
   "bugs": "https://github.com/eslint/eslint/issues/",
   "dependencies": {
+    "ajv": "^5.2.0",
     "babel-code-frame": "^6.22.0",
     "chalk": "^1.1.3",
     "concat-stream": "^1.6.0",
@@ -50,7 +51,6 @@
     "ignore": "^3.3.3",
     "imurmurhash": "^0.1.4",
     "inquirer": "^3.0.6",
-    "is-my-json-valid": "^2.16.0",
     "is-resolvable": "^1.0.0",
     "js-yaml": "^3.8.4",
     "json-stable-stringify": "^1.0.1",

diff --git a/tests/conf/config-schema.js b/tests/conf/config-schema.js
@@ -0,0 +1,26 @@
+/**
+ * @fileoverview Tests for config-schema.
+ * @author Evgeny Poberezkin
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const ajv = require("../../lib/util/ajv"),
+    configSchema = require("../../conf/config-schema.js"),
+    assert = require("assert");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+describe("config-schema", () => {
+    it("should be valid against meta-schema", () => {
+        const valid = ajv.validateSchema(configSchema);
+
+        assert.strictEqual(valid, true);
+    });
+});