Bug 258510 - [plan] Remove evil mutator methods from the type abstraction
Summary: [plan] Remove evil mutator methods from the type abstraction
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: DEVELOPMENT   Edit
Hardware: PC Windows Vista
: P3 normal (vote)
Target Milestone: 1.6.4   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-12-11 13:26 EST by Andrew Clement CLA
Modified: 2009-01-20 17:42 EST (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Clement CLA 2008-12-11 13:26:20 EST
The type abstraction that exists in org.aspectj.matcher includes mutators for:
- adding annotations
- changing superclass
- adding superinterfaces

These exist because we sometimes need to temporarily add them whilst matching at compilation time (eg. a declare @type that would subsequently cause a declare parents to match).  Before weaving the types involved are 'reset' to remove anything done at compile time.  But it means anyone providing an implementation for their type system (eg. writing a JDTWorld) sees these wierd methods and doesnt know what to do about them.  They should be removed, they should be pushed up into the matchers own infrastructure and the implementation of the type abstraction interfaces (ReferenceType, ResolvedMember) should be immutable objects.
Comment 1 Andrew Clement CLA 2008-12-11 13:26:54 EST
quite serious so not going to do this in 1.6.3 - but definetly for 1.6.4
Comment 2 Andrew Clement CLA 2008-12-11 13:34:06 EST
getJoinpointSignatures() shouldn't be in the resolvedmember hierarchy i dont think - an implementor of resolvedmember hasn't a clue what to do.
Comment 3 Andrew Clement CLA 2008-12-11 13:45:16 EST
joinpoint processing casts the member to ResolvedMemberImpl - shouldn't really do that unless subclassing ResolvedmemberImpl is the required pattern...
Comment 4 Andrew Clement CLA 2008-12-17 21:20:07 EST
moving the annotations from the delegate to the referencetype is straightforward.  moving the superclass and implemented interfaces is more challenging - and the logic to create a new generic signature based on these changes doesn't seem entirely correct, I'll write some tests to flush out the bugs.

I also think we should move the aspect type specific methods into a separate interface.  getting declares/itds/etc from a referencetype will only make sense for very few types - the aspect types - so I dont think the typical delegate implementor should be forced to implement those get methods.
Comment 5 Andrew Clement CLA 2009-01-20 17:42:23 EST
They have been removed.  It is always nice to delete code like this:

public void addAnnotation(AnnotationAJ annotationX) {
	throw new UnsupportedOperationException(
			"What on earth do you think you are doing???");
}

Extra annotations and temporary replacement superclasses and new interfaces are held at the ReferenceType level.  So the delegates themselves now have no mutators and no need to implement a horrible method called 'ensureConsistent()'.

It is *possible* we might see a regression with declare annotation or declare parents, but all our tests are OK so hopefully not...