Re: [Clirr-devel] adding methods to interfaces

[Simon K. <si...@ec...>]

On Mon, 2004-06-28 at 17:54, Lars K=FChne wrote:
> Simon Kitching wrote:
>=20
> >>
> >
> >The issue is that binary incompatibilities are *always* failures; just
> >having two incompatible classes together in the classpath causes a
> >failure.
> >
> > =20
> >
>=20
> You mean "having two incompatible classes in the same classloader", rig=
ht?

Yep.=20

>=20
> >For "warning" level issues, the app *may* run or *may* cause a runtime
> >exception depending on exactly how the classes are used.
> > =20
> >
>=20
> Hmmm... I think the situation for commons-collections is a bit special.=
=20
> I like to think of binary compatibility as "binary compatible in all=20
> possible scenarios".
>=20
>  From the Eclipse paper: "Adding a new method to an API interface that=20
> is implemented by Clients (e.g., a callback, listener, or visitor=20
> interface) breaks compatibility because hypothetical pre-existing=20
> implementations do not implement the new method."
>=20
> There are many libraries where users are actually expected to provide=20
> interface implementations. Think PropertyChangeListener in the JDK.=20
> Think Appender in log4j. Think all kinds of Decorators. Clirr can't fin=
d=20
> out whether the interface is meant to be implemented externally (maybe=20
> in JDK 1.5 some class metadata could be used to express that...)
>=20
> My current thinking is this should be flagged as an error, and clirr=20
> users who modify interfaces that are typically not implemented by a use=
r=20
> (like in commons-collection) can somehow filter those messages.
>=20
> The effect is that if you introduce new methods in an interface you are=
=20
> forced to make a decision and actively approve the change. You cannot=20
> ignore the error (like you can ignore a warning) because it will break=20
> your build.

Ok, I'm happy with that as long as we can document this, so when people
see ERROR being reported on their interfaces they know why, and know
that we know that it is "probably" an error, not "definitely" one.

The new message framework (I18N) should help, as messages will now have
codes that can link to further explanations.

> >>
> >>There are theoretically four combinations of final and abstract. One =
of=20
> >>them (abstract method in a non-final class) creates a compatibility=20
> >>problem and the code reports that correctly.
> >>
> >>The remaining three cases do not create a compatibility problem. One =
of=20
> >>those remaining three cases (abstract method and final class) cannot =
be=20
> >>achieved with a correct java compiler - but that doesn't matter here,=
=20
> >>because it is still not a compatibility problem: there can't be any=20
> >>subclasses where the method is missing because the class is final.=20
> >>Reporting INFO in that case is OK, no?
> > =20
> >
>=20
> Yep, change the code as you like, I think we agree on the calculation=20
> result for the relevant three cases :-)

I see:
  non-final + non-abstract --> info
  non-final + abstract --> ERROR
  final + non-abstract --> info
  final + abstract --> impossible

ie the final/non-final state of the class is irrelevant.

I have just committed a patch to MethodSetCheck to rewrite the "reportAdd=
ed" method using if-stmts. I hope you can check this for me.


> PS: will you start working on the i18n improvements you suggested or=20
> should I do it?

I'm right in the middle of committing some patches now - literally.

PS: I'm logged in to IRC if you wish to catch up. Of course, design
discussions may well be better held by email so they are archived.

Cheers,

Simon