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