Bug 265993 - [handles] no fully qualified parameters types in ITD handles
Summary: [handles] no fully qualified parameters types in ITD handles
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Build (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 1.6.4   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 265986
Blocks:
  Show dependency tree
 
Reported: 2009-02-24 13:25 EST by Andrew Eisenberg CLA
Modified: 2009-03-27 16:53 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Eisenberg CLA 2009-02-24 13:25:59 EST
Currently, AspectJ always uses fully qualified parameter types in the handles of ITDs.  This should not be.  The handles should mimic the text, so that if the text does not fully qualify the parameter, neither should the handle.

See Bug 265985 c1 for an example.  Also attached to that bug is a sample project to use that recreates the bug.
Comment 1 Andrew Clement CLA 2009-02-24 13:43:03 EST
interesting.
Comment 2 Andrew Clement CLA 2009-02-25 01:47:18 EST
dont think you mean bug 265985, think you bean bug 265986.  However that bug is now closed - so just want to make sure this work needs to be done?  Does always including fully qualified names in the handles cause problems when locating the JDT elements using them? Or are you just saying you'd like this for absolute consistency?

it should be possible for full source compilation to mimic this for AspectJ - but changing the binary class file format to persist whether the user did or didn't qualify the original reference is not something i would like to do so it is not going to be possible for aspects arriving in binary form (from aspectpath or inpath).
Comment 3 Andrew Eisenberg CLA 2009-02-25 11:39:40 EST
(In reply to comment #2)
> dont think you mean bug 265985, think you bean bug 265986.  
Yes.

Should be fine to mimic this for source compilation only.  I believe that in binary files *all* parameter and return types should be fully qualified.

Now that I think about it, then, this will cause problems when converting from a binary handle in an aspectj project to a source handle in another aspectj project.  I will have to walk through the handle and convert from fully qualified to simple names (although this won't be perfect since if a name happens to be fully qualified in a method, then it will be fully qualified in the source handle too, but I won't be able to tell this when transforming from binary handles).
Comment 4 Andrew Clement CLA 2009-03-23 12:42:18 EDT
I'm guessing you want to have your cake and eat it... you want fully qualified parameter signatures when you call 'getParameterSignatures()' on an IProgramElement, but when you retrieve the handle you want the short form depending on what was in the source. nice...
Comment 5 Andrew Clement CLA 2009-03-23 13:07:20 EDT
tricky to do in a performant way, I either remember the source references and greatly increase the model size or compare the source with the resolved signature and increase the time taken to build the model.  In fact I don't see why we even do any type resolution in model building, we are doing it 'just in case' the data will be needed later - seems such a waste.
Comment 6 Andrew Eisenberg CLA 2009-03-23 16:14:35 EDT
(In reply to comment #4)
> I'm guessing you want to have your cake and eat it... you want fully qualified
> parameter signatures when you call 'getParameterSignatures()' on an
> IProgramElement, but when you retrieve the handle you want the short form
> depending on what was in the source. nice...
> 

Never said this was going to be easy.  :-)  


(In reply to comment #5)
> tricky to do in a performant way, I either remember the source references and
> greatly increase the model size or compare the source with the resolved
> signature and increase the time taken to build the model.  In fact I don't see
> why we even do any type resolution in model building, we are doing it 'just in
> case' the data will be needed later - seems such a waste.
> 

When I create the mock JavaElement that is inserted into the target Java class, we need to know the fully qualified type because the parameter types may not be imported in the target class.  If this is not done, ITDs with unqualified parameters will cause reconciling errors in the target types.

There might be some shortcuts you can do that can save AspectJ time and push it forward to AJDT so that it can do some extra processing.  For example, getParameterTypes can return fully qualified parameters, and the handle can be based on that.  But, somewhere, AJDT would need to get a list of booleans that tells what is fully parameterized and what isn't.  Then AJDT can map back and forth between AJ and JDT handles (I know this is a little ugly).

Alternatively, AJ could create handles based on  what is in the source (and have getParameterTypes follow that).  And then when needed, AJDT can ask the World for the fully qualified types of parameters.
Comment 7 Andrew Clement CLA 2009-03-23 16:29:16 EDT
I know what the options are, but none of them are ideal. it would be far easier if generics did not exist because generics mean multiple references within a single parameter may or may not be qualified. 

I want to recover some processing time to cover what I am losing for this - can you tell me what you do with the parameter signatures?  Do you pass them back to AspectJ for resolution, or?
Comment 8 Andrew Eisenberg CLA 2009-03-23 18:06:13 EDT
The fully qualified signatures are used by IntertypeElement.createMockDeclaration() to generate the fully qualified signature that is inserted into the target type.  These mock elements are then consumed as part of the reconciling process (it is a JDT thing, not an aspectj thing).  So, they are not passed back to AspectJ.  So this occurs during reconciling (ie- whenever a target type is edited or whenever a compilation unit that references a target type is edited).  This will probably happen multiple times between compiles.

The unqualified names are required so that it is possible to translate between JDT and AspectJ handles.

Not sure if this really answers your question, though.
Comment 9 Andrew Clement CLA 2009-03-23 20:51:10 EDT
yes it does answer my question and it is good news :)
Comment 10 Andrew Clement CLA 2009-03-23 21:55:07 EDT
ok - all done and changes are in (AspectJ) - will be in ajdt a bit later.  A important side effect is that full parameter signatures are no longer 'P' style, they are plain Java style.  *This means AJDT won't need to translate them from P to plain java style* and the P style can remain an internal thing.

All my tests are passing but I find it hard to believe they cover all scenarios so please let me know what methods you discover are revealing wrong handles and what those handles should be.

I will do some profiling before 1.6.4 ships to verify the construction of these things is not a performance problem.

the code is written to catch any problems and report issues to stderr (and try to carry on processing) - so watch out for any messages or stack traces in stderr.
Comment 11 Andrew Clement CLA 2009-03-27 16:53:10 EDT
believed fixed