Refactor _rules and _tests. For #1987

lib/common.js

@@ -119,17 +119,12 @@ exports.extend = function (Class, prop, methods) {
 
  for (const name of Reflect.ownKeys(methods)) {
  let method = methods[name];
- if (method &&
- typeof name === 'string' &&
- typeof method !== 'function') {
-
- const args = method.args;
- method = method.method;
- if (!method) {
- method = () => null; // Set placeholder method for dummy rules
+ if (prop === '_rules') {
+ if (typeof method === 'function') {
+ method = { method };
  }
 
- method.args = args;
+ Hoek.assert(typeof method === 'object', 'Invalid rule definition for', Class, prop, name);
  }
 
  Class.prototype[prop][name] = method;

lib/extend.js

@@ -176,6 +176,6 @@ internals.rule = function (joi, root, rule, type) {
  return rule.validate.call(this, args, value, state, prefs);
  };
 
- return schema._test(rule.name, args, validate, { description: rule.description });
+ return schema._test(rule.name, args, validate);
  };
 };

lib/types/any.js

@@ -466,10 +466,10 @@ module.exports = internals.Any = class {
  // Combine tests
 
  for (const test of source._tests) {
- if (test.rule &&
- !test.rule.multi) {
+ if (!test.func &&
+ !source._rules[test.method].multi) {
 
- obj._uniqueRules.set(test.name, test.rule._options);
+ obj._uniqueRules.set(test.name, test.options);
  }
 
  obj._tests.push(test);
@@ -654,7 +654,7 @@ module.exports = internals.Any = class {
 
  Hoek.assert(code && typeof code === 'string', 'Invalid warning code');
 
- return this._rule('warning', { args: { code, local }, multi: true, warn: true });
+ return this._rule({ name: 'warning', args: { code, local }, warn: true });
  }
 
  // Internals
@@ -711,7 +711,7 @@ module.exports = internals.Any = class {
  const rules = [];
  for (const test of this._tests) {
  if (test.name === name) {
- rules.push(test.rule._options);
+ rules.push(test.options);
  }
  }
 
@@ -771,31 +771,45 @@ module.exports = internals.Any = class {
  this._ids.reset();
  }
 
- _rule(name, options = {}) {
+ _rule(options) {
+
+ // Normalize rule
+
+ if (typeof options === 'string') {
+ options = { name: options };
+ }
+
+ Hoek.assert(options && typeof options === 'object', 'Invalid options');
+ Hoek.assert(options.name && typeof options.name === 'string', 'Invalid rule name');
 
  const rule = {
- rule: name,
- alias: name,
+ name: options.name,
+ method: options.method || options.name,
+ args: options.args,
  resolve: [],
- ...options, // args, refs, multi, convert, priority, ...rule-specific
- _options: options // The original options
+ warn: options.warn,
+ options
  };
 
- Hoek.assert(this._rules[rule.rule], 'Unknown rule', name);
- Hoek.assert(!options.args || Object.keys(options.args).length === 1 || Object.keys(options.args).length === this._rules[rule.rule].args.length, 'Invalid rule definition for', this._type, name);
+ const definition = this._rules[rule.method];
+
+ Hoek.assert(definition, 'Unknown rule', rule.method);
+ Hoek.assert(!rule.args || Object.keys(rule.args).length === 1 || Object.keys(rule.args).length === this._rules[rule.name].args.length, 'Invalid rule definition for', this._type, rule.name);
 
- if (!options.multi &&
- this._uniqueRules.has(name) &&
- Hoek.deepEqual(options, this._uniqueRules.get(name))) {
+ // Check for unique changes
+
+ if (!definition.multi &&
+ this._uniqueRules.has(rule.name) &&
+ Hoek.deepEqual(rule.options, this._uniqueRules.get(rule.name))) {
 
  return this;
  }
 
- const obj = this.clone();
-
  // Args
 
- const args = options.args;
+ const obj = this.clone();
+
+ const args = rule.args;
  if (args) {
  for (const key in args) {
  let arg = args[key];
@@ -804,8 +818,8 @@ module.exports = internals.Any = class {
  continue;
  }
 
- if (options.refs) {
- const resolver = options.refs[key];
+ if (definition.refs) {
+ const resolver = definition.refs[key];
  if (resolver) {
  if (Common.isResolvable(arg)) {
  rule.resolve.push(key);
@@ -814,7 +828,7 @@ module.exports = internals.Any = class {
  else {
  if (resolver.normalize) {
  arg = resolver.normalize(arg);
- options.args[key] = arg;
+ rule.args[key] = arg;
  }
 
  Hoek.assert(resolver.assert(arg), resolver.message);
@@ -826,32 +840,22 @@ module.exports = internals.Any = class {
  }
  }
 
- if (!options.multi) {
- obj._ruleRemove(name, { clone: false });
- obj._uniqueRules.set(name, rule._options);
+ // Unique rules
+
+ if (!definition.multi) {
+ obj._ruleRemove(rule.name, { clone: false });
+ obj._uniqueRules.set(rule.name, rule.options);
  }
 
  if (obj._ruleset === false) {
  obj._ruleset = null;
  }
 
- const test = { rule, name };
-
- if (args &&
- Object.keys(args).length) {
-
- test.args = args;
- }
-
- if (rule.warn) {
- test.warn = true;
- }
-
- if (options.priority) {
- obj._tests.unshift(test);
+ if (definition.priority) {
+ obj._tests.unshift(rule);
  }
  else {
- obj._tests.push(test);
+ obj._tests.push(rule);
  }
 
  return obj;
@@ -911,15 +915,15 @@ module.exports = internals.Any = class {
  return Validator.validate(value, this, state, prefs);
  }
 
- _test(name, args, func, options) {
+ _test(name, args, func) {
 
  const obj = this.clone();
 
  if (obj._ruleset === false) {
  obj._ruleset = null;
  }
 
- obj._tests.push({ func, name, args, options });
+ obj._tests.push({ func, name, args });
 
  return obj;
  }
@@ -965,6 +969,7 @@ Common.extend(internals.Any, 'rules', {
 
  return helpers.error(code, local);
  },
- args: ['code', 'local']
+ args: ['code', 'local'],
+ multi: true
  }
 });