clirr-devel — Discussion about the source code of Clirr

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/cli Clirr.java,1.3,1.4

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

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

Modified Files:
	Clirr.java 
Log Message:
Add "--include-pkg" option, to specify that only certain java packages
are to be checked.


Index: Clirr.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/cli/Clirr.java,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- Clirr.java	18 Jun 2004 07:35:22 -0000	1.3
+++ Clirr.java	21 Jun 2004 08:48:13 -0000	1.4
@@ -61,12 +61,14 @@
 
     private void run(String[] args)
     {
-        System.out.println("reporting diffs..");
         BasicParser parser = new BasicParser();
         Options options = new Options();
-        options.addOption("o", "oldversion", true, "jar files of old version");
-        options.addOption("n", "newversion", true, "jar files of new version");
-        options.addOption("s", "style", true, "output style");
+        options.addOption("o", "old-version", true, "jar files of old version");
+        options.addOption("n", "new-version", true, "jar files of new version");
+        options.addOption("s", "style", true, "output style [text|xml]");
+        options.addOption("i", "include-pkg", true,
+            "include only classes from this package and its subpackages");
+        options.addOption("p", "show-pkg-scope", false, "show package scope classes");
         options.addOption("a", "show-all-scopes", false, "show private and package classes");
         options.addOption("f", "output-file", true, "output file name");
 
@@ -82,16 +84,13 @@
             System.exit(-1);
         }
 
-        // TODO: provide commandline options that allow the user to
-        // specify which packages/classes should be checked
-        //
-        ClassSelector classSelector = new ClassSelector(ClassSelector.MODE_UNLESS);
-
         String oldPath = cmdline.getOptionValue('o');
         String newPath = cmdline.getOptionValue('n');
         String style = cmdline.getOptionValue('s', "text");
         String outputFileName = cmdline.getOptionValue('f');
+        String[] includePkgs = cmdline.getOptionValues('i');
         boolean showAll = cmdline.hasOption('a');
+        boolean showPkg = cmdline.hasOption('p');
 
         if ((oldPath == null) || (newPath == null))
         {
@@ -104,6 +103,25 @@
         {
             checker.getScopeSelector().setScope(ScopeSelector.SCOPE_PRIVATE);
         }
+        else if (showPkg)
+        {
+            checker.getScopeSelector().setScope(ScopeSelector.SCOPE_PACKAGE);
+        }
+
+        ClassSelector classSelector;
+        if ((includePkgs != null) && (includePkgs.length > 0))
+        {
+            classSelector = new ClassSelector(ClassSelector.MODE_IF);
+            for (int i = 0; i < includePkgs.length; ++i)
+            {
+                classSelector.addPackageTree(includePkgs[i]);
+            }
+        }
+        else
+        {
+            // a selector that selects everything
+            classSelector = new ClassSelector(ClassSelector.MODE_UNLESS);
+        }
 
         DiffListener diffListener = null;
         if (style.equals("text"))

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/checks GenderChangeCheck.java,1.8,1.9

[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-serv21812

Modified Files:
	GenderChangeCheck.java 
Log Message:
Removed methods that are never called.


Index: GenderChangeCheck.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/checks/GenderChangeCheck.java,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -r1.8 -r1.9
--- GenderChangeCheck.java	18 Jun 2004 06:52:10 -0000	1.8
+++ GenderChangeCheck.java	21 Jun 2004 08:31:56 -0000	1.9
@@ -64,42 +64,4 @@
 
         return true;
     }
-
-    // TODO: This should be a method in BCEL's ClassSet !!!
-    private JavaClass findClass(String className, ClassSet classSet)
-    {
-        JavaClass[] classes = classSet.toArray();
-        for (int i = 0; i < classes.length; i++)
-        {
-            JavaClass clazz = classes[i];
-            if (clazz.getClassName().equals(className))
-            {
-                return clazz;
-            }
-        }
-        throw new IllegalStateException();
-    }
-
-    private String[] intersectionClassNames(ClassSet setA, ClassSet setB)
-    {
-        String[] aNames = setA.getClassNames();
-        String[] bNames = setB.getClassNames();
-        Arrays.sort(aNames);
-        Arrays.sort(bNames);
-
-        List helper = new ArrayList();
-
-        for (int i = 0; i < aNames.length; i++)
-        {
-            String aName = aNames[i];
-            if (Arrays.binarySearch(bNames, aName) >= 0)
-            {
-                helper.add(aName);
-            }
-        }
-
-        String[] retVal = new String[helper.size()];
-        helper.toArray(retVal);
-        return retVal;
-    }
 }

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

[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-serv14085

Modified Files:
	ClassModifierCheck.java 
Log Message:
Implemented TODO item: it is not an error to add final to a class if that
class previously had no public or protected constructors.


Index: ClassModifierCheck.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/checks/ClassModifierCheck.java,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -r1.9 -r1.10
--- ClassModifierCheck.java	18 Jun 2004 06:52:10 -0000	1.9
+++ ClassModifierCheck.java	21 Jun 2004 07:26:58 -0000	1.10
@@ -23,7 +23,11 @@
 import net.sf.clirr.framework.AbstractDiffReporter;
 import net.sf.clirr.framework.ApiDiffDispatcher;
 import net.sf.clirr.framework.ClassChangeCheck;
+import net.sf.clirr.framework.CheckerException;
+import net.sf.clirr.event.ScopeSelector;
+
 import org.apache.bcel.classfile.JavaClass;
+import org.apache.bcel.classfile.Method;
 
 /**
  * Detects changes in class modifiers (abstract, final).
@@ -46,6 +50,25 @@
     /** {@inheritDoc} */
     public boolean check(JavaClass compatBaseLine, JavaClass currentVersion)
     {
+        final String className = compatBaseLine.getClassName();
+
+        try
+        {
+            ScopeSelector.Scope currentScope = ScopeSelector.getClassScope(currentVersion);
+            if (currentScope == ScopeSelector.SCOPE_PRIVATE)
+            {
+                // for private classes, we don't care if they are now final,
+                // or now abstract, or now an interface.
+                return true;
+            }
+        }
+        catch (CheckerException ex)
+        {
+            log("Unable to determine whether class is private",
+                    Severity.ERROR, className, null, null);
+            return true;
+        }
+
         final boolean currentIsFinal = currentVersion.isFinal();
         final boolean compatIsFinal = compatBaseLine.isFinal();
         final boolean currentIsAbstract = currentVersion.isAbstract();
@@ -53,13 +76,6 @@
         final boolean currentIsInterface = currentVersion.isInterface();
         final boolean compatIsInterface = compatBaseLine.isInterface();
 
-        final String className = compatBaseLine.getClassName();
-
-        // TODO: There are cases when nonfinal classes are effectively final
-        // because they do not have public or protected ctors. For such
-        // classes we should not emit errors when a final modifier is
-        // introduced.
-
         if (compatIsFinal && !currentIsFinal)
         {
             log("Removed final modifier in class " + className,
@@ -67,8 +83,17 @@
         }
         else if (!compatIsFinal && currentIsFinal)
         {
-            log("Added final modifier in class " + className,
-                    Severity.ERROR, className, null, null);
+            if (isEffectivelyFinal(compatBaseLine))
+            {
+                log("Added final modifier in class " + className
+                        + " (but class was effectively final anyway)",
+                        Severity.INFO, className, null, null);
+                }
+            else
+            {
+                log("Added final modifier in class " + className,
+                        Severity.ERROR, className, null, null);
+            }
         }
 
         // interfaces are always abstract, don't report gender change here
@@ -86,4 +111,36 @@
         return true;
     }
 
+    /**
+     * There are cases where nonfinal classes are effectively final
+     * because they do not have public or protected ctors. For such
+     * classes we should not emit errors when a final modifier is
+     * introduced.
+     */
+    private boolean isEffectivelyFinal(JavaClass clazz)
+    {
+        if (clazz.isFinal())
+        {
+            return true;
+        }
+
+        // iterate over all constructors, and detect whether any are
+        // public or protected. If so, return false.
+        Method[] methods = clazz.getMethods();
+        for (int i = 0; i < methods.length; ++i)
+        {
+            Method method = methods[i];
+            final String methodName = method.getName();
+            if (methodName.equals("<init>"))
+            {
+                if (method.isPublic() || method.isProtected())
+                {
+                    return false;
+                }
+            }
+        }
+
+        // no public or protected constructor found
+        return true;
+    }
 }

[Clirr-devel] CVS: clirr/src/test/net/sf/clirr/checks ClassModifierCheckTest.java,NONE,1.1

[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-serv12981

Added Files:
	ClassModifierCheckTest.java 
Log Message:
Add tests for ClassModifierCheck.


--- NEW FILE ---
package net.sf.clirr.checks;

import net.sf.clirr.framework.ClassChangeCheck;
import net.sf.clirr.event.ApiDifference;
import net.sf.clirr.event.Severity;
import net.sf.clirr.event.ScopeSelector;
import net.sf.clirr.framework.ClassSelector;

/**
 * Tests for the ClassModifierCheck class.
 */
public class ClassModifierCheckTest extends AbstractCheckTestCase
{
    public void testAll()
    {
        ApiDifference[] expected = new ApiDifference[] {
            new ApiDifference("Added final modifier in class testlib.modifiers.NonFinalBecomesFinal", Severity.ERROR, "testlib.modifiers.NonFinalBecomesFinal", null, null),
            new ApiDifference("Added final modifier in class testlib.modifiers.EffectivelyFinal (but class was effectively final anyway)", Severity.INFO, "testlib.modifiers.EffectivelyFinal", null, null),
        };
        verify(expected);
    }

    protected ClassChangeCheck createCheck(TestDiffListener tdl)
    {
        return new ClassModifierCheck(tdl);
    }

    protected ClassSelector createClassSelector()
    {
        // only apply the check to classes in the testlib.modifiers package.
        ClassSelector classSelector = new ClassSelector(ClassSelector.MODE_IF);
        classSelector.addPackage("testlib.modifiers");
        return classSelector;
    }
}

[Clirr-devel] CVS: clirr/src/test/net/sf/clirr/checks ClassModifierCheck.java,1.1,NONE

[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-serv12081

Removed Files:
	ClassModifierCheck.java 
Log Message:
Oops - committed wrong file.

[Clirr-devel] CVS: clirr/src/test/net/sf/clirr/checks ClassModifierCheck.java,NONE,1.1

[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-serv10004

Added Files:
	ClassModifierCheck.java 
Log Message:
ClassModifierCheck tests.


--- 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.checks;

import net.sf.clirr.event.Severity;
import net.sf.clirr.framework.AbstractDiffReporter;
import net.sf.clirr.framework.ApiDiffDispatcher;
import net.sf.clirr.framework.ClassChangeCheck;
import net.sf.clirr.framework.CheckerException;
import net.sf.clirr.event.ScopeSelector;

import org.apache.bcel.classfile.JavaClass;
import org.apache.bcel.classfile.Method;

/**
 * Detects changes in class modifiers (abstract, final).
 *
 * @author lkuehne
 */
public final class ClassModifierCheck
        extends AbstractDiffReporter
        implements ClassChangeCheck
{
    /**
     * Create a new instance of this check.
     * @param dispatcher the diff dispatcher that distributes the detected changes to the listeners.
     */
    public ClassModifierCheck(ApiDiffDispatcher dispatcher)
    {
        super(dispatcher);
    }

    /** {@inheritDoc} */
    public boolean check(JavaClass compatBaseLine, JavaClass currentVersion)
    {
        final String className = compatBaseLine.getClassName();

        try
        {
            ScopeSelector.Scope currentScope = ScopeSelector.getClassScope(currentVersion);
            if (currentScope == ScopeSelector.SCOPE_PRIVATE)
            {
                // for private classes, we don't care if they are now final,
                // or now abstract, or now an interface.
                return true;
            }
        }
        catch (CheckerException ex)
        {
            log("Unable to determine whether class is private", 
                    Severity.ERROR, className, null, null);
            return true;
        }
        
        final boolean currentIsFinal = currentVersion.isFinal();
        final boolean compatIsFinal = compatBaseLine.isFinal();
        final boolean currentIsAbstract = currentVersion.isAbstract();
        final boolean compatIsAbstract = compatBaseLine.isAbstract();
        final boolean currentIsInterface = currentVersion.isInterface();
        final boolean compatIsInterface = compatBaseLine.isInterface();

        if (compatIsFinal && !currentIsFinal)
        {
            log("Removed final modifier in class " + className,
                    Severity.INFO, className, null, null);
        }
        else if (!compatIsFinal && currentIsFinal)
        {
            if (isEffectivelyFinal(compatBaseLine))
            {
                log("Added final modifier in class " + className
                        + " (but class was effectively final anyway)",
                        Severity.INFO, className, null, null);
                }
            else
            {
                log("Added final modifier in class " + className,
                        Severity.ERROR, className, null, null);
            }
        }

        // interfaces are always abstract, don't report gender change here
        if (compatIsAbstract && !currentIsAbstract && !compatIsInterface)
        {
            log("Removed abstract modifier in class " + className,
                    Severity.INFO, className, null, null);
        }
        else if (!compatIsAbstract && currentIsAbstract && !currentIsInterface)
        {
            log("Added abstract modifier in class " + className,
                    Severity.ERROR, className, null, null);
        }

        return true;
    }
    
    /**
     * There are cases where nonfinal classes are effectively final
     * because they do not have public or protected ctors. For such
     * classes we should not emit errors when a final modifier is
     * introduced.
     */
    private boolean isEffectivelyFinal(JavaClass clazz)
    {
        if (clazz.isFinal())
        {
            return true;
        }

        // iterate over all constructors, and detect whether any are
        // public or protected. If so, return false.
        Method[] methods = clazz.getMethods();
        for(int i=0; i<methods.length; ++i)
        {
            Method method = methods[i];
            final String methodName = method.getName();
            if (methodName.equals("<init>"))
            {
                if (method.isPublic() || method.isProtected())
                {
                    return false;
                }
            }
        }
        
        // no public or protected constructor found
        return true;
    }
}

[Clirr-devel] CVS: clirr/src/testinput/testlib-v1/testlib/modifiers EffectivelyFinal.java,NONE,1.1 NonFinalBecomesFinal.java,NONE,1.1

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

Update of /cvsroot/clirr/clirr/src/testinput/testlib-v1/testlib/modifiers
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv9524

Added Files:
	EffectivelyFinal.java NonFinalBecomesFinal.java 
Log Message:
tests for ClassModifierCheck


--- NEW FILE ---
package testlib.modifiers;

/**
 * Because this class has a package-scope constructor, no subclass of this
 * class can ever be created. And because of that, it is not a problem if
 * this class is declared "final" in a later version.
 * <p>
 * Classes with only private constructors are commonly used to implement
 * an "enumerated type" in java, as is done here.
 */

public class EffectivelyFinal
{
    int val;
    
    public static final EffectivelyFinal ZERO = new EffectivelyFinal(0);
    public static final EffectivelyFinal ONE = new EffectivelyFinal(1);
    
    private EffectivelyFinal(int i) 
    {
        val = i;
    }
    
    public int getValue() 
    {
        return val;
    }
}

--- NEW FILE ---
package testlib.modifiers;

/**
 * It is a binary compatibility error for a non-final class to become
 * final in a later version, because users may have created classes which
 * are derived from it. Such classes will fail to load with the new version
 * of the class present.
 */

public class NonFinalBecomesFinal
{
}

[Clirr-devel] CVS: clirr/src/testinput/testlib-v1/testlib/modifiers - New directory

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

Update of /cvsroot/clirr/clirr/src/testinput/testlib-v1/testlib/modifiers
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv9267/modifiers

Log Message:
Directory /cvsroot/clirr/clirr/src/testinput/testlib-v1/testlib/modifiers added to the repository

[Clirr-devel] CVS: clirr/src/testinput/testlib-v2/testlib/modifiers EffectivelyFinal.java,NONE,1.1 NonFinalBecomesFinal.java,NONE,1.1

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

Update of /cvsroot/clirr/clirr/src/testinput/testlib-v2/testlib/modifiers
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv8917

Added Files:
	EffectivelyFinal.java NonFinalBecomesFinal.java 
Log Message:
tests for ClassModifiersCheck


--- NEW FILE ---
package testlib.modifiers;

/**
 * Because this class has a package-scope constructor, no subclass of this
 * class can ever be created. And because of that, it is not a problem if
 * this class is declared "final" in a later version.
 * <p>
 * Classes with only private constructors are commonly used to implement
 * an "enumerated type" in java, as is done here.
 */

public final class EffectivelyFinal
{
    int val;
    
    public static final EffectivelyFinal ZERO = new EffectivelyFinal(0);
    public static final EffectivelyFinal ONE = new EffectivelyFinal(1);
    
    private EffectivelyFinal(int i) 
    {
        val = i;
    }
    
    public int getValue() 
    {
        return val;
    }
}

--- NEW FILE ---
package testlib.modifiers;

/**
 * It is a binary compatibility error for a non-final class to become
 * final in a later version, because users may have created classes which
 * are derived from it. Such classes will fail to load with the new version
 * of the class present.
 */

public final class NonFinalBecomesFinal
{
}

[Clirr-devel] CVS: clirr/src/testinput/testlib-v2/testlib/modifiers - New directory

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

Update of /cvsroot/clirr/clirr/src/testinput/testlib-v2/testlib/modifiers
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv8753/modifiers

Log Message:
Directory /cvsroot/clirr/clirr/src/testinput/testlib-v2/testlib/modifiers added to the repository

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/checks ClassHierarchyCheck.java,1.10,1.11

[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-serv5208

Modified Files:
	ClassHierarchyCheck.java 
Log Message:
Use CoIterator.


Index: ClassHierarchyCheck.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/checks/ClassHierarchyCheck.java,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -r1.10 -r1.11
--- ClassHierarchyCheck.java	18 Jun 2004 06:52:10 -0000	1.10
+++ ClassHierarchyCheck.java	21 Jun 2004 07:09:50 -0000	1.11
@@ -26,6 +26,8 @@
 import net.sf.clirr.framework.AbstractDiffReporter;
 import net.sf.clirr.framework.ApiDiffDispatcher;
 import net.sf.clirr.framework.ClassChangeCheck;
+import net.sf.clirr.framework.CoIterator;
+import net.sf.clirr.framework.JavaClassNameComparator;
 import org.apache.bcel.classfile.JavaClass;
 
 /**
@@ -71,35 +73,42 @@
     /** {@inheritDoc} */
     public boolean check(JavaClass compatBaseline, JavaClass currentVersion)
     {
-        JavaClass[] compatSuper = compatBaseline.getSuperClasses();
-        JavaClass[] currentSuper = currentVersion.getSuperClasses();
+        JavaClass[] compatSupers = compatBaseline.getSuperClasses();
+        JavaClass[] currentSupers = currentVersion.getSuperClasses();
 
         boolean isThrowable = false;
-        for (int i = 0; i < compatSuper.length; i++)
+        for (int i = 0; i < compatSupers.length; i++)
         {
-            JavaClass javaClass = compatSuper[i];
+            JavaClass javaClass = compatSupers[i];
             if ("java.lang.Throwable".equals(javaClass.getClassName()))
             {
                 isThrowable = true;
             }
         }
 
-        List added = getSetDifference(currentSuper, compatSuper);
-        List removed = getSetDifference(compatSuper, currentSuper);
-
         final String className = compatBaseline.getClassName();
-        for (int i = 0; i < added.size(); i++)
-        {
-            String s = (String) added.get(i);
-            log("Added " + s + " to the list of superclasses of " + className,
-                    isThrowable ? Severity.WARNING : Severity.INFO, className, null, null);
-        }
 
-        for (int i = 0; i < removed.size(); i++)
+        CoIterator iter = new CoIterator(
+            JavaClassNameComparator.COMPARATOR, compatSupers, currentSupers);
+
+        while (iter.hasNext())
         {
-            String s = (String) removed.get(i);
-            log("Removed " + s + " from the list of superclasses of " + className,
-                    Severity.ERROR, className, null, null);
+            iter.next();
+            JavaClass baselineSuper = (JavaClass) iter.getLeft();
+            JavaClass currentSuper = (JavaClass) iter.getRight();
+
+            if (baselineSuper == null)
+            {
+                log("Added " + currentSuper.getClassName()
+                    + " to the list of superclasses of " + className,
+                        isThrowable ? Severity.WARNING : Severity.INFO, className, null, null);
+            }
+            else if (currentSuper == null)
+            {
+                log("Removed " +  baselineSuper.getClassName()
+                    + " from the list of superclasses of " + className,
+                        Severity.ERROR, className, null, null);
+            }
         }
 
         return true;

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

[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-serv789

Modified Files:
	FieldSetCheck.java 
Log Message:
Checkstyle stuff.


Index: FieldSetCheck.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/checks/FieldSetCheck.java,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -r1.11 -r1.12
--- FieldSetCheck.java	21 Jun 2004 06:24:28 -0000	1.11
+++ FieldSetCheck.java	21 Jun 2004 07:00:59 -0000	1.12
@@ -19,7 +19,6 @@
 
 package net.sf.clirr.checks;
 
-import java.util.Arrays;
 import java.util.Comparator;
 
 import net.sf.clirr.framework.ClassChangeCheck;
@@ -79,10 +78,10 @@
 
             Field bField = (Field) iter.getLeft();
             Field cField = (Field) iter.getRight();
-            
+
             if (bField == null)
             {
-                if (scopeSelector.isSelected(cField)) 
+                if (scopeSelector.isSelected(cField))
                 {
                     final String name = cField.getName();
                     String scope = ScopeSelector.getScopeDesc(cField);
@@ -91,7 +90,7 @@
             }
             else if (cField == null)
             {
-                if (scopeSelector.isSelected(bField)) 
+                if (scopeSelector.isSelected(bField))
                 {
                     final String name = bField.getName();
                     fireDiff("Field " + name + " has been removed", Severity.ERROR, baselineClass, bField);
@@ -105,7 +104,7 @@
                 checkForConstantValueChange(bField, cField,  currentClass);
             }
         }
-        
+
         return true;
     }
 

[Clirr-devel] CVS: clirr/src/test/net/sf/clirr/checks FieldSetCheckTest.java,1.5,1.6

[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-serv31527

Modified Files:
	FieldSetCheckTest.java 
Log Message:
Changes due to FieldSetCheck change; a field changed from private to public
is now reported as "accessability changed" rather than "field added".


Index: FieldSetCheckTest.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/test/net/sf/clirr/checks/FieldSetCheckTest.java,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- FieldSetCheckTest.java	5 Jun 2004 16:19:07 -0000	1.5
+++ FieldSetCheckTest.java	21 Jun 2004 06:59:36 -0000	1.6
@@ -18,7 +18,7 @@
             new ApiDifference("Field stat7 has been removed in testlib.MembersChange", Severity.ERROR, "testlib.MembersChange", null, "stat7"),
             new ApiDifference("Accessibility of field fin2 has been weakened from public to protected in testlib.MembersChange", Severity.ERROR, "testlib.MembersChange", null, "fin2"),
             new ApiDifference("Accessibility of field stat4 has been weakened from public to protected in testlib.MembersChange", Severity.ERROR, "testlib.MembersChange", null, "stat4"),
-            new ApiDifference("Added public field priv2 in testlib.MembersChange", Severity.INFO, "testlib.MembersChange", null, "priv2"),
+            new ApiDifference("Accessability of field priv2 has been increased from private to public in testlib.MembersChange", Severity.INFO, "testlib.MembersChange", null, "priv2"),
             new ApiDifference("Accessibility of field stat5 has been weakened from public to private in testlib.MembersChange", Severity.ERROR, "testlib.MembersChange", null, "stat5"),
             new ApiDifference("Field stat2 is now final in testlib.MembersChange", Severity.ERROR, "testlib.MembersChange", null, "stat2"),
             new ApiDifference("Field pub3 is now final in testlib.MembersChange", Severity.ERROR, "testlib.MembersChange", null, "pub3"),
@@ -30,8 +30,8 @@
             new ApiDifference("Field pub2 is now static in testlib.MembersChange", Severity.ERROR, "testlib.MembersChange", null, "pub2"),
             new ApiDifference("Changed type of field obj1 from java.lang.Object to java.lang.String in testlib.MembersChange", Severity.ERROR, "testlib.MembersChange", null, "obj1"),
             new ApiDifference("Changed type of field obj2 from java.lang.Boolean to java.lang.String in testlib.MembersChange", Severity.ERROR, "testlib.MembersChange", null, "obj2"),
-            new ApiDifference("Value of compile time constant fin5 has been changed in testlib.MembersChange", Severity.WARNING, "testlib.MembersChange", null, "fin5"),
             new ApiDifference("Value of fin6 is no longer a compile time constant in testlib.MembersChange", Severity.WARNING, "testlib.MembersChange", null, "fin6"),
+            new ApiDifference("Value of compile time constant fin5 has been changed in testlib.MembersChange", Severity.WARNING, "testlib.MembersChange", null, "fin5"),
         };
         verify(expected);
     }

[Clirr-devel] CVS: clirr/src/java/net/sf/clirr/checks FieldSetCheck.java,1.10,1.11

[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-serv5492/src/java/net/sf/clirr/checks

Modified Files:
	FieldSetCheck.java 
Log Message:
* now uses CoIterator
* when a field was private and now is public, the report now says
  "Accessability of field X increased from private to public"
  instead of "Added field X".


Index: FieldSetCheck.java
===================================================================
RCS file: /cvsroot/clirr/clirr/src/java/net/sf/clirr/checks/FieldSetCheck.java,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -r1.10 -r1.11
--- FieldSetCheck.java	18 Jun 2004 07:36:06 -0000	1.10
+++ FieldSetCheck.java	21 Jun 2004 06:24:28 -0000	1.11
@@ -25,6 +25,7 @@
 import net.sf.clirr.framework.ClassChangeCheck;
 import net.sf.clirr.framework.AbstractDiffReporter;
 import net.sf.clirr.framework.ApiDiffDispatcher;
+import net.sf.clirr.framework.CoIterator;
 import net.sf.clirr.event.ApiDifference;
 import net.sf.clirr.event.Severity;
 import net.sf.clirr.event.ScopeSelector;
@@ -55,7 +56,7 @@
         }
     }
 
-    private Comparator comparator = new FieldNameComparator();
+    private static final Comparator COMPARATOR = new FieldNameComparator();
     private ScopeSelector scopeSelector;
 
     public FieldSetCheck(ApiDiffDispatcher dispatcher, ScopeSelector scopeSelector)
@@ -64,64 +65,48 @@
         this.scopeSelector = scopeSelector;
     }
 
-    public final boolean check(JavaClass compatBaseline, JavaClass currentVersion)
+    public final boolean check(JavaClass baselineClass, JavaClass currentClass)
     {
-        final Field[] baselineFields = compatBaseline.getFields();
-        final Field[] currentFields = currentVersion.getFields();
+        final Field[] baselineFields = baselineClass.getFields();
+        final Field[] currentFields = currentClass.getFields();
 
-        // Sigh... BCEL 5.1 hands out it's internal datastructure,
-        // so we have to make a copy here to make sure we don't mess up BCEL by sorting
+        CoIterator iter = new CoIterator(
+            COMPARATOR, baselineFields, currentFields);
 
-        final Field[] bFields = createSortedCopy(baselineFields);
-        final Field[] cFields = createSortedCopy(currentFields);
-
-        checkForChanges(bFields, cFields, compatBaseline, currentVersion);
-
-        return true;
-    }
-
-    private void checkForChanges(
-            Field[] bFields, Field[] cFields, JavaClass baseLineClass, JavaClass currentClass)
-    {
-        boolean[] newInCurrent = new boolean[cFields.length];
-        Arrays.fill(newInCurrent, true);
-
-        // check for deleted fields and modified fields
-        for (int i = 0; i < bFields.length; i++)
+        while (iter.hasNext())
         {
-            Field bField = bFields[i];
-            if (!scopeSelector.isSelected(bField))
+            iter.next();
+
+            Field bField = (Field) iter.getLeft();
+            Field cField = (Field) iter.getRight();
+            
+            if (bField == null)
             {
-                continue;
+                if (scopeSelector.isSelected(cField)) 
+                {
+                    final String name = cField.getName();
+                    String scope = ScopeSelector.getScopeDesc(cField);
+                    fireDiff("Added " + scope + " field " + name, Severity.INFO, currentClass, cField);
+                }
             }
-            int cIdx = Arrays.binarySearch(cFields, bField, comparator);
-            if (cIdx < 0)
+            else if (cField == null)
             {
-                final String name = bField.getName();
-                fireDiff("Field " + name + " has been removed", Severity.ERROR, baseLineClass, bField);
+                if (scopeSelector.isSelected(bField)) 
+                {
+                    final String name = bField.getName();
+                    fireDiff("Field " + name + " has been removed", Severity.ERROR, baselineClass, bField);
+                }
             }
-            else
+            else if (scopeSelector.isSelected(bField) || scopeSelector.isSelected(cField))
             {
-                Field cField = cFields[cIdx];
-                newInCurrent[cIdx] = false;
                 checkForModifierChange(bField, cField, currentClass);
                 checkForVisibilityChange(bField, cField, currentClass);
-                checkForReturnTypeChange(bField, cField,  currentClass);
+                checkForTypeChange(bField, cField,  currentClass);
                 checkForConstantValueChange(bField, cField,  currentClass);
             }
         }
-
-        // check for added fields
-        for (int i = 0; i < newInCurrent.length; i++)
-        {
-            Field field = cFields[i];
-            if (newInCurrent[i] && scopeSelector.isSelected(field))
-            {
-                String scope = ScopeSelector.getScopeDesc(field);
-                final String fieldName = field.getName();
-                fireDiff("Added " + scope + " field " + fieldName, Severity.INFO, currentClass, field);
-            }
-        }
+        
+        return true;
     }
 
     private void checkForConstantValueChange(Field bField, Field cField, JavaClass currentClass)
@@ -157,7 +142,7 @@
         }
     }
 
-    private void checkForReturnTypeChange(Field bField, Field cField, JavaClass currentClass)
+    private void checkForTypeChange(Field bField, Field cField, JavaClass currentClass)
     {
         final String bSig = bField.getType().toString();
         final String cSig = cField.getType().toString();
@@ -230,12 +215,4 @@
         getApiDiffDispatcher().fireDiff(diff);
 
     }
-
-    private Field[] createSortedCopy(final Field[] orig)
-    {
-        final Field[] fields = new Field[orig.length];
-        System.arraycopy(orig, 0, fields, 0, orig.length);
-        Arrays.sort(fields, comparator);
-        return fields;
-    }
 }

Re: [Clirr-devel] comments on recent commits

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

On Mon, 2004-06-21 at 17:50, Lars K=FChne wrote:
> Simon Kitching wrote:
>=20
> >On Mon, 2004-06-21 at 05:13, Lars K=FChne wrote:
> > =20
> >
> >
> >So I would be quite keen for clirr to move to ASM sooner or later. BCE=
L
> >is working for now, so there's no urgency, but I think it would be a
> >good idea in the long run. Building a "wrapper" that allows BCEL or AS=
M
> >to be "plugged in" is an option, but it would be significant work and =
I
> >don't really see any benefits over simply using ASM directly.
> >
> > =20
> >
>=20
>     * It would minimize our external dependencies, check implementation=
s
>       would only reference our own stuff.
>     * Some IDEs provide their own API for accessing the class info we
>       need, at least for the current version of the code that is
>       checked. For example I know that IDEA provides a ProgramStructure
>       interface (PSI) to access the source code. PSI isn't based on
>       class files, but it provides all info required for clirr, like
>       inheritance hierarchy, method signature, etc. Typically the IDE
>       caches that information, minimizing disk activity. If clirr is
>       used interactively from an IDE plugin, using PSI instead of
>       accessing class files could make a real difference in terms of
>       performance.

The PSI stuff is a good point.

If IDEA have gone to the effort of designing a nice API for accessing
class structure, then can we use *that* as our API too? ie write
adaptors from PSI->BCEL, PSI->ASM, PSI->PSI? It seems silly to invent
our own API...and presumably APIs are in the public domain (unlike
implementations).

>=20
>=20
> >And one last issue: java 1.4 includes the BCEL classes, because it
> >includes Xalan; try "jar tf rt.jar | grep BCEL".
> >
>=20
> No results for me (1.4.2-b28), it seems Sun has fixed it within 1.4.2 a=
s=20
> well (?). Still it's worrying. I've checked the bcel-devel archives, an=
d=20
> that's not too encouraging either...

Sorry,my mistake. I'm sure I saw it there, but can't find it now. I must
have had sun-stroke or something :-)

Bcel is in java 1.5, but isn't an issue because with 1.5 sun now make
sure any non-sun software they distribute is given a sun package prefix,
eg 1.5 contains:
  com.sun.org.apache.bcel...
  com.sun.org.apache.xerces....

This is a reasonably nice solution; they don't hide the real authors of
the code, but do allow downloads direct from apache or others to be used
without conflicting with the classes in the java std libs.


> >>>
> >>Maybe we should start an Open Source Java Profiler tool (competitor t=
o=20
> >>JProbe, Optimizeit, etc.) so we get to read "Inside the Java Virtual=20
> >>Machine" as well ;-)
> >>   =20
> >>
> >
> >Hey, why not??
> >
> >I have noticed a need for a tool that determines whether code is
> >invoking any methods not present in Java version XXX. A few Apache
> >commons releases have accidentally gone out with code that doesn't wor=
k
> >in java 1.3.=20
> >
>=20
> That would probably be in scope for clirr, but I'm not sure it's=20
> feasible: Things like reflection and casting may turn out to be=20
> significant road blocks to detect these problems reliably. But maybe we=
=20
> could start by checking that the class file format version of each clas=
s=20
> has stayed the same?

Yes, there could be gotchas.=20

>=20
> On a related note, I have recently found out that messing around with a=
=20
> method's declared exceptions will not result in binary compatibility=20
> problems (surprising to me). Should we extend clirr's scope to check fo=
r=20
> source compatibility as well?

I'm +1 on reporting compile-time incompatibilities too.=20

I agree that changing the declared exceptions should be reported; a
WARNING seems most appropriate to me. What do you think?


Regards,

Simon

Re: [Clirr-devel] comments on recent commits

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

Simon Kitching wrote:

>On Mon, 2004-06-21 at 05:13, Lars Kühne wrote:
>  
>
>>BTW, if we wrote our own BCEL adaptor I mentioned on IRC, this would be 
>>one of the methods for a class, so we'd have a more OO API to work with. 
>>No big deal however, and the work involved in such an abstraction would 
>>not be justified...
>>    
>>
>
>The ASM library appears to handle this stuff automatically; see
>
>http://asm.objectweb.org/current/doc/javadoc/user/org/objectweb/asm/tree/InnerClassNode.html
>
>I also think that BCEL's OO design is odd in places. AccessFlags? Should
>a JavaClass really have an "is-a" relationship with (ie inherit from)
>"AccessFlags"? A has-a relationship seems much more sensible.
>
>  
>

Yes, I have found that the BCEL design is ... errrmmhhh ... a bit 
strange in places. You mentioned one example, the classloader/repository 
handling is another one. I've also found that bcel accessors tend to 
hand out their private data structures, e.g. JavaClass.getFields()

>And I've also subscribed to the BCEL user email list, and can report it
>is absolutely *silent*.
>
>So I would be quite keen for clirr to move to ASM sooner or later. BCEL
>is working for now, so there's no urgency, but I think it would be a
>good idea in the long run. Building a "wrapper" that allows BCEL or ASM
>to be "plugged in" is an option, but it would be significant work and I
>don't really see any benefits over simply using ASM directly.
>
>  
>

    * It would minimize our external dependencies, check implementations
      would only reference our own stuff.
    * Some IDEs provide their own API for accessing the class info we
      need, at least for the current version of the code that is
      checked. For example I know that IDEA provides a ProgramStructure
      interface (PSI) to access the source code. PSI isn't based on
      class files, but it provides all info required for clirr, like
      inheritance hierarchy, method signature, etc. Typically the IDE
      caches that information, minimizing disk activity. If clirr is
      used interactively from an IDE plugin, using PSI instead of
      accessing class files could make a real difference in terms of
      performance.


>And one last issue: java 1.4 includes the BCEL classes, because it
>includes Xalan; try "jar tf rt.jar | grep BCEL".
>

No results for me (1.4.2-b28), it seems Sun has fixed it within 1.4.2 as 
well (?). Still it's worrying. I've checked the bcel-devel archives, and 
that's not too encouraging either...

> So it is not possible
>to provide an updated BCEL distribution with clirr (classes in rt.jar
>override the user classpath, unless serious trickery is applied). Note
>that this is a problem *only* for java1.4; sun have fixed this in 1.5.
>
>  
>
>>>I'm slowly working my way through chapter 13 of the java specification
>>>(the binary compatibility bit); I presume you've been though that too.
>>>That's something I would never have done if I hadn't started working on
>>>clirr!
>>>
>>> 
>>>
>>>      
>>>
>>Maybe we should start an Open Source Java Profiler tool (competitor to 
>>JProbe, Optimizeit, etc.) so we get to read "Inside the Java Virtual 
>>Machine" as well ;-)
>>    
>>
>
>Hey, why not??
>
>I have noticed a need for a tool that determines whether code is
>invoking any methods not present in Java version XXX. A few Apache
>commons releases have accidentally gone out with code that doesn't work
>in java 1.3. 
>
>This should be pretty simple to implement: 
>(a) 
>get a list of all accessable classes, methods,fields in release X of
>java
>(b) 
>iterate over all the code in a set of jars, determining whether
>there are any references to classes/methods/fields that aren't present
>in the list from step (a).
>
>I don't know whether this is within the scope of the clirr project or
>not....it is sort of a "binary compatibility test", but this time
>testing whether program W is compatible with library X, rather than
>whether there are any differences between library X version 1 and
>version 2.
>  
>

That would probably be in scope for clirr, but I'm not sure it's 
feasible: Things like reflection and casting may turn out to be 
significant road blocks to detect these problems reliably. But maybe we 
could start by checking that the class file format version of each class 
has stayed the same?

On a related note, I have recently found out that messing around with a 
method's declared exceptions will not result in binary compatibility 
problems (surprising to me). Should we extend clirr's scope to check for 
source compatibility as well?

Cheers,
Lars

Re: [Clirr-devel] comments on recent commits

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

On Mon, 2004-06-21 at 05:13, Lars K=FChne wrote:
> BTW, if we wrote our own BCEL adaptor I mentioned on IRC, this would be=
=20
> one of the methods for a class, so we'd have a more OO API to work with=
.=20
> No big deal however, and the work involved in such an abstraction would=
=20
> not be justified...

The ASM library appears to handle this stuff automatically; see

http://asm.objectweb.org/current/doc/javadoc/user/org/objectweb/asm/tree/=
InnerClassNode.html

I also think that BCEL's OO design is odd in places. AccessFlags? Should
a JavaClass really have an "is-a" relationship with (ie inherit from)
"AccessFlags"? A has-a relationship seems much more sensible.

And I've also subscribed to the BCEL user email list, and can report it
is absolutely *silent*.

So I would be quite keen for clirr to move to ASM sooner or later. BCEL
is working for now, so there's no urgency, but I think it would be a
good idea in the long run. Building a "wrapper" that allows BCEL or ASM
to be "plugged in" is an option, but it would be significant work and I
don't really see any benefits over simply using ASM directly.

And one last issue: java 1.4 includes the BCEL classes, because it
includes Xalan; try "jar tf rt.jar | grep BCEL". So it is not possible
to provide an updated BCEL distribution with clirr (classes in rt.jar
override the user classpath, unless serious trickery is applied). Note
that this is a problem *only* for java1.4; sun have fixed this in 1.5.

>=20
> >I'm slowly working my way through chapter 13 of the java specification
> >(the binary compatibility bit); I presume you've been though that too.
> >That's something I would never have done if I hadn't started working o=
n
> >clirr!
> >
> > =20
> >
>=20
> Maybe we should start an Open Source Java Profiler tool (competitor to=20
> JProbe, Optimizeit, etc.) so we get to read "Inside the Java Virtual=20
> Machine" as well ;-)

Hey, why not??

I have noticed a need for a tool that determines whether code is
invoking any methods not present in Java version XXX. A few Apache
commons releases have accidentally gone out with code that doesn't work
in java 1.3.=20

This should be pretty simple to implement:=20
(a)=20
get a list of all accessable classes, methods,fields in release X of
java
(b)=20
iterate over all the code in a set of jars, determining whether
there are any references to classes/methods/fields that aren't present
in the list from step (a).

I don't know whether this is within the scope of the clirr project or
not....it is sort of a "binary compatibility test", but this time
testing whether program W is compatible with library X, rather than
whether there are any differences between library X version 1 and
version 2.

Regards,

Simon

Re: [Clirr-devel] comments on recent commits

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

Simon Kitching wrote:

>Hi Lars,
>
>Thought I'd gone a bit quiet on the clirr development? Nope, just was
>working on a rather tricky bit of code :-).
>
>It took me a while to figure out how to determine the real scope of a
>nested class - see method ScopeSelector.getClassScope, it's a beauty!
>
>  
>

Yep, nice work! I once wrote a sth similar for checkstyle to ensure that 
you don't have to Javadoc public methods of anonymous inner classes. 
Funny that this information is not available directly in BCEL...

BTW, if we wrote our own BCEL adaptor I mentioned on IRC, this would be 
one of the methods for a class, so we'd have a more OO API to work with. 
No big deal however, and the work involved in such an abstraction would 
not be justified...

>I'm slowly working my way through chapter 13 of the java specification
>(the binary compatibility bit); I presume you've been though that too.
>That's something I would never have done if I hadn't started working on
>clirr!
>
>  
>

Maybe we should start an Open Source Java Profiler tool (competitor to 
JProbe, Optimizeit, etc.) so we get to read "Inside the Java Virtual 
Machine" as well ;-)

>Hopefully these changes are OK by you. There isn't anything
>controversial in there, I think, except possibly having the check method
>return a boolean. As you can see from the ClassScopeCheck method, this
>allows a check to indicate that there isn't any point applying further
>changes. Maybe this feature could be used in the GenderCheck, to avoid
>having to have this rather ugly check in MethodSetCheck?
>
>  if (compatBaseline.isInterface() ^ currentVersion.isInterface())
>  {
>      return true;
>  }
>
>
>  
>

Tried to work on it for a few minutes. Changing the code is easy and I 
think Clirr works correctly. But getting the unit tests to run is 
trickier than I first thought: Because our unit test framework only 
works with one check at a time, the GenderChange check does not abort 
the MethodSetCheck for ClassBecomesInterface (like it would in the real 
world), and MethodSetCheck reports a missing constructor.

This demonstrates that our test framework really needs to use your class 
selector better, MethodSetCheckTest should not check that class. I'll 
look into that this week.

Lars

Re: [Clirr-devel] Test failure

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

Simon Kitching wrote:

>Hi Lars,
>
>I tried to run the unit tests for the latest clirr today, and got an
>error in the MethodSetCheckTest test. By reverting one version
>(1.6->1.5) of MethodSetCheckTest.java the problem goes away.
>
>Maybe you forgot to check in a change to the test-input dirs
>corresponding to the change you checked in to MethodSetCheckTest.java?
>
>  
>

Yes, fixed now.

Thanks,
Lars

[Clirr-devel] CVS: clirr/src/testinput/testlib-v2/testlib AbstractImpl.java,NONE,1.1 BaseInterface.java,NONE,1.1

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

Update of /cvsroot/clirr/clirr/src/testinput/testlib-v2/testlib
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv12578

Added Files:
	AbstractImpl.java BaseInterface.java 
Log Message:
added test data for recent methodcheck changes

--- NEW FILE ---
package testlib;

public abstract class AbstractImpl implements BaseInterface
{
    // public abstract void method(); not available here directly
}

--- NEW FILE ---
package testlib;

public interface BaseInterface
{
    void method();
}

[Clirr-devel] CVS: clirr/src/testinput/testlib-v1/testlib AbstractImpl.java,NONE,1.1 BaseInterface.java,NONE,1.1

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

Update of /cvsroot/clirr/clirr/src/testinput/testlib-v1/testlib
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv12167

Added Files:
	AbstractImpl.java BaseInterface.java 
Log Message:
added test data for recent methodcheck changes

--- NEW FILE ---
package testlib;

public abstract class AbstractImpl implements BaseInterface
{
    public abstract void method();
}

--- NEW FILE ---
package testlib;

public interface BaseInterface
{
    void method();
}

[Clirr-devel] FieldSetCheck: the truth is relative?

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

Hi,

Currently, if a field is private in the old version, and public in the
new one, this is reported as the field being "added".

While this makes sense from the user perspective, it doesn't from the
developer perspective.

Would you object to me changing FieldSetCheck so that clirr reports the
actual truth: that the field has changed from "private" to "public"
(even when the user is only reporting on the public/protected API)?

Thanks,

Simon

[Clirr-devel] comments on recent commits

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

Hi Lars,

Thought I'd gone a bit quiet on the clirr development? Nope, just was
working on a rather tricky bit of code :-).

It took me a while to figure out how to determine the real scope of a
nested class - see method ScopeSelector.getClassScope, it's a beauty!

I'm slowly working my way through chapter 13 of the java specification
(the binary compatibility bit); I presume you've been though that too.
That's something I would never have done if I hadn't started working on
clirr!

Hopefully these changes are OK by you. There isn't anything
controversial in there, I think, except possibly having the check method
return a boolean. As you can see from the ClassScopeCheck method, this
allows a check to indicate that there isn't any point applying further
changes. Maybe this feature could be used in the GenderCheck, to avoid
having to have this rather ugly check in MethodSetCheck?

  if (compatBaseline.isInterface() ^ currentVersion.isInterface())
  {
      return true;
  }


Catch you later,

Simon

[Clirr-devel] CVS: clirr/src/testinput/testlib-v1/testlib/scope ClassScopeChange.java,NONE,1.1

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

Update of /cvsroot/clirr/clirr/src/testinput/testlib-v1/testlib/scope
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv8197/testinput/testlib-v1/testlib/scope

Added Files:
	ClassScopeChange.java 
Log Message:
Added check which determines whether a class has changed its scope
(public/protected/package/private) between releases. Also added
unit tests for this change.


--- NEW FILE ---
package testlib.scope;

public class ClassScopeChange
{
    // public class is unchanged
    public static class A1 {}
    
    // public class becomes protected
    public static class A2 {}

    // public class becomes package
    public static class A3 {}

    // public class becomes private
    public static class A4 {}
    
    // protected class is unchanged
    protected static class B1 {}
    
    // protected class becomes public
    protected static class B2 {}

    // protected class becomes package
    protected static class B3 {}

    // protected class becomes private
    protected static class B4 {}
    
    // package class is unchanged
    static class C1 {}

    // package class becomes public
    static class C2 {}

    // package class becomes protected
    static class C3 {}

    // package class becomes private
    static class C4 {}
    
    // private class is unchanged
    private static class D1 {}
    
    // private class becomes public
    private static class D2 {}

    // private class becomes protected
    private static class D3 {}

    // private class becomes package
    private static class D4 {}
}

[Clirr-devel] CVS: clirr/src/testinput/testlib-v2/testlib/scope ClassScopeChange.java,NONE,1.1

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

Update of /cvsroot/clirr/clirr/src/testinput/testlib-v2/testlib/scope
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv8197/testinput/testlib-v2/testlib/scope

Added Files:
	ClassScopeChange.java 
Log Message:
Added check which determines whether a class has changed its scope
(public/protected/package/private) between releases. Also added
unit tests for this change.


--- NEW FILE ---
package testlib.scope;

public class ClassScopeChange
{
    // public class is unchanged
    public static class A1 {}
    
    // public class becomes protected
    protected static class A2 {}

    // public class becomes package
    static class A3 {}

    // public class becomes private
    private static class A4 {}
    
    // protected class is unchanged
    protected static class B1 {}
    
    // protected class becomes public
    public static class B2 {}

    // protected class becomes package
    static class B3 {}

    // protected class becomes private
    private static class B4 {}
    
    // package class is unchanged
    static class C1 {}

    // package class becomes public
    public static class C2 {}

    // package class becomes protected
    protected static class C3 {}

    // package class becomes private
    private static class C4 {}
    
    // private class is unchanged
    private static class D1 {}
    
    // private class becomes public
    public static class D2 {}

    // private class becomes protected
    protected static class D3 {}

    // private class becomes package
    static class D4 {}
}