clirr-devel — Discussion about the source code of Clirr

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/event Message.java,1.1,1.2

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/java/net/sf/clirr/event
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv14103

Modified Files:
	Message.java 
Log Message:
Provide alternative constructor that doesn't register the new
Message object with the MessageManager [needed for unit testing].


Index: Message.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/event/Message.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- Message.java	28 Jun 2004 06:51:21 -0000	1.1
+++ Message.java	2 Jul 2004 02:24:27 -0000	1.2
@@ -27,10 +27,36 @@
 {
     private int id;
 
+    /**
+     * This constructor is equivalent to new Message(id, true).
+     */
     public Message(int id)
     {
+        this(id, true);
+    }
+
+    /**
+     * Create an instance of this object with the specified message id
+     *
+     * @param id is an integer which is used to look up the appropriate
+     * text string for this message from a resource file. The id of a
+     * message should be unique.
+     *
+     * @param register determines whether the new Message object should be
+     * registered with the central MessageManager object. This is normally
+     * desirable, as this allows the unit tests associated with clirr to
+     * verify that message ids are unique and that translations exist for
+     * all registered messages. However false can be useful in some
+     * circumstances, eg when creating Message objects for the purposes
+     * of unit tests.
+     */
+    public Message(int id, boolean register)
+    {
         this.id = id;
-        MessageManager.getInstance().addMessage(this);
+        if (register)
+        {
+            MessageManager.getInstance().addMessage(this);
+        }
     }
 
     public int getId()

[Clirr-devel] CVS: clirr/src/conf event-messages.properties,1.3,1.4

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/conf
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv22710

Modified Files:
	event-messages.properties 
Log Message:
Added entries for all messages currently generated by clirr


Index: event-messages.properties
===================================================================
RCS file: /cvsroot/clirr/clirr/src/conf/event-messages.properties,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- event-messages.properties	29 Jun 2004 06:55:45 -0000	1.3
+++ event-messages.properties	1 Jul 2004 23:54:02 -0000	1.4
@@ -5,7 +5,61 @@
 # {2} --> full field declaration of affected field
 # {3}..{n} --> check-specific parameters
 
+
+#-----------------------------------------------------------------------------
+# ClassScopeCheck messages
+#-----------------------------------------------------------------------------
+m1000=Increased visibility of class from {3} to {4}
+m1001=Decreased visibility of class from {3} to {4}
+m1002=Unable to determine class scope: {3} in old class version
+m1003=Unable to determine class scope: {3} in new class version
+
+#-----------------------------------------------------------------------------
+# GenderChangeCheck messages
+#-----------------------------------------------------------------------------
+m2000=Changed from class to interface
+m2001=Changed from interface to class
+
+#-----------------------------------------------------------------------------
+# ClassModifierCheck messages
+#-----------------------------------------------------------------------------
+m3000=Unable to determine whether class is private
+m3001=Removed final modifier from class
+m3002=Added final modifier to class, but class was effectively final anyway
+m3003=Added final modifier to class
+m3004=Removed abstract modifier from class
+m3005=Added abstract modifier to class
+
+#-----------------------------------------------------------------------------
+# InterfaceSetCheck messages
+#-----------------------------------------------------------------------------
+m4000=Added {3} to the set of implemented interfaces
+m4001=Removed {3} from the set of implemented interfaces
+
+#-----------------------------------------------------------------------------
+# ClassHierarchyCheck messages
+#-----------------------------------------------------------------------------
+m5000=Added {3} to the list of superclasses
+m5001=Removed {3} from the list of superclasses
+
+#-----------------------------------------------------------------------------
+# FieldSetCheck messages
+#-----------------------------------------------------------------------------
+m6000=Added {3} field {2}
+m6001=Removed field {2}
+m6002=Value of field {2} is no longer a compile-time constant
+m6003=Value of compile-time constant {2} has been changed
+m6004=Changed type of field {2} from {3} to {4}
+m6005=Field {2} is now non-final
+m6006=Field {2} is now final
+m6007=Field {2} is now non-static
+m6008=Field {2} is now static
+m6009=Accessability of field {2} has been increased from {3} to {4}
+m6010=Accessability of field {2} has been weakened from {3} to {4}
+
+#-----------------------------------------------------------------------------
 # MethodSetCheck messages
+#-----------------------------------------------------------------------------
 m7000=Method ''{1}'' is now implemented in superclass {3}
 m7001=Abstract method ''{1}'' is now specified by implemented interface {3}
 m7002=Method ''{1}'' has been removed
@@ -21,3 +75,9 @@
 m7012=Method ''{1}'' has been added to an interface
 m7013=Abstract method ''{1}'' has been added
 
+
+#-----------------------------------------------------------------------------
+# Core Check messages
+#-----------------------------------------------------------------------------
+m8000=Class {0} added
+m8001=Class {0} removed

Re: [Clirr-devel] Message framework

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

Hi Lars,

I'm just about to commit another batch of Message-related patches.
This email explains what they're about.

On Wed, 2004-06-30 at 11:27, Simon Kitching wrote:
> On Wed, 2004-06-30 at 08:47, Lars K=FChne wrote:
> >=20
> >     * I would probably have kept all Message instances in the Message
> >       class itself (typesafe enum idiom), not in the MethodSetCheck. =
The
> >       event package will be part of clirr's public API (for IDE plugi=
ns)
> >       and I think we don't want to have a public constructor for
> >       Messages. Typesafe enum also provides a central point of refere=
nce
> >       for all possible changes Clirr can find.
>=20
> By putting the Message objects on the check classes, I was trying to
> avoid "class version churn". With all the possible messages declared
> "enum-style" on the central Message class, whenever any test is added t=
o
> any check, the Message class needs to gets updated. With the design I
> committed, that doesn't need to happen.
>=20
> However there are other points of "churn" anyway: the .properties file
> and the "message description" html file. So one more is probably not a
> significant problem.
>=20
> So I'm happy to move the Message members from MethodSetCheck to the
> Message class itself. There are definite benefits to doing this
> (including being able to make the constructor private as you said).

I had a go at moving all the Message object declarations into the
Message class (enum-style) but it just didn't feel right. In the end I
felt my original approach of having the Message objects declared on the
check classes was better. Here's my reasons for this:

(a) As mentioned earlier, having all the Message declarations on the
Message class causes the Message.java class to be very frequently
changed in CVS. This just isn't tidy.

(b) Message objects really aren't ENUM constants. They are only ever
referred to "by name" from the point where the ApiDifference is created
and fired. So I don't feel the "enum" design pattern really applies
here.

(c) Having the Message class "know" about all the possible messages that
can be generated really is an un-necessary coupling. Yes, this is=20
probably only theoretical but I feel it isn't tidy.

(d) With all Message objects declared as members of the Message class,
it isn't possible for users to add any custom checks, or subclass
existing checks to add extra tests that can generate new messages. Note
that the current message framework doesn't currently make this simple
either, but it is easier to implement this in future if we don't expect
all Message objects to be members of the Message class.

(e) The current design allows the Message objects to be private members
of the check classes. Moving them to the Message class means they would
have to be public, just so they can then be referenced from exactly one
place in the code: the line in the check class where the message is
reported. Not only is this slightly untidy, but checkstyle requires each
public member to have a separate javadoc tag :-(

>=20
>=20
> >     * For testing I think it would be better if the translation outpu=
t
> >       language would be controlled by the DiffListener (add a Locale
> >       param do ApiDifference.getReport?) and not in the MessageManage=
r.
> >       Maybe such a move would even allows us to get rid of the
> >       MessageManager completely? I usually try to avoid singletons
> >       wherever I can...
>=20
> Yep. Having the translation step done by the DiffListener was my
> original design too. It's just a little more difficult to
> "incrementally" introduce this change, as it is necessary to update
> every DiffListener implementation. There's also the issue of what objec=
t
> holds a reference to the ResourceBundle (ie the loaded properties file).
> We certainly don't want to reload this file each time we generate a
> message. And the ApiDifference can't hold it because these are very
> short-lived objects.=20
>=20
> I think perhaps the best solution is to have each DiffListener hold the
> relevant ResourceBundle. I will have a think about this.=20
>=20
> I agree singletons should generally be avoided. However in this case
> Message objects being created as static members of classes are
> registering themselves with a "central" object, so singleton was really
> the only way to go. The fact that the MessageManager also holds the
> ResourceBundle for the translation is kind of an "after-thought".
>=20
> The MessageManager does provide a point where all the Message objects
> register themselves, so that it is possible to print out the list of al=
l
> Message objects, check there are no duplicates, check that all
> translations are present. I like having this facility available, so
> would like to keep the MessageManager and its singleton behaviour, even
> if we move the ResourceBundle elsewhere (eg on to the DiffListener
> classes).

What I've done is split out the resource-related stuff in the
MessageManager class into a new MessageTranslator class.

The MessageManager still exists, and is still a singleton. But now its
only role is as a central registry of all the (static final) Message
objects. In this sense, it is really an extension of the ClassLoader.
And as a singleton is by definition 1:1 with its ClassLoader, using a
singleton seems nice and consistent to me.

The MessageTranslator class is *not* a singleton. Each DiffReporter
class now has a MessageTranslator member which it uses as a paramter to
the ApiDifference.getReport method. This means that potentially we can
have multiple DiffReporter objects each using a different locale. I
don't know why that would ever be useful, but it's cool :-)


-------------

Because the ApiDifference constructor now requires Message objects
instead of literal report strings, there were some resulting problems
with the unit test framework.

One option would be for the unit tests to specify message IDs, eg
  ApiDifference[] expected =3D=20
  {
    new ApiDifference( new Message(1000), ....)
  }

However I think it is valuable for the tests to have a literal
message string in them, because this tests the message expansion
process; I had problems earlier with messages looking like
  "Added method {1} to class {2}"
instead of=20
  "Added method foo to class Bar".

So what I did instead was create an ExpectedDiff class, and change the
unit tests over to using:
  ExpectedDiff[] expected =3D
  {=20
    new ExpectedDiff("Added method foo to class Bar", .....)
  }

There is a method ExpectedDiff.matches(ApiDifference) which determines
whether the reported difference matches the expected one.

This also allowed removal of all the custom hashCode and equals code in
the ApiDifference class. Unfortunately, it does mean that the method
  TestDiffListener.checkExpected
isn't quite as efficient as it used to be. If you can think of a better
implementation for this, that would be nice. But in the end the new
implementation works ok, and it is only a unit test.


I hope all this is OK with you. Any and all of it can be changed...

Cheers,

Simon

Re: [Clirr-devel] Message framework

[<lak...@t-...>]

Simon Kitching wrote:

>[...]
>  
>
>>    * Using english language properties files without language extension
>>      (i.e. x.properties instead of x_en.properties) does bring some
>>      nasty surprises, see
>>      https://sourceforge.net/tracker/index.php?func=detail&aid=594469&group_id=29721&atid=397078
>>      <>I have a solution that will work without copying the properties
>>      file during the build (like we do in checkstyle). You just need to
>>      use a valid class name as the ressource name, e.g.
>>      EventMessages_en.properties, I can do the rest.
>>      <><> 
>>    
>>
> <>
> I understand the problem described by your email (and yes that is
> nasty!). I don't understand your comment above, though.
>
> Isn't the correct solution just to copy event_messages.properties to
> event_messages_en.properties as part of the build?


> <>With that:
> * if the user asks for "en" then "en" is found.
> * if the user asks for an unsupported locale then they get the
> properties for the machine's default locale (if it exists)
> * if there are no properties for the machine's default locale,
> then they get the base version (which is also english).
>


Basically yes, that's what we do in checkstyle, and it works. My point 
probably is that Maven is great for doing standard things, but it's a 
bit tricky to convince it to do additional things (at least for me), and 
I have a better solution that doesn't require build time tricks or 
duplicate data in the jar file.

>So what do you mean by "You just need to use a valid class name as the
>resource name"? Maybe it is just easier if you commit the fix then I can
>see what you meant :-)
>
>  
>
:-)

OK, I'll fix it, but probably not tonight. European Football 
Championship Semifials on TV - sorry, I just have to set priorities ;-).

Cheers,
Lars

Re: [Clirr-devel] Message framework

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

On Wed, 2004-06-30 at 08:47, Lars K=FChne wrote:
> Simon Kitching wrote:
>=20
> >Hi Lars,
> >
> >Ok, message framework all committed. Please have a look and let me kno=
w
> >what you think.
> >
> > =20
> >
>=20
> In general the approach of using message IDs is great, and I can see=20
> where you are heading with the docs.
>=20
> A few things concerning the concrete implementation:
>=20
>     * I would probably have kept all Message instances in the Message
>       class itself (typesafe enum idiom), not in the MethodSetCheck. Th=
e
>       event package will be part of clirr's public API (for IDE plugins=
)
>       and I think we don't want to have a public constructor for
>       Messages. Typesafe enum also provides a central point of referenc=
e
>       for all possible changes Clirr can find.

By putting the Message objects on the check classes, I was trying to
avoid "class version churn". With all the possible messages declared
"enum-style" on the central Message class, whenever any test is added to
any check, the Message class needs to gets updated. With the design I
committed, that doesn't need to happen.

However there are other points of "churn" anyway: the .properties file
and the "message description" html file. So one more is probably not a
significant problem.

So I'm happy to move the Message members from MethodSetCheck to the
Message class itself. There are definite benefits to doing this
(including being able to make the constructor private as you said).


>     * For testing I think it would be better if the translation output
>       language would be controlled by the DiffListener (add a Locale
>       param do ApiDifference.getReport?) and not in the MessageManager.
>       Maybe such a move would even allows us to get rid of the
>       MessageManager completely? I usually try to avoid singletons
>       wherever I can...

Yep. Having the translation step done by the DiffListener was my
original design too. It's just a little more difficult to
"incrementally" introduce this change, as it is necessary to update
every DiffListener implementation. There's also the issue of what object
holds a reference to the ResourceBundle (ie the loaded properties file).
We certainly don't want to reload this file each time we generate a
message. And the ApiDifference can't hold it because these are very
short-lived objects.=20

I think perhaps the best solution is to have each DiffListener hold the
relevant ResourceBundle. I will have a think about this.=20

I agree singletons should generally be avoided. However in this case
Message objects being created as static members of classes are
registering themselves with a "central" object, so singleton was really
the only way to go. The fact that the MessageManager also holds the
ResourceBundle for the translation is kind of an "after-thought".

The MessageManager does provide a point where all the Message objects
register themselves, so that it is possible to print out the list of all
Message objects, check there are no duplicates, check that all
translations are present. I like having this facility available, so
would like to keep the MessageManager and its singleton behaviour, even
if we move the ResourceBundle elsewhere (eg on to the DiffListener
classes).

>     * Using english language properties files without language extensio=
n
>       (i.e. x.properties instead of x_en.properties) does bring some
>       nasty surprises, see
>       https://sourceforge.net/tracker/index.php?func=3Ddetail&aid=3D594=
469&group_id=3D29721&atid=3D397078
>       <>I have a solution that will work without copying the properties
>       file during the build (like we do in checkstyle). You just need t=
o
>       use a valid class name as the ressource name, e.g.
>       EventMessages_en.properties, I can do the rest.
>       <><>=20

I understand the problem described by your email (and yes that is
nasty!). I don't understand your comment above, though.

Isn't the correct solution just to copy event_messages.properties to
event_messages_en.properties as part of the build?

With that:
 * if the user asks for "en" then "en" is found.
 * if the user asks for an unsupported locale then they get the
   properties for the machine's default locale (if it exists)
 * if there are no properties for the machine's default locale,
   then they get the base version (which is also english).

So what do you mean by "You just need to use a valid class name as the
resource name"? Maybe it is just easier if you commit the fix then I can
see what you meant :-)


>=20
> Hope this doesn't come across too negative... as I said, I think this i=
s=20
> great work!

No problems at all. It's great to get feedback. One of the things I miss
with my current job is the chance to debate design a little; I
essentially work alone on the projects I am responsible for.

Your comments are all high-quality too; I'm happy to get them. And if I
disagree, I'll simply let you know :-)

>=20
> >I've only updated the MethodSetCheck class to use it, so that if there
> >are changes I don't need to redo/revert everything. If you like it, th=
en
> >I will go ahead and convert the other check classes over.
> >
> > =20
> >
>=20
> Go ahead, we can clean up the above points later.
>=20

Will do!

Cheers,

Simon

Re: [Clirr-devel] Message framework

[<lak...@t-...>]

Simon Kitching wrote:

>Hi Lars,
>
>Ok, message framework all committed. Please have a look and let me know
>what you think.
>
>  
>

In general the approach of using message IDs is great, and I can see 
where you are heading with the docs.

A few things concerning the concrete implementation:

    * I would probably have kept all Message instances in the Message
      class itself (typesafe enum idiom), not in the MethodSetCheck. The
      event package will be part of clirr's public API (for IDE plugins)
      and I think we don't want to have a public constructor for
      Messages. Typesafe enum also provides a central point of reference
      for all possible changes Clirr can find.
    * For testing I think it would be better if the translation output
      language would be controlled by the DiffListener (add a Locale
      param do ApiDifference.getReport?) and not in the MessageManager.
      Maybe such a move would even allows us to get rid of the
      MessageManager completely? I usually try to avoid singletons
      wherever I can...
    * Using english language properties files without language extension
      (i.e. x.properties instead of x_en.properties) does bring some
      nasty surprises, see
      https://sourceforge.net/tracker/index.php?func=detail&aid=594469&group_id=29721&atid=397078
      <>I have a solution that will work without copying the properties
      file during the build (like we do in checkstyle). You just need to
      use a valid class name as the ressource name, e.g.
      EventMessages_en.properties, I can do the rest.
      <><> 

Hope this doesn't come across too negative... as I said, I think this is 
great work!

>I've only updated the MethodSetCheck class to use it, so that if there
>are changes I don't need to redo/revert everything. If you like it, then
>I will go ahead and convert the other check classes over.
>
>  
>

Go ahead, we can clean up the above points later.

>Sorry the commit to MethodSetCheck.java has 2 separate changes in it
>(scopeSelector, checkVisibility and Message support).
>

No problem.

> I don't have
>internet access at home (too cheap to pay when I've got it for free at
>work :-). So over a weekend I tend to sometimes end up implementing
>several features, and they sometimes touch the same files. If we were
>using a "distributed" version control system like "monotone", this
>wouldn't be a problem...
>
>  
>

Hey, I had never heard of monotone before - thanks for mentioning it!

Lars

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/event MessageManager.java,1.1,1.2

[<lk...@us...>]

Update of /cvsroot/clirr/clirr/src/java/net/sf/clirr/event
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv8598

Modified Files:
	MessageManager.java 
Log Message:
fixed javadoc problem

Index: MessageManager.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/event/MessageManager.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- MessageManager.java	28 Jun 2004 06:51:21 -0000	1.1
+++ MessageManager.java	29 Jun 2004 20:22:35 -0000	1.2
@@ -157,7 +157,7 @@
      * Message ids in the properties file should be prefixed with an 'm',
      * eg "m1000", "m5003".
      * <p>
-     * @throws MissingResourceException if there is no entry in the
+     * @throws java.util.MissingResourceException if there is no entry in the
      * message translation resource bundle for the specified message.
      */
     public String getDesc(Message msg)

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/checks MethodSetCheck.java,1.16,1.17

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/java/net/sf/clirr/checks
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv14507/src/java/net/sf/clirr/checks

Modified Files:
	MethodSetCheck.java 
Log Message:
Minor change to comment


Index: MethodSetCheck.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/checks/MethodSetCheck.java,v
retrieving revision 1.16
retrieving revision 1.17
diff -u -r1.16 -r1.17
--- MethodSetCheck.java	29 Jun 2004 07:01:25 -0000	1.16
+++ MethodSetCheck.java	29 Jun 2004 07:40:58 -0000	1.17
@@ -533,15 +533,15 @@
         Type bReturnType = baselineMethod.getReturnType();
         Type cReturnType = currentMethod.getReturnType();
 
-        // TODO: Check assignability...
+        // TODO: Check assignability. If the new return type is
+        // assignable to the old type, then the code is source-code
+        // compatible even when binary-incompatible.
         if (!bReturnType.toString().equals(cReturnType.toString()))
         {
             fireDiff(MSG_METHOD_RETURNTYPE_CHANGED,
                     Severity.ERROR, compatBaseline, baselineMethod,
                     new String[] {cReturnType.toString()});
         }
-
-
     }
 
     private void checkDeclaredExceptions(

[Clirr-devel] CVS: clirr/xdocs navigation.xml,1.6,1.7

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/xdocs
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv13818

Modified Files:
	navigation.xml 
Log Message:
Added links for running clirr from cli + message explanations


Index: navigation.xml
===================================================================
RCS file: /cvsroot/clirr/clirr/xdocs/navigation.xml,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- navigation.xml	5 Oct 2003 17:58:20 -0000	1.6
+++ navigation.xml	29 Jun 2004 07:37:16 -0000	1.7
@@ -13,8 +13,14 @@
                 href="/index.html"/>
             <item name="Download"
                 href="/download.html"/>
-            <item name="Running Clirr"
+            <item name="Running Clirr">
+              <item name="From Ant"
                 href="/anttask.html"/>
+              <item name="From a command-line"
+                href="/runcli.html"/>
+            </item>
+            <item name="Message Explanations"
+                href="/exegesis.html"/>
             <item name="Contributing"
                 href="/contributing.html"/>
             <item name="Related tools"

[Clirr-devel] CVS: clirr/xdocs runcli.xml,NONE,1.1

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/xdocs
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv13750

Added Files:
	runcli.xml 
Log Message:
Information on running Clirr via the commandline.


--- NEW FILE ---
<document>
  <properties>
    <title>Clirr Command-Line Usage</title>
    <author>Lars Kühne</author>
  </properties>
  <body>
    <section name="Running Clirr from the Command Line">
      <p>
        Clirr provides an "uberjar" download, which bundles all the
        necessary code, including all required libraries, into a single
        executable jar file. Clirr can then be run from a commandline
        via the command:
        <code>
          java -jar clirr-1.0-uber.jar
        </code>
        Running clirr with no command-line arguments will display the
        available options.
      </p>
    </section>
  </body>
</document>

[Clirr-devel] CVS: clirr/xdocs exegesis.xml,NONE,1.1

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/xdocs
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv13007

Added Files:
	exegesis.xml 
Log Message:
Verbose explanations of MethodSetCheck messages.


--- NEW FILE ---
<?xml version="1.0" encoding="ISO-8859-1"?>

<document>
  <properties>
    <title>Difference Message Reference Manual</title>
    <author>The Clirr Development Team</author>
  </properties>
  <body>
    <section name="Introduction">
        <p>
            When clirr generates an ERROR, WARNING or INFO message about
            a change in the jars being compared, there is an associated
            message reference code. This document contains an explanation
            of the meaning of that message which may contain information
            which could not be fitted into the brief message summary.
        </p>
        <p>
            Messages are separated into three severity levels:
            <ul>
              <li>ERROR</li>
              <li>WARNING</li>
              <li>INFO</li>
            </ul>
        </p>
        <p>
            Errors come in two flavours: 
            <ul>
              <li>
                Link-time failures, where an exception will be thrown as soon
                as code compiled against an old version of a class and the
                new version of the class are loaded into the same classloader
                hierarchy.
              </li>
              <li>
                Run-time failures, where an exception is thrown when code
                compiled against the old version of a class attempts to call a 
                method on a new version of the class, or vice versa.
              </li>
            </ul>
            <p>
                Clirr reports "errors" for cases where it is <i>possible</i>
                to get a run-time failure. Whether one actually occurs can
                depend upon the way the library is called, ie changes reported
                as an error may in fact work when used as long as the patterns
                of use of the library do not trigger the failure situation.
            </p>
        </p>
        <p>
            Warnings are issued for situations where no link or runtime
            exception will occur, but where the application may behave
            unexpectedly due to the changes that have occurred.
        </p>
        <p>
            Information messages provide users with information about
            new features which have been added without breaking backward
            compatibility in any way.
        </p>
        <p>
            In the following sections, the term "old" is used to refer to
            a class, interface, method or field from the set of jars which
            represent the old/previous/original/baseline version of the
            library being inspected. The term "new" is used to refer to
            a class, interface, method or field from the set of jars
            which represent the new/current/latest version of the library
            being inspected.
        </p>
    </section>
    <section name="Messages">
      <section name="7000 - Method now in Superclass">
        <p>Severity: <code>INFO</code></p>
        <p>
          The old class had a method named X. The new class no longer has this 
          method, but a parent class does define this method, so no binary or 
          source incompatibility has occurred.
        </p>
        <p>
          Note that this change may have the effect of forcing the new
          class to become 'abstract'. If this is the case, then this change
          is reported separately.
        </p>
      </section>
      <section name="7001 - Method now in Interface">
        <p>Severity: <code>INFO</code></p>
        <p>
          The old class or interface previously had a method named X. The 
          new class or interface no longer has this method, but a parent 
          interface does define this method, so no binary or source 
          incompatibility has occurred.
        </p>
        <p>
          Note that this change may have the effect of forcing the new
          class to become 'abstract'. If this is the case, then this change
          is reported separately.
        </p>
      </section>
      <section name="7002 - Method Removed">
        <p>Severity: <code>ERROR</code></p>
        <p>
          The old class or interface had a method named X. The new class or
          interface no longer has this method, and this method is not defined 
          on any parent class or interface.
        </p>
        <p>
          Whether an error actually occurs at runtime for this change depends
          on usage patterns. The modified class can be used with existing
          code as long as that existing code does not attempt to call the
          removed method. If a call to the missing method is made, then
          a NoSuchMethodError exception is generated when the method
          invocation occurs.
        </p>
      </section>
      <section name="7003 - Unused">
        <p>This message code is not currently used.</p>
      </section>
      <section name="7004 - Method Argument Count Changed">
        <p>Severity: <code>ERROR</code></p>
        <p>
          The specified method has had arguments added or removed. This means
          that code which previously invoked it will no longer invoke the same
          method.
        </p>
        <p>
          If there is an inherited method definition with the old prototype,
          then there is no binary incompatibility; code which was compiled
          against the old version of this class will now invoke the inherited
          implementation. <em>In this situation, clirr should output an INFO
          message rather than an error. However at the current date, clirr does 
          not check for this situation</em>.
        </p>
        <p>
          If there is no inherited method definition with the old prototype,
          then the change is a binary incompatibility.
        </p>
      </section>
      <section name="7005 - Method Argument Type changed">
        <p>Binary Severity: <code>INFO or ERROR</code></p>
        <p>Source Severity: <code>ERROR</code></p>
        <p>
          The specified method has had the type of one or more of its arguments
          modified. This means that code compiled against the old version of
          the class will no longer invoke the same method. However exactly
          the same old source code, when compiled against the new class
          version <i>may</i> invoke this method if the argument types are
          assignment-compatible.
        </p>
        <p>
          If there is an inherited method definition with the old prototype,
          then there is no binary incompatibility; code which was compiled
          against the old version of this class will now invoke the inherited
          implementation.
          <em>At the current date, clirr does not check for this situation</em>.
        </p>
        <p>
          If there is no inherited method definition with the old prototype,
          then the change is a binary incompatibility.
        </p>
        <p>
          If the parameter types changed were all changed to <i>supertypes</i>
          of their previous declared types, or for primitive parameter types if
          they were changed to "larger" types in every case, then the new
          code is <i>source-code-compatible</i> with the previous release
          even if it is not binary-compatible. Note that in this situation,
          recompiling code which uses the library may change its behaviour
          from calling an inherited method to calling a method on the class
          which has a slightly different prototype.
          <em>At the current date, clirr does not check for this situation</em>.
        </p>
      </section>
      <section name="7006 - Method Return Type changed">
        <p>Binary Severity: <code>ERROR</code></p>
        <p>Source Severity: <code>INFO or ERROR</code></p>
        <p>
          The specified method has had its declared return type changed. Whether
          a problem actually occurs at runtime when using code compiled against
          the old version of this library depends upon usage patterns. Old
          code may call other methods on this class. However any attempt to
          call the method whose return type has changed will result in a
          NoSuchMethodError being thrown when the method is invoked, because
          the return type is part of the "method signature".
        </p>
        <p>
          The change is <i>source-code-compatible</i> if and only if the new 
          return type is <i>assignable to</i> the old return type. This means 
          that:
          <ul>
            <li>
              if the old return type was a primitive type, then the new return 
              type must be <i>narrower</i> than the old type.
            </li>
            <li>
              if the old return type was an interface, then the new return type
              must be a class or interface which implements the old return type. 
            </li>
            <li>
              if the old return type was a class, then the new return type must
              be a subclass of the previously returned type.
            </li>
          </ul>
          Clirr does not currently check for source-code compatibility for
          changes in method return types; currently these are simply
          reported as an ERROR.
        </p>
      </section>
      <section name="7007 - Method has been Deprecated">
        <p>Severity: <code>INFO</code></p>
        <p>
          The specified method has been declared as "deprecated". This is
          always a binary-compatible change as well as a source-code-compatible
          change.
        </p>
      </section>
      <section name="7008 - Method has been Undeprecated">
        <p>Severity: <code>INFO</code></p>
        <p>
          The specified method was declared "deprecated" in the previous 
          version, but is no longer deprecated in the current release. While
          slightly unusual, this is permitted. This change is always a
          binary-compatible change as well as a source-code-compatible change.
        </p>
      </section>
      <section name="7009 - Method is now Less Accessable">
        <p>Severity: <code>ERROR</code></p>
        <p>
          The access permissions associated with the specified method have
          been tightened to permit less user code to access the method.
        </p>
        <p>
          Whether this change is truly a binary-compatibility error depends
          upon patterns of usage. If the change is from public to protected,
          and the protected method is only invoked from code which subclasses
          the modified class then the new accessability for the method is still
          adequate and no problem will occur. Old code will also continue to
          work correctly as long as the modified method is not invoked. If it
          is invoked, then an IllegalAccessException is thrown at the time of
          method invocation.
        </p>
        <p>
          Whether this change is a source-code compatibility issue or not
          also depends upon patterns of usage. This change is therefore
          always reported as an ERROR.
        </p>
      </section>
      <section name="7010 - Method is now More Accessable">
        <p>Severity: <code>INFO</code></p>
        <p>
          The access permissions associated with the specified method have
          been loosened to permit more user code to access the method. This
          is always a binary and source-code compatible change.
        </p>
      </section>
      <section name="7011 - Method Added">
        <p>Severity: <code>INFO</code></p>
        <p>
          A non-abstract method has been added to the specified class. This is
          always a binary-compatible change.
        </p>
        <p>
          It is also a source-code compatible change.
        </p>
        <p>
         Q: if the new method overrides an inherited one, then which version
         does code compiled against the old library invoke?
        </p>
      </section>
      <section name="7012 - Method Added to Interface">
        <p>Binary Severity: <code>ERROR</code></p>
        <p>Source Severity: <code>ERROR</code></p>
        <p>
          A method declaration has been added to the specified interface. This
          is always reported as a binary-compatibility error, but in practice
          the changed class <i>might</i> be used successfully with code compiled
          against the old interface depending upon usage patterns.
        </p>
        <p>
          Old code which invokes methods upon code compiled against the new
          (expanded) interface will continue to work without issues. And old 
          code which implements the old version of the interface will also 
          continue to work correctly as long as no code attempts to invoke
          any of the newly-added methods against that instance. But code which
          (validly) invokes one of the new methods in the interface against
          an object which implements only the old version of the interface will
          cause an AbstractMethodError to be thrown at the time the method
          invocation is attempted.
        </p>
        <p>
          Adding a method to an interface is always reported as an ERROR,
          because classes that implement that interface must now be modified
          to implement the declared method.
        </p>
      </section>
      <section name="7013 - Abstract Method Added to Class">
        <p>Binary Severity: <code>ERROR</code></p>
        <p>Source Severity: <code>ERROR</code></p>
        <p>
          An abstract method declaration has been added to the specified class.
          This is always reported as a binary-compatibility error, but in practice
          the changed class <i>might</i> be used successfully with code compiled
          against the old class depending upon usage patterns.
        </p>
        <p>
          If instances of objects compiled against the old class are created,
          then their methods can be invoked without problems. But if the
          newly-added abstract method is ever invoked, then an AbstractMethodError
          is thrown at the time the method invocation is attempted.
        </p>
      </section>
    </section>
  </body>
</document>

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/event PlainDiffListener.java,1.5,1.6

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/java/net/sf/clirr/event
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv8805/event

Modified Files:
	PlainDiffListener.java 
Log Message:
Output message-id (if any) on front of each report.
Output affected class name on front of each report.


Index: PlainDiffListener.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/event/PlainDiffListener.java,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- PlainDiffListener.java	27 Jun 2004 14:21:35 -0000	1.5
+++ PlainDiffListener.java	29 Jun 2004 07:06:30 -0000	1.6
@@ -35,6 +35,15 @@
     {
         PrintStream out = getOutputStream();
         out.print(difference.getMaximumSeverity().toString());
+
+        Message m = difference.getMessage();
+        if (m != null)
+        {
+            out.print(": ");
+            out.print(m.getId());
+        }
+        out.print(": ");
+        out.print(difference.getAffectedClass());
         out.print(": ");
         out.println(difference.getReport());
     }

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/event ApiDifference.java,1.12,1.13

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/java/net/sf/clirr/event
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv7875/event

Modified Files:
	ApiDifference.java 
Log Message:
Provide getter method for associated Message object.


Index: ApiDifference.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/event/ApiDifference.java,v
retrieving revision 1.12
retrieving revision 1.13
diff -u -r1.12 -r1.13
--- ApiDifference.java	28 Jun 2004 06:53:00 -0000	1.12
+++ ApiDifference.java	29 Jun 2004 07:02:06 -0000	1.13
@@ -205,6 +205,17 @@
     }
 
     /**
+     * Return the message object (if any) associated with this difference.
+     * <p>
+     * Checks which support the "new" message API will provide ApiDifference
+     * objects with non-null message objects.
+     */
+    public Message getMessage()
+    {
+        return message;
+    }
+
+    /**
      * The Severity of the API difference in terms of binary compatibility.
      * ERROR means that clients will definitely break, WARNING means that
      * clients may break, depending on how they use the library.

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/checks MethodSetCheck.java,1.15,1.16

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/java/net/sf/clirr/checks
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv7696/checks

Modified Files:
	MethodSetCheck.java 
Log Message:
Swap message ids 7002 and 7003 for tidiness


Index: MethodSetCheck.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/checks/MethodSetCheck.java,v
retrieving revision 1.15
retrieving revision 1.16
diff -u -r1.15 -r1.16
--- MethodSetCheck.java	28 Jun 2004 06:57:28 -0000	1.15
+++ MethodSetCheck.java	29 Jun 2004 07:01:25 -0000	1.16
@@ -49,8 +49,8 @@
 {
      private static final Message MSG_METHOD_NOW_IN_SUPERCLASS = new Message(7000);
      private static final Message MSG_METHOD_NOW_IN_INTERFACE = new Message(7001);
-     // 7002 unused
-     private static final Message MSG_METHOD_REMOVED = new Message(7003);
+     private static final Message MSG_METHOD_REMOVED = new Message(7002);
+     // 7003 unused
      private static final Message MSG_METHOD_ARGCOUNT_CHANGED = new Message(7004);
      private static final Message MSG_METHOD_PARAMTYPE_CHANGED = new Message(7005);
      private static final Message MSG_METHOD_RETURNTYPE_CHANGED = new Message(7006);

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/ant AntLogger.java,1.7,1.8

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/java/net/sf/clirr/ant
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv7325/ant

Modified Files:
	AntLogger.java 
Log Message:
Add affected class name to the front of each message.


Index: AntLogger.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/ant/AntLogger.java,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -r1.7 -r1.8
--- AntLogger.java	27 Jun 2004 14:21:34 -0000	1.7
+++ AntLogger.java	29 Jun 2004 07:00:02 -0000	1.8
@@ -46,6 +46,10 @@
     {
         final Severity severity = difference.getMaximumSeverity();
         final Integer prio = (Integer) severityPrioMap.get(severity);
-        task.log(severity.toString() + ": " + difference.getReport(), prio.intValue());
+        task.log(
+            severity.toString()
+            + ": " + difference.getAffectedClass()
+            + ": " + difference.getReport(),
+            prio.intValue());
     }
 }

[Clirr-devel] CVS: clirr/src/test/net/sf/clirr/checks MethodSetCheckTest.java,1.9,1.10

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/test/net/sf/clirr/checks
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv6863/net/sf/clirr/checks

Modified Files:
	MethodSetCheckTest.java 
Log Message:
Removed the "..in {classname}" from the end of each generated message.


Index: MethodSetCheckTest.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/test/net/sf/clirr/checks/MethodSetCheckTest.java,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -r1.9 -r1.10
--- MethodSetCheckTest.java	28 Jun 2004 05:09:46 -0000	1.9
+++ MethodSetCheckTest.java	29 Jun 2004 06:56:46 -0000	1.10
@@ -17,50 +17,50 @@
         ApiDifference[] expected = new ApiDifference[] {
 
             // method addition and removal
-            new ApiDifference("Method 'public void removedMethod(java.lang.String)' has been removed in testlib.MethodsChange",
+            new ApiDifference("Method 'public void removedMethod(java.lang.String)' has been removed",
                     Severity.ERROR, "testlib.MethodsChange", "public void removedMethod(java.lang.String)", null),
-            new ApiDifference("Accessability of method 'public int getPriv2()' has been decreased from public to private in testlib.MethodsChange",
+            new ApiDifference("Accessability of method 'public int getPriv2()' has been decreased from public to private",
                     Severity.ERROR, "testlib.MethodsChange", "public int getPriv2()", null),
-            new ApiDifference("Method 'protected MethodsChange(int, boolean)' has been added in testlib.MethodsChange",
+            new ApiDifference("Method 'protected MethodsChange(int, boolean)' has been added",
                     Severity.INFO, "testlib.MethodsChange", "protected MethodsChange(int, boolean)", null),
-            new ApiDifference("Method 'public java.lang.Long getPrivSquare()' has been added in testlib.MethodsChange",
+            new ApiDifference("Method 'public java.lang.Long getPrivSquare()' has been added",
                     Severity.INFO, "testlib.MethodsChange", "public java.lang.Long getPrivSquare()", null),
 
-            new ApiDifference("Method 'public void moveToSuper()' has been added in testlib.ComplexMethodMoveBase",
+            new ApiDifference("Method 'public void moveToSuper()' has been added",
                     Severity.INFO, "testlib.ComplexMethodMoveBase", "public void moveToSuper()", null),
-            new ApiDifference("Method 'public void moveToSuper()' is now implemented in superclass testlib.ComplexMethodMoveBase in testlib.ComplexMethodMoveSub",
+            new ApiDifference("Method 'public void moveToSuper()' is now implemented in superclass testlib.ComplexMethodMoveBase",
                     Severity.INFO, "testlib.ComplexMethodMoveSub", "public void moveToSuper()", null),
 
-            new ApiDifference("Abstract method 'public void method()' is now specified by implemented interface testlib.BaseInterface in testlib.AbstractImpl",
+            new ApiDifference("Abstract method 'public void method()' is now specified by implemented interface testlib.BaseInterface",
                     Severity.INFO, "testlib.AbstractImpl", "public void method()", null),
 
             // Constructor changes
-            new ApiDifference("Parameter 1 of 'protected MethodsChange(int)' has changed its type to java.lang.Integer in testlib.MethodsChange",
+            new ApiDifference("Parameter 1 of 'protected MethodsChange(int)' has changed its type to java.lang.Integer",
                     Severity.ERROR, "testlib.MethodsChange", "protected MethodsChange(int)", null),
 
             // return type changes
-            new ApiDifference("Return type of method 'public java.lang.Number getPrivAsNumber()' has been changed to java.lang.Integer in testlib.MethodsChange",
+            new ApiDifference("Return type of method 'public java.lang.Number getPrivAsNumber()' has been changed to java.lang.Integer",
                     Severity.ERROR, "testlib.MethodsChange", "public java.lang.Number getPrivAsNumber()", null),
             // TODO: INFO if method is final
-            new ApiDifference("Return type of method 'public java.lang.Integer getPrivAsInteger()' has been changed to java.lang.Number in testlib.MethodsChange",
+            new ApiDifference("Return type of method 'public java.lang.Integer getPrivAsInteger()' has been changed to java.lang.Number",
                     Severity.ERROR, "testlib.MethodsChange", "public java.lang.Integer getPrivAsInteger()", null),
 
             // parameter list changes
             // Note: This is the current behaviour, not necessarily the spec of the desired behaviour
             // TODO: need to check assignability of types (and check if method or class is final?)
-            new ApiDifference("In method 'public void printPriv()' the number of arguments has changed in testlib.MethodsChange",
+            new ApiDifference("In method 'public void printPriv()' the number of arguments has changed",
                     Severity.ERROR, "testlib.MethodsChange", "public void printPriv()", null),
-            new ApiDifference("Parameter 1 of 'public void strengthenParamType(java.lang.Object)' has changed its type to java.lang.String in testlib.MethodsChange",
+            new ApiDifference("Parameter 1 of 'public void strengthenParamType(java.lang.Object)' has changed its type to java.lang.String",
                     Severity.ERROR, "testlib.MethodsChange", "public void strengthenParamType(java.lang.Object)", null),
-            new ApiDifference("Parameter 1 of 'public void weakenParamType(java.lang.String)' has changed its type to java.lang.Object in testlib.MethodsChange",
+            new ApiDifference("Parameter 1 of 'public void weakenParamType(java.lang.String)' has changed its type to java.lang.Object",
                     Severity.ERROR, "testlib.MethodsChange", "public void weakenParamType(java.lang.String)", null),
-            new ApiDifference("Parameter 1 of 'public void changeParamType(java.lang.String)' has changed its type to java.lang.Integer in testlib.MethodsChange",
+            new ApiDifference("Parameter 1 of 'public void changeParamType(java.lang.String)' has changed its type to java.lang.Integer",
                     Severity.ERROR, "testlib.MethodsChange", "public void changeParamType(java.lang.String)", null),
 
             // deprecation changes
-            new ApiDifference("Method 'public void becomesDeprecated()' has been deprecated in testlib.MethodsChange",
+            new ApiDifference("Method 'public void becomesDeprecated()' has been deprecated",
                     Severity.INFO, "testlib.MethodsChange", "public void becomesDeprecated()", null),
-            new ApiDifference("Method 'public void becomesUndeprecated()' is no longer deprecated in testlib.MethodsChange",
+            new ApiDifference("Method 'public void becomesUndeprecated()' is no longer deprecated",
                     Severity.INFO, "testlib.MethodsChange", "public void becomesUndeprecated()", null),
 
             // declared exceptions

[Clirr-devel] CVS: clirr/src/conf event-messages.properties,1.2,1.3

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/conf
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv6539

Modified Files:
	event-messages.properties 
Log Message:
Removed the "..in {classname}" string from the end of each message;
the reporting classes can add this if they want.
Also moved the "removed method" id from 7003 to 7002 for tidiness.


Index: event-messages.properties
===================================================================
RCS file: /cvsroot/clirr/clirr/src/conf/event-messages.properties,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- event-messages.properties	28 Jun 2004 06:52:03 -0000	1.2
+++ event-messages.properties	29 Jun 2004 06:55:45 -0000	1.3
@@ -6,18 +6,18 @@
 # {3}..{n} --> check-specific parameters
 
 # MethodSetCheck messages
-m7000=Method ''{1}'' is now implemented in superclass {3} in {0}
-m7001=Abstract method ''{1}'' is now specified by implemented interface {3} in {0}
-#m7002 no longer used
-m7003=Method ''{1}'' has been removed in {0}
-m7004=In method ''{1}'' the number of arguments has changed in {0}
-m7005=Parameter {3} of ''{1}'' has changed its type to {4} in {0}
-m7006=Return type of method ''{1}'' has been changed to {3} in {0}
-m7007=Method ''{1}'' has been deprecated in {0}
-m7008=Method ''{1}'' is no longer deprecated in {0}
-m7009=Accessability of method ''{1}'' has been decreased from {3} to {4} in {0}
-m7010=Accessability of method ''{1}'' has been increased from {3} to {4} in {0}
-m7011=Method ''{1}'' has been added in {0}
-m7012=Method ''{1}'' has been added to an interface in {0}
-m7013=Abstract method ''{1}'' has been added in {0}
+m7000=Method ''{1}'' is now implemented in superclass {3}
+m7001=Abstract method ''{1}'' is now specified by implemented interface {3}
+m7002=Method ''{1}'' has been removed
+#m7003 no longer used
+m7004=In method ''{1}'' the number of arguments has changed
+m7005=Parameter {3} of ''{1}'' has changed its type to {4}
+m7006=Return type of method ''{1}'' has been changed to {3}
+m7007=Method ''{1}'' has been deprecated
+m7008=Method ''{1}'' is no longer deprecated
+m7009=Accessability of method ''{1}'' has been decreased from {3} to {4}
+m7010=Accessability of method ''{1}'' has been increased from {3} to {4}
+m7011=Method ''{1}'' has been added
+m7012=Method ''{1}'' has been added to an interface
+m7013=Abstract method ''{1}'' has been added
 

[Clirr-devel] MethodSetCheck: changing argument count

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

Hi Lars,

Re the check for "Method argument count changed".

I suggest it would be better to remove the "heuristic" code in the
"similarity" section which tries to pair up methods which have different
numbers of parameters.

If this code was removed, then 

old class:  
  foo(int i);

new class: 
  foo(String s, Object o);

would report
 * removed method "foo(int i)"
 * added method "foo(String s, Object o)"
rather than
 * changed # of arguments in method foo

Are these two different versions of method Foo *really* "the same method
with different number of arguments"? I think the answer is "sometimes
yes, sometimes no".

And what do we report if the old version of Foo was overriding an
inherited method, and the new version is not?

By reporting a method removed + a method added, we may miss an
opportunity to be "clever", but we won't ever report "method changed
argument count" when the user sees the two variants as really being
different methods.

I think this is particularly important when there are a number of
overloaded variants with the same method name.

One possible compromise is to report "method changed argument count"
only when new arguments have been added to the end of the list of
arguments for the old method version. But frankly even this seems to
introduce more complexity than needed. 

What do you think?

Regards,

Simon

[Clirr-devel] CVS: clirr project.properties,1.6,1.7

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv27985

Modified Files:
	project.properties 
Log Message:
Added entry to enable "maven uberjar" target.


Index: project.properties
===================================================================
RCS file: /cvsroot/clirr/clirr/project.properties,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- project.properties	25 May 2004 04:43:29 -0000	1.6
+++ project.properties	29 Jun 2004 05:40:20 -0000	1.7
@@ -18,5 +18,8 @@
 maven.junit.sysproperties=testinput
 testinput=${basedir}/target/testinput
 
+# class to execute from all-in-one uberjar
+maven.uberjar.main=net.sf.clirr.cli.Clirr
+
 # used by test pregoal to generate test input, see maven.xml
 clirr.testlibs=testlib-v1, testlib-v2

Re: [Clirr-devel] Message framework

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

On Mon, 2004-06-28 at 19:01, Simon Kitching wrote:
> Hi Lars,
> 
> Ok, message framework all committed. Please have a look and let me know
> what you think.
> 
> I've only updated the MethodSetCheck class to use it, so that if there
> are changes I don't need to redo/revert everything. If you like it, then
> I will go ahead and convert the other check classes over.
> 
> Sorry the commit to MethodSetCheck.java has 2 separate changes in it
> (scopeSelector, checkVisibility and Message support). I don't have
> internet access at home (too cheap to pay when I've got it for free at
> work :-). So over a weekend I tend to sometimes end up implementing
> several features, and they sometimes touch the same files. If we were
> using a "distributed" version control system like "monotone", this
> wouldn't be a problem...

Just to state the obvious, the existance of this Message class with a
unique number in each one allows us to make this kind of change to the
report generators:

    public void reportDiff(ApiDifference difference)
    {
	...
        Message m = difference.getMessage();
        if (m != null)
        {
            out.print(" #");
            out.print(m.getId());
        }
        ....
    }

resulting in this kind of output:

INFO #7010: org.apache.commons.collections.BagUtils: Accessability of
method 'private BagUtils()' has been increased from private to public in
org.apache.commons.collections.BagUtils
INFO #7011: org.apache.commons.collections.BagUtils: Method 'public
org.apache.commons.collections.Bag
transformedBag(org.apache.commons.collections.Bag,
org.apache.commons.collections.Transformer)' has been added in
org.apache.commons.collections.BagUtils

Where the user can then look up 7010 and 7011 in the supplementary
documentation. Or for HTML reports, we can just generate a link to:
  http://clirr.sourceforge.net/doc/message-info.html#{msgid}

Regards,

Simon

[Clirr-devel] Message framework

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

Hi Lars,

Ok, message framework all committed. Please have a look and let me know
what you think.

I've only updated the MethodSetCheck class to use it, so that if there
are changes I don't need to redo/revert everything. If you like it, then
I will go ahead and convert the other check classes over.

Sorry the commit to MethodSetCheck.java has 2 separate changes in it
(scopeSelector, checkVisibility and Message support). I don't have
internet access at home (too cheap to pay when I've got it for free at
work :-). So over a weekend I tend to sometimes end up implementing
several features, and they sometimes touch the same files. If we were
using a "distributed" version control system like "monotone", this
wouldn't be a problem...

Cheers,

Simon

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/checks MethodSetCheck.java,1.14,1.15

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/java/net/sf/clirr/checks
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv4253

Modified Files:
	MethodSetCheck.java 
Log Message:
This commit includes 3 separate significant changes:
* add checkVisibility method to detect changes in method
  scope, eg protected->public.
* minor changes to way scopeSelector is used to filter which
  changes are reported
* added support for new Message infrastructure.


Index: MethodSetCheck.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/checks/MethodSetCheck.java,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -r1.14 -r1.15
--- MethodSetCheck.java	28 Jun 2004 05:58:13 -0000	1.14
+++ MethodSetCheck.java	28 Jun 2004 06:57:28 -0000	1.15
@@ -22,6 +22,7 @@
 import net.sf.clirr.event.ApiDifference;
 import net.sf.clirr.event.Severity;
 import net.sf.clirr.event.ScopeSelector;
+import net.sf.clirr.event.Message;
 import net.sf.clirr.framework.AbstractDiffReporter;
 import net.sf.clirr.framework.ApiDiffDispatcher;
 import net.sf.clirr.framework.ClassChangeCheck;
@@ -46,6 +47,21 @@
         extends AbstractDiffReporter
         implements ClassChangeCheck
 {
+     private static final Message MSG_METHOD_NOW_IN_SUPERCLASS = new Message(7000);
+     private static final Message MSG_METHOD_NOW_IN_INTERFACE = new Message(7001);
+     // 7002 unused
+     private static final Message MSG_METHOD_REMOVED = new Message(7003);
+     private static final Message MSG_METHOD_ARGCOUNT_CHANGED = new Message(7004);
+     private static final Message MSG_METHOD_PARAMTYPE_CHANGED = new Message(7005);
+     private static final Message MSG_METHOD_RETURNTYPE_CHANGED = new Message(7006);
+     private static final Message MSG_METHOD_DEPRECATED = new Message(7007);
+     private static final Message MSG_METHOD_UNDEPRECATED = new Message(7008);
+     private static final Message MSG_METHOD_LESS_ACCESSABLE = new Message(7009);
+     private static final Message MSG_METHOD_MORE_ACCESSABLE = new Message(7010);
+     private static final Message MSG_METHOD_ADDED = new Message(7011);
+     private static final Message MSG_METHOD_ADDED_TO_INTERFACE = new Message(7012);
+     private static final Message MSG_ABSTRACT_METHOD_ADDED = new Message(7013);
+
     private ScopeSelector scopeSelector;
 
     /** {@inheritDoc} */
@@ -65,14 +81,6 @@
             return true;
         }
 
-        // The main problem here is to figure out which old method corresponds to which new method.
-
-        // Methods that are named differently are trated as unrelated
-        //
-        // For Methods that differ only in their parameter list we build a similarity table, i.e.
-        // for new method i and old method j we have number that charaterizes how similar
-        // the method signatures are (0 means equal, higher number means more different)
-
         Map bNameToMethod = buildNameToMethodMap(compatBaseline);
         Map cNameToMethod = buildNameToMethodMap(currentVersion);
 
@@ -156,11 +164,11 @@
             JavaClass currentVersion,
             List currentMethods)
     {
-        for(Iterator bIter = baselineMethods.iterator(); bIter.hasNext(); )
+        for (Iterator bIter = baselineMethods.iterator(); bIter.hasNext();)
         {
             Method bMethod = (Method) bIter.next();
 
-            for(Iterator cIter = currentMethods.iterator(); cIter.hasNext(); )
+            for (Iterator cIter = currentMethods.iterator(); cIter.hasNext();)
             {
                 Method cMethod = (Method) cIter.next();
 
@@ -229,6 +237,10 @@
         // ok, we now have to deal with the tricky cases, where it is not
         // immediately obvious which old methods correspond to which new
         // methods.
+        //
+        // Here we build a similarity table, i.e. for new method i and old
+        // method j we have number that charaterizes how similar the method
+        // signatures are (0 means equal, higher number means more different)
 
         while (!baselineMethods.isEmpty() && !currentMethods.isEmpty())
         {
@@ -372,6 +384,11 @@
             Method oldMethod,
             JavaClass currentClass)
     {
+        if (!scopeSelector.isSelected(oldMethod))
+        {
+            return;
+        }
+
         String methodSignature = getMethodId(oldClass, oldMethod);
         String superClassName = findSuperClassWithSignature(methodSignature, currentClass);
         String superInterfaceName = null;
@@ -382,24 +399,20 @@
 
         if (superClassName != null)
         {
-            fireDiff("Method '"
-                    + getMethodId(oldClass, oldMethod)
-                    + "' is now implemented in superclass " + superClassName,
-                    Severity.INFO, oldClass, oldMethod);
+            fireDiff(MSG_METHOD_NOW_IN_SUPERCLASS,
+                    Severity.INFO, oldClass, oldMethod,
+                    new String[] {superClassName});
         }
         else if (superInterfaceName != null)
         {
-            fireDiff("Abstract method '"
-                    + getMethodId(oldClass, oldMethod)
-                    + "' is now specified by implemented interface " + superInterfaceName,
-                    Severity.INFO, oldClass, oldMethod);
+            fireDiff(MSG_METHOD_NOW_IN_INTERFACE,
+                    Severity.INFO, oldClass, oldMethod,
+                    new String[] {superInterfaceName});
         }
         else
         {
-            fireDiff("Method '"
-                    + getMethodId(oldClass, oldMethod)
-                    + "' has been removed",
-                    Severity.ERROR, oldClass, oldMethod);
+            fireDiff(MSG_METHOD_REMOVED,
+                    Severity.ERROR, oldClass, oldMethod, null);
         }
     }
 
@@ -422,26 +435,25 @@
      */
     private void reportMethodAdded(JavaClass newClass, Method newMethod)
     {
+        if (!scopeSelector.isSelected(newMethod))
+        {
+            return;
+        }
+
         if (newClass.isInterface())
         {
-            fireDiff("Method '"
-                    + getMethodId(newClass, newMethod)
-                    + "' has been added to an interface",
-                    Severity.ERROR, newClass, newMethod);
+            fireDiff(MSG_METHOD_ADDED_TO_INTERFACE,
+                    Severity.ERROR, newClass, newMethod, null);
         }
         else if (newMethod.isAbstract())
         {
-            fireDiff("Abstract method '"
-                    + getMethodId(newClass, newMethod)
-                    + "' has been added",
-                    Severity.ERROR, newClass, newMethod);
+            fireDiff(MSG_ABSTRACT_METHOD_ADDED,
+                    Severity.ERROR, newClass, newMethod, null);
         }
         else
         {
-            fireDiff("Method '"
-                    + getMethodId(newClass, newMethod)
-                    + "' has been added",
-                    Severity.INFO, newClass, newMethod);
+            fireDiff(MSG_METHOD_ADDED,
+                    Severity.INFO, newClass, newMethod, null);
         }
     }
 
@@ -456,11 +468,6 @@
         {
             Method method = methods[i];
 
-            if (!scopeSelector.isSelected(method))
-            {
-                continue;
-            }
-
             final String name = method.getName();
             List set = (List) retVal.get(name);
             if (set == null)
@@ -475,10 +482,16 @@
 
     private void check(JavaClass compatBaseline, Method baselineMethod, Method currentMethod)
     {
+        if (!scopeSelector.isSelected(baselineMethod) && !scopeSelector.isSelected(currentMethod))
+        {
+            return;
+        }
+
         checkParameterTypes(compatBaseline, baselineMethod, currentMethod);
         checkReturnType(compatBaseline, baselineMethod, currentMethod);
         checkDeclaredExceptions(compatBaseline, baselineMethod, currentMethod);
         checkDeprecated(compatBaseline, baselineMethod, currentMethod);
+        checkVisibility(compatBaseline, baselineMethod, currentMethod);
     }
 
     private void checkParameterTypes(JavaClass compatBaseline, Method baselineMethod, Method currentMethod)
@@ -488,9 +501,8 @@
 
         if (bArgs.length != cArgs.length)
         {
-            fireDiff("In Method '" + getMethodId(compatBaseline, baselineMethod)
-                    + "' the number of arguments has changed",
-                    Severity.ERROR, compatBaseline, baselineMethod);
+            fireDiff(MSG_METHOD_ARGCOUNT_CHANGED,
+                    Severity.ERROR, compatBaseline, baselineMethod, null);
             return;
         }
 
@@ -506,9 +518,13 @@
             }
 
             // TODO: Check assignability...
-            fireDiff("Parameter " + (i + 1) + " of '" + getMethodId(compatBaseline, baselineMethod)
-                    + "' has changed it's type to " + cArg,
-                    Severity.ERROR, compatBaseline, baselineMethod);
+            String[] args =
+            {
+                "" + (i + 1),
+                cArg.toString()
+            };
+            fireDiff(MSG_METHOD_PARAMTYPE_CHANGED,
+                    Severity.ERROR, compatBaseline, baselineMethod, args);
         }
     }
 
@@ -520,9 +536,9 @@
         // TODO: Check assignability...
         if (!bReturnType.toString().equals(cReturnType.toString()))
         {
-            fireDiff("Return type of Method '" + getMethodId(compatBaseline, baselineMethod)
-                    + "' has been changed to " + cReturnType,
-                    Severity.ERROR, compatBaseline, baselineMethod);
+            fireDiff(MSG_METHOD_RETURNTYPE_CHANGED,
+                    Severity.ERROR, compatBaseline, baselineMethod,
+                    new String[] {cReturnType.toString()});
         }
 
 
@@ -541,15 +557,44 @@
 
         if (bIsDeprecated && !cIsDeprecated)
         {
-            fireDiff(
-                "Method '" + getMethodId(compatBaseline, baselineMethod) + "' is no longer deprecated",
-                    Severity.INFO, compatBaseline, baselineMethod);
+            fireDiff(MSG_METHOD_UNDEPRECATED,
+                    Severity.INFO, compatBaseline, baselineMethod, null);
         }
         else if (!bIsDeprecated && cIsDeprecated)
         {
-            fireDiff(
-                "Method '" + getMethodId(compatBaseline, baselineMethod) + "' has been deprecated",
-                    Severity.INFO, compatBaseline, baselineMethod);
+            fireDiff(MSG_METHOD_DEPRECATED,
+                    Severity.INFO, compatBaseline, baselineMethod, null);
+        }
+    }
+
+    /**
+     * Report changes in the declared accessability of a method
+     * (public/protected/etc).
+     */
+    private void checkVisibility(JavaClass compatBaseline, Method baselineMethod, Method currentMethod)
+    {
+        ScopeSelector.Scope bScope = ScopeSelector.getScope(baselineMethod);
+        ScopeSelector.Scope cScope = ScopeSelector.getScope(currentMethod);
+
+        if (cScope.isLessVisibleThan(bScope))
+        {
+            String[] args =
+            {
+                bScope.getDesc(),
+                cScope.getDesc()
+            };
+            fireDiff(MSG_METHOD_LESS_ACCESSABLE,
+                    Severity.ERROR, compatBaseline, baselineMethod, args);
+        }
+        else if (cScope.isMoreVisibleThan(bScope))
+        {
+            String[] args =
+            {
+                bScope.getDesc(),
+                cScope.getDesc()
+            };
+            fireDiff(MSG_METHOD_MORE_ACCESSABLE,
+                    Severity.INFO, compatBaseline, baselineMethod, args);
         }
     }
 
@@ -602,12 +647,12 @@
         }
     }
 
-    private void fireDiff(String report, Severity severity, JavaClass clazz, Method method)
+    private void fireDiff(Message msg, Severity severity, JavaClass clazz, Method method, String[] args)
     {
         final String className = clazz.getClassName();
         final ApiDifference diff =
-                new ApiDifference(report + " in " + className,
-                        severity, className, getMethodId(clazz, method), null);
+                new ApiDifference(
+                    msg, severity, className, getMethodId(clazz, method), null, args);
         getApiDiffDispatcher().fireDiff(diff);
 
     }

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/event ApiDifference.java,1.11,1.12

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/java/net/sf/clirr/event
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv3709/src/java/net/sf/clirr/event

Modified Files:
	ApiDifference.java 
Log Message:
Changes to support new Message framework (all backwards-compatible).


Index: ApiDifference.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/event/ApiDifference.java,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -r1.11 -r1.12
--- ApiDifference.java	27 Jun 2004 14:21:35 -0000	1.11
+++ ApiDifference.java	28 Jun 2004 06:53:00 -0000	1.12
@@ -29,6 +29,12 @@
 {
     private static final int HASHCODE_MAGIC = 29;
 
+    /**
+     * Object representing the message text to be output (or null if
+     * the constructor which takes a message string directly is used).
+     */
+    private Message message = null;
+
     /** human readable change report. */
     private String report;
 
@@ -118,6 +124,78 @@
         this.affectedMethod = method;
     }
 
+    /**
+     * Invokes the two-severity-level version of this constructor.
+     */
+    public ApiDifference(
+        Message message,
+        Severity severity,
+        String clazz,
+        String method,
+        String field,
+        String[] args)
+    {
+        this(message, severity, severity, clazz, method, field, args);
+    }
+
+    /**
+     * Create a new API difference representation.
+     *
+     * @param message is the key of a human readable string describing the
+     * change that was made.
+     *
+     * @param binarySeverity the severity in terms of binary compatibility,
+     * must be non-null.
+     *
+     * @param sourceSeverity the severity in terms of source code compatibility,
+     * must be non-null.
+     *
+     * @param clazz is the fully-qualified name of the class in which the
+     * change occurred.
+     *
+     * @param method the method signature of the method that changed,
+     * <code>null</code> if no method was affected.
+     *
+     * @param field the field name where the change occured, <code>null</code>
+     * if no field was affected.
+     *
+     * @param args is a set of additional change-specific strings which are
+     * made available for the message description string to reference via
+     * the standard {n} syntax.
+     */
+    public ApiDifference(
+        Message message,
+        Severity binarySeverity,
+        Severity sourceSeverity,
+        String clazz,
+        String method,
+        String field,
+        String[] args)
+    {
+        String desc = MessageManager.getInstance().getDesc(message);
+        int nArgs = 0;
+        if (args != null)
+        {
+            nArgs = args.length;
+        }
+        String[] strings = new String[nArgs + 3];
+        strings[0] = clazz;
+        strings[1] = method;
+        strings[2] = field;
+        for (int i = 0; i < nArgs; ++i)
+        {
+            strings[i + 3] = args[i];
+        }
+
+        this.message = message;
+        this.report = java.text.MessageFormat.format(desc, strings);
+        this.binaryCompatibilitySeverity = binarySeverity;
+        this.sourceCompatibilitySeverity = sourceSeverity;
+        this.affectedClass = clazz;
+        this.affectedField = field;
+        this.affectedMethod = method;
+    }
+
     private void checkNonNull(Object o)
     {
         if (o == null)
@@ -156,6 +234,9 @@
         return sourceCompatibilitySeverity;
     }
 
+    /**
+     * Return the maximum of the binary and source compatibility severities.
+     */
     public Severity getMaximumSeverity()
     {
         final Severity src = getSourceCompatibilitySeverity();

[Clirr-devel] CVS: clirr/src/conf event-messages.properties,1.1,1.2

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/conf
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv3580/src/conf

Modified Files:
	event-messages.properties 
Log Message:
New messages added for MethodSetCheck


Index: event-messages.properties
===================================================================
RCS file: /cvsroot/clirr/clirr/src/conf/event-messages.properties,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- event-messages.properties	28 Jun 2004 05:06:04 -0000	1.1
+++ event-messages.properties	28 Jun 2004 06:52:03 -0000	1.2
@@ -8,7 +8,7 @@
 # MethodSetCheck messages
 m7000=Method ''{1}'' is now implemented in superclass {3} in {0}
 m7001=Abstract method ''{1}'' is now specified by implemented interface {3} in {0}
-m7002=Method ''{1}'' has been added in {0}
+#m7002 no longer used
 m7003=Method ''{1}'' has been removed in {0}
 m7004=In method ''{1}'' the number of arguments has changed in {0}
 m7005=Parameter {3} of ''{1}'' has changed its type to {4} in {0}
@@ -17,4 +17,7 @@
 m7008=Method ''{1}'' is no longer deprecated in {0}
 m7009=Accessability of method ''{1}'' has been decreased from {3} to {4} in {0}
 m7010=Accessability of method ''{1}'' has been increased from {3} to {4} in {0}
+m7011=Method ''{1}'' has been added in {0}
+m7012=Method ''{1}'' has been added to an interface in {0}
+m7013=Abstract method ''{1}'' has been added in {0}
 

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/event Message.java,NONE,1.1 MessageManager.java,NONE,1.1

[Simon K. <s_k...@us...>]

Update of /cvsroot/clirr/clirr/src/java/net/sf/clirr/event
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv3450/src/java/net/sf/clirr/event

Added Files:
	Message.java MessageManager.java 
Log Message:
Message translation framework


--- NEW FILE ---
//////////////////////////////////////////////////////////////////////////////
// Clirr: compares two versions of a java library for binary compatibility
// Copyright (C) 2003 - 2004  Lars Kühne
//
// This library is free software; you can redistribute it and/or
// modify it under the terms of the GNU Lesser General Public
// License as published by the Free Software Foundation; either
// version 2.1 of the License, or (at your option) any later version.
//
// This library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
// Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public
// License along with this library; if not, write to the Free Software
// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
//////////////////////////////////////////////////////////////////////////////

package net.sf.clirr.event;

/**
 * Class which manages API Difference messages, including expanding message
 * codes into strings and descriptions.
 */
public final class Message
{
    private int id;

    public Message(int id)
    {
        this.id = id;
        MessageManager.getInstance().addMessage(this);
    }

    public int getId()
    {
        return id;
    }
}

--- NEW FILE ---
//////////////////////////////////////////////////////////////////////////////
// Clirr: compares two versions of a java library for binary compatibility
// Copyright (C) 2003 - 2004  Lars Kühne
//
// This library is free software; you can redistribute it and/or
// modify it under the terms of the GNU Lesser General Public
// License as published by the Free Software Foundation; either
// version 2.1 of the License, or (at your option) any later version.
//
// This library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
// Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public
// License along with this library; if not, write to the Free Software
// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
//////////////////////////////////////////////////////////////////////////////

package net.sf.clirr.event;

import java.util.Locale;
import java.util.Iterator;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.ResourceBundle;

/**
 * Class which manages API Difference messages, including expanding message
 * codes into strings and descriptions.
 */
public final class MessageManager
{
    /**
     * The base name of the resource bundle from which message descriptions
     * are read.
     */
    public static final String RESOURCE_NAME = "event-messages";

    private static MessageManager instance;
    private ArrayList messages = new ArrayList();
    private Locale locale;
    private ResourceBundle messageText;

    /**
     * Utility class to sort messages by their numeric ids.
     */
    private static class MessageComparator implements Comparator
    {
        public int compare(Object o1, Object o2)
        {
            Message m1 = (Message) o1;
            Message m2 = (Message) o2;
            return m1.getId() - m2.getId();
        }
    }

    /**
     * This is a singleton class; to get an instance of this class, use
     * the getInstance method.
     */
    private MessageManager()
    {
    }

    /**
     * Return the singleton instance of this class.
     */
    public static MessageManager getInstance()
    {
        if (instance == null)
        {
            instance = new MessageManager();
        }
        return instance;
    }

    /**
     * Add a message to the list of known messages.
     */
    public void addMessage(Message msg)
    {
        messages.add(msg);
    }

    /**
     * Define the local language etc. Future calls to the getDesc method
     * will attempt to use a properties file which is appropriate to that
     * locale to look the message descriptions up in.
     * <p>
     * @param locale may be a valid Locale object, or null to indicate
     * that the default locale is to be used.
     */
    public void setLocale(Locale locale)
    {
        if (locale == null)
        {
            locale = Locale.getDefault();
        }
        this.locale = locale;
        this.messageText = null;
    }

    /**
     * Verify that the list of known messages contains no two objects
     * with the same numeric message id. This method is expected to be
     * called from the unit tests, so that if a developer adds a new
     * message and accidentally uses the message id of an existing
     * message object, then this will be reported as an error.
     * <p>
     * @throws IllegalArgumentException if any duplicate id is found.
     */
    public void checkUnique()
    {
        java.util.Collections.sort(messages, new MessageComparator());
        int lastId = -1;
        for (Iterator i = messages.iterator(); i.hasNext();)
        {
            // check for any duplicates
            Message m = (Message) i.next();
            int currId = m.getId();
            if (currId <= lastId)
            {
                throw new IllegalArgumentException(
                    "Message id [" + currId + "] is not unique.");
            }
        }
    }

    /**
     * Verify that the resource bundle for the currently set locale has
     * a translation string available for every registered message object.
     * This method is expected to be called from the unit tests, so that
     * if a developer adds a new message the unit tests will fail until
     * translations are also available for that new message.
     * <p>
     * @throws java.util.MissingResourceException if there is a registered
     * message for which no description is present in the current locale's
     * resources.
     */
    public void checkComplete()
    {
        java.util.Collections.sort(messages, new MessageComparator());
        for (Iterator i = messages.iterator(); i.hasNext();)
        {
            Message m = (Message) i.next();
            getDesc(m);
        }
    }

    /**
     * Given a Message object (containing a unique message id), look up
     * that id in the appropriate resource bundle (properties file) for
     * the set locale and return the text string associated with that
     * message id.
     * <p>
     * Message ids in the properties file should be prefixed with an 'm',
     * eg "m1000", "m5003".
     * <p>
     * @throws MissingResourceException if there is no entry in the
     * message translation resource bundle for the specified message.
     */
    public String getDesc(Message msg)
    {
        // load resource bundle
        if (locale == null)
        {
            locale = Locale.getDefault();
        }

        if (messageText == null)
        {
            messageText = ResourceBundle.getBundle(RESOURCE_NAME, locale);
        }

        return messageText.getString("m" + msg.getId());
    }
}