Add Fully-Qualified class name in UnrecognizedField exception

[Kern046]

Currently if we use a field that is not properly registered in our mapping, we end up with the following exception from the BasicEntityPersister:

Unrecognized field: {field}

As with a complete stack trace we may be able to dig enough to find the right mapping, it can take more time to find the concerned entity in a context with less informations, such as logs.

This PR adds the entity FQCN to the exception message to ease debugging in such cases, using the class metadata available in BasicEntityPersister::class property. We end up with:

Unrecognized field: {fqcn}::{field}

As I didn't encountered any tests for the existing UnrecognizedException class behavior, I didn't add some but I can if needed.

[greg0ire]

Great idea, I think I remember losing time over this a few years ago.

[SenseException]

As far as I can see were these the only occurrences of UnrecognizedField::byName() and this method is now unused. Exceptions' named constructors are for internal usage, but can be used elsewhere in userland. Should it be removed now or set as deprecated first?

[greg0ire]

I'm OK with it being removed now.

[Kern046]

I'm OK with it being removed now.

I thought it would be a BC to remove it, some users may use this method. Maybe we should deprecate it instead ?

[derrabus]

Sorry if I'm nit-picking too much here, but if the exception message is that important to you, I believe we should have a test that makes sure the exception message is as you expect it to be.

[derrabus]

I thought it would be a BC to remove it, some users may use this method. Maybe we should deprecate it instead ?

This PR is filed as a bugfix, so I'd really vote against removing any methods. Yes, I believe we should deprecate the method, but we cannot introduce deprecations in a bugfix release either. So let's do that in a follow-up on 2.15.x.

[greg0ire]

Regarding what is a BC and what is not : https://xkcd.com/1172/

I always fear I'm going to be too strict, only to find my fellow maintainers think otherwise. The BC is small, but this isn't just any library, so let's avoid unnecessary risks.

[Kern046]

Sorry if I'm nit-picking too much here, but if the exception message is that important to you, I believe we should have a test that makes sure the exception message is as you expect it to be.

Done :)

Regarding what is a BC and what is not : https://xkcd.com/1172/

I always fear I'm going to be too strict, only to find my fellow maintainers think otherwise. The BC is small, but this isn't just any library, so let's avoid unnecessary risks.

I added a deprecation notice with a link to the new method.