Community
Participate
Working Groups
Aspects which come from injars are currently not in the model. Instead, whenever the model is asked for the IPE corresponding to advice which is in an injar then an IPE for the source file is returned. The consequence is within AJDT the relationships are marked as coming from "injar aspect <filename>".
Created attachment 43978 [details] zip containing first installment of implementation The attached zip contains three patches: - pr145963-asm-patch.txt: apply to the asm project - pr145963-tests-patch.txt: apply to the tests project - pr145963-weaver-patch.txt: apply to the weaver project Together these patches are the start of filling in the model for aspects which are contained in jar files. Currently, it creates a hierarchy for advice and deow statements. I've also created a separate testclass to the Ajc152Tests and ajc152.xml. Currently they don't hold many tests however this will grow in the future as more cases are covered (ITD's,@AJ, declare annotation etc) and I felt it was cleaner to keep them separate.
A further note.....to flag that this part of the hierarchy has been faked the node corresponding to the source file (which comes under the package node) has "(binary)" added to it's name. So, we expect something like: - pkg - A.aj (binary) - import declarations - A - pcd - before Also note that we currently assume there are no imports. Other cases which need testing are abstract aspects. Finally, there is a boolean flag World.createInjarHierarchy which is used to determine whether or not we want to fake in the hierarchy. This is by default set to false.
Created attachment 44793 [details] ensuring filled in ipes are in relationship map The attached zip contains 3 patches: - pr145963-asm-patch.txt: apply to the asm project - pr145963-tests-patch.txt: apply to the tests project - pr145963-weaver-patch.txt: apply to the weaver project Together these patches ensure that the relationship map is correct i.e. includes relationships for the filled in nodes (still only advice and deow)
patches from comment #3 in
Created attachment 45664 [details] ensure the label of filled in advice is correct This zip contains 3 patches: - pr145963-weaver-patch.txt: apply to the weaver project - pr145963-tests-patch.txt: apply to the tests project - pr145963-ajdt-core-patch.txt: apply to the org.aspectj.ajdt.core project Together these patches ensure the labels of the filled in advice is correct as well as ensuring only one file node is created for each aspect rather than one per piece of advice/deow (ooooops :-)).
the zip files in the tests patch are corrupt when I apply them - so the tests fail. And unfortunately there is no README in the directory (or a build.bat file) that tells me how to reconstruct those jars. Ideally we should always have a way to rebuild the jars (even if it is a manual step) in case the binary format of the aspect changes mid-release.
Created attachment 46709 [details] tests patch containing readme detailing how to recreate the jar files Apply this patch to the tests project. The difference between this patch and the tests patch in comment #5 is the addition of a readme detailing how to regenerate the jar files.
patches are in... I'd rather have had the commands listed in the readme that rebuild the jars than some AJDT instructions. I've added what they might be to the readme but don't know for certain because the latter compiles might need earlier jars on the classpath when building. helen ... can you please change your template for creating new files so it doesnt say 'iniital version', i've changed it toooooooooooo many times ;)
Firstly, just to note that the flag World.createInjarHierarchy now defaults to true in AspectJ, although ajdt.core currently sets it to false on startup, until we're ready to use it. With the flag enabled I see the nodes I expect to, for advice coming from a separate project (via aspectpath). However the binary node has a source of "bar\MyBar.aj:7::0" whereas source nodes have full absolute paths for their location. I don't think there is enough information in the binary node for AJDT to determine which project (if any) it comes from. I was expecting to see a full path, and for AJDT to have to try to map that to a project.
Created attachment 47843 [details] zip containing patches for getting sloc correct The attached zip contains the following patches/files: * pr145963-ajde.txt: Apply to the ajde project. This adds two more tests to the ShowWeaveMessagesTestCase to include the case where the binary aspect is in a package and when the aspectpath contains a directory rather than a jar file. There have also been changes to the info messages and the corresponding 'expected' files have been updated * AspectInPackage.clss: This is part of the ajde testdata and is in case it was corrupted whilst creating the patch. Create a folder called 'pkg' within ajde\testdata\WeaveInfoMessagesTest and put this class file in there * AspectInPackage.jar: This is part of the ajde testdata and again is in case it was corrupted whilst creating the patch. Place within the ajde\testdata\WeaveInfoMessagesTest folder. * pr145963-ajdt-core.txt: Apply to the org.aspectj.ajdt.core project. Provides default implementation of the new ISourceLocation method getSourceFileName() * pr145963-asm.txt: apply to the asm project. This adds a reset method for the AsmManager reporting mechansim to be used by the new model tests, as well as promoting removeChild(IPE) to the IProgramElement interface and providing a bit more manipulation when creating the file nodes within AspectJelementHierarchy (this is so more heavy weight manipulation isn't required when filling in the nodes) * pr145963-bridge.txt: apply to the bridge project. This adds an extra method getSourceFileName() onto the ISourceLocation interface and an implementation for SourceLocation (so we can get hold of the filename XXX.aj to give this info in weaveinfo messages) * pr145963-testing.txt: apply to the testing project. This provides default implementation of the new ISourceLocation method getSourceFileName(). Also, removes the call to ajc.setShouldEmptySandbox(true) in the runTest() method. With this call in the testcase can't decide not to empty the sandbox. This is required for the new model test infrastructure. * pr145963-tests.txt: apply to the tests project. Creates new model test infrastructure and provides tests for this feature * pr145963-weaver.txt: apply to the weaver project. Adds private field 'binaryPath' and corresponding setter/getting to ResolvedType so we can keep hold of where the aspect came from if its binary. This is set within BcelWeaver.addAspectsFromJarFile() and BcelWeaver.addIfAspect(). Also, changes the implementation of faulting in the hierarchy for binary aspects to ensure the sourcelocation points to the .class file. Together these patches ensure that the model for a BinaryAspect will be something like this: BinaryAspect.class (binary) [class] C:\temp\ajcSandbox\pr145963R\ajcTest14018.tmp\simple.jar!pkg\BinaryAspect.class:1::0 import declarations [import reference] BinaryAspect [aspect] C:\temp\ajcSandbox\pr145963R\ajcTest14018.tmp\simple.jar!pkg\BinaryAspect.class:1::0 p() [pointcut] C:\temp\ajcSandbox\pr145963R\ajcTest14018.tmp\simple.jar!pkg\BinaryAspect.class:7::0 before(): p.. [advice] C:\temp\ajcSandbox\pr145963R\ajcTest14018.tmp\simple.jar!pkg\BinaryAspect.class:9::0 declare warning: "There should be n.." [declare warning] C:\temp\ajcSandbox\pr145963R\ajcTest14018.tmp\simple.jar!pkg\BinaryAspect.class:5::0 This fix has introduced changes to weaveinfo messages. Now, if we're advised by a binary aspect in a .jar file we have messages like: Join point 'method-execution(void Simple.m1())' in Type 'Simple' (Simple.java:5) advised by before advice from 'AspectAdvice' (AspectAdvice.jar!AspectAdvice.class:8(from AspectAdvice.aj)) and if it comes from a folder: Join point 'method-execution(void Simple.m1())' in Type 'Simple' (Simple.java:5) advised by before advice from 'pkg.AspectInPackage' (AspectInPackage.class:7(from AspectInPackage.aj))
I've integrated this. Switched things around a bit... I didn't like having the sandbox in the text for the expected output files - so I added a ModelFilter that can be used to transform the model when it is dumped. Tests use this to transform the sandbox directory to a fixed TEST_SANDBOX. I expected the comparison logic for the model to compare the models line by line, rather than the approach that is taken here which is set comparison (ie. order of lines is irrelevant) - obviously we get away with it here, but it just means we can't have a nice error that says "the models differ on line XXX, I expected <this> line but got <this> line". If we did do line comparison I know we'd have to change the relmap dumping to produce a reliable ordering. Some of the showweaveinfo messages look odd, things like: Softening exceptions in type 'Simple' (no debug info available) as defined by "aspect 'AspectDeclareSoft' (AspectDeclareSoft_nodebug.jar!AspectDeclareSoft.class:4(from AspectDeclareSoft.aj))" but let's go with this for now - may need to change it if users get confused. i'm not keen on the getSourceFile() and getSourceFileName() returning entirely different things (in ISourceLocation) but can't immediately think of a better name for the latter. It is a shame so much had to change in order for this to work, I hope AJDT can benefit!!!!!!!
Created attachment 48269 [details] patch to ensure model tests work on linux Apply this patch to the tests project. This patch ensures that the model tests pass on both windows and linux regardless of which os was used to generate the expected output.
patch committed. Also had to change it because at home my temp dir is C:\TEMP and the dir created for me is C:\temp - so I made it case insensitive for the sandbox.
Matt - these changes are now in the latest AJDT
can close once Matt confirms they do vaguely what he wants.
AJDT is now using this support, and it does indeed appear to be doing vaguely the right thing :)
great :-) Closing as per comment #15 and comment #16.
