Chai: Rename `.string` to `.substring` (with alias `.substr`)

[meeber]

The .string assertion is very dangerous because it can easily be misused:

expect(42).to.be.a.string;  // Doesn't do anything but passes anyway, even with 4.0 proxies

The above assertion wrongly passes because it has nothing to do with type-checking; instead, string is a method assertion that's supposed to be invoked to verify that the given substring is present in the target:

expect('foobar').to.contain.string('bar');

Recommended (breaking) change:

expect('foobar').to.contain.substring('bar');
expect('foobar').to.contain.substr('bar');

[keithamus]

Could we instead do something with the assertion expect(42).to.be.a.string?

[vieiralucas]

We already have the to.be.a('string') way of type-checking
If we opt in to use it for type-checking, then we might need to add .number, .array...

@keithamus do you have any idea besides type-checking?

[meeber]

To be clear, .to.be.a.string isn't currently a valid way of type-checking. Instead, it does nothing at all, which unfortunately means that the assertion simply passes no matter what. The problem is that it visually appears to be a valid assertion.

We could add logic to the proxy protection to prevent a.string, but it wouldn't be compatible with @shvaikalesh's idea to refactor the proxy protection as detailed here.

Alternatively, we could easily add a getter to all chainable method assertions that intercepts a.string and throws an error, but this logic path would get entered anytime something was chained off of any uninvoked chainable method assertion, not just a, and would involve adding a-specific code to the addChainableMethod function.

Another alternative is to make some more substantial architectural changes to enable newly created assertions to define custom getters to be applied only to their assertion, which would then allow the a.string detection logic to be located with the definition of the a assertion, and to be only be fired when something is chained off of a, not other uninvoked chainable method assertions.

[keithamus]

This feels like just another nail in coffin of the whole "property assertions" concept. I think its high time we started to think about removing them, again, but this time for good and in a non-backwards-compatible way.

[meeber]

This isn't a critical issue; let's punt it to post-v4.

[keithamus]

We'll be deprecating to.be.a.string and other matchers for chai 5. The plan is to keep things uniform for chai 5 and only have .to.be.a('')