Escaping dot should be taken in deep property

[umireon]

According to getPathInfo.js, deep.property(name) is assumed to support escaping dot . by prepending backslash \. However, escaping dot in deep.property seems to work in the unexpected way. Here is the example:

> expect({"foo.bar": "baz"}).to.have.deep.property("foo.bar", "baz");
AssertionError: expected { 'foo.bar': 'baz' } to have a deep property 'foo.bar'
// fail expectedly

> expect({"foo.bar": "baz"}).to.have.deep.property("foo\\.bar", "baz");
AssertionError: expected { 'foo.bar': 'baz' } to have a deep property 'foo\\.bar'
// fail unexpectedly

> expect({"foo\\.bar": "baz"}).to.have.deep.property("foo\\.bar", "baz");
// success unexpectedly

The current implementation of deep.propertynever match {"foo\\.bar": "baz"} because backslash \ is not removed on processing escape. This PR fixes this problem. In addition, it includes the test case to ensure the problem fixed.

Noted that this PR obviously breaks some backward compatibility.

[keithamus]

Thanks for the PR @umireon!

While it does strictly break backwards compatibility - I'd be shocked if anyone was bitten by this - so I'm on the fence whether or not to merge it here or wait for 3.x.x. Having said that - as you mentioned, trying to escape results in wild behaviour so likely if people have seen this, they've seen bugged out behaviour - so I'd consider this a bug fix.

Irregardless, a few notes:

• If we're escaping .s we should probably also escape []s. Would you be happy to implement [] escaping too?

[umireon]

Hi, @keithamus !
I've written some documentations and implemented escaping [].

In fact, the path strings which the current implementation of .deep.property accepts is very wide.
I assumed the input of .deep.property matches /^(?:(?:[.[]]|[^.[]])+|[\d+])(?:.(?:[.[]]|[^.[]])+|[\d+])$/ to keep the implementation clean.
Therefore, any path string which does not may match the expression results in an unexpected way.

I'm looking forward to hearing from you,

[umireon]

I need to use while instead of Array#map to avoid matching /[(\d+)]/ twice.

[keithamus]

Well, .[] isnt officially supported - it was more a by-product of implementation, and I'm not sure its something we need to explicitly support (especially as its not familiar syntax for javascript). Would that not simplify parsing a lot?

[umireon]

I'm sorry if I didn't get your point, but you mean it would be nice to implement
escaping using shorter regular expression with modifying the string?

[keithamus]

Pretty much yes. The regexp seems a little complex - and unless I'm mistaken a fair chunk of the complexity comes from detecting . vs \\. vs .[] vs \\.[] vs .\\[], but in reality we just need to find ., [] and escape on \\. and \\[].

Please correct me if I'm wrong though.

[umireon]

Frankly speaking, I'm not confident to keep the behavior unchanged with parsing the regular language (= the simple syntax of .deep.property's name) by the mechanism more complex than regular expression.
Regular expression is so simple syntax that it can easily be proved to match the desired string.
Therefore, I unintentionally wrote the code whose everything is in one regular expression but it is not concern of this PR.
Finally, it's my fault to mangling the regular expression in this PR, very sorry 😢.

[umireon]

Without (([^\\]), $1), a\\[results in accessing a.[.

[umireon]

Actually, ] don't have to be escaped.

[umireon]

The regexp seems a little complex - and unless I'm mistaken a fair chunk of the complexity comes from detecting . vs . vs .[] vs .[] vs .[], but in reality we just need to find ., [] and escape on . and [].

The expression was complex simply because three regular expressions and memory rewriting were combined.

[keithamus]

@umireon that now looks much clearer to me. Good job 😄

I'd like to pester you for one more bit:

• Add documentation for .deep.property escaping in the deep flag docs (assertion.jsL68-80).

Once that's done I'm happy to wrap up this PR 😄

[umireon]

@keithamus I've made it done!

[keithamus]

Great work @umireon! 🎉

[keithamus]

Linking #398