Bug 240021 - [jdt-weaving] Gaining control of CompilationUnit creation in JDT through the use of Aspects
Summary: [jdt-weaving] Gaining control of CompilationUnit creation in JDT through the ...
Status: RESOLVED FIXED
Alias: None
Product: AJDT
Classification: Tools
Component: Core (show other bugs)
Version: DEVELOPMENT   Edit
Hardware: Macintosh Mac OS X - Carbon (unsup.)
: P3 enhancement (vote)
Target Milestone: 1.6.2   Edit
Assignee: AJDT-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 177733 (view as bug list)
Depends on: 240047
Blocks: 254431
  Show dependency tree
 
Reported: 2008-07-08 12:42 EDT by Andrew Eisenberg CLA
Modified: 2008-12-11 14:59 EST (History)
2 users (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 2008-07-08 12:42:24 EDT
One of the major barriers for having a truly deep integration of AJDT with the JDT is the fact that we don't always have control over how CompilationUnits are created.  Hence there is a little hack that AJDT uses such that each .aj file has two compilation units associated with it (CompilationUnit for the JDT and an AJCompilationUnit for the AJDT).  This works in many cases, but means that we have to manage both and provide work arounds so that both are not displayed at the same time.

A solution to this problem that I have been experimenting with is to weave into JDT, providing around advice for org.eclipse.jdt.internal.core.CompilationUnit constructor calls so that we can have control over which type of compilation unit to create.

Exactly how this weaving will be done when this is shipped is still up in the air (suggestions please!).  It needs to be robust as well as non-invasive.  I have some ideas and will document them later.

The following is a list of bugs that are fixed because of this functionality.

1. no need to filter duplicate CUs
2. organize imports
3. show in editor bug 133119
4. bug 188845 (import statements not being removed
5. bug 106589 (inner class and import)
6. bug 132370--dups when synchronizing works
7. bug 106485 seems to work.  But, I don't know if it was working before
8. bug 111329 works, but I think it worked before
9 bug 155860 works
10. bug 166210 works (but would work anyway even without the weaving)

The following are bugs that still do not work, even though it seems like they might:

1. quick fix for imports
2. sometimes a .aj file is being opened in a java editor
3. wrong icon for .aj files being used in all views (shows up looking like a Java compilation unit)
4. Bug 78341 Copy and paste of AJ unit inside of a package.  Now throws an exception, whereas before would fail nicely.  interesting, though...works if the top level element is an aspect, but does not work if there is an aspect contained inside of a class file.
5. Bug 131467 not working (Stack trace following) must use different parser because of search engine
6. Bug 159853 not working Indexing problem As above.  Works when aspect is not the top-level type.     
7. Bug 159867
8. Bug 236352  organizing imports sometimes puts an import above the package line

So, it is plain that this kind of weaving of compilation unit constructor calls helps to some extent, but does not solve all the problems.  I believe that many of the problems still occurring are all related in that they use org.eclipse.jdt.internal.core.search.indexing.IndexManager to help with searching for types.  Aspect files are not indexed because they cannot be parsed by a Java parser.

This experiment is a first attempt to weave into the JDT.  It doesn't seem to break anything major and does seem to fix quite a few problems.  The next step would be to do a similar thing for the Java parser.
Comment 1 Andrew Clement CLA 2008-07-08 14:05:14 EDT
> Exactly how this weaving will be done when this is shipped is still up in the
> air (suggestions please!).  It needs to be robust as well as non-invasive.  I
> have some ideas and will document them later.

Initial brainstorming on this is:
1. loadtime weave on eclipse startup.  Makes it easy to switch it on/off but will impact every startup
2. one time weave after downloading AJDT using a batch script. Adds a step for the user after installing AJDT.
3. one time weave after downloading AJDT using a button in eclipse.  Whether the necessary jars could be woven whilst Eclipse is up, I'm not certain but at least this would mean everything is contained inside eclipse rather than asking users to go to the command line to complete installation.  It may be possible to implement some existing callback that when eclipse is restarting post install we can be called to do our thing.

AJDT base functionality must be OK without this being applied (ie. the basic create/edit/compile/run/debug/etc) - it is just that with jdt woven other facilities will be available.

The aspect must be bullet proof, it must not affect JDT in any way outside of an AspectJ project (possibly this includes creation of .aj files created in non-aspectj projects).
Comment 2 Andrew Eisenberg CLA 2008-07-08 14:16:09 EDT
(In reply to comment #1)
Thanks Andy.  Shipping the woven code is in a separate bug. Bug 240047.  I am copy your comment to that bug.
Comment 3 Andrew Eisenberg CLA 2008-07-12 17:22:48 EDT
I've been really struggling to find a clean way (or any way that is not a complete mess) to plug in a parser for indexing purposes.  The problem is that it's not just the parser that needs to be plugged in, but also the scanner, the source requestor, the problem reporter, and a bunch of other things.  AspectJ alone won't do the trick, I would additionally need to use generous amounts of reflection.  So, I am backing away from this technique.

Instead, I am going to try this:

1. advise Scanner.setSource() so that the source that gets set is always syntactically correct Java (we can probably get away with before advice.)
2. This will mean that the scanner (and hence the parser) will not fail if the input is valid AspectJ
3. The assumption is that the translation from AJ to Java will retain much of the likeness of the original AspectJ.  This shouldn't be too hard.  Perhaps it can be a translation from code style to annotation style aspectj.

Once an aspect compilation unit is able to be indexed, things like Open Type and add import should just work.
Comment 4 Andrew Clement CLA 2008-08-27 16:53:09 EDT
*** Bug 177733 has been marked as a duplicate of this bug. ***
Comment 5 Andrew Eisenberg CLA 2008-11-12 13:19:40 EST
Here is some explanation of how to use the current implementation of jdt weaving.

=====================================


Actually, all you need to do is use this update site and install ajdt
and ajdt source features into a clean version of 3.4.1:
http://download.eclipse.org/tools/ajdt/34/dev/weaving/

Then restart eclipse and make sure that AJDT is properly installed.

What I have here is standard AJDT and the 3 bundles for equinox
aspects.  Plus there is an extra new bundle that I called
org.eclipse.contribution.weaving.jdt.  The best thing to do is to look
at the code for this plugin and see what it does and how AJDT uses it.
 Here is a brief description

It defines 3 extension points.  You don't need to implement all of them.

1. compilation unit provider
Extend this so that JDT will instantiate a compilation unit of your
kind whenever it tries to open a file of the given extension.

You must extend *CompilationUnit* (not ICompilationUnit) in order to
fit in with the type system when weaving around constructors.  It is
up to your plugin to ensure that the structure is built properly (see
the class AspectJCore for how AJDT does it).

2. image descriptor selector
Extend this so that you can provide your own icons (in search dialogs,
etc) where appropriate.

3. source transformer
Extend this so that you have control over what code the Java indexer
sees.  This allows your stuff to show up in things like Java search,
hierarchies, imports, etc.  A source transformer will convert from
your language to syntactically correct Java.  Source positions are
important, so ensure that all indexed things (classes, methods and
fields, eg) have the same text offset and length in the original as in
the transformed.

See AspectsConvertingParser.java for an example of this (a bit of a
hack, but it works for AJDT).

This last extension point, I am less happy with.  I would prefer to
plug directly into the Java indexer, but I couldn't find an easy way
to do it.
Comment 6 Andrew Eisenberg CLA 2008-12-11 14:59:19 EST
We are about to ship a version of AJDT that does this, all is well.  Closing this bug.