Re: [jdt-ui-dev] UseSupertypeWherePossible released

[Timothy Halloran <thallora@xxxxxxxxxxxxxxxxx>]

On Tue, 2002-10-01 at 17:27, Adam Kiezun wrote:
> Thank you Tim,
> I had a look at your paper and run your tool.
> It surely is very interesting stuff that you're working on.
> And the tool is really good too - I'm planning to run it on jdt ui to do the analysis
> (and fix the problems :-)).
> 
> [Btw, compared to other tools - especially competition - Eclipse does not look too bad in yout metrics, eh?]

Yes, but I was thinking of posting the "unnecessary import" data to the
developer list.  Since this is so easy to detect and remove in Eclipse,
there should really have been 0!

Have you considered "mass" refactorings for this--sort-of like a
"organize imports" at the project level?

> And your work is somewhat related to our refactorings
> (except you only analyze - and our tool also modifies the code)

Correct, we are really interested in nudging the programmer toward using
your refactoring tool, and assurance of properties across the code base.

> I see a couple of differences between our approaches:
> a. you analyze the whole workspace in a lint-kind of way while we perform our analysis for a selected type
> b. (consequence of a. really) your tool opens every file and creates the ast for it.
> ours opens only those files that need to be opened.
> c. you do not consider method return type and never suggests to update them - our tool does
> d. or method parameters - we do

Yes, these are all being corrected and are artifacts of our inexperience
with Eclipse and limitations we were willing to stomach to investigate
our ideas.  We are working to: (1) add incremental analysis, and (2)
treating return type as a variable. We do get method parameters, but not
when it would cause a "supertype" change and I think only on final
classes (but I need to check this).  Your insight (above and below) on
method parameters is interesting and better thought out than what I had
considered.

> e. the analysis that your tool performs seems to be a bit limited - consider the following:
> class A{}
> class B extends A{
>    void f(B b0){
>        b0.f();
>        B b1= null;
>        B b2= b1;
>        B b3= b2;
>    }
> }
> now, eclipse's refactoring will notice that it's ok to update declarations of b1, b2 and b3 to A
> (but not b0, naturally)
> while i need to run Tallyho 3 times (run, modify, run, modify,  run, modify) to achieve the same effect.
> The first time round it thinks that only the declaration of b3 can be modified.

Agreed, I'll have to look at how you approach this -- because I want to
assure absence of "overspecific variables" the "step-by-step" approach
seemed logical.  After the first change our (yet to be finished), 
incremental analysis (today you sadly rerun it all) would update its
state based upon the change and make further "suggestions".

> f. having said all that - our tool is, at this point, a bit over-enthusiastic about what can be changed
> (whereas i could not find a way to fool yours :-) ).
> For now you have avoided it but it gets really interesting when method parameters come into the picture - overloading, overriding
> and related problems are not far behind.

Yes, this adds complexity to the problem and really raises a to need to
examine each class to see if the change is "legal."  You want to be able
to correlate results up and down the type hierarchy and identify the
opportunity to improve the users type choice.  Another dimension is
respecting what I would term the "JAR-wall" -- if the analysis gets into
a "non-source" class then forget the possible change since they are
extending some sort of "COTS" API (this maybe could be configurable, but
would be true for the vast majority of users).

> Btw, thanks for the insight about Throwable - updating these would trully be a _bad_ idea :-)
> (far too much is dependent on run-time types of the exception objects).

No problem, hopefully will save you some time.

> So, I think there's room for improvement in both your tool and ours - and we could utilize each other's experience and insights..
> Tim, thanks very much again for your interest and sharing your ideas with us.We really, really appreciate it.
> Hope you will stay in touch.

Agreed, two questions:

(1) Can you send me a code pointer on your refactoring.  I downloaded a
nightly build and couldn't find it?  I also couldn't find it in the UI. 
I probably got the wrong "stream", do I need to get it from CVS?

(2) Is there any interest in enhancing the "Tasks" view to allow richer
explanation of "why" a task is on the list (sort-of like our "Code
Quality Advice" rationale, but probably not exactly) -- this would be a
big help for assurance type tools within Eclipse and I think would be a
nice incremental improvement on the current "filter" the table approach
when dealing with huge numbers of tasks (via flexible categorization).
My team at CMU would be interested in contributing on this effort, but
has been leery of changing "built-in" components to avoid "code-fork"
issues.

> Best regards
> -adam
> 
> ps. Where's the paper going to be published? And when?
> pps. In figure 1 'newEntry' is of type Object but then t(newEntry) = {String}. This seems strange.
> (the + operator does not imply that the second argument is of type String.
> Rather,  string conversion is used, which for reference types means that toString() is called - as spec'd in JLS 15.18.1.1) .
> ppps.  The ancestor(ListIterator)  (section 2.1) misses Object, i think
> pppps. The ancestor function is defined only for types but then it's also used for sets of types (2.1 also)
> (as in 'ancestor(t)')

Thanks, I will correct these

> (i guess everybody will understand it, though)

I hope so :-> but I'll put in the fixes.

> ppppps. It was good not to see an Eclipse example in the 3.2.1 section on ignored exceptions and interesting comments in empty catch
> blocks. Is there none or were they not interesting? Or too embarassing for eclipse developers <g>?

The worst was "// should not happen" (in TextViewer.java) and other
similar non-absolute comments that probably make sense in context.  I
didn't note any TBDs or such -- we were surprised by the ignored
exception results being so high (I expected far fewer cases) and are
starting to investigate beyond the "default-try-ignore" pattern we noted
as quite common in the paper.  The empirical data in this case suggest a
possible mismatch between common Java practice and current "rules of
thumb." We are suffering, somewhat, by the lack of a good AST pattern
detector--does one exist within your refactoring infrastructure we
should have a look at?

Take Care,
Tim Halloran