feat: add 'rename' context-item

[fedeci]

Description

This allows to rename a property.

// req: { body: { name: 'foo' } } becomes 
// req: { body: { username: 'foo' } }
body('name').rename('username').isString()

// req: { body: { names: ['foo', 'bar'] } } throws
body('names.*').rename('username').isString()

// req: { body: { name: 'foo', surname: 'bar' } } throws
body(['name', 'surname']).rename('username')

// req: { body: { name: 'foo', surname: 'bar' } } throws
body('name').rename('surname')

Closes #785

To-do list

• Work on returned errors
• Wildcards?
• Dotted property names (e.g. 1.0)
• Context runner instance.path editing
• Nested properties

---

• I have added tests for what I changed.

• This pull request is ready to merge.

[fedeci]

These errors are thrown at runtime, should we use context.addError() even if it is not a validation error?

[coveralls]

Coverage remained the same at 100.0% when pulling db52eb4 on fedeci:rename-sanitizer into 0b0b6c8 on express-validator:master.

[fedeci]

Is it correct to change instance.path?

[gustavohenke]

I'll guess no because of the below:

express-validator/src/context.ts

Lines 66 to 70 in 6715a6a

	setData(path: string, value: any, location: Location) {
	const instance = this.dataMap.get(getDataMapKey(path, location));
	if (!instance) {
	throw new Error('Attempt to write data that did not pre-exist in context');
	}

but we can always change behaviour.

And side comment, I'd just find it a bit more classy if we could use context's APIs instead of checking for the specific implementation of Rename here.

[gustavohenke]

ohey only 1.5 years later

[gustavohenke]

WDYT of moving this to the constructor/.rename()?

Just thinking that failing on server start might be a better developer experience than on request.

[gustavohenke]

I'm guessing that a client might, due to a bug, send more information than required and litter logs with 5XX errors (as I think it'll probably flow all the way to next()).

Perhaps it's best to remove this?

[gustavohenke]

These should obey the dryRun option

[gustavohenke]

nitpicking, but I usually like to make test names read nicely with the it.
For example:

Suggested change

	it('the new path is identical to the old one', () => {
	it('does nothing when the new path is identical to the old one', () => {

[gustavohenke]

Suggested change

	expect(new Rename('foo').run(context, 'value', meta)).resolves;
	await expect(new Rename('foo').run(context, 'value', meta)).resolves;

though I'm afraid that .resolves might not be asserting anything 🤔

[gustavohenke]

I'll guess no because of the below:

express-validator/src/context.ts

Lines 66 to 70 in 6715a6a

	setData(path: string, value: any, location: Location) {
	const instance = this.dataMap.get(getDataMapKey(path, location));
	if (!instance) {
	throw new Error('Attempt to write data that did not pre-exist in context');
	}

but we can always change behaviour.

And side comment, I'd just find it a bit more classy if we could use context's APIs instead of checking for the specific implementation of Rename here.