Internal rules consistency and bugs

lib/base.js

@@ -11,7 +11,6 @@ const Compile = require('./compile');
 const Errors = require('./errors');
 const Extend = require('./extend');
 const Manifest = require('./manifest');
-const Messages = require('./messages');
 const Modify = require('./modify');
 const Ref = require('./ref');
 const Validator = require('./validator');
@@ -213,11 +212,6 @@ internals.Base = class {
         return obj;
     }
 
-    keep() {
-
-        return this.rule({ keep: true });
-    }
-
     label(name) {
 
         Assert(name && typeof name === 'string', 'Label name must be a non-empty string');
@@ -426,7 +420,7 @@ internals.Base = class {
 
         for (const name of source._singleRules.keys()) {
             if (obj._singleRules.has(name)) {
-                obj._rules = obj._rules.filter((target) => target.name !== name);
+                obj._rules = obj._rules.filter((target) => target.keep || target.name !== name);
                 obj._singleRules.delete(name);
             }
         }
@@ -442,7 +436,7 @@ internals.Base = class {
 
         for (const test of source._rules) {
             if (!source._definition.rules[test.method].multi) {
-                obj._singleRules.set(test.name, test.options);
+                obj._singleRules.set(test.name, test);
             }
 
             obj._rules.push(test);
@@ -518,33 +512,35 @@ internals.Base = class {
         return obj;
     }
 
-    message(message) {
-
-        return this.rule({ message });
-    }
-
     rule(options) {
 
-        Common.assertOptions(options, ['keep', 'message', 'warn']);
+        const def = this._definition;
+        Common.assertOptions(options, Object.keys(def.modifiers));
 
         Assert(this._ruleset !== false, 'Cannot apply rules to empty ruleset');
         const start = this._ruleset === null ? this._rules.length - 1 : this._ruleset;
         Assert(start >= 0 && start < this._rules.length, 'Cannot apply rules to empty ruleset');
 
-        options = Object.assign({}, options);                   // Shallow cloned
-
-        if (options.message) {
-            options.message = Messages.compile(options.message);
-        }
-
         const obj = this.clone();
 
         for (let i = start; i < obj._rules.length; ++i) {
-            obj._rules[i] = Object.assign({}, obj._rules[i], options);
+            const original = obj._rules[i];
+            const rule = Clone(original);
+
+            for (const name in options) {
+                def.modifiers[name](rule, options[name]);
+                Assert(rule.name === original.name, 'Cannot change rule name');
+            }
+
+            obj._rules[i] = rule;
+
+            if (obj._singleRules.get(rule.name) === original) {
+                obj._singleRules.set(rule.name, rule);
+            }
         }
 
         obj._ruleset = false;
-        return obj;
+        return obj.$_mutateRebuild();
     }
 
     get ruleset() {
@@ -593,11 +589,6 @@ internals.Base = class {
         return Validator.entryAsync(value, this, options);
     }
 
-    warn() {
-
-        return this.rule({ warn: true });
-    }
-
     // Extensions
 
     $_addRule(options) {
@@ -611,35 +602,26 @@ internals.Base = class {
         Assert(options && typeof options === 'object', 'Invalid options');
         Assert(options.name && typeof options.name === 'string', 'Invalid rule name');
 
-        const rule = {
-            name: options.name,
-            method: options.method || options.name,
-            args: options.args,
-            resolve: [],
-            warn: options.warn,
-            options
-        };
+        for (const key in options) {
+            Assert(key[0] !== '_', 'Cannot set private rule properties');
+        }
+
+        const rule = Object.assign({}, options);        // Shallow cloned
+        rule._resolve = [];
+        rule.method = rule.method || rule.name;
 
         const definition = this._definition.rules[rule.method];
+        const args = rule.args;
 
         Assert(definition, 'Unknown rule', rule.method);
-        Assert(!rule.args || Object.keys(rule.args).length === 1 || Object.keys(rule.args).length === this._definition.rules[rule.name].args.length, 'Invalid rule definition for', this.type, rule.name);
-
-        // Check for unique changes
-
-        if (!definition.multi &&
-            this._singleRules.has(rule.name) &&
-            DeepEqual(rule.options, this._singleRules.get(rule.name))) {
-
-            return this;
-        }
 
         // Args
 
         const obj = this.clone();
 
-        const args = rule.args;
         if (args) {
+            Assert(Object.keys(args).length === 1 || Object.keys(args).length === this._definition.rules[rule.name].args.length, 'Invalid rule definition for', this.type, rule.name);
+
             for (const key in args) {
                 let arg = args[key];
                 if (arg === undefined) {
@@ -653,13 +635,13 @@ internals.Base = class {
                     if (resolver.ref &&
                         Common.isResolvable(arg)) {
 
-                        rule.resolve.push(key);
+                        rule._resolve.push(key);
                         obj._refs.register(arg);
                     }
                     else {
                         if (resolver.normalize) {
                             arg = resolver.normalize(arg);
-                            rule.args[key] = arg;
+                            args[key] = arg;
                         }
 
                         if (resolver.assert) {
@@ -677,7 +659,7 @@ internals.Base = class {
 
         if (!definition.multi) {
             obj._ruleRemove(rule.name, { clone: false });
-            obj._singleRules.set(rule.name, rule.options);
+            obj._singleRules.set(rule.name, rule);
         }
 
         if (obj._ruleset === false) {
@@ -720,7 +702,7 @@ internals.Base = class {
         const rules = [];
         for (const test of this._rules) {
             if (test.name === name) {
-                rules.push(test.options);
+                rules.push(test);
             }
         }
 
@@ -787,12 +769,12 @@ internals.Base = class {
 
             const override = each(rule);
             if (override) {
-                override.args = override.options.args;
                 obj = obj || this.clone();
                 obj._rules[i] = override;
 
-                if (obj._singleRules.has(name)) {
-                    obj._singleRules.set(name, override.options);
+                const existingUnique = obj._singleRules.get(name);
+                if (existingUnique === rule) {
+                    obj._singleRules.set(name, override);
                 }
             }
         }

lib/extend.js

@@ -114,6 +114,28 @@ exports.type = function (from, options) {
 
     def.rules = rules;
 
+    // Modifiers
+
+    const modifiers = Object.assign({}, parent.modifiers);
+    if (def.modifiers) {
+        for (const name in def.modifiers) {
+            Assert(!prototype[name], 'Rule conflict in', def.type, name);
+
+            const modifier = def.modifiers[name];
+            Assert(typeof modifier === 'function', 'Invalid modifier definition for', def.type, name);
+
+            const method = function (arg) {
+
+                return this.rule({ [name]: arg });
+            };
+
+            prototype[name] = method;
+            modifiers[name] = modifier;
+        }
+    }
+
+    def.modifiers = modifiers;
+
     // Overrides
 
     if (def.overrides) {
@@ -135,12 +157,12 @@ exports.type = function (from, options) {
 
     def.build = internals.build(def.build, parent.build);
 
-    // Modify
+    // Fork
 
-    def.modify = internals.modify(def.modify, parent.modify);
+    def.fork = internals.fork(def.fork, parent.fork);
     def.rebuild = internals.rebuild(def.rebuild, parent.rebuild);
 
-    schema._ids._reachable = !!def.modify;
+    schema._ids._reachable = !!def.fork;
 
     return schema;
 };
@@ -206,7 +228,7 @@ internals.coerce = function (child, parent) {
 };
 
 
-internals.modify = function (child, parent) {
+internals.fork = function (child, parent) {
 
     if (!child ||
         !parent) {

lib/manifest.js

@@ -61,7 +61,8 @@ exports.describe = function (schema) {
     // Rules
 
     for (const rule of schema._rules) {
-        if (rule.name === 'items') {
+        const def = schema._definition.rules[rule.name];
+        if (def.manifest === false) {                           // Defaults to true
             continue;
         }
 

lib/modify.js

@@ -51,7 +51,7 @@ exports.Ids = internals.Ids = class {
 
         for (const node of chain) {
             const def = node.schema._definition;
-            adjusted = { id: node.id, schema: def.modify(node.schema, adjusted.id, adjusted.schema) };
+            adjusted = { id: node.id, schema: def.fork(node.schema, adjusted.id, adjusted.schema) };
         }
 
         return adjusted.schema;
@@ -136,7 +136,7 @@ exports.Ids = internals.Ids = class {
             return nodes;
         }
 
-        Assert(node.schema._definition.modify, 'Schema node', [...behind, ...path].join('.'), 'does not support manipulation');
+        Assert(node.schema._definition.fork, 'Schema node', [...behind, ...path].join('.'), 'does not support manipulation');
         return node.schema._ids._collect(forward, [...behind, current], nodes);
     }
 };

lib/schemas.js

@@ -49,8 +49,6 @@ internals.nameRx = /^[a-zA-Z0-9]+$/;
 
 internals.rule = Joi.object({
     alias: Joi.array().items(Joi.string().pattern(internals.nameRx)).single(),
-    method: Joi.func().allow(false),
-    validate: Joi.func(),
     args: Joi.array().items(
         Joi.string(),
         Joi.object({
@@ -61,8 +59,11 @@ internals.rule = Joi.object({
             message: Joi.string().when('assert', { is: Joi.func(), then: Joi.required() })
         })
     ),
+    convert: Joi.boolean(),
+    manifest: Joi.boolean(),
+    method: Joi.func().allow(false),
     multi: Joi.boolean(),
-    convert: Joi.boolean()
+    validate: Joi.func()
 });
 
 
@@ -78,14 +79,15 @@ exports.extension = Joi.object({
     ],
     initialize: Joi.func().arity(1),
     messages: [Joi.object(), Joi.string()],
-    modify: Joi.func().arity(3),
+    fork: Joi.func().arity(3),
+    modifiers: Joi.object().pattern(internals.nameRx, Joi.func().minArity(1).maxArity(2)),
     overrides: Joi.object().pattern(internals.nameRx, Joi.func()),
     prepare: Joi.func().maxArity(3),
     rebuild: Joi.func().arity(1),
     rules: Joi.object().pattern(internals.nameRx, internals.rule),
     validate: Joi.func().maxArity(3)
 })
-    .and('modify', 'rebuild')
+    .and('fork', 'rebuild')
     .strict();
 
 

lib/types/alternatives.js

@@ -266,9 +266,9 @@ module.exports = Any.extend({
         return obj;
     },
 
-    // Modify
+    // Fork
 
-    modify(schema, id, replacement) {
+    fork(schema, id, replacement) {
 
         let i = 0;
         for (const match of schema.$_terms.matches) {

lib/types/any.js

@@ -3,6 +3,7 @@
 const Assert = require('@hapi/hoek/lib/assert');
 
 const Base = require('../base');
+const Messages = require('../messages');
 
 
 const internals = {};
@@ -53,6 +54,26 @@ module.exports = Base.extend({
         }
     },
 
+    // Modifiers
+
+    modifiers: {
+
+        keep(rule, enabled = true) {
+
+            rule.keep = enabled;
+        },
+
+        message(rule, message) {
+
+            rule.message = Messages.compile(message);
+        },
+
+        warn(rule, enabled = true) {
+
+            rule.warn = enabled;
+        }
+    },
+
     // Errors
 
     messages: {

lib/types/array.js

@@ -333,7 +333,8 @@ module.exports = Any.extend({
 
                 return errors.length ? errors : value;
             },
-            priority: true
+            priority: true,
+            manifest: false
         },
 
         length: {
@@ -570,9 +571,9 @@ module.exports = Any.extend({
         return obj;
     },
 
-    // Modify
+    // Fork
 
-    modify(schema, id, replacement) {
+    fork(schema, id, replacement) {
 
         for (const set of ['items', 'ordered']) {
             for (let i = 0; i < schema.$_terms[set].length; ++i) {