Community
Participate
Working Groups
Build ID: I20080502-0100 Steps To Reproduce: I am creating a structure bridge for AspectJ. When CoreContextPlugin.getStructureBridge() is determining which structure bridge to use for a particular element, the JavaStructureBridge.acceptObject() method is always used instead of my AspectJStructureBridge. AspectJ elements are modelled as special subclasses of IJavaElement. JavaStructureBridge.acceptObject() will return true if the object passed in is a IJavaElement, even if it is an aspect, pointcut, advice, etc. This is a problem because the AspectJStructureBridge can never be used since JavaStructureBridge always steals the object away. Thanks. More information:
Andrew: you may be able to address this using the child bridge mechanism. The Java bridge is a child of the resource bridge, and it may be possible to make the AspectJ bridge a child of the Java bridge. If the mechanism is not sufficient, it probably makes sense to extend it to support this scenario. Give that a shot and let me know how ti goes.
My initial implementation was to subclass the JavaStructureBridge class to make AspectStructureBridge. That's when I came up with the problem. I started playing around a bit and I commented out 2 extension points in the Mylyn's Java plugin: the structure bridge and ui bridge extensions. Obviously, this is not meant to be a "fix", but I was able to get the AspectJ bridge working. What I am saying is that I don't think the mechanism that you are suggesting will work (unless I am misunderstanding you), and that I don't think it will be possible to do this without changing JavaStructureBridge/JavaUIBridge. Also, not much code is required to get AspectJStructureBridge working if the Java bridge is already there. I'll think about it some more see what I can get to work.
The challenge is that both the Java and AspectJ structure bridges need to work in together at the same time. This means that either we need an extension point that will know when to delegate something to the AspectJ structure bridge instead of the Java structure bridge, or that the Java structure bridge will need to know how to test that an element is a subtype of IJavaElement but not an actual Java element. I suppose that we could do that via a call to IJavaElmenet.exists(), but that might be too expensive.
Created attachment 101080 [details] mylyn/context/zip
(In reply to comment #3) > I suppose that we could do that via a call to > IJavaElmenet.exists(), but that might be too expensive. > We would want to use something like IJavaElement.getElementType(). But, now that I investigate it further, it seems that all aspect elements return either METHOD or TYPE. So, this wouldn't work either... Another possibility is to check the file extension of the associated resource. If .java, then use JavaStructureBridge. If .aj, then use AspectJStructureBridge. This would probably work, but there are 2 potential problems: 1. I don't know if the call to get the associated resource's file extension is quick or slow. 2. Aspect elements in Java files will not use the correct structure bridge. This isn't quite as bad as it sounds because there are quite a few other things that will not work aspect entities are in java files.
(In reply to comment #5) > This would probably work, but there are 2 potential problems: > 1. I don't know if the call to get the associated resource's file extension is > quick or slow. > 2. Aspect elements in Java files will not use the correct structure bridge. > This isn't quite as bad as it sounds because there are quite a few other things > that will not work aspect entities are in java files. > Actually, this would need to be tested a bit because I don't know what the behavior would be for class files. Another possibility (which is admittedly kludgy) is to check the package declaration of the IJavaElement to see if it is in the expected package. The test would be something like: ((IJavaElement) object).getClass().getPackage().getName().startsWith("org.eclipse.jdt.core");
Andrew: consider exploring this by making the "aspectj" structure bridge shadow the "java" bride. See attached context for the relevant methods. Don't bother with extension points yet, it probably makes more sense to just hack it into getStructureBridge() for now.
Created attachment 101567 [details] mylyn/context/zip
I have something that seems to work. I subclassed JavaStructureBridge and overriode a few methods. Not much code in it. I am attaching a patch, a context, and a project that contains the mylyn-aspectj bridge.
Created attachment 101608 [details] mylyn/context/zip This context contains a first attempt at adding an AspectJStructureBridge. I hacked the ContextCorePlugin.getStructureBridge(Object) method to test for AspectJStructureBridge before the JavaStructureBridge.
Created attachment 101610 [details] AspectJStructureBridge Contains the plugin and code for AspectJStructureBridge
Created attachment 101612 [details] patch for changing ContextCorePlugin This patch changes ContextCorePlugin as specified in the comments section.
Created attachment 101613 [details] mylyn/context/zip
I need the equivalent of this: public AbstractContextStructureBridge getStructureBridge(Object object) { BridgesExtensionPointReader.initExtensions(); // ADE---hack here. // must check aspectj structure bridge *before* java structure bridge. // get aspectj bridge first AbstractContextStructureBridge aspectjBridge = bridges.get("aspectj"); if (aspectjBridge != null) { if (aspectjBridge.acceptsObject(object)) { return aspectjBridge; } } for (AbstractContextStructureBridge structureBridge : bridges.values()) { if (structureBridge.acceptsObject(object)) { return structureBridge; } } // use the default if not found return (defaultBridge != null && defaultBridge.acceptsObject(object)) ? defaultBridge : DEFAULT_BRIDGE; }
Andrew: could you consider providing a patch for an extension point that achieves this? I would like to keep the extension point internal for now, since we have to consider the overlap with the parent attribute on the bridge further. But for now an extension point along the following lines should provide what you want: <extension point="org.eclipse.mylyn.context.core.internalBridges"> <shadow baseContent="java" shadowedByContent="aspectj"/> </extension> Then your code above would be updated to take look up the bridge shadowing similar to how bridge parenting is looked up.
Sure. Let me see if I understand you correctly... 1) InternalBridges is a new extension point that indicates for internal use only (and in the future it may be made external). 2) The method "public AbstractContextStructureBridge getStructureBridge(Object object)" will check whatever bridge is returned and see if there is a bridge that shadows it.
What happens if there there are multiple content types that shadow a single content type? Eg- AspectJ shadows Java ArchJava shadows Java Which one gets precedence? We are back to the original problem. One solution is to keep this as an internal extension point so that mylyn developers have control over the extension point and can prevent this from happening.
(In reply to comment #16) > Sure. Let me see if I understand you correctly... > > 1) InternalBridges is a new extension point that indicates for internal use only > (and in the future it may be made external). > > 2) The method "public AbstractContextStructureBridge getStructureBridge(Object > object)" will check whatever bridge is returned and see if there is a bridge > that shadows it. Yup and yup. (In reply to comment #17) > What happens if there there are multiple content types that shadow a single > content type? ... > One solution is to keep this as an internal extension point so that mylyn > developers have control over the extension point and can prevent this from > happening. Exactly. Don't worry about collisions right now (just override previous values multiple extensions have been read), and when we make the extension point public we can figure that out. For an example of how to read extension points see the landmarks (bold things) in the attached context.
Created attachment 104027 [details] Patch that adds shadows extension point Attaching a patch that adds the shadows extension point and adds the appropriate behavior to the ContextCorePlugin as discussed above.
Created attachment 104028 [details] mylyn/context/zip
Andrew: great, thanks. I'll apply this once we get 3.0 out the door. For now please just continue to work with your local patch.
Has this patch been commited to Mylyn?
No, it hasn't yet, it's scheduled for Mylyn 3.0.2 which will be released on September 17th. Bumping priority.
Thanks. It's not a show stopper for releasing AJDT 1.6, but it would be very, very nice. We can't advertise this feature if it only half works.
Mik, Wanted to make sure that this will be making it in to Mylyn 3.0.2. We are holding off on our release of AJDT 1.6 final until we have the mylyn structure bridge. Thanks.
Created attachment 111321 [details] patch is reworked to be based off of tag R_3_0_1 Same patch, but reworked to mesh with R_3_0_1. This can be applied directly to the code base and will work with the AspectJ Structure bridge. As discussed before, I am using the extension point internalBridge. When this patch is applied and verified, the internalBridge extension point should be merged into the bridge extension point.
Good stuff Andrew. I have tentatively applied it and incorporated some naming changes of the internals which you should look over. Before resolving this we need a test that exercises the shadowing of the two getStructureBridge(..) methods.
Created attachment 111753 [details] mylyn/context/zip
Thanks for applying the patch. I'll send you the test soon. Probably tomorrow.
Andrew: For an example of a test see the ContextExternalizerTest in the attached context. But your test should be simpler, since it only needs to exercise ContextCorePlugin.
Created attachment 111837 [details] mylyn/context/zip
Noticed this comment in ContextCorePlugin: /** * Recommended bridge registration is via extension point, but bridges can also be added at runtime. Note that only * one bridge per content type is supported. Overriding content types is not supported. */ This is no longer strictly true with Shadowing going on. Should probably change this comment.
Created attachment 111889 [details] passing test case for shadowing structure bridges Thanks for the pointer. I have created my own passing test case.
Has this patch been committed?
The patch with the test case is still pending. I have put an item on next week's conference call to go through the outstanding patches.
I have applied the last patch. Thanks Andrew!
