Lombok: [FEATURE] Optional Parameters
I hope this is not a duplicate, but I could not find anything similar. It would be great if Lombok could be used to generate different versions of a method with and without optional parameters. For this, a new annotation @optional(default) could be introduced that can be added to parameters of methods (or constructors). For example, consider the "lomboked" method:
public void doSomething(String a, @optional(42) int b, @optional(false) boolean c) {
// method body
}
The default value itself could be optional, too, resulting in defaults like 0, false, or null depending on the type of the parameter. This would result in 4 different versions of the method (in general: 2^(number of @optional) versions):
public void doSomething(String a) {
doSomething(a, 42, false);
}
public void doSomething(String a, int b) {
doSomething(a, b, false);
}
public void doSomething(String a, boolean c) {
doSomething(a, 42, b);
}
public void doSomething(String a, int b, boolean c) {
// method body
}
Whereas the following would result in an error and thus would not be allowed, as there are two optional parameters of the same type (without a non-optional parameter in between) so the different methods would have the same signature:
public void doesNotWork(A a, @optional(42) int b, @optional(23) int c) {
Optionally, optional parameters could also be paired or grouped. E.g. in the following example, there would only be two versions of the method: One with all parameters, and one with only the required parameters, but none with the individual optional parameters. (This would also allow for multiple optional parameters of the same type in a row.)
public void doSomething(String a, @optional(42, group=0) int b, @optional(false, group=0) boolean c) {
// method body
}
Often Java code contains many versions of the same methods (or constructors) with different parameters, some of which are optional. Usually, there is one "full" version of the method that gets called by all the others with appropriate default values. With the @optional(defaultValue) annotation this would not be needed any more.
t-kuester
All 31 comments
I like the idea. However, it seems that it is impossible because you can't have "untyped" annotation parameters.
There has been a discussion on Google Groups a few years ago on this suggestion.
janrieke
on 19 Dec 2019
I understand, that's a pity. I assume a parameter type of "Object" would not be possible? Still, my main point was about the parameters being optional, the default value (the main point of the Google groups thread) was actually an afterthought, so I guess one could still have a parameter-free @optional annotation and default-values such as 0, false, and null? (Not sure if it's all that much use then, though. Or maybe multiple (optional) parameters, e.g. @optional(defaultInt=42) and @optional(defaultStr="Foo")? Or maybe use a (generic) wrapper class for either the optional parameters themselves or their defaults? @optional(OptVal<Integer>(42)) int b
t-kuester
on 19 Dec 2019
Is it more elegant?
I'm interested in what you said, it feels great.
So I added this feature to my plugin.
Implementing this is simple, just like above, although this statement may not apply to most people.
2300 Ignored for a long time, they look too busy.
So I plan to completely replace lombok with my plugin.
Unfortunately, everything I wrote was based on the latest JDK version. (Currently JDK13, JDK14 will be followed soon)
NekoCaffeine
on 22 Dec 2019
I understand, that's a pity. I assume a parameter type of "Object" would not be possible?
No, annotations are very limited in what arguments are allowed.
Or maybe use a (generic) wrapper class for either the optional parameters themselves or their defaults?
@optional(OptVal<Integer>(42)) int b
This isn't allowed either.
@Optional(defaultInt=42)
This is somewhat ugly, but correct.
Is it more elegant?
Up to a point when there are multiple arguments of the same type. Or castable to the same type. Then it gets messy pretty soon.
I'd by much more conservative and allow "optional tails" only (i.e., you can omit up to n last arguments).
What you're doing with
void func(String name[] = {"foo", "bar")) {...}
won't work in Lombok nor in any annotation processor as it's a Java syntax error (annotation processors can't fix it as they don't get run in such a case). This unfortunately means that you have to integrate your plugin in every tool you use, which may be too much work.
Maaartinus
on 24 Dec 2019
You are right, so I plan to provide two modes, one is power set derivation and the other is optional tail, the complexity of which is 2 ^ n and n + 1 respectively.
which may be too much work.
Yes, but I already support Javac and IDEA. In addition to this syntax change, I also support global method extensions and operator overloading.
I will push this implementation soon, which may be a reference for you or others to modify the syntax level.
NekoCaffeine
on 24 Dec 2019
I have a counterproposal to the syntax, which might solve the issue of annotation parameters being limited to certain types. Instead of one fixed annotation (@Default), there could be several, for all the common types (all primitive types, String, Array). The annotation would then e.g. be @DefaultInt(42).
For all other types, a possible solution would be referencing a static final field, that is declared somewhere in the class (or anywhere else, available at compile time):
private static final MagicalProperty magProp = Something.magicalProperty;
public String foo(String someString, @DefaultVal("magProp") MagicalProperty magProp, ...) {
// ...
}
For such cases, an @DefaultNull could also be useful, as null is more often than not an allowed parameter fallback.
Up to a point when there are multiple arguments of the same type. Or castable to the same type. Then it gets messy pretty soon.
Correct. This is indeed a problem, when deciding on which parameter can be omitted. I would agree that tail-first should be used. In cases e.g. where there are two optional String-parameters directly after one another, the later should be omitted, when only one String is passed. However, for edge cases, I would suggest an @AllDefaults annotation, which generates different method names, thus preventing collision. An example could be:
@AllDefaults
public String foo(@DefaultNull String someString, @DefaultNull String someOtherString) { ...
could generate the signatures:
public String foo(String someString, String someOtherString) { ...
public String foo(String someString) { ...
public String foo_SomeOtherString(String someOtherString) { ...
Smarter people than I will most likely find a more suitable pattern to generate the post fixes on.
TreffnonX
on 30 Dec 2019
We're never, ever, going to add @DefaultInt and friends. What if you want to have a default value for a type that isn't allowed in annotations (which only allows class literals, string literals, primitive literals, specifically typed annotation literals, and arrays of any of the previous, and that is all... and null is never allowed either)? Also, making that many @DefaultX is ugly as sin.
The one way I can foresee is something like:
private static final LocalDate SENTINEL_DATE = LocalDate.of(1800, 1, 1);
public void whatever(String a, String b, LocalDate c, LocalDate d) {
Lombok.defaultValue(b, "Hello");
Lombok.defaultValue(d, SENTINEL_DATE);
}
or perhaps:
private static final LocalDate SENTINEL_DATE = LocalDate.of(1800, 1, 1);
public void whatever(String a, String b, LocalDate c, LocalDate d) {
defaultValues: {
b = "Hello";
d = SENTINEL_DATE;
}
and this would generate the following 2 additional methods:
whatever(String, String, LocalDate) // d is defaulted
whatever(String, LocalDate) // b and d is defaulted
it would not generate the variant with b defaulted; that is a combinatorial explosion and type clash I don't wanna deal with.
I'm open for other proposals on syntax, but please first check that it is syntactically valid (It can be semantic gobbledygook, but if it doesn't parse, there's nothing we can do!) – and that it caters to this SENTINEL_DATE case I'm working on here (which, as far as I can tell, @DefaultInt and friends cannot do, thus, those are vetoed).
rzwitserloot
on 8 Jan 2020
@rzwitserloot I like your version better too, but assumed, it would be simpler to implement, and more in the spirit of lombok to solve it via annotations alone. That said, I didn't suggest invalid syntax. I suggested DefaultXonly for:
all the common types (all primitive types, String, Array)
Nevertheless, the code block and static-function call you suggested offer a great benefit. The block you suggested:
private static final LocalDate SENTINEL_DATE = LocalDate.of(1800, 1, 1);
public void whatever(String a, String b, LocalDate c, LocalDate d) {
Lombok.defaultValue(b, "Hello");
Lombok.defaultValue(d, SENTINEL_DATE);
}
Could also be written as:
public void whatever(String a, String b, LocalDate c, LocalDate d) {
Lombok.staticDefault(b, "Hello");
Lombok.staticDefault(d, LocalDate.of(1800, 1, 1));
}
where the static final LocalDate-Object could be implicitly generated, if required for compilation, or ommitted alltogether. Unless someone tries to use dynamic values to create LocalDate, this will work, otherwise, it could report a syntactical error.
There could be a separate method dynamicDefault which would not trigger the generation of a static field, but instead generate the value dynamically on each function call:
public void whatever(String a, String b, LocalDate c, LocalDate d) {
Lombok.staticDefault(b, "Hello");
Lombok.dynamicDefault(d, c.plusDays(1));
}
Lombok.dynamicDefault(d, c.plusDays(1)); could just "resolve" to a simple assignment.
TreffnonX
on 8 Jan 2020
I'm already regretting how complex this gets; it's not very discoverable.
What happens if we find a halfway solution:
public void whatever(String a, @DefaultValue String b, LocalDate c, @DefaultValue LocalDate d) {
.....
}
Here the @DV marked params will result in overloads, and if you write nothing further, the values passed in will be null/0/0.0/'\0'/false.
You may optionally specify different values, using Lombok.defaultValue() statements as written here. lombok automatically figures out whether the defaults are static or dynamic, by checking if you reference any other field. So, Lombok.defaultValue(d, c.plusDays(1)) is dynamic because you use c.
rzwitserloot
on 15 Jan 2020
@rzwitserloot
You may optionally specify different values, using
Lombok.defaultValue()statements as written here. lombok automatically figures out whether the defaults are static or dynamic, by checking if you reference any other field. So,Lombok.defaultValue(d, c.plusDays(1))is dynamic because you usec.
I'm unsure about the need for static default values. Most of time, they're primitives or stings, i.e., usually compile time constants. Cases like LocalDate.of(1800, 1, 1) or Math.sqrt(2) are rare enough so that the user can always defined them as static constants themselves.
I wonder what's the intended semantics of
public void whatever(String a, @DefaultValue String b, LocalDate c, @DefaultValue LocalDate d) ...
Note that the third argument lacks the annotation.
(1) As we don't want the combinatorial explosion, it can't mean that methods should be generated by leaving out b, d and both of them.
(2) It could be understood as a request to generate
public void whatever(String a, String b, LocalDate c) ...
public void whatever(String a) ...
and leave out the intermediate
public void whatever(String a, String b) ...
as there's no annotation on LocalDate c. This could be interpreted as having no default unless when forced by leaving out also b.
(3) Another possibility could be to ignore the lack of annotation, but this leads to multiple ways of achieving the same outcome. Not nice.
(4) Making it mandatory could be a way (when an argument is annotated, the all following must be annotated as well). This would leave the path to (2) open.
(5) Forbidding it sounds better (at most one argument can be annotated, and the behavior for the following ones is implied).
Maaartinus
on 15 Jan 2020
Here the @DV marked params will result in overloads, and if you write nothing further, the values passed in will be null/0/0.0/'0'/false.
This is actually a very plausible idea. I like it a lot. Only one question should be answered beforehand. What is the empty default for a String? Is it null or ""?
lombok automatically figures out whether the defaults are static or dynamic, by checking if you reference any other field. So, Lombok.defaultValue(d, c.plusDays(1)) is dynamic because you use c.
While I would like the simplicity of the idea, I am unsure, if this hides a relevant information from the developer. If my value is dynamically generated (as opposed to statically) then one invokation of whatever might (or might not) return a different default value than another one, even if the input is the same, which might cause issues with equals, == and therefore mappings, as the developer would not know, whether the returned value is actually identical, equal, or neither. Also, in some cases, a developer might require an explicit new instance, for example, if an ID is implicitly generated. For the same reason, in other cases, an accidental new instance might cause the same kind of problems.
An explicit static setter would ensure compile time transparency.
(1) As we don't want the combinatorial explosion, it can't mean that methods should be generated by leaving out b, d and both of them.
Actually, that probably was the intention, but I agree with the problem. I offer this instead:
If one or two default values are set for a method, one or three methods are generated (so all of those combinations ab, a, b, {} exist). Above that, automatic 'guessing' makes no sense. At that point the developer must specify which signatures are needed. While this causes additional code, it lifts the combinatorial dillema:
@DefaultSignatures({"a, b", "a, c", "a, c, d", "c", "c, d"})
public void whatever(@Default String a, @Default String b, @Default LocalDate c, @Default LocalDate d) {
Lombok.staticDefault(b, "Hello");
Lombok.staticDefault(b, "I <3 lombok");
Lombok.dynamicDefault(d, LocalDate.now());
Lombok.dynamicDefault(d, c.plusDays(1));
...
}
This could generate 5 additional methods, matching the specified signatures. However this can only work, if all of the specified combinations have distinct type-sequences. If for example two signatures would result in a 'String x, String y' signature, this would need to fail with a speaking error. This would yield very clear code, though.
To work around the limitation of colliding signatures, and the issue of readability of the resulting methods, I suggest the following:
@DefaultSignatures({"TextOnly: a, b", "StartOnly: a, c", "OnlyC: c", "OnlyDates: c, d"})
public void whatever(@Default String a, @Default String b, @Default LocalDate c, @Default LocalDate d) { ...
This could generate the methods with postfixes:
public void whateverTextOnly(String a, String b) { ...
public void whateverStartOnly(String a, LocalDate c) { ...
public void whateverOnlyC(LocalDate c) { ...
public void whateverOnlyDates(LocalDate c, LocalDate d) { ...
Sadly annotations do not allow nested arrays or mapping types as parameters, otherwise the signatures could be assembled more elegantly. This way, string parsing is required, which is probably disfavorable. An alternate syntax could be stacking the same Annotation (@DefaultSignature) multiple times, and settings specific values for parameters and postfix each time:
@DefaultSignature(postfix="TextOnly", value={"a", "b"})
@DefaultSignature(postfix="StartOnly", value={"a", "c"})
@DefaultSignature(postfix="OnlyC", value={"c"})
@DefaultSignature(postfix="OnlyDates", value={"c", "d"})
public void whatever(@Default String a, @Default String b, @Default LocalDate c, @Default LocalDate d) { ...
I am unsure what I like better, to be honest. Maybe mix them?:
@DefaultSignature("TextOnly: a, b")
@DefaultSignature("StartOnly: a, c")
@DefaultSignature("OnlyC: c")
@DefaultSignature("OnlyDates: c, d")
public void whatever(@Default String a, @Default String b, @Default LocalDate c, @Default LocalDate d) { ...
Only one question should be answered beforehand. What is the empty default for a String? Is it
nullor""?
I'm afraid that while "" is more useful, it must be null as this is what Java does. Otherwise, the users would be surprised by the unexpected magic. Moreover, it opens a can of worms for collections. Making this configurable is probably not worth it as you can save just a single line.
OTOH I could imagine @DefaultValue @NonNull String s defaulting to the empty string.
lombok automatically figures out whether the defaults are static or dynamic, by checking if you reference any other field. So, Lombok.defaultValue(d, c.plusDays(1)) is dynamic because you use c.
While I would like the simplicity of the idea, I am unsure, if this hides a relevant information from the developer.....
Agreed; that's why I proposed to implement dynamic defaults only. The user can always define a constant themselves, which is exactly what they have to do in all cases where they want to reuse an instance.
@DefaultSignatures({"a, b", "a, c", "a, c, d", "c", "c, d"})
@DefaultSignatures({"TextOnly: a, b", "StartOnly: a, c", "OnlyC: c", "OnlyDates: c, d"})
@DefaultSignature(postfix="TextOnly", value={"a", "b"})
@DefaultSignature("TextOnly: a, b")
IMHO it's not useful enough to warrant such crazy constructs. While optional parameters are useful, Java lives well without them for 20+ years. While many other languages provide them, they do it in the simplest form: some mandatory args followed by some optional args. That's a well-known construct covering most cases. Whenever you want more, you have to write multiple method manually.... and that's no big deal, given how rare such a need is (I'm sure, I could use optional parameters in a few places, but I doubt, I could ever use your more flexible variant).
Maaartinus
on 16 Jan 2020
Moreover, it opens a can of worms for collections. Making this configurable is probably not worth it as you can save just a single line.
My thought as well. Defaulting null to empty is a solved issue. null should be the default.
The user can always define a constant themselves, which is exactly what they have to do in all cases where they want to reuse an instance.
Also a good point. Static constants can be dynamically included, without drawbacks. Therefore dynamic resolution makes more sense.
While many other languages provide them, they do it in the simplest form: some mandatory args followed by some optional args. That's a well-known construct covering most cases.
Many modern languages that do this have some form of named parameters, which allow you to specify which parameters you are supplying, when calling a method or function. As Java does not have this feature, default values generate a lot of intransparency as to which parameter is being supplied. Therefore I believe that specifying postfixes offers a great benefit to work around this restriction. I wouldn't force them on the developer, though. If specified, it could be used. If nothing is provided, there is no postfix.
TreffnonX
on 16 Jan 2020
Let's not go overboard. We generate all variants by lopping off default values 'from the back', so, this:
void m(String a, @Optional String b, String c, @Optional String d) {}
results in signatures: a, b, c, d, a,b,c, and a,c. The varargs (if present) cannot be optional of course.
I'd also be chuffed to bits to re-use optional here because, well, j.u.s.Optional is so bad, but I guess that's not actually convenient. At this point @DefaultValue doesn't make sense, I think. Maybe @OptionalParam?
You can put the following statements as first line(s) in the method:
Lombok.defaultValue(b, "");
Except now we have a problem. I don't think lombok has the ability to verbatim copy entire expressions (which can contain the world, as they can contain anonymous inner classes). So how _DO_ we do the whole dynamic constants thing? We can more or less move them. But copying is hard.
rzwitserloot
on 17 Jan 2020
NB: How about we support @lombok.NonNull on a few hardcoded data types (Let's say String, Set, List, Map, and that's it), producing empties instead, and on anything else, unless a default is being set, it is a compile time error (cannot @OptionalParam a @NonNull marked parameter without a defaultValue statement).
One solution for copying the defaulting expression is to generate a method. We can copy typerefs, and move expressions. We can also attempt to detect extremely simple constant values and actually copy those. Thus, putting it all together resulting in:
void m(
String a,
@OptionalParam LocalDate start,
@OptionalParam LocalDate end,
@OptionalParam @NonNull String x,
@OptionalParam int y) {
// Let's say method invokes are too complicated to copy.
Lombok.defaultValue(start, LocalDate.now());
Lombok.defaultValue(end, start.plusDays(1));
// But let's at least allow constants like this to work; refs to constants are presumably common.
Lombok.defaultValue(y, Integer.MIN_VALUE);
}
then we generate:
void m(String a) {
LocalDate start = m$paramDefault$start();
LocalDate end = m$paramDefault$end(start);
m(a, start, end, "", Integer.MIN_VALUE);
}
void m(String a, LocalDate start) {
LocalDate end = m$paramDefault$end(start);
m(a, start, end, "", Integer.MIN_VALUE);
}
void m(String a, LocalDate start, LocalDate end) {
m(a, start, end, "", Integer.MIN_VALUE);
}
void m(String a, LocalDate start, LocalDate end, @NonNull String x) {
m(a, start, end, x, Integer.MIN_VALUE);
}
private LocalDate m$paramDefault$start() {
return /* START MOVE of expression */ LocalDate.now(); /* END MOVE */
}
private LocalDate m$paramDefault$end(LocalDate start) {
return /* START MOVE of expression */ start.plusDays(1); /* END MOVE */
}
NB: How about we support @lombok.NonNull on a few hardcoded data types (Let's say String, Set, List, Map, and that's it), producing empties instead, and on anything else, unless a default is being set, it is a compile time error (cannot @OptionalParam a @NonNull marked parameter without a defaultValue statement).
I love that. Leaving the decision to the developer ensures a high level of acceptance and forcing a decision where one is required seems the right way to prevent wild guessing.
One solution for copying the defaulting expression is to generate a method.
That is a smart idea, because as you stated, anonymous inner classes would at least create a distinct class, if just copied plainly.
We can more or less move them. But copying is hard.
Could (and should) the original Lombok.defaultValue(...) statements be removed from the original method? They would just be empty placeholders anyway...
// But let's at least allow constants like this to work; refs to constants are presumably common.
Great!
Let's not go overboard. We generate all variants by lopping off default values 'from the back', so, this:
void m(String a, @Optional String b, String c, @Optional String d) {}results in signatures: a, b, c, d, a,b,c, and a,c. The varargs (if present) cannot be optional of course.
Why not make this the default behavior, but allow for more flexibility, if required? _I am willing to put some effort into developing this subfeature in a branch_, if it seems too much extra effort. The impulse to have generated methods based on defaults is huge, but if they are only distinguishable in their parameter signature rather than their name, I fear that people will get issues with their IDEs and autocompletion, since long parameter-signatures are often abbreviated. Also, distinguishing methods by name is an underestimated feature of any API, which we would give up on, and as a result the acceptance might drop. In cases where only one default parameter is specified, this is not a huge problem, but two will already be hard to isolate, and I know from experience, that method cascading with up to four or more variable parameters is not uncommon.
If you let me implement this on top of the rest, I would go with a mix of my suggestions, actually, as it seems the best tradeoff between clear syntax and brevity:
@DefaultSignature("b") // this would generate signature a, b without a postfix, because a is not default.
@DefaultSignature("b, c")
@DefaultSignature(postfix="Simple") // this would generate a signature with only 'a', because no other parameters are specified.
@DefaultSignature(postfix="WithC", value="c")
@DefaultSignature(postfix="BothDates", value="c, d")
public void whatever(String a, @Default String b, @Default LocalDate c, @Default LocalDate d) { ...
I believe that this is a great tradeoff between a readable API and a good result on compilation. If any @DefaultSignature annotations are present, those signatures should be the only ones generated. Otherwise the default behavior (drop from tail) should be applied.
TreffnonX
on 17 Jan 2020
I want to hold off on @DefaultSignature for now; I don't like having to put field names in strings, because IDEs dont auto-complete on these, typoes are harder to find, etc. In the end, @DefaultSignature(postFix = "BothDates", value = "c, d") is not that much shorter vs. public void whateverBothDates(String a, String b) {return whateverBothDates(a, b, null, null); }. Yes, it can get more convoluted if default values and very long signatures are involved, but we're really diving into exotic deep ends here.
My plan is to keep this in mind, and ensure that whatever we release does not negatively impact a potential future where we do add this.
rzwitserloot
on 17 Jan 2020
Let's not go overboard. We generate all variants by lopping off default values 'from the back', so, this:
void m(String a, @Optional String b, String c, @Optional String d) {}
results in signatures:a, b, c, d,a,b,c, anda,c. The varargs (if present) cannot be optional of course.
That's better than my ideas.
I'd also be chuffed to bits to re-use optional here because, well,
j.u.s.Optionalis so bad, but I guess that's not actually convenient. At this point@DefaultValuedoesn't make sense, I think. Maybe@OptionalParam?
IMHO reusing Optional would be a pain forcing people to use FQNs. For me, both @DefaultValue and @OptionalParam are fine.
Except now we have a problem. I don't think lombok has the ability to verbatim copy entire expressions (which can contain the world, as they can contain anonymous inner classes). So how DO we do the whole dynamic constants thing? We can more or less move them. But copying is hard.
You need each such expression exactly once, when you chain-delegate like
void m(String a, @Optional String b, String c, @Optional String d) {}
void m(String a, @Optional String b, String c) {
m(a, b, c, defaultValueForD);
}
void m(String a, String c) {
m(a, defaultValueForB, c);
}
However, this only works without the combinatorial explosion.
NB: How about we support
@lombok.NonNullon a few hardcoded data types (Let's say String, Set, List, Map, and that's it), producing empties instead, and on anything else, unless a default is being set, it is a compile time error (cannot@OptionalParama@NonNullmarked parameter without a defaultValue statement).
IMHO that's pretty cool.
I don't like the @DefaultSignature idea, but I might like the following syntax (working for "lopping from the back" only):
void m(String a, @OptionalParam("m1") String b, String c, @OptionalParam("m2") String d) {}
generating the methods as above, except that the one leaving out d only would be name "m2" and the one leaving out also b would be named "m1". The rule is that the name in the annotation on parameter P stands for the name of the method leaving out P and all following annotated fields.
Maaartinus
on 17 Jan 2020
Ooh, you're right, if we chain-delegate we don't need copies. I hope the performance impact of a long chain is mitigated by hotspot, because I was really not looking forward to introducing a bunch of dollar-festooned private methods to tie it all together.
Unfortunately, now we ARE going against the principle of keeping the door open for @DefaultSignature. I guess we can work around that by saying that if we ever do go and add @DS, then depending on which sigs you want, we may have to farm out the expression to a dollarized helper.
I find naming the method that you're NOT in a little confusing, but maybe that's okay. What's a bigger issue is that without a DSL construct you can't overload at all. A lot of java programmers overload, so that's not a workable way to do it (Imagine I want, given sig m(int, String, double), the entire combinatorial explosion, and all the methods must be named m. If we're going to support combinatorial explosions at all, we must support this.
rzwitserloot
on 18 Jan 2020
Ooh, you're right, if we chain-delegate we don't need copies. I hope the performance impact of a long chain is mitigated by hotspot, because I was really not looking forward to introducing a bunch of dollar-festooned private methods to tie it all together.
It might happen that the stuff gets too heavy for inlining. However, with your beloved dollar methods, it could happen, too (the same amount of calls and the same method after full inlining). OTOH, simple expressions need no dollar methods and then the non-chaining variant wins.
The chaining could be restricted to cases when a non-simple expression is used.
Obviously, static default values (which I argued against) would convert things like LocalDate.of(2020, 1.18) to a simple expression. A possible syntax could be
Lombok.defaultValue(d, Lombok.defineConst(LocalDate.of(2020, 1.18));
where defineConst could be useful per se (sometimes, I want to avoid declaring a constant used just once).
Unfortunately, now we ARE going against the principle of keeping the door open for @DefaultSignature. I guess we can work around that by saying that if we ever do go and add @DS, then depending on which sigs you want, we may have to farm out the expression to a dollarized helper.
Yes, it's a pity that it could lead different implementations, but let's hope, nobody relies on such implementation details.
I find naming the method that you're NOT in a little confusing, but maybe that's okay.
That's pretty strange, but logical: @OptionalParam("m1") b allows you leave out b and gives a name to the resulting method.
Imagine I want, given sig m(int, String, double), the entire combinatorial explosion, and all the methods must be named m. If we're going to support combinatorial explosions at all, we must support this.
Maybe with the following syntax
m(@OptionalParam(explode=true) int a,
@OptionalParam(explode=true) String b,
@OptionalParam double c) {}
where exploding means "generate methods including following optional params even when this one is left out". I don't think, I like it.
Or maybe using this syntax
m(@OptionalParam int a, @OptionalParam String b, @OptionalParam double c) {
Lombok.overload(a, c);
Lombok.overload(b, c);
Lombok.overload(c);
// Lombok.overload("someMethodName", a, c);
}
I don't think, I like it either. I guess, the stringly typed @DefaultSignature is best so far.
Maaartinus
on 18 Jan 2020
Actually, I like the Lombok.overload() strategy more; at least now you're not stringly-typing (putting java code in strings), which we did once with the exclude/of properties of @ToString and friends and regretted it ever since.
Not for the first release in any case, and you have the same 'we might have to make some dollarized methods to carry the initializers' problem that the @DS annotation would have. But it avoids stringly typing and I really like that.
rzwitserloot
on 18 Jan 2020
Actually, I like the
Lombok.overload()strategy more; at least now you're not stringly-typing
What I dislike is burying the list in the method body, but given that each IDE has an outline, it doesn't really matter. Anyway, I'd require that this spec comes at the top of the method.
But then, we mayn't need @OptionalParam at all, instead of
void m(@OptionalParam String a,
@OptionalParam int b,
@OptionalParam long c,
@OptionalParam double d) {
}
just write
void m(String a, int b, long c, double d) {
Lombok.overload(a, b, c);
Lombok.overload(a, b);
Lombok.overload(a);
Lombok.overload();
}
It's longer (depending on the formatting and argument name lengths) and somewhat repetitive, but not terribly much. We could also define
Lombok.overload().and(a).and(b).and(c);
as a shortcut for the four above lines.
When we wanted to generate all 16-1 methods, it'd get pretty long, but still much shorter and more readable than manual overloads. Using the shortcut is half as long but a real mess:
Lombok.overload().and(a).and(b).and(c); // 4 methods
Lombok.overload(a, b, d); // 1 method
Lombok.overload(a, c).and(d); // 2 methods
Lombok.overload(a, d); // 1 method
Lombok.overload(b).and(c).and(d); // 3 methods
Lombok.overload(b, d); // 1 method
Lombok.overload(c).and(d); // 2 methods
Lombok.overload(d); // 1 method
The funny thing is that there'd be no annotation at all. It'd probably require to scan all method bodies, which is probably not that funny. We could require some dummy annotation on the method (ugly).
When we preserve @OptionalParam, then it's unclear how these two should mix.
Maaartinus
on 19 Jan 2020
Whilst this overload business is a lot more flexible, and we can talk about funky syntax to make it easier to configure combinatorial explosions, I think the annotation based solution is still far superior. Yes, it is less flexible, but it is much simpler to follow, much less impactful (it's just.. doing stuff when annotations show up in signatures, it's what lombok already does in spades), easier to maintain, and (this one I really like), the 'dont think about it too long and just use it' tends towards the right behaviour, which is to not quite combinatorially explode, but offer some convenient shortcuts. If we go with the overload syntax only, surely we'll have some tools to address the combinatorial explosion, but then that tool (maybe Lombok.overloadAllCombinations()) or whatever will inevitably be treated as the right dont-think-about-it-too-long default, and whilst that's not a showstopper (I can't stop bad coders from writing bad code no matter how hard I try), it is quite a downside.
It's decided: We move forward with @OptionalParam for now, and tackle the other overloads after receiving feedback on the feature being used in anger.
rzwitserloot
on 19 Jan 2020
Talked about it with @rspilker tonight; the feature is looking good!
However, the name is a difficult issue; the aim is that a signature has a reasonable chance that a random java coder who is not familiar with this feature, or possibly not familiar with lombok at all, at least groks what it is trying to do, even if they are then completely befuddled as to how the heck that would work.
I think this:
void foo(@OptionalParam int a, int b, @OptionalParam String c) {}
has that chance. This:
void foo(@Default int a, int b, @Default String c) {}
does not.
And yet, @Optional as a name is 'taken' (by a core java type that shouldn't exist in the first place, that really hurts, but it's just taken, no way around it), and @OptionalParam is quite long. long is bad; by design these annotations are going to appear on very elaborate signatures!
So what then? @Opt? Is that still as legible? I doubt it.
Talking about with @rspilker and in the spirit of keeping it simple, isn't the most common case that ALL the params are optional? In that case, a very succint option becomes available. What if we do:
@OptionalParams
void foo(int a, @Required int b, String c) {}
as your paramslist grows, assuming the (vast) majority of them are optional, this wins out on shortness easily. This (@OptionalParams on the method, with if you really need it @Required on whatever is required) would be the _only_ way to do it.
Some pointed concerns also came up:
What if varargs is involved?
Our answer is: Those just aren't optional. Every variant we generate has the varargs as last argument, no way around it. I bet this just works. If you really need the varargsy explosion that e.g. log frameworks have to avoid the performance penalty of the created arrays, look, we're just beyond boilerplate here, or at least in 'that needs a custom annotation then' territory.
What about overloads?
Imagine you have:
@OptionalParams
void ex(int a, String dateAsISO8601, int c, int d) { this.ex(a, parse(dateAsISO8601), c, d); }
@OptionalParams
void ex(int a, LocalDate date, int c, int d) { ... }
with the intent that you want signatures (int, String, int), (int, Date, int), (int, String), (int, Date), (int), and (). Then a few questions pop up:
- How do we ensure that lombok doesn't try to generate
(int)and()twice? - If we detect that we shouldn't generate those twice, which of the two explicit overloads should the
(int)and()forms even call? Which of the two 'wins'? - How do we get this to compile? We can't just make the
()method do:this.ex(0, null, 0, 0)as that is an ambiguous method invocation. If we have somehow decided that the date variant wins, how do we write an AST node that unambiguously resolves to the proper invocation?
Note that it is extremely hard for lombok to attempt to figure out if there is a signature clash. With int it's easy, but imagine types, hierarchies, fully qualified types vs. import statements, generics, etc.
One way is to just dump it with the users: You can get it done as follows:
@OptionalParams
void ex(@Required int a, @Required String dateAsISO8601, int c, int d) {
this.ex(a, parse(dateAsISO8601), c, d);
}
@OptionalParams
void ex(int a, LocalDate date, int c, int d) { ... }
and now issue 1 and 2 is solved.
To solve 3, we add a cast. Fortunately, null can be cast to e.g. (T) or (List<T>) and it is not a compile-time warning (we suppresswarnings those, but still). So that's awesome. The one place where we get stuck is if the method itself has a generics arg. Something like: @OptionalParams public <Z> void foo(Z arg). We can't cast to (Z), it is not valid there. The solution seems to be to just disregard it as a use case: If you attempt to use any parameter as optional where the parameter has a method-local type parameter anywhere inside it... just means lombok generates an error and won't do it. This seems rare enough to just not cater to it (other than by erroring out and saying: Nope, sorry, make that one required).
The one remaining concern is that this trick of 'make the first few of the 'non-preferred path' required' is a bit hard to discover. any ideas on that one?
rzwitserloot
on 31 Jan 2020
If you put javadoc on the method, we can generate javadoc on the generated methods with the right @param and @return.
We need to copy the throws clause.
What about annotations on the method? Probably copy those as well.
We should not allow @Builder on a method also annotated with @OptionalParams, OR just not "copy" that one specifically. The interaction with @OptionalParams and @Builder is at least a bit strange regarding the default values.
What about method type annotations?
@OptionalParams
<T extends Duplicate> int calc(
int value, IntFunction<T> onDuplicate,
ToIntFunction<? super T> resolve,
Function<T,T> onError) {
}
Should that generate the methods that don't use any of the type parameters?
<T extends Duplicate> int calc() {}
<T extends Duplicate> int calc(int value) {}
rspilker
on 31 Jan 2020
So what then?
@Opt? Is that still as legible? I doubt it.
It could mean tons of other things, but I doubt anyone would expect it means "optimal" or "optical". For me, it would do.
Talking about with @rspilker and in the spirit of keeping it simple, isn't the most common case that ALL the params are optional?
IMHO the most common case is that ALL params after one optional param are optional. What's needed then, is a separator between the mandatory and the optional parts. Instead of
@OptionalParams
void foo(@Required int a, @Required int b, @Required int c, int d, int e, int f) {}
I could imagine
void foo(int a, int b, int c, @OptionalParams int d, int e, int f) {}
Unfortunately, that's not how annotations usually work.
What if varargs is involved?
Agreed with everything here.
What about overloads?
- If we detect that we shouldn't generate those twice, which of the two explicit overloads should the
(int)and()forms even call? Which of the two 'wins'?
IMHO that's a question Lombok shouldn't event try to answer. Let the user handle it.
Should that generate the methods that don't use any of the type parameters?
<T extends Duplicate> int calc() {}
<T extends Duplicate> int calc(int value) {}
I guess so. It's probably not what the user wanted, but it's surely what they requested. In the method body, you have to call the original method and, without the otherwise useless type parameter, this may be impossible (try it with <E extends Enum<E>>).
I guess, usually, the user doesn't want these two overloads, with our without the type parameters.
Maaartinus
on 31 Jan 2020
Not intending to start up a discussion regarding the merits of it, but I do
wonder how long it will take for someone to pop up with a feature request
asking for Optional parameters to be implemented as Optional containers
defaulting to Optional.empty(), rather than null.
That being said - I'd request that lombok generate @Nullable annotations
on these optional parameters, at least, so that checker frameworks have a
chance of catching this.
On Fri, Jan 31, 2020 at 10:32 AM Maaartinus notifications@github.com
wrote:
So what then? @Opt? Is that still as legible? I doubt it.
It could mean tons of other things, but I doubt anyone would expect it
means "optimal" or "optical". For me, it would do.Talking about with @rspilker https://github.com/rspilker and in the
spirit of keeping it simple, isn't the most common case that ALL the params
are optional?IMHO the most common case is that ALL params after one optional param
are optional. What's needed then, is a separator between the mandatory
and the optional parts. Instead of@OptionalParams
void foo(@Required int a, @Required int b, @Required int c, int d, int e, int f) {}I could imagine
void foo(int a, int b, int c, @OptionalParams int d, int e, int f) {}
Unfortunately, that's not how annotations usually work.
What if varargs is involved?Agreed with everything here.
What about overloads?
- If we detect that we shouldn't generate those twice, which of the
two explicit overloads should the (int) and () forms even call? Which
of the two 'wins'?IMHO that's a question Lombok shouldn't event try to answer. Let the user
handle it.
Should that generate the methods that don't use any of the type parameters?
int calc() {}
int calc(int value) {} I guess so. It's probably not what the user wanted, but it's surely what
they requested. In the method body, you have to call the original method
and, without the otherwise useless type parameter, this may be impossible
(try it with>). I guess, usually, the user doesn't want these two overloads, with our
without the type parameters.—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2316?email_source=notifications&email_token=AABIERODA73IGOV3SGYLC2DRAPV4RA5CNFSM4J5F3XSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKOCC3A#issuecomment-580657516,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIEROM7OHEWFFKP56T7B3RAPV4RANCNFSM4J5F3XSA
.
--
"Don't only practice your art, but force your way into it's secrets, for it
and knowledge can raise men to the divine."
-- Ludwig von Beethoven
randakar
on 31 Jan 2020
@rspilker We can't generate the cast required if you have any generics that are from a method local generics arg. So, 'do we copy the <T extends Duplicate> to generated methods that have no T in them is irrelevant: Lombok would just error, demanding you make those required.
@randakar We'd just answer that one the usual way: Optional? No. Not in lombok. Optional bad. See Brian Goetz' opinion and search the issue tracker for various debates on this, my yearly allotment of spending 10 hours to explain has been exhausted already 😩
rzwitserloot
on 31 Jan 2020
@randakar No, generating a @Nullable annotation is precisely the wrong thing to do (almost every day I am amazed at how much more convoluted nullable in the type system is, yet another case where it is rather complicated).
Let's say this debate is relevant. Then we have the following facts:
- The method has an
@OptionalParametersannotation, - No default is being set for that parameter,
- No
@lombok.NonNullannotation is on that parameter, - the type is not primitive
Then, yeah, that means we are going to generate code that passes null here. However, is that implied by the above set of actions? I don't see how; nowhere do you actually _SAY_: "Yes, I am the programmer, I am aware of what the above means, I understand null is allowed now, I shall code accordingly".
And that is _THE POINT_ of a nullable annotation. It's like a cast: An explicit statement by the coder that they are on top of it. The point of not having the annotation is that the compiler will inform you that you evidently forgot about it, and that you should address that.
In other words, the behaviour I _WANT_ is that, if you set up your IDE right (so, presumably, a @NonNullByDefault mechanism applying to the file we're editing, and IDE-sourced warnings or errors on violations), then you'll get a warning/error on the @OptionalParams annotation that it is passing null to a method that effectively says it doesn't allow that. That's good. It means that you are now reminded to [A] explicitly annotate your method to say that this param can be null (or add a non-default or add a @lombok.NonNull annotation so that lombok attempts to make an empty constant for you instead if ti can), and [B] write your method's code to ensure that it can deal with the fact that the param can be null, and by adding the annotation, the IDE will help you out on this front.
It might feel like a modicum of boilerplate but it is not. 'the programmer is aware that this can be null' is not already present in the source file anywhere else (well, other than that list of 4 facts I started with, but I do not believe this is sufficient evidence the programmer is implicitly aware of it).
As a separate concern, right now the elegance of this proposal is that we don't mess with the longest-form method at all, other than removing the Lombok.defaultParamValue calls if you have any. That's nice. Lombok doesn't inject @Nullable annotations anywhere else; it's quite a bit of weird magic if we did it here.
does, sure, imply that the parameter is therefore de facto nullable, as we shall be passing null to it, but my point is: That's not IMPLIED; that is merely a result of what you wrote. In fact, generating a null anno here is actively BAD: If you are in the habit of explicitly nullable-annotating params that can be null and you dont, that means you misunderstand how the feature works. If you have compile-time checking of this stuff on, you WILL get a warning stating that null is being passed to a param that is (via a @ParamsNonNullByDefault mechanism presumably then) not annotated as being capable of handling it. That's good: It tells you: Oh, right. I need to think about the fact that this param can be null if it is omitted.
rzwitserloot
on 31 Jan 2020
I was forced to delete some utterly off topic whining about java and grass is greener oversimplifications about nullity systems in other languages. This is not the venue to delve into the nuances of these statements.
I'll allow it on the google groups, or for example ##java on freenode, but not here, I think.
rzwitserloot
on 1 Feb 2020
(And I do apologize to @randakar and @TreffnonX ; I think I perhaps invited these comments. That was my mistake – my intent is not to stifle this discussion, but I should have thought better of inviting it here in the first place!)
rzwitserloot
on 4 Feb 2020
Related issues
ownaginatious
·
31Comments
kevcodez
·
58Comments
ilgrosso
·
76Comments
lombokissues
·
34Comments
michaelboyles
·
52Comments
Most helpful comment
(And I do apologize to @randakar and @TreffnonX ; I think I perhaps invited these comments. That was my mistake – my intent is not to stifle this discussion, but I should have thought better of inviting it here in the first place!)