Bug 389752 - declare parents & @type not matching on annotation properties of enum types
Summary: declare parents & @type not matching on annotation properties of enum types
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: 1.7.1   Edit
Hardware: Macintosh Mac OS X
: P3 normal (vote)
Target Milestone: 1.7.2   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-17 14:47 EDT by Matthew Adams CLA
Modified: 2012-09-20 12:23 EDT (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 Matthew Adams CLA 2012-09-17 14:47:17 EDT
NOTE:  real project private git repo demonstrating bug has been shared with Andy Clement privately.  I'm reporting bug as Andy requested with as much info as I feel I can.

I have a case where 'declare parents' and 'declare @type' instructions are not being applied because their type patterns, which use annotations whose properties are of enumerated types, are not matching.

Below find the relevant artifacts and their shapes.  The lines in the aspect PersistableJpaAspect that are not matching as they should are the 'declare parents' and 'declare @type'.  If I change the annotation properties to be of type String and change the type patterns to use string literals (see "matches" comments in annotation:  StoreType.JPA => "JPA" and IdType.LONG => "LONG"), the matching works properly.

enums:
======

public enum StoreType {
	MONGO, JDO, JPA;
}

=====

public enum IdType {
	LONG, STRING;
}

annotation:
===========

public @interface Persistable {

	StoreType in() default StoreType.MONGO; // doesn't match
	// matches: String in() default "MONGO";

	IdType id() default IdType.STRING; // doesn't match
	// matches: String id() default "STRING";
}

aspect:
=======

public abstract privileged aspect PersistableAspect {

	public interface I extends ....trait.interfaces.persistence.Persistable {
		long version();
		void version(long version);
	}

	public interface L extends I {
		Long idLong();
		void idLong(Long id);
	}

	public interface S extends I {
		String idString();
		void idString(String id);
	}
	
	declare @type : I+ : @Configurable;

	// ...
}

=====

public privileged aspect PersistableJpaAspect extends PersistableAspect {

	public interface JL extends L {
	}

	public interface JS extends S {
	}

	declare parents :
        (@Persistable(id = IdType.LONG, in = StoreType.JPA) *)
        implements JL;

	declare parents :
        (@Persistable(id = IdType.STRING, in = StoreType.JPA) *)
        implements JS;

	declare @type : @Persistable(in="JPA") JL+ : @Entity;
	declare @type : @Persistable(in="JPA") JS+ : @Entity;

	// ...
}
Comment 1 Andrew Clement CLA 2012-09-19 13:56:48 EDT
can you possibly give me a commit hash that shows the problem? I don't really want to go editing too much of the app to recreate.  I can change the decps but surely I then need to change all the targets to be using enum values too? Or do all the targets have double annotations on right now for the enum and string variant values?
Comment 2 Matthew Adams CLA 2012-09-19 13:58:37 EDT
I think HEAD can be used.  I'll add a quick test and let you know.  Hang tight & I'll advise when committed.
Comment 3 Matthew Adams CLA 2012-09-19 14:22:24 EDT
Ok, pushed commit e0a11efa3c6adc8d6ccc26b4229e7ae32772fd91 on master.  You should see failing tests.
Comment 4 Andrew Clement CLA 2012-09-19 14:49:17 EDT
thanks. I have checked that out and have failures but am having a little trouble understanding the setup.

I see the PersistableAspect, doing things like:

	declare parents :
        (@Persistable(idAsEnum = IdType.LONG, in = StoreType.JPA) *)
        implements JS;

But I don't see anywhere annotated with 

@Persistable(idAsEnum = IdType.LONG, in = StoreType.JPA) 

I only see:

@Persistable(id = "LONG", in = "JPA")

do I need to change those places myself to idAsEnum=IdType.LONG?
Comment 5 Matthew Adams CLA 2012-09-19 14:55:58 EDT
Committed change 9f3d93c66884 on master.  Sorry 'bout that!
Comment 6 Andrew Clement CLA 2012-09-19 19:41:59 EDT
All fixed.  This was due to a couple of things - bad code paths when multiple annotation values specified and an ordering issue too (if the annotated element hadn't yet gone through the compilation pipeline, the eclipse version of the annotation value was not creating the right value that would match the pattern).

I still had to change a couple of things in your codebase though.  One of your Mongo test classes still specified it was JPA in the annotation.  Also in your patterns you must be careful because right now this will not give you an error:

String in();
EnumType inAsEnum();

declare parents: (@Persistable(in = StoreType.JPA) *) ...

note that a string value will accept anything, it will not give you the error saying can't use enum value there.  This is tricky to fix (but I've created a testcase for the problem).

With the fixes and the changes to correct the app, the tests all pass.
Comment 7 Andrew Clement CLA 2012-09-19 19:42:12 EDT
closing
Comment 8 Matthew Adams CLA 2012-09-19 20:44:23 EDT
Omg -- sorry again. I was trying to hurry while supporting production for another client.  Copy/paste error.  Thanks for fixing.
Comment 9 Andrew Clement CLA 2012-09-19 22:46:02 EDT
no problem. It is my fault that the error messages aren't coming out. If they were then you would have known it was broken!
Comment 10 Matthew Adams CLA 2012-09-20 12:23:26 EDT
I grabbed 1.7.2.BUILD-SNAPSHOT this morning & everything tested ok.