nhibernate-core icon indicating copy to clipboard operation
nhibernate-core copied to clipboard

Obsoletes with warnings should be made into errors if their usage breaks code

Open bdrajer opened this issue 1 year ago • 1 comments

I've upgraded from NHibernate 4.1 to 5.3 and soft obsoletes are starting to become a problem. For example, a line in my code like

if (obj is IProxy)

doesn't work anymore since IProxy is now unused - it doesn't detect proxies anymore. The IProxy interface has been marked as Obsolete but only produces a warning during build. It seems it would have been better if the interface were removed if it does nothing, or at least the Obsolete attribute set to produce a build error... Otherwise it becomes a trap.

The same goes for the QueueOperation() method in AbstractPersistentCollection, I've overridden it ages ago and haven't noticed it has become obsolete. As a result of the upgrade, my override doesn't get called and my code is broken. In such cases it would have been better for my build to break.

bdrajer avatar May 24 '23 11:05 bdrajer

Thanks for submitting the issue. Unfortunately, we cannot travel back in time and change that postmortem. We'll try to communicate more clearly the obsolete behaviors in the future.

Upgrading from 4.1 to 5.3 is a long way and a lot of things has changed. It is always a good idea to review the release notes and enable "treat warnings as errors" at least temporarily while upgrade is in progress.

if (obj is IProxy) -- this was always invalid code working incidentally. IProxy was an internal-ish interface, a part of implementation details, a remain of copied LinFu.DynamicProxy. The correct check, from the dawn of times, was to check for implementation of INHibernateProxy, or calling NHibernateProxyHelper.IsProxy(obj) method. If you cannot change your code you should be able to switch back using DynamicProxyFactoryFactory.

As for AbstractPersistentCollection.QueueOperation I think we could have better clarified the changed behavior.

hazzik avatar May 24 '23 12:05 hazzik