testcode and patches for pr141730 comments #13,14: "new handleprovider"

diff --git a/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java b/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java
index 8c525b083fd165e8a026d29d2b19045682735f99..cb483acd0bc1cb711d75f40d1ffca1d930ddc39e 100644 (file)
--- a/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java
+++ b/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java
@@ -35,13 +35,7 @@ public class AspectJElementHierarchy implements IHierarchy {
     private Map typeMap = null;
     
        public IProgramElement getElement(String handle) {
-               IProgramElement cachedEntry = (IProgramElement)handleMap.get(handle);
-               if (cachedEntry!=null) return cachedEntry;
-               IProgramElement ret = findElementForHandle(handle);
-               if (ret!=null) {
-                       cache(handle,ret);
-               }
-               return ret;
+               return findElementForHandleOrCreate(handle, false);
        }
 
     public IProgramElement getRoot() {
@@ -332,13 +326,22 @@ public class AspectJElementHierarchy implements IHierarchy {
                this.configFile = configFile;
        }
        
-       // TODO: optimize this lookup
        public IProgramElement findElementForHandle(String handle) {
+               return findElementForHandleOrCreate(handle,true);
+       }
+       
+       // TODO: optimize this lookup
+       // only want to create a file node if can't find the IPE if called through
+       // findElementForHandle() to mirror behaviour before pr141730
+       private IProgramElement findElementForHandleOrCreate(String handle, boolean create) {
                // try the cache first...
                IProgramElement ret = (IProgramElement) handleMap.get(handle);
                if (ret != null) return ret;
                
                ret = findElementForHandle(root,handle);
+               if (ret == null && create) {
+                       ret = createFileStructureNode(getFilename(handle));
+               }
                if (ret != null) {
                        cache(handle,(ProgramElement)ret);
                }

diff --git a/asm/src/org/aspectj/asm/internal/FullPathHandleProvider.java b/asm/src/org/aspectj/asm/internal/FullPathHandleProvider.java
index 6ab254d453df003f20a350fb188d3e3dcf9d55eb..cd7f350b7eb257343bc4e6bdebed6e7340bf86f8 100644 (file)
--- a/asm/src/org/aspectj/asm/internal/FullPathHandleProvider.java
+++ b/asm/src/org/aspectj/asm/internal/FullPathHandleProvider.java
@@ -75,6 +75,7 @@ public class FullPathHandleProvider implements IElementHandleProvider {
        }
 
        public String createHandleIdentifier(IProgramElement ipe) {
+               if (ipe == null) return null;
                if (ipe.getHandleIdentifier(false) != null) {
                        return ipe.getHandleIdentifier(false);
                }

diff --git a/asm/src/org/aspectj/asm/internal/OptimizedFullPathHandleProvider.java b/asm/src/org/aspectj/asm/internal/OptimizedFullPathHandleProvider.java
index e7b36424166d5a52d5d753b9e5c19a0e7ea0db88..c8a8499057540cf189b2e7edad564c7e8920f246 100644 (file)
--- a/asm/src/org/aspectj/asm/internal/OptimizedFullPathHandleProvider.java
+++ b/asm/src/org/aspectj/asm/internal/OptimizedFullPathHandleProvider.java
@@ -99,6 +99,7 @@ public class OptimizedFullPathHandleProvider implements IElementHandleProvider {
        }
 
        public String createHandleIdentifier(IProgramElement ipe) {
+               if (ipe == null) return null;
                if (ipe.getHandleIdentifier(false) != null) {
                        return ipe.getHandleIdentifier(false);
                }

diff --git a/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java b/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java
index 6114bef57fa1c8bfd6d95625ae1c4eb0d08642b6..2d0e1ba6d70975f08cf7cc6ea217fd84943663d6 100644 (file)
--- a/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java
+++ b/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java
@@ -17,12 +17,17 @@ import java.io.FilenameFilter;
 import java.io.IOException;
 import java.io.PrintStream;
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 import java.util.StringTokenizer;
 
 import junit.framework.AssertionFailedError;
 
 
+import org.aspectj.asm.AsmManager;
+import org.aspectj.asm.IProgramElement;
+import org.aspectj.asm.IRelationshipMap;
+import org.aspectj.asm.internal.Relationship;
 import org.aspectj.bridge.AbortException;
 import org.aspectj.bridge.ICommand;
 import org.aspectj.bridge.IMessage;
@@ -431,6 +436,46 @@ public class Ajc {
                }
                return ret;
        }
+       
+       public static void dumpAJDEStructureModel(String prefix) {
+               dumpAJDEStructureModel(prefix, false);
+       }
+       
+       public static void dumpAJDEStructureModel(String prefix, boolean useHandles) {
+               System.out.println("======================================");//$NON-NLS-1$
+               System.out.println("start of AJDE structure model:"+prefix); //$NON-NLS-1$
+
+               IRelationshipMap asmRelMap = AsmManager.getDefault().getRelationshipMap();
+               for (Iterator iter = asmRelMap.getEntries().iterator(); iter.hasNext();) {
+                       String sourceOfRelationship = (String) iter.next();
+                       System.err.println("Examining source relationship handle: "+sourceOfRelationship);
+                       List relationships = null;
+                       if (useHandles) {
+                               relationships = asmRelMap.get(sourceOfRelationship);
+                       } else {
+                               IProgramElement ipe = AsmManager.getDefault().getHierarchy()
+                                                                               .findElementForHandle(sourceOfRelationship);
+                               relationships = asmRelMap.get(ipe);
+                       }
+                       if (relationships != null) {
+                               for (Iterator iterator = relationships.iterator(); iterator.hasNext();) {
+                                       Relationship rel = (Relationship) iterator.next();
+                                       List targets = rel.getTargets();
+                                       for (Iterator iterator2 = targets.iterator(); iterator2.hasNext();) {
+                                               String t = (String) iterator2.next();
+                                               IProgramElement link = AsmManager.getDefault().getHierarchy().findElementForHandle(t);
+                                               System.out.println(""); //$NON-NLS-1$
+                                               System.out.println("      sourceOfRelationship " + sourceOfRelationship); //$NON-NLS-1$
+                                               System.out.println("          relationship " + rel.getName()); //$NON-NLS-1$
+                                               System.out.println("              target " + link.getName()); //$NON-NLS-1$
+                                       }
+                               }
+                               
+                       }
+               }
+               System.out.println("End of AJDE structure model"); //$NON-NLS-1$
+               System.out.println("======================================");//$NON-NLS-1$
+       }
 }
 
 /*

diff --git a/tests/bugs152/pr141730/aspectpath/MyBar.aj b/tests/bugs152/pr141730/aspectpath/MyBar.aj
new file mode 100644 (file)
index 0000000..3a38885
--- /dev/null
+++ b/tests/bugs152/pr141730/aspectpath/MyBar.aj
@@ -0,0 +1,9 @@
+package bar;
+
+public aspect MyBar {
+
+       before() : call(* main(..)) {
+               System.out.println("about to call a main method");
+       }
+       
+}

diff --git a/tests/bugs152/pr141730/aspectpath/MyFoo.java b/tests/bugs152/pr141730/aspectpath/MyFoo.java
new file mode 100644 (file)
index 0000000..d733097
--- /dev/null
+++ b/tests/bugs152/pr141730/aspectpath/MyFoo.java
@@ -0,0 +1,11 @@
+package foo;
+
+public class MyFoo {
+
+       public void callMain() {
+               new MyFoo().main();
+       }
+       
+       public void main() {
+       }
+}

diff --git a/tests/bugs152/pr141730/aspectpath/aspectpath.jar b/tests/bugs152/pr141730/aspectpath/aspectpath.jar
new file mode 100644 (file)
index 0000000..27ccd5a
Binary files /dev/null and b/tests/bugs152/pr141730/aspectpath/aspectpath.jar differ

diff --git a/tests/bugs152/pr141730/inpath/MyAnnotation.java b/tests/bugs152/pr141730/inpath/MyAnnotation.java
new file mode 100644 (file)
index 0000000..16a690c
--- /dev/null
+++ b/tests/bugs152/pr141730/inpath/MyAnnotation.java
@@ -0,0 +1,5 @@
+package bar;
+
+public @interface MyAnnotation {
+
+}

diff --git a/tests/bugs152/pr141730/inpath/MyBar.aj b/tests/bugs152/pr141730/inpath/MyBar.aj
new file mode 100644 (file)
index 0000000..50ea6ef
--- /dev/null
+++ b/tests/bugs152/pr141730/inpath/MyBar.aj
@@ -0,0 +1,18 @@
+package bar;
+
+import foo.*;
+
+public aspect MyBar {
+
+       before() : call(* main(..)) {}
+
+       declare warning: get( * System.out ) : "System.out should not be called";
+       
+       declare parents : *Foo extends NewClass;
+       
+       declare @type : *Foo* : @MyAnnotation;
+       declare @method : public * *Foo.anotMethod(..) : @MyAnnotation;
+       declare @constructor : *Foo.new(String) : @MyAnnotation;
+       declare @field : int *Foo.* : @MyAnnotation ;
+       
+}

diff --git a/tests/bugs152/pr141730/inpath/MyFoo.java b/tests/bugs152/pr141730/inpath/MyFoo.java
new file mode 100644 (file)
index 0000000..d9aba23
--- /dev/null
+++ b/tests/bugs152/pr141730/inpath/MyFoo.java
@@ -0,0 +1,25 @@
+package foo;
+
+public class MyFoo {
+
+       public int i;
+       
+       public MyFoo() {
+               super();
+       }
+       
+       public MyFoo(String s) {
+               super();
+       }
+       
+       public void callMain() {
+               new MyFoo().main();
+       }
+       
+       public void main() {
+               System.out.println("blah");
+       }
+       
+       public void anotMethod() {      
+       }
+}

diff --git a/tests/bugs152/pr141730/inpath/NewClass.java b/tests/bugs152/pr141730/inpath/NewClass.java
new file mode 100644 (file)
index 0000000..cdbce29
--- /dev/null
+++ b/tests/bugs152/pr141730/inpath/NewClass.java
@@ -0,0 +1,5 @@
+package bar;
+
+public class NewClass {
+
+}

diff --git a/tests/bugs152/pr141730/inpath/inpath.jar b/tests/bugs152/pr141730/inpath/inpath.jar
new file mode 100644 (file)
index 0000000..ba262c5
Binary files /dev/null and b/tests/bugs152/pr141730/inpath/inpath.jar differ

diff --git a/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java b/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java
index 3b23af93a14516e64e7bf6392ff7bb3e117cde00..c33abb8ab740ff66e2bd20d53d081d4d19a26427 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java
@@ -12,6 +12,7 @@ package org.aspectj.systemtest.ajc152;
 
 import java.io.File;
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 
 import junit.framework.Test;
@@ -19,9 +20,11 @@ import junit.framework.Test;
 import org.aspectj.asm.AsmManager;
 import org.aspectj.asm.IHierarchy;
 import org.aspectj.asm.IProgramElement;
+import org.aspectj.asm.IRelationshipMap;
 import org.aspectj.asm.internal.Relationship;
 import org.aspectj.testing.XMLBasedAjcTestCase;
 import org.aspectj.util.CharOperation;
+import org.aspectj.weaver.World;
 
 public class Ajc152Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
 
@@ -185,8 +188,105 @@ public class Ajc152Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
          checkSignatureOfIPE("MyClass.MyClass()",IProgramElement.Kind.INTER_TYPE_CONSTRUCTOR);
   }
 
-  
+  // if not filling in the model for aspects contained in jar files then
+  // want to ensure that the relationship map is correct and has nodes
+  // which can be used in AJDT - ensure no NPE occurs for the end of
+  // the relationship with aspectpath
+  public void testAspectPathRelWhenNotFillingInModel_pr141730() {
+         World.createInjarHierarchy = false;
+         try {
+                 runTest("ensure aspectpath injar relationships are correct when not filling in model");
+                 
+                 // expecting:
+                 //    sourceOfRelationship main in MyFoo.java
+          //           relationship advised by
+          //                   target MyBar.aj
+                 // 
+                 // and
+                 //
+                 //    sourceOfRelationship MyBar.aj
+          //           relationship advises
+          //                   target main in MyFoo.java
+
+                 
+                 IHierarchy top = AsmManager.getDefault().getHierarchy();
+                 IRelationshipMap asmRelMap = AsmManager.getDefault().getRelationshipMap();
+                 assertEquals("expected two sources of relationships but only found " 
+                                 + asmRelMap.getEntries().size(),2,asmRelMap.getEntries().size());
+                 for (Iterator iter = asmRelMap.getEntries().iterator(); iter.hasNext();) {
+                         String sourceOfRelationship = (String) iter.next();
+                         IProgramElement ipe = top.findElementForHandle(sourceOfRelationship);
+                         List relationships = asmRelMap.get(ipe);
+                         if (ipe.getName().equals("MyBar.aj")) {
+                                 assertEquals("expected MyBar.aj to have one relationships but found "
+                                                 + relationships.size(),1,relationships.size());       
+                                 Relationship rel = (Relationship)relationships.get(0);
+                                 assertEquals("expected relationship to be 'advises' but was " 
+                                                 + rel.getName(), "advises", rel.getName());
+                                 List targets = rel.getTargets();
+                                 assertEquals("expected one target but found " + targets.size(),1,targets.size());
+                                 IProgramElement link = top.findElementForHandle((String)targets.get(0));
+                                 assertEquals("expected target 'method-call(void foo.MyFoo.main())' but target " + link.getName(),
+                                                 "method-call(void foo.MyFoo.main())",link.getName());
+                                 String fileName = link.getSourceLocation().getSourceFile().toString();
+                                 assertTrue("expected 'main' to be in class MyFoo.java but found it " +
+                                               "in " + fileName,fileName.indexOf("MyFoo.java") != -1);
+                         } else if (ipe.getName().equals("method-call(void foo.MyFoo.main())")) {
+                                 String fileName = ipe.getSourceLocation().getSourceFile().toString();
+                                 assertTrue("expected 'method-call(void foo.MyFoo.main())' to be in " +
+                                               "class MyFoo.java but found it in"
+                                               + fileName,fileName.indexOf("MyFoo.java") != -1);
+                                 assertEquals("expected 'method-call(void foo.MyFoo.main())' " +
+                                               "to have one relationships but found "
+                                                 + relationships.size(),1,relationships.size());       
+                                 Relationship rel = (Relationship)relationships.get(0);
+                                 assertEquals("expected relationship to be 'advised by' but was " 
+                                                 + rel.getName(), "advised by", rel.getName());
+                                 List targets = rel.getTargets();
+                                 assertEquals("expected one target but found " + targets.size(),1,targets.size());
+                                 IProgramElement link = top.findElementForHandle((String)targets.get(0));
+                                 assertEquals("expected target 'MyBar.aj' but target " + link.getName(),
+                                                 "MyBar.aj",link.getName());
+                                 
+                         } else {
+                                 fail("unexpected element " + ipe.getName() + " in the relationship map");
+                         }
+                 }
+         } finally {
+                 World.createInjarHierarchy = true;
+         }
+  }
 
+  // if not filling in the model for classes contained in jar files then
+  // want to ensure that the relationship map is correct and has nodes
+  // which can be used in AJDT - ensure no NPE occurs for the end of
+  // the relationship with inpath
+  public void testNoNPEWithInpathWhenNotFillingInModel_pr141730() {
+         World.createInjarHierarchy = false;
+         try {
+                 runTest("ensure inpath injar relationships are correct when not filling in model");
+                 // the aspect used for this test has advice, declare parents, deow,
+                 // and declare @type, @constructor, @field and @method. We only expect
+                 // there to be relationships in the map for declare parents and declare @type
+                 // (provided the model isn't being filled in) because the logic in the other
+                 // addXXXRelationship methods use AspectJElementHierarchy.findElementForType().
+                 // This method which returns null because there is no such ipe. The relationship is
+                 // therefore not added to the model. Adding declare @type and declare parents
+                 // uses AspectJElementHierarchy.findElementForHandle() which returns the file
+                 // node ipe if it can't find one for the given handle. Therefore the relationships
+                 // are added against the file node. Before change to using ipe's to create handles
+                 // we also had the deow relationship, however, now we don't because this also
+                 // uses findElementForType to find the targetNode which in the inpath case is null.
+                 
+                 // just check that the number of entries is what we expect:
+                 // We expect 3 (the declare @type and declare parents statements plus the filenode)
+                 IRelationshipMap relMap = AsmManager.getDefault().getRelationshipMap();
+                 assertEquals("expected 3 entries in the relationship map but found " 
+                                 + relMap.getEntries().size(), 3, relMap.getEntries().size());
+         } finally {
+                 World.createInjarHierarchy = true;
+         }
+  }
   
 //  public void testFunkyGenericErrorWithITDs_pr126355_2() { 
 //       runTest("bizarre generic error with itds - 2");

diff --git a/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml b/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml
index b91caf7ead4a4cd90831049538f3bba6e72d4230..77e80c7943841baba718d4e443b93ebf1ed9f97e 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml
+++ b/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml
@@ -667,6 +667,16 @@
             </stderr>
         </run>
     </ajc-test>
+
+       <ajc-test dir="bugs152/pr141730/aspectpath" title="ensure aspectpath injar relationships are correct when not filling in model">
+      <compile files="MyFoo.java" aspectpath="aspectpath.jar" options="-emacssym"/>
+    </ajc-test>
+
+       <ajc-test dir="bugs152/pr141730/inpath" title="ensure inpath injar relationships are correct when not filling in model">
+      <compile files="MyBar.aj, MyAnnotation.java, NewClass.java" inpath="inpath.jar" options="-1.5 -emacssym">
+                       <message kind="warning" line="20" text="System.out should not be called"/>
+         </compile>
+    </ajc-test>
     
    <ajc-test dir="ltw" title="weaveinfo messages with include and exclude">
       <compile files="EmptyTest1.java, EmptyTest2.java"/>

diff --git a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
index 1271a6bea6c3730fd5f6a61fbd5b160617dff0fd..1f5793688480c0fc899615e4fb9533ac5fe92d0a 100644 (file)
--- a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
+++ b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
@@ -31,6 +31,7 @@ import org.aspectj.asm.IRelationship;
 import org.aspectj.asm.IRelationshipMap;
 import org.aspectj.asm.internal.Relationship;
 import org.aspectj.bridge.IMessage;
+import org.aspectj.tools.ajc.Ajc;
 import org.aspectj.weaver.World;
 
 /**
@@ -79,14 +80,14 @@ public class MultiProjectIncrementalTests extends AbstractMultiProjectIncrementa
                        configureBuildStructureModel(true);
                        initialiseProject("P4");
                        build("P4");
-                       dumpAJDEStructureModel("after full build where advice is applying");
+                       Ajc.dumpAJDEStructureModel("after full build where advice is applying");
                        // should be 4 relationship entries
        
                        // In inc1 the first advised line is 'commented out'
                        alter("P4","inc1");
                        build("P4");
                        checkWasntFullBuild();
-                       dumpAJDEStructureModel("after inc build where first advised line is gone");
+                       Ajc.dumpAJDEStructureModel("after inc build where first advised line is gone");
                        // should now be 2 relationship entries
                        
                        // This will be the line 6 entry in C.java
@@ -1440,38 +1441,6 @@ public class MultiProjectIncrementalTests extends AbstractMultiProjectIncrementa
                                        " in the aop.xml file but found " + aspectCount + " occurrences");
                }
        }
-       
-       
-       private void dumpAJDEStructureModel(String prefix) {
-               System.out.println("======================================");//$NON-NLS-1$
-               System.out.println("start of AJDE structure model:"+prefix); //$NON-NLS-1$
-
-               IRelationshipMap asmRelMap = AsmManager.getDefault().getRelationshipMap();
-               for (Iterator iter = asmRelMap.getEntries().iterator(); iter.hasNext();) {
-                       String sourceOfRelationship = (String) iter.next();
-                       IProgramElement ipe = AsmManager.getDefault().getHierarchy()
-                                                                       .findElementForHandle(sourceOfRelationship);
-                       System.err.println("Examining source relationship handle: "+sourceOfRelationship);
-                       List relationships = asmRelMap.get(ipe);
-                       if (relationships != null) {
-                               for (Iterator iterator = relationships.iterator(); iterator.hasNext();) {
-                                       Relationship rel = (Relationship) iterator.next();
-                                       List targets = rel.getTargets();
-                                       for (Iterator iterator2 = targets.iterator(); iterator2.hasNext();) {
-                                               String t = (String) iterator2.next();
-                                               IProgramElement link = AsmManager.getDefault().getHierarchy().findElementForHandle(t);
-                                               System.out.println(""); //$NON-NLS-1$
-                                               System.out.println("      sourceOfRelationship " + sourceOfRelationship); //$NON-NLS-1$
-                                               System.out.println("          relationship " + rel.getName()); //$NON-NLS-1$
-                                               System.out.println("              target " + link.getName()); //$NON-NLS-1$
-                                       }
-                               }
-                               
-                       }
-               }
-               System.out.println("End of AJDE structure model"); //$NON-NLS-1$
-               System.out.println("======================================");//$NON-NLS-1$
-       }
 
        private File getProjectRelativePath(String p,String filename) {
                File projDir = new File(getWorkingDir(),p);

diff --git a/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java b/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java
index 033503e5354b964493e37e18e6a34a82f7462a56..e68696a77cc313be9a13a9cdf5f0a1214fd8654e 100644 (file)
--- a/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java
+++ b/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java
@@ -54,27 +54,29 @@ public class AsmRelationshipProvider {
                if (shadow.getSourceLocation() == null || checker.getSourceLocation() == null) return;
                
                // Ensure a node for the target exists
-               IProgramElement targetNode = getNode(AsmManager.getDefault().getHierarchy(), shadow);
+               IProgramElement targetNode = getNode(AsmManager.getDefault().getHierarchy(),shadow);
+               if (targetNode == null) return;
+               String targetHandle = AsmManager.getDefault().getHandleProvider()
+                               .createHandleIdentifier(targetNode);
+               if (targetHandle == null) return;
+
                IProgramElement sourceNode = AsmManager.getDefault().getHierarchy()
                                .findElementForSourceLine(checker.getSourceLocation());
                String sourceHandle = AsmManager.getDefault().getHandleProvider()
                                .createHandleIdentifier(sourceNode);
-               String targetHandle = AsmManager.getDefault().getHandleProvider()
-                               .createHandleIdentifier(targetNode);
+               if (sourceHandle == null) return;
                                
                if (World.createInjarHierarchy) {
                        checker.createHierarchy();
                }
 
                IRelationshipMap mapper = AsmManager.getDefault().getRelationshipMap();
-               if (sourceHandle != null && targetHandle != null) {
-                       IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE, MATCHED_BY,false,true);
-                       foreward.addTarget(targetHandle);
+               IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE, MATCHED_BY,false,true);
+               foreward.addTarget(targetHandle);
                                
-                       IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE, MATCHES_DECLARE,false,true);
-                       if (back != null && back.getTargets() != null) {
-                               back.addTarget(sourceHandle);
-                       }
+               IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE, MATCHES_DECLARE,false,true);
+               if (back != null && back.getTargets() != null) {
+                       back.addTarget(sourceHandle);
                }
        }
 
@@ -85,34 +87,32 @@ public class AsmRelationshipProvider {
                ResolvedType originatingAspect) {
 
          if (!AsmManager.isCreatingModel()) return;
-               String sourceHandle = "";
-               if (munger.getSourceLocation()!=null && munger.getSourceLocation().getOffset()!=-1) {
-                       IProgramElement sourceNode = AsmManager.getDefault().getHierarchy()
-                                       .findElementForSourceLine(munger.getSourceLocation());
-                       sourceHandle = AsmManager.getDefault().getHandleProvider()
-                                       .createHandleIdentifier(sourceNode);
-               } else {
-                       IProgramElement sourceNode = AsmManager.getDefault().getHierarchy()
-                                       .findElementForSourceLine(originatingAspect.getSourceLocation());
-                       sourceHandle = AsmManager.getDefault().getHandleProvider()
-                                       .createHandleIdentifier(sourceNode);
-               }
                if (originatingAspect.getSourceLocation() != null) {
+                       String sourceHandle = "";
+                       if (munger.getSourceLocation()!=null && munger.getSourceLocation().getOffset()!=-1) {
+                               IProgramElement sourceNode = AsmManager.getDefault().getHierarchy()
+                                               .findElementForSourceLine(munger.getSourceLocation());
+                               sourceHandle = AsmManager.getDefault().getHandleProvider()
+                                               .createHandleIdentifier(sourceNode);
+                       } else {
+                               IProgramElement sourceNode = AsmManager.getDefault().getHierarchy()
+                                               .findElementForSourceLine(originatingAspect.getSourceLocation());
+                               sourceHandle = AsmManager.getDefault().getHandleProvider()
+                                               .createHandleIdentifier(sourceNode);
+                       }
+                       if (sourceHandle == null) return;
                        IProgramElement targetNode = AsmManager.getDefault().getHierarchy()
                                        .findElementForSourceLine(onType.getSourceLocation());
                        String targetHandle = AsmManager.getDefault().getHandleProvider()
                                        .createHandleIdentifier(targetNode);
-                               
+                       if (targetHandle == null) return;
+                       
                        IRelationshipMap mapper = AsmManager.getDefault().getRelationshipMap();
-                       if (sourceHandle != null && targetHandle != null) {
-                               IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE_INTER_TYPE, INTER_TYPE_DECLARES,false,true);
-                               foreward.addTarget(targetHandle);
-//                             foreward.getTargets().add(targetHandle);
-                               
-                               IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE_INTER_TYPE, INTER_TYPE_DECLARED_BY,false,true);
-                               back.addTarget(sourceHandle);
-//                             back.getTargets().add(sourceHandle);  
-                       }
+                       IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE_INTER_TYPE, INTER_TYPE_DECLARES,false,true);
+                       foreward.addTarget(targetHandle);
+                       
+                       IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE_INTER_TYPE, INTER_TYPE_DECLARED_BY,false,true);
+                       back.addTarget(sourceHandle);
                }
        }
        
@@ -123,20 +123,20 @@ public class AsmRelationshipProvider {
                                .findElementForSourceLine(decp);
                String sourceHandle = AsmManager.getDefault().getHandleProvider()
                                .createHandleIdentifier(sourceNode);
+               if (sourceHandle == null) return;
+               
                IProgramElement targetNode = AsmManager.getDefault().getHierarchy()
                                .findElementForSourceLine(targetType.getSourceLocation());
                String targetHandle = AsmManager.getDefault().getHandleProvider()
                                .createHandleIdentifier(targetNode);
+               if (targetHandle == null) return;
                
                IRelationshipMap mapper = AsmManager.getDefault().getRelationshipMap();
-               if (sourceHandle != null && targetHandle != null) {
-                       IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE_INTER_TYPE, INTER_TYPE_DECLARES,false,true);
-                       foreward.addTarget(targetHandle);
+               IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE_INTER_TYPE, INTER_TYPE_DECLARES,false,true);
+               foreward.addTarget(targetHandle);
                                
-                       IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE_INTER_TYPE, INTER_TYPE_DECLARED_BY,false,true);
-                       back.addTarget(sourceHandle);
-               }
-               
+               IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE_INTER_TYPE, INTER_TYPE_DECLARED_BY,false,true);
+               back.addTarget(sourceHandle);
        }
        
        /**
@@ -150,20 +150,20 @@ public class AsmRelationshipProvider {
                        .findElementForSourceLine(declareAnnotationLocation);
            String sourceHandle = AsmManager.getDefault().getHandleProvider()
                        .createHandleIdentifier(sourceNode);
+           if (sourceHandle == null) return;
 
            IProgramElement targetNode = AsmManager.getDefault().getHierarchy()
                                .findElementForSourceLine(annotatedLocation);
            String targetHandle = AsmManager.getDefault().getHandleProvider()
                                .createHandleIdentifier(targetNode);
-                               
+               if (targetHandle == null) return;
+           
                IRelationshipMap mapper = AsmManager.getDefault().getRelationshipMap();
-               if (sourceHandle != null && targetHandle != null) {
-                       IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATES,false,true);
-                       foreward.addTarget(targetHandle);
+               IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATES,false,true);
+               foreward.addTarget(targetHandle);
                                
-                       IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATED_BY,false,true);
-                       back.addTarget(sourceHandle);
-               }
+               IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATED_BY,false,true);
+               back.addTarget(sourceHandle);
        }
        
        public void adviceMunger(IHierarchy model, Shadow shadow, ShadowMunger munger) {
@@ -179,12 +179,14 @@ public class AsmRelationshipProvider {
 
                        IRelationshipMap mapper = AsmManager.getDefault().getRelationshipMap();
                        IProgramElement targetNode = getNode(AsmManager.getDefault().getHierarchy(), shadow);
+                       if (targetNode == null) return;
                        boolean runtimeTest = ((BcelAdvice)munger).hasDynamicTests();
                        
                        // Work out extra info to inform interested UIs !
                        IProgramElement.ExtraInformation ai = new IProgramElement.ExtraInformation();
 
                        String adviceHandle = advice.getHandle(); 
+                       if (adviceHandle == null) return;
                        
                        // What kind of advice is it?
                        // TODO: Prob a better way to do this but I just want to
@@ -195,27 +197,20 @@ public class AsmRelationshipProvider {
                        if (adviceElement != null) {
                                adviceElement.setExtraInfo(ai); 
                        }
-                       
-                       if (adviceHandle != null && targetNode != null) {
-               
-                               if (targetNode != null) {
-                    String targetHandle = targetNode.getHandleIdentifier(); 
-                    if (advice.getKind().equals(AdviceKind.Softener)) {
-                        IRelationship foreward = mapper.get(adviceHandle, IRelationship.Kind.DECLARE_SOFT, SOFTENS,runtimeTest,true);
-                        if (foreward != null) foreward.addTarget(targetHandle);//foreward.getTargets().add(targetHandle);
+            String targetHandle = targetNode.getHandleIdentifier(); 
+            if (advice.getKind().equals(AdviceKind.Softener)) {
+               IRelationship foreward = mapper.get(adviceHandle, IRelationship.Kind.DECLARE_SOFT, SOFTENS,runtimeTest,true);
+                if (foreward != null) foreward.addTarget(targetHandle);//foreward.getTargets().add(targetHandle);
                         
-                        IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE, SOFTENED_BY,runtimeTest,true);
-                        if (back != null)     back.addTarget(adviceHandle);//back.getTargets().add(adviceHandle);
-                    } else {
-                                       IRelationship foreward = mapper.get(adviceHandle, IRelationship.Kind.ADVICE, ADVISES,runtimeTest,true);
-                                       if (foreward != null) foreward.addTarget(targetHandle);//foreward.getTargets().add(targetHandle);
+                IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE, SOFTENED_BY,runtimeTest,true);
+                if (back != null) back.addTarget(adviceHandle);//back.getTargets().add(adviceHandle);
+            } else {
+               IRelationship foreward = mapper.get(adviceHandle, IRelationship.Kind.ADVICE, ADVISES,runtimeTest,true);
+                       if (foreward != null) foreward.addTarget(targetHandle);//foreward.getTargets().add(targetHandle);
                                        
-                                       IRelationship back = mapper.get(targetHandle, IRelationship.Kind.ADVICE, ADVISED_BY,runtimeTest,true);
-                                       if (back != null)     back.addTarget(adviceHandle);//back.getTargets().add(adviceHandle);
-                    }
-                }
-                       }
-
+                       IRelationship back = mapper.get(targetHandle, IRelationship.Kind.ADVICE, ADVISED_BY,runtimeTest,true);
+                       if (back != null)     back.addTarget(adviceHandle);//back.getTargets().add(adviceHandle);
+            }
                }
        }
 
@@ -381,22 +376,22 @@ public class AsmRelationshipProvider {
          if (methodElem == null) return;
          
          try {
+                 String targetHandle = methodElem.getHandleIdentifier();
+                 if (targetHandle == null) return; 
                  
                  IProgramElement sourceNode = AsmManager.getDefault().getHierarchy().findElementForSourceLine(sourceLocation);
                  String sourceHandle = AsmManager.getDefault().getHandleProvider().createHandleIdentifier(sourceNode);
-                               
-           String targetHandle = methodElem.getHandleIdentifier();
-           IRelationshipMap mapper = AsmManager.getDefault().getRelationshipMap();
-               if (sourceHandle != null && targetHandle != null) {
-                               IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATES,false,true);
-                               foreward.addTarget(targetHandle);
+                 if (sourceHandle == null) return;     
+                 
+                 IRelationshipMap mapper = AsmManager.getDefault().getRelationshipMap();
+                 IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATES,false,true);
+                 foreward.addTarget(targetHandle);
                                        
-                               IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATED_BY,false,true);
-                               back.addTarget(sourceHandle);
-               }
-        } catch (Throwable t) { // I'm worried about that code above, this will make sure we don't explode if it plays up
-          t.printStackTrace(); // I know I know .. but I don't want to lose it!
-        }
+                 IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATED_BY,false,true);
+                 back.addTarget(sourceHandle);
+       } catch (Throwable t) { // I'm worried about that code above, this will make sure we don't explode if it plays up
+          t.printStackTrace(); // I know I know .. but I don't want to lose it!
+       }
        }
        
     /**
@@ -422,19 +417,19 @@ public class AsmRelationshipProvider {
         IProgramElement fieldElem = AsmManager.getDefault().getHierarchy().findElementForSignature(typeElem,IProgramElement.Kind.FIELD,field.getName());
         if (fieldElem== null) return;
 
-                 IProgramElement sourceNode = AsmManager.getDefault().getHierarchy().findElementForSourceLine(sourceLocation);
-                 String sourceHandle = AsmManager.getDefault().getHandleProvider().createHandleIdentifier(sourceNode);
-                       
                String targetHandle = fieldElem.getHandleIdentifier();
+               if (targetHandle == null) return;
+        
+               IProgramElement sourceNode = AsmManager.getDefault().getHierarchy().findElementForSourceLine(sourceLocation);
+               String sourceHandle = AsmManager.getDefault().getHandleProvider().createHandleIdentifier(sourceNode);
+               if (sourceHandle == null) return;       
                
         IRelationshipMap mapper = AsmManager.getDefault().getRelationshipMap();
-               if (sourceHandle != null && targetHandle != null) {
-                       IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATES,false,true);
-                       foreward.addTarget(targetHandle);
+               IRelationship foreward = mapper.get(sourceHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATES,false,true);
+               foreward.addTarget(targetHandle);
                                
-                       IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATED_BY,false,true);
-                       back.addTarget(sourceHandle);
-               }
+               IRelationship back = mapper.get(targetHandle, IRelationship.Kind.DECLARE_INTER_TYPE, ANNOTATED_BY,false,true);
+               back.addTarget(sourceHandle);
        }
        
 }