Hi, it seems the reason for the build fail it is that there's some Tests that requires that the process failed. Right now as explained in #1978 the process is failing in the last round... it seems this is not being detected...

I've run several tests in projects and it seams to be working fine.

Could you anyway please check why they are failing exactly?.. maybe it is something else.

Thanks

Thank you for testing and enhancing my PR. I'll try to test this on my project and check whats wrong with the tests as soon as possible. But currently I have a lot of projects for my job and in my free time. I hope I can get into it this weekend! :)

I just closed my version of this PR as we should not have two PR for the same issue. Please ensure that you have checked the Allow edits from maintainers. checkbox on the bottom of the right menu in this PR. so we can directly contribute to your PR to fix it.

Yes, the option is checked.. Didn't know it existed, sorry maybe I was able to work in your PR.

I think this option is fairly new. but you would not have been able to work in my PR as it requires write access to the androidannoations repo to get permission to write on PR branches when this option is checked.

It looks like the problems with the test are related to the Messager used in our MessagerAppender.

When its printMessage is called during processing it internally has a JavaFileObject as source that is used to set the required source field in the Diagnostic that is checked in ProcessorTestHelper to check if the error is present in the correct source file.

I'll see if i can find a way to keep this source state for the deffered adding of the erros that you implemented. But I'm afraid that this might limit our compiler support as the implementation of the Messager interface is part of the compiler used (in my test case it is com.sun.tools.javac.processing.JavacMessager of the javac compiler).

Ok, I guess sacrificing the tests is not an option neither... I simply didn't find a way to keep the compiler working at least that the report was done in that last round... not sure if making it in the prev-last round would make difference, but that would be hard to detect from within the framework.

I'm not sure of this, but isn't there a way to send information to the test from within the annotation processor?... of course that would require to change those tests...

The problem is that we not only break the tests with this behaviour but also e.g. eclipse would no longer be able to show the error on the annotation as it currently is as it is missing the information about the source file that has the error.

Making it like this for Android Studio only would work? The real problem that we are solving with this PR is exactly there.

Well, as soon as IntelliJ / Android Studio starts to show errors on the source/annotation it would not be able to do this for AndroidAnnotations as the required information here is missing. :(

Well, I guess if it is applied to Android Studio, it would be to IntellyJ in general.. not sure how to detect that exactly, but well, between eclipse and IntellyJ the biggest difference is the folder structure..

Mmm.. well, in that moment, this fixed is not needed anymore... right now IMHO, this is something serious, to be no able to detect the bugs, or have to make that process of watching the console, or going to the log file (I have it opened all the time in NotePad++ for instance) it is really annoying for programming... I really wouldn't like to omit this fix.. of course, maybe there's some other way to make it work..

Maybe not completely good idea, and it sounds more like a "patch" to this issue... but well, it can be created a Flag to pass as parameter to the framework which activates this behavior..

It is even an issue in CLI builds as the javac does not know the files where the error is added.

See this screenshot as an example. the blue line should be part of the MainActivity.java block. building on CLI looks the same.

Mmm.. well, in that moment, this fixed is not needed anymore...

well it would still be required as on bigger projects not generating source might again turn the whole project red and makes the source of the issue hard to find.

Yes that's right.. but this was something that I didn't understand, but now that you mention it, maybe there's a simple solution to it.. in the code, what was done, it is that the error is reported, by a mechanism in which I don't see the Element..

This is the original code (which I didn't modify):

for (Element annotatedElement : annotatedElements) {
	ElementValidation elementValidation = annotationHandler.validate(annotatedElement);

	AnnotationMirror annotationMirror = elementValidation.getAnnotationMirror();
	for (ElementValidation.Error error : elementValidation.getErrors()) {
		LOGGER.error(error.getMessage(), error.getElement(), annotationMirror);
	}

	for (String warning : elementValidation.getWarnings()) {
		LOGGER.warn(warning, elementValidation.getElement(), elementValidation.getAnnotationMirror());
	}

	if (elementValidation.isValid()) {
		validatedAnnotatedElements.add(annotatedElement);
	} else {
		LOGGER.warn("Element {} invalidated by {}", annotatedElement, annotationName);
	}
}

Maybe it would be enough if we instead of reporting all the errors separated and the warning separated, we could create a single String and reported in the last step with the major Level (so if there's Info and Warns, a Warn would be reported, if there's Error, then this is what would be reported).

I will try this I will let you know what happens then.

Mmm.. no, I see they are almost the same... the main difference is that the last one uses de annotated alement and a Warn, which is reported correctly...

Ok, I got it completely now.. there's no easy work around.. since the JavaFileObject exists only in that round, probably you cannot reference it in a different round...

But in general JavaFileObjects can be created and passed to other methods.. not sure if that could be a possible solution, I would have to see more of your research about it...

Well, a different approach it is to report all as a Warning and fail in the last round (this will solve what you point of the place the Error is being generate)..

But of course, it still doesn't solve what the Tests do....

Please note that I simply passed a reference to the Element and Annotation Mirror obtained in the first round, to the last one.... I'm just thinking if trying to create an an element in the last round pointing to the same place would make any difference...

I do not think that creating a Element or something is a solution as we are in java compiler (java/ecj/whatever) internal api where the relevant stuff happens... :(

I would have to see more of your research about it...

well there is not much to know. I found that the Messager is com.sun.tools.javac.processing.JavacMessager and it sets the related source in lines 98-105. and this source is required by ides and the test to know the file related to the warning/error.

unfortunately i haven't found a solution for this problem yet. :(

I got here some good result with something, I will let you know if I can get more improvements ;)

What have you done to reach this? maybe I'll can try too

I'm still testing this... what I did was to get the elements from the TypeElement:

I use getElement to send the message

        private class Message {
		Kind kind;
		String message;
		AnnotationMirror annotationMirror;
		
		List <String> elements = new LinkedList<>();

		Message(Kind kind, String message, Element element,
				AnnotationMirror annotationMirror) {
			super();
			this.kind = kind;
			this.message = message;
			this.annotationMirror = annotationMirror;
			
			Element enclosingElement = element;
			do { 
				elements.add(0, enclosingElement.toString());
				enclosingElement = enclosingElement.getEnclosingElement();
			} while (!enclosingElement.getKind().equals(ElementKind.PACKAGE));
			
			System.out.println("ELEMENTS: " + elements);
		}

		public Element getElement() {
			Element element = processingEnv.getElementUtils().getTypeElement(elements.get(0));
			elements.remove(0);
			
			while (elements.size() > 0) {
				for (Element elem : element.getEnclosedElements()) {
					if (elem.toString().equals(elements.get(0))) {
						System.out.println("FOUND: " + elem);
						elements.remove(0);
						element = elem; 
						break;
					}
				}
			}
			
			return element;
		}

	}

The test is getting stuck.. I'm checking this:

To be honest I'm not sure why is needed the AnnotationMirror... well, I will try to generate it also from the element

I think we could tackle this quite easily. Where we are depending on generated code, we should introduce a new check, if the generating element is valid.
I think we can do this, because generating handlers are always running before the decorating handlers. So in the decorating handlers, we could check if the referenced element has any errors, and if yes, we signal an error on the decorated element as well.

I think we have to do this for @App, @Bean, @RestService and @Pref.

It is good idea, but I'm not sure to be honest if this will solve the initial problem better... I mean, if you make this validation this means that the activity will be not generated due to its dependency (a Bean) will not be generated. You could be using that Activity (or Fragment, or Service, or... ) from many other places, which will generate as well a chain of errors (this is what this PR was trying to reduce, the huge amount of errors generated while compiling)...

I'm not saying it is incorrect, just that it would be nice to weight what is more important here... once again, the PR was to reduce the amount of errors and let the programmer to focus in a few of them...

@smaugho the Activity itself will be generated. The code which injects the bean into the activity will not be generated in it.

@smaugho again, if we introduce these checks, we do not leak error to the user, because the Activity will be generated. Only we do not generate the bean injection code inside. So there will be no MyActivity_ not found error.

yeap yeap, got it :).. sounds nice, just didn't see the first message... so basically the @bean will do nothing if the generating handler for the @ebean failed...

Yep. I am wondering if there are other cases where on annotation is depending on the other?

From AA, I don't remember another...

@WonderCsabo are you implementing this already?

It would be good to invalidate @bean when the "linked" @ebean validation failed... but I don't find a way to know this at priori just in the validation phase... at least that you pass the "validatedAnnotatedElements" as parameter to the validations in the Annotation Handler...

Any better idea?

I am experimenting with this, yeah.

Ok, I guess you have it cracked :)... probably enough getting the validated elements from the AndroidAnnotationEnvironment..

@smaugho i think i managed to do it. Here is my branch. Can you reset your branch to mine?

Also @dodgex @smaugho please review my changes.

@WonderCsabo you should have write access to the branch of this PR.

@WonderCsabo Does your code work with incremental compilation?

@dodgex thanks, i did not know that, it is cool!

@dodgex yes it works, i actually checked it, that is why i introduced the extractedModel into the AndroidAnnotationsEnvironment. (If you compile using IntelliJ "make" and not maven, it will use incremental).

Did we update all licences? :)

I think all licenses are up to date. but I can check that when updating the PR. how to go with the getElement?

You missed to update the licenses but i fixed that. Also I made the ElementDetails static.

Finally merged. Let's hope it will not introduce problems. Thanks for your work on this, @smaugho !

You're are welcome! Would like to be able to contribute much more to AA... lacking of time :'( ...