Re: @Inject on generated constructors

[Reinier Zwitserloot]

We're still not convinced our hack is worth doing, because it feels like the kind of thing that is going to fail in an upcoming java version.

We did write up an entire patch for javac (including updated documentation and java language spec, tests, and analysis of impact) to make this an official part of java, but this was shot down under the motto 'we don't have time for this'. After which Joe Darcy introduced a bunch of annotations anyway, so I guess Oracle just says they want contributions, but they don't actually mean it. Shame.

http://mail.openjdk.java.net/pipermail/coin-dev/2012-February/003452.html

Roel might be able to cheerlead our proposal at Devoxx this week. Joe Darcy will be there, and surely some other movers and shakers from Oracle will also be there. If we can get this sponsored as a JEP, that would be a _much_ better path to (re)introducing the onX feature.

Let's await the results at devoxx before making further plans.

On Monday, November 12, 2012 10:28:17 AM UTC+1, David Barri wrote:

Hi! When using @RequiredArgsConstructor there currently doesn't seem to be a way to have an @Inject annotation added to the generated constructor.

I see that a year ago, the functionality was in place but it was temporarily removed due to an issue with Java 7.

Ref: https://groups.google.com/d/topic/project-lombok/u4rfrhPlKns/discussion

What is the current status of this, does anyone know? Does it live on a unmerged branch somewhere? Is it coming anytime soon? Is there a way I can enable it for Java 6?

Thanks,

David

[David Barri]

Wow, that's some great work! I'm impressed at the evident careful consideration!

Hopefully all goes well at Devoxx this week, I'd love to hear how it goes.

If things don't work out, would a viable workaround be to provide the annotation class itself (rather than an annotation instance as in the proposal), or even plain text? I'm thinking that way the @XxxArgs annotations could just reproduce the pass the generated/given text to javac when generating the constructor.

I'm thinking something like

@RequiredArgsConstructor(annotateWith = javax.annotation.Nonnull.class)

or

@RequiredArgsConstructor(annotateWith = "@SuppressWarnings(\"all\")")

[Roel Spilker]

Using a class means you can only use marker annotations, but you already understood that. The "we parse the String" also has a few problems. The developer cannot use code completion AND needs to use fully qualified names, so "@java.lang.SuppressWarnings(\"all\")", which, due to the lack of code completions is suboptimal.

--
 
 

[David Barri]

I whole-heartedly agree that the string approach would be suboptimal, and that the class approach has limitations.

At the same time, I also think suboptimal would be better than nothing. Without the perfect solution and without any workarounds people are only really left with two choices: don't use the otherwise-awesome lombok constructor-functionality, or modify the preferred class design to avoid the problem. I'd love the spec you've proposed to be approved, I just hope there's a solution #2 if that road is closed.

Re workarounds, admittedly there is also a third option: patch lombok! :D

Which I just did hehehe. I made a quick hack very specific to my current project's requirements:

https://github.com/japgolly/lombok/commit/e1d3a5678e203ae9bd951bc24e044dd4d68b4eae

--
 
 

[Maaartin G]

On Tuesday, November 13, 2012 1:24:41 PM UTC+1, Roel Spilker wrote:

Using a class means you can only use marker annotations, but you already understood that. The "we parse the String" also has a few problems. The developer cannot use code completion AND needs to use fully qualified names, so "@java.lang.SuppressWarnings(\"all\")", which, due to the lack of code completions is suboptimal.

You can combine it and get something nearly readable:

// the easy case

@RequiredArgsConstructor(annotation=Inject.class)

// no problem IMHO

// single argument packed in a String

@RequiredArgsConstructor(annotation=SuppressWarnings.class, args="\"all\"")

// costs additional escaped quotes

// multiple arguments as a String[]

@RequiredArgsConstructor(annotation=Sequence.class, args={"1", "2", "4", "8"})

// this generates @Sequence(1, 2, 4, 8)

// with nested annotations it gets worse

@RequiredArgsConstructor(annotation=Top.class, args={"Bottom(\"a\")", "Bottom(\"b\")"})

// this generates @Top(Bottom("a"), Bottom("b"))

But the most common cases are quite fine and as already said "suboptimal would be better than nothing".

[Reinier Zwitserloot]

We'd have to create a new lombok. package, probably 'lombok.workaround' or something, to make it crystal clear that this thing is going away, preferably sooner rather than later.

Is there a way we can do this with a 'proxy' - a field / method / type that you make JUST so you can shove the annotations on there, so that you can then reference that and say: That stuff? I guess it would be far too much boilerplate to create a dummy method just as a place to dump annotations.

[Maaartin G]

On Saturday, November 17, 2012 4:52:24 AM UTC+1, Reinier Zwitserloot wrote:

Is there a way we can do this with a 'proxy' - a field / method / type that you make JUST so you can shove the annotations on there, so that you can then reference that and say: That stuff? I guess it would be far too much boilerplate to create a dummy method just as a place to dump annotations.

Probably yes, unless the annotation is extremely complicated. This thread started with `@Inject`, which can be specified simply like

@AllArgsConstructor(annotateWith="Inject")

class C {

...

}

and the placeholder idea could look like

@AllArgsConstructor

class C {

@Inject

private lombok.ConstructorAnnotationPlaceholder placeholder;

...

}

i.e. simply terrible (moreover, it doesn't work for annotations which are allowed on constructors only).

The second most important Guice annotation is probably `@Named("someName")`. You may want to generate

class FileHelper {

@Inject

FileHelper(@Named("from") File from, @Named("to") File to) {

this.from = from;

this.to = to;

}

private final File from;

private final File to;

...

}

The most natural way is

@AllArgsConstructor(annotateWith="Inject", moveAnnotations={"Named"})

class FileHelper {

@Named("from") private final File from;

@Named("to") private final File to;

}

which would work as `@Named` is allowed on fields too. If it wasn't, something like

@AllArgsConstructor(annotateWith="Inject")

class FileHelper {

@AnnotateConstructorArgumentWith(value=Named.class, args="\"from\"") private final File from;

@AnnotateConstructorArgumentWith(value=Named.class, args="\"to\"")  private final File to;

}

would be acceptable (1). As a template, you'd need something like

@AllArgsConstructor(annotateWith="Inject")

class FileHelper {

private lombok.ConstructorArgumentAnnotationPlaceholder placeholder(@Named("from") File from, @Named("to") File to) {}

private final File from;

private final File to;

}

which is way worse than writing the constructor manually. I can imagine that the placeholder idea can work well somewhere, but I'm afraid I've just demonstrated it to be unusable in these two important cases.

___

(1) I was assuming a simplified notation in case there's only the "value" argument; otherwise something like

args="value=\"from\""

or in the general  case

args={"min=1", "max=10", "name=\"abc\""}

would be needed. Not very nice.

[Reinier Zwitserloot]

We've got this tentatively working on javac, but the syntax is horribly cartoon sweary. The point is, _IT WORKS_, and it is the one and only thing we could come up with that actually works in javac ('works' defined as: We get called. Other alternatives simply error out without ever triggering annotation processors, i.e. lombok):

@AllArgsConstructor(onConstructor=@_({@Inject, @Named("atUnderscoreReallyYesReally")})

The good news is, unlike earlier reports, we CAN actually put onConstructor (and onMethod and onParam) into the annotations, so you'll see them with auto-complete and such. The bad news is that you need to use @_. This is syntax-technically an annotation of type '_', which does not exist and is not provided by lombok. The fact that it does not exist means javac will trigger APs just in case one of the APs generates the _ annotation class. None will, but it gives us the opportunity to fix things. Hence the @_.

To avoid confusion, lombok enforces @_, even though technically it could be @WhateverIdentifierYouWantAsLongAsItIsNotAnExistingType. Just in case _ is an actual type in your system, any number of underscores is allowed.

We're still writing more tests and have to produce the eclipse variant. Right now if you get it wrong you get a ridiculous amount of errors from javac (with a lombok-provided error listing the proper syntax buried amongst them) and we're going to try and address that if at all possible.

This feature will be in 'workaround' state - we expect that in future javac versions this may no longer work, and we may remove it with no reasonable deprecation period. Because, boy, it's ugly.

 --Reinier Zwitserloot

--
 
 

[Maaartin G]

On Monday, November 19, 2012 12:20:23 PM UTC+1, Reinier Zwitserloot wrote:

This feature will be in 'workaround' state - we expect that in future javac versions this may no longer work, and we may remove it with no reasonable deprecation period. Because, boy, it's ugly.

Indeed, it is, but it's surely nice enough for a workaround.

Looking at the commit "onX-removal" I see that there were no support for placing annotations on constructor arguments. I guess, something like

@OnConstructorParam(@_={@Named("from")})

private final File from;

should be possible, right? Now comes the question on "which constructor?". Concerning Guice, it's needed on the one annotated with `@Inject`, but placing it elsewhere too does no harm.

A side note: There's `NoArgsConstructor` but there was `Setter.onParam`. Wouldn't choosing one of "arg" and "param" make sense?

[David Barri]

Haha wow, thanks amazing! Nice work Reinier and lombok team :D

--
 
 

[Reinier Zwitserloot]

On Tuesday, November 20, 2012 12:05:37 AM UTC+1, Maaartin G wrote:

Looking at the commit "onX-removal" I see that there were no support for placing annotations on constructor arguments. I guess, something like

@OnConstructorParam(@_={@Named("from")})

private final File from;

We weren't planning on this, but, you make a good point. I'm guessing that at least for now we'll just roll with: That would apply to any and all generated constructors. If you want an annotation on a field-based parameter, but only on some but not all generated constructors, I think we can safely say we moved beyond boilerplate territory.

 

A side note: There's `NoArgsConstructor` but there was `Setter.onParam`. Wouldn't choosing one of "arg" and "param" make sense?

Huh, hadn't thought about that. 'no arguments constructor' is java jargon so we can't mess with that one, but we can change onParam to onArg. (and thus, @OnConstructorParam to @OnConstructorArg). The JLS itself uses the terms mostly interchangeably  but to be precise, 'parameter' refers to the 'foo' in "public void println(String foo)", and 'argument' refers to the "foo" in "System.out.println("foo")". In that sense, onParam is actually correct, but calling it a @NoParamConstructor is a bad idea because 'no args constructor' is the terminology that is used waaaay more. It makes sense too, even: As a caller of the constructor I care that I don't have to pass anything, and as caller, I call them 'arguments'. You, as declarer, calls these things 'parameters', of course.

So, our current style uses common jargon if available, and if not, the most appropriate term, but, unfortunately, that means there's a mix. I think I prefer our current mix, but, maybe I'm wrong. Anyone else have feelings on this matter?

[Krzysztof Makowski]

Hi,

I would like also to have Lombok generated constructor with javax.inject.Inject annotation but I do not like @_(@ ) syntax. I wonder if it would be more sensible to have @lombok.InjectingRequiredArgsConstructor annotation with no need to provide onConstructor parameter. I would expect also that after adding this annotation to my class, all annotations provided to my private final fields (like @Named("blabla")) would be copied as generated constructor arguments annotations.

So class

@InjectingRequiredArgsConstructor

class Sample {

    @Named("blabla")

    private final SampleA sample1;

    private final SampleB sample2;

}

would be equivalent to:

class Sample {

    private final SampleA sample1;

    private final SampleB sample2;

    public Sample( @Named("blabla") SampleA sample1, SampleB sample2) {

        this.sample1 = sample1;

        this.sample2 = sample2;

    }

}

PS. This is my first post in this group so I would like to say thank you to all lombok commiters - this is a great library :)

[Martin Grajcar]

On Fri, May 30, 2014 at 12:00 PM, Krzysztof Makowski <krz...@gmail.com> wrote:

Hi,

I would like also to have Lombok generated constructor with javax.inject.Inject annotation but I do not like @_(@ ) syntax. I wonder if it would be more sensible to have @lombok.InjectingRequiredArgsConstructor annotation with no need to provide onConstructor parameter.

It would be perfect, if custom annotations (issue 567) would work, but given how complicated the compiler hacking is, I can't see it happening anytime soon. I'm curious what the project authors say.

Maybe Lombok Configuration could help here, though it's unclear what the syntax should be.

A hardwired annotation would work, but I can imagine such requests flooding Lombok. Additionally, the behavior of @InjectingRequiredArgsConstructor isn't obvious. Should it add @javax.inject.Inject like you wrote or rather @com.google.inject.Inject which I use (for whatever reason)? This could be easily solved via Configuration.

I would expect also that after adding this annotation to my class, all annotations provided to my private final fields (like @Named("blabla")) would be copied as generated constructor arguments annotations.

Concerning @Named, it should be most probably moved it to the constructor, rather than copied. Again, this could be hardwired, but what about other @BindingAnnotations? I'm afraid that recognizing them is impossible.

 

Maybe the more verbose and more general solution like I proposed here would be good enough (vote for issue 661). I guess it so simple that I could probably hack it myself.

[Martin Grajcar]

I implemented a part of it in

https://github.com/Maaartinus/lombok-clone/commit/ae1b6787e0860649689dda9f9a73b7395b7f8c62

However, it doesn't work in two cases as the next commit shows

https://github.com/Maaartinus/lombok-clone/commit/de877aa1415d159fe12f4bd5fe6f6fd39efc73d4

I'm using unboxAndRemoveAnnotationParameter, which eats the data. Because of that, it fails in cases like

@OnConstructor(onParams=@__(SomeAnnotation)) int x, y;

and also when multiple constructors set the annotated field.

I need either to write a version of the method not removing the parameter or to store the data elsewhere (FieldAugment?). What do you recommend?

Finally, I should remove the whole annotation. This is related to Issue 146. I guess the best solution would be a final pass removing all Lombok annotations? The question is how?

Could creating some FakeAnnotationHandler running last be the way? As there's no @FakeAnnotation, will it get run at all?

Or should I in createConstructor check both the annotation and the FieldAugment (or wherever the information gets move to)?

Or should I write HandleOnConstructor, which runs early, removes the annotation, and stores it elsewhere?

So many questions, but actually I just need to show the direction.

Regards,

Martin.

[Martin Grajcar]

Hi Reinier and Roel,

I really could use some advice. Could you kindly tell me if removing the annotation upfront and storing the information in a FieldAugment on the class node is a good idea. If not, please point me to another direction.

Regards,

Martin.

[Roel Spilker]

Hi Martin, 

Sorry we didn't react to you before.

I Without looking at the code I don't feel confortable to give you any advice. I think I'll be able to look at it tomorrow. Maybe Reinier can already answer you.

Roel

--
You received this message because you are subscribed to the Google Groups "Project Lombok" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-lombo...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Reinier Zwitserloot]

The FieldAugment trick works; another way to do this is to make 2 layers of handlers: One handler (or possibly set of handlers if you smear out the duties of this job to for example the HandleConstructor inner classes) does all the copy work and does NOT delete any of it, and another handler with an extremely high priority (just below @PrintAST and above everything else most likely) just deletes them. That avoids having to augment.