guillaumepotier / validator.js

Powerful objects and strings validation in javascript for Node and the browser
http://validatorjs.org
MIT License
255 stars 39 forks source link

Fix pretty asserts and static methods not updated on extends #67

Closed rplopes closed 5 years ago

rplopes commented 5 years ago

When we add extra asserts to validator.js, they get added to the list of asserts both in the PascalCase version (e.g. Foobar) and in the prettified camelCase version (e.g. foobar), and both as a prototype method (e.g. new Assert().Foobar()) and static method (e.g. Assert.foobar()).

However, when those asserts match the name of an existing assert, the PascalCase version gets correctly overwritten, but the prettified camelCase version, along with the static versions of both, don't.

This leads to an inconsistency in behaviour that might result in unexpected and hard to trace down bugs.

This PR fixes that by applying the new assert to all generated methods. This allows us to extend existing asserts in case we need to add additional clauses or adapt existing ones (e.g. we could extend the Email assert to include special edge cases for specific providers).

Closes https://github.com/guillaumepotier/validator.js/issues/57.

How to test

// Example of an assert that always returns true:
fn = function() { this.__class__ = 'Email'; this.validate = function(value) { return true; }; return this; }

// Overwriting the Email assert to use that custom assert that always returns true:
Extended = Assert.extend({ Email: fn });

// Should throw as before:
Assert.Email().check('foo');
Assert.email().check('foo');
new Assert().Email().check('foo');
new Assert().email().check('foo');

// Should return a Violation as before:
Assert.Email().validate('foo');
Assert.email().validate('foo');
new Assert().Email().validate('foo');
new Assert().email().validate('foo');

// Static methods were still using the original assert, this PR fixes them to use the new one instead.
// Should throw/return a Violation on master
// Should return true with this PR:
Extended.Email().check('foo');
Extended.email().check('foo');
Extended.Email().validate('foo');
Extended.email().validate('foo');

// Prettified camelCase methods were still using the original assert, this PR fixes them to use the new one instead.
// Should throw/return a Violation on master
// Should return true with this PR:
new Extended().email().check('foo');
new Extended().email().validate('foo');

// PascalCase methods were already being overwritten.
// Should use the new assert both on master and with this PR:
new Extended().Email().check('foo');
new Extended().Email().validate('foo');