Bad news: onMethod/onParam/onConstructor are going away.

[Reinier Zwitserloot]

Evidently Joe Darcy and crew fixed the problem of those 3 generating a bunch of fake error messages in javac6.... by generating a bunch of _ACTUAL_ error messages in java 7.

We did some basic checking and it turns out javac 7 will report the errors and quit without ever even invoking lombok. Given that, I don't think we can fix this short of requiring something like: javac -Jjavaagent:lombok.jar to boot up your javac, and as that's a rather strange thing to ask for, and very difficult to integrate in ant and maven and such, not acceptable.

In other words: As nice as the idea was, we're going to remove it from 0.10.0-final ("Burning Emu") unless someone has a bright idea. The feature was technically new for Burning Emu so in theory nobody should be affected. Of course, I know many of you are running on one beta/RC/devoxx prerelease version or other of Burning Emu so we are sorry that we have to drop support for it.

We do have one really weird hacky idea to try and add support back in but it's so hacky we're not sure we want to take on the support burden, and it would definitely take far too long to try and build to delay the release of 0.10.0.

We're planning to release 0.10.0-final "Burning Emu" on Friday 12th of August.... with onX removed entirely.

If we've overlooked the importance of this feature or, even better, someone has an idea about how to make this work on javac 7 we'd love to hear it before then.

NB: Another plan is to ask Joe Darcy about Project Coin-Flip Side, and submitting a proposal for it (or if that won't happen, submit a straight JSR) to allow 'Annotation' as a valid annotation field type, which can take any annotation. That would solve all problems, though it would obviously have to wait until at least Java 8, and getting these kinds of features into java, no matter how simple and useful it might seem, is not exactly something you should be planning for.

[Philipp Eichhorn]

I think it would be nice if we could support marker annotations or
annotations with a very very limited amount of key-value pairs in
future versions (not 0.10.0).
So one can at least do something like this:

@OnMethod({
@Add(VisibleForTesting.class),
@Add(Inject.class)
})

Still this is not as straight forward as I would like it to be. :/

NB: The first idea, on fixing this bad boy, that comes to mind, would
be to do some hacky nasty business with
com.sun.tools.javac.comp.Check#checkType(...) or
com.sun.tools.javac.comp.Annotate#enterAnnotation(...) but I have no
clue if it would actually work.

[Maaartin G]

On Aug 9, 1:46 am, Philipp Eichhorn <peich...@web.de> wrote:
> I think it would be nice if we could support marker annotations or
> annotations with a very very limited amount of key-value pairs in
> future versions (not 0.10.0).
> So one can at least do something like this:
>
> @OnMethod({
>   @Add(VisibleForTesting.class),
>   @Add(Inject.class)
> })

Isn't this just a longer version of

@OnMethod({
VisibleForTesting.class,
Inject.class

})

> Still this is not as straight forward as I would like it to be. :/

Simply placing the annotations on the field and specifying what should
happen to them (place them on getter, setter, setter's argument, ctor,
or leave them at the field, or some combination of the above) would be
more powerful, as it'd allow annotation arguments. The proper syntax
for this is a bit problematic, as discussed in
https://groups.google.com/d/topic/project-lombok/sTff5NiFwWw/discussion

[Reinier Zwitserloot]

On Tuesday, August 9, 2011 1:46:04 AM UTC+2, Philipp Eichhorn wrote:

NB: The first idea, on fixing this bad boy, that comes to mind, would
be to do some hacky nasty business with
com.sun.tools.javac.comp.Check#checkType(...) or
com.sun.tools.javac.comp.Annotate#enterAnnotation(...) but I have no
clue if it would actually work.

It would, but lombok is never even called, _at all_, if javac detects unfixable errors (unfixable by 'standard' APTs at any rate). So, the only way we could mess with checkType/enterAnnotation is to either field our own version of javac, which kind of defeats the point, or require users to spin up lombok as an agent via -Jjavaagent:lombok.jar, but that too mostly defeats the point (though that does give us the freedom to compile, I dunno, QBasic with javac if we want to. We could change whatever we want).

 

[Reinier Zwitserloot]

For no-args annotations, we can just do this:

@Getter(onMethod=Deprecated.class)

no problem. However, this is a bit ugly (very minor concern), occupies the 'onMethod' name for the future if ever we can go back to @Getter(onMethod=@Deprecated) (medium concern), and doesn't let you put any arguments on the annotation (major concern).

We could try and hack this together using string literals, like so:

@Getter(onMethod={@Add(Deprecated.class, arg1="5", arg2="\"Hello World\"")})

but that's so ridiculously ugly, that'll never happen as long as I have some sort of veto right :P

What's really needed here is for a way to mark an annotation 'field' to take any X, where X is either anything legal for an annotation (a primitive, a String, a class, another annotation, or a homogenous array of any of those), or at least to be able to say that any annotation would do. We will try and fight for that as a feature because it seems like a useful idea for not just lombok but annotations in general. That's one of the reasons I'd be mildly concerned if we set in stone a less nice interface for onMethod now; we'd either have to come up with a munged name later or we'd have to break backwards compatibility, which is on the table, but never a nice thing to do).

[Maaartin G]

On Aug 9, 4:04 pm, Reinier Zwitserloot <reini...@gmail.com> wrote:
> For no-args annotations, we can just do this:
>
> @Getter(onMethod=Deprecated.class)
>
> no problem. However, this is a bit ugly (very minor concern), occupies the
> 'onMethod' name for the future if ever we can go back to
> @Getter(onMethod=@Deprecated) (medium concern), and doesn't let you put any
> arguments on the annotation (major concern).

I think, it can be solved by combining two things:

For parameterless annotations use arguments of the lombok annotations
like

Class<? extends Annotation>[] onMethod() default {};

Combine them with annotations on fields like

@Named("blue") private Something a;

in order to specify parameterized annotations. So for example

@Setter(Deprecated.class) private Something a;

leads to placing @Deprecated on the setter, while

@Setter(onMethod=Named.class) @Named("blue") private Something a;

leads to placing @Named("blue") on the setter. At the same time the
annotation gets removed from the field, since AFAIK there's no tool
requiring such duplicities.

This doesn't allow all combinations, e.g, you can't place
@Named("blue") on the setter and @Named("red") on the getter. But
again, I don't think anybody needs it. Compared to the state before
the onX-removal it's a bit more verbose, but not much. It doesn't
allow some strange combinations previously possible, but it allows
some additional things like annotating constructor arguments:

@RequiredArgsConstructor(onParam=Named.class, onMethod=Inject.class)
class SomeFileProcessor {
@Named("src") private final File srcFile;
@Named("dst") private final File dstFile;
}

leads to

class SomeFileProcessor {
@Inject
public SomeFileProcessor(@Named("src") File srcFile, @Named("dst")
File dstFile) {
this.srcFile = srcFile;
this.dstFile = dstFile;
}
private final File srcFile;
private final File dstFile;
}

Something like this is necessary whenever you want to constructor-
inject two different instances of a single class.

[Reinier Zwitserloot]

Won't work; if there's a method-only annotation then javac will still abort with a type error before lombok ever runs.

[Maaartin G]

On Aug 22, 8:10 am, Reinier Zwitserloot <reini...@gmail.com> wrote:
> Won't work; if there's a method-only annotation then javac will still abort
> with a type error before lombok ever runs.

That's sad. Then the only thing remaining for now seems to be using
strings

@Getter(onMethodAsString="@Something(42)")

It's ugly like hell, the autocompletion doesn't work, but I'm sure you
can compile-time check it. It could be combined with parameterless
annotations like

@Getter(onMethodAsString="@Something(42)" onMethod=Deprecated.class)

It could be actually combined with all the above, so it wouldn't get
ugly unless you need to place parametrized on-fields-forbidden
annotations on methods. It's not very nice, I know.

[Reinier Zwitserloot]

The string bit can be done, but, boy, that's ugly. Let's try and petition Joe for a way to add any annotation to annotations instead, see if that pans out.

[joewalp]

Reinier:

What was Joe Darcy's response to the petition to allow 'Annotation' as a valid annotation field type?  Is there a discussion thread about the proposal somewhere?

[Reinier Zwitserloot]

First the good news, well, more or less, because we aren't committed to abusing this hack we found:

The problem is that, in java7, @foo(bar=X), where bar is a real parameter of annotation foo, and X is *NOT* a legal value for it, java6 would still invoke lombok, thus letting us fix the problem, but java7 will abort with a fatal error before invoking annotation processors, which is why the feature went away.

Turns out that if bar _does not exist at all_ as a parameter for foo, java7 WILL invoke annotation processors BEFORE erroring out. And, yes, we can fix it in lombok and compilation will finish normally. This is pretty hacky, crappy from a documentation / auto-complete standpoint (your IDE will act as if @foo has no parameters, or at least, not 'onMethod' and 'onParameter' even though it secretly does), so we're not actually sure we want to take this route to bring it back.

Secondly, on Joe's response: Well, turns out Joe made a bit of a brain fart when he designed the annotation feature and prematurely optimized. The way annotations are stored in classfiles, only the param values of an annotation-in-an-annotation are stored; the annotation's type itself isn't. At class file read time, this information is gleaned from the host annotation. So, this:

@Setter(onMethod=@SuppressWarnings("all")) is stored as:

ANN 'lombok.Setter' {

    onMethod = "all"

}

where reflection / classloader will have to figure out that's supposed to be @SuppressWarnings("all") by looking back at the definition of @Setter itself.

That means our proposal is impossible to implement simply. "old" code that guesstimates intended annotation by looking at the host annotation's members will have to stick around forever to preserve backwards compatibility, and a hack involving extended fields (a feature of the class file format where you can shove any random key/value pair on most nodes which are defined to be ignored unless you as a class-reading entity know what you are doing) would have to be designed to make it work. Joe deems this a big enough change that it's basically guaranteed to NOT happen for 1.8 (as 1.8 already has its plate full with jigsaw and lambda), but he had been thinking about it before and isn't opposed to the idea in general, so, who knows what might happen with 1.9. Still, that's a long wait, and that wasn't exactly a resounding cheer in favour of the idea either.

Joe's response on this is in the coin-dev archives, here: http://mail.openjdk.java.net/pipermail/coin-dev/2011-October/003432.html

My original request is here: http://mail.openjdk.java.net/pipermail/coin-dev/2011-October/003422.html. That's not a real coin request, more a request-to-request, as a real coin request is quite a bit of effort and I wanted to check if there was room in the first place first, and, evidently, there's not.

[joewalp]

Thanks for the comprehensive reply. Like other people [1], I'd be
happy with a solution specific to Guice. If I hack something
together, I'll post details.


[1]
http://groups.google.com/group/project-lombok/msg/4baace6ea7bd0bfe
http://groups.google.com/group/project-lombok/msg/38263474db4ba1da