projectlombok / lombok

Very spicy additions to the Java programming language.
https://projectlombok.org/
Other
12.86k stars 2.38k forks source link

@ToString formatting 'language'. #1297

Open rzwitserloot opened 7 years ago

rzwitserloot commented 7 years ago

This is the umbrella issue for any requests to improve or modify the way @ToString prints in some fashion.

The general notion would be to have a regex like pattern language to specify exactly what you want. Some examples, given:

package foo;
public class Test {
    String name;
    int age;
    int[] values;

    public String getSummary() { return "hello"; }")
}

1: new Test("Joe", 50, new int[] {5, 12, 14}) 2: new Test(null, 50, new int[0]); 3: new Test(null, 50, null);

@ToString("<class>{<#name>: <#getSummary()>}")
1: Test{Joe: hello}
2,3: Test{null: hello}

@ToString("<fqn>{<#values: [a, b]>: <fieldNames: n1=v1 n2=v2>}")
1: foo.Test{[5, 12, 14]: name=Joe age=50}
2: foo.Test{[]: name=Joe age=50}
3: foo.Test{null: name=Joe age=50}

etcetera.

Concerns:

This issue supercedes #876 #1216 #1276 #918

Not sure if we actually want this; there's a lot to cover, toString() is a debugging aid (if it's something else, you're doing it wrong), and it feels like reinventing an entire programming language when you can just.. program your toString() method in java by.. just typing java code in the editor you're already in.

askoning commented 7 years ago

This issue also supercedes #256 #529 #1211

gmathias commented 7 years ago

I don't think hideNulls (#256 #529 #1211) should be covered (only) by pattern. The pattern is for formatting while hideNulls is for filtering content. Also it would be over-complicated to write a pattern just to say "hide nulls". Please keep the boolean flag separated from the pattern.

jefferyyuan commented 7 years ago

a separate boolean flag for hideNulls is very useful.

wheredevel commented 7 years ago

Is there a way of weaving methods in Lombok? (Traits...?) (Mixins...?)

Why do I ask under this issue at all: Because I have a method called toJson(). The implementation is something like this:

protected String toJson() {
    return new ObjectMapper().writeValueAsString(this);
}

Now, the only way to have it in my classes of interest - is to inherit from some class implementing it. And this is ugly. Not a Lombok way!!! Lombok does Object. ;) So, instead, I would prefer a way to weave this implementation into classes of interest.

Finally, I'd like to use it as a toString() replacement. For that I would do something like @ToString(delegate=toJson) - which should result in something like:

@Override
public String toString() {
    return toJson();
}

And this is my suggestion as to how to solve the toString() formatting issue discussed here: in an imperative, instead of a declarative way.

What do you think?

gmathias commented 7 years ago

It's disappointing to not have even a reply. Lombok made Java great again, please continue to improve it. Please reopen one of the hideNulls issue ! Or explain why this unrealistic issue supercedes the easy-to-implement really-useful hideNulls issue) !

Maaartinus commented 7 years ago

It's disappointing to not have even a reply.

I'm not a project owner, but my answer may be better than nothing.


Lombok doesn't support traits. There's @Delegate and @ExtensionMethod. The latter could help. It add s no methods, but let's you write yourObject.toJson().

@ToString(delegate=toJson)

This could be a nice feature. It may be better to allow placing @ToString (or @ToString.Delegate) on a method (avoiding stringly-typed arguments). This obviously won't work with @ExtensionMethod.

But maybe you can define an @ExtensionMethod for toString() directly? No idea if it takes precedence over an inherited method (I guess so, as lombok knows nothing about inheritance).

And this is my suggestion as to how to solve the toString() formatting issue discussed here: in an imperative, instead of a declarative way.

This is similar in spirit to my proposal #1393.

zbstof commented 6 years ago

In my opinion, merging 'skipNulls' feature requests into this issue is misleading. Having boolean flag should be sufficient, checking for nulls before on fields directly should be enough.

I guess this subroutine has to be rewritten to use StringBuilder.append https://github.com/rzwitserloot/lombok/blob/master/src/core/lombok/javac/handlers/HandleToString.java#L141 And insert maker.If that checks for nulls before appending each field, or something along these lines EDIT: @rzwitserloot , I see you already came to the same conclusion 4 years ago: https://groups.google.com/d/msg/project-lombok/9on7mFH3z34/Y9HvIJxn0JAJ

oblodgett commented 4 years ago

Any progress on this issue?

mjustin commented 4 years ago
  • Print classname, fully qualified classname, or nothing.

I think it would be useful to choose a custom class name to be printed out as well. I view this as being analogous to the member-level annotation @ToString.Include(name = "nameToUse").

In my current use case, I have a private inner class that implements a public interface, and I'd rather my debugging logs not contain ReallyLongOuterClassName.KindOfLongInnerClassName whenever I'm logging that particular class.

rzwitserloot commented 4 years ago

We explicitly cover this in our various presentations as one of those issues where many are adamant that juuuust one more straw on this particular camel's back is all that you need to be happy, but surely all that does is open the door to the next batch. If null needs to be skipped, why not sentinel blank values? How do we identify those, etc, etc, etc.

There is no progress on this issue because there is no design that is suitable, and we (@rspilker and I) have little hope that a suitable design even exists.

We do not believe 'just add hideNulls' is the correct 80/20 balance.

wheredevel commented 4 years ago

There is no progress on this issue because there is no design that is suitable, and we (@rspilker and I) have little hope that a suitable design even exists.

We do not believe 'just add hideNulls' is the correct 80/20 balance.

I suggested a design back in 2017 (almost 3 years ago!!!) https://github.com/rzwitserloot/lombok/issues/1297#issuecomment-289988646 Why isn't it good enough? What could be simpler than delegating toString with an annotation to developer-provided method?! @ToString(delegate=toJson) In this case, Jackson would take over all the additional configurations....

I'm doing a workaround for quite sometime. It goes something like this:


@UtilityClass
public class Printer {
  public String toString(Object object) { 
    //// print the object
  }
}

//workaround
interface ToString {

  public static class Mixin implements ToString {

    private Object origin;

    public Mixin(Object object) {
      this.origin = object;
    }

    public String toString() {
      Printer.toString(origin);
    }
  }
}

//than apply it to the class
class Something implements ToString {

  @Delegate
  private ToString _toString = new ToString.Mixin(this);

  //....
}

//instead of simply...
@ToString(delegate = Printer.class)
class Something {
  //....
}
Maaartinus commented 4 years ago

@wheredevel

Why isn't it good enough?

IMHO it's not bad, but it solves none of the other problems (just look at the links above) of people having no method to delegate to.

Disclaimer: I'm not a project owner.

wheredevel commented 4 years ago

@Maaartinus ,

My workaround is good enough for me. In my opinion, @ToString(delegate=Printer.class) is just a short for defining interface, mixin, and delegate. This is where Lombok shines: reducing the boilerplate code.

I see my workaround as a solution design, which @rzwitserloot said is missing.

To me, it solves many, if not all, the issues people mentioned related to toString() fine-tuning.
As I already sugested, the delegated Printer could utilize the power of libraries such as Jackson to drive how fields are marshaled during the delegate toString() - giving all the power to the developer to modify toString() behavior, including the choice of the "fine-printing" library.

rzwitserloot commented 4 years ago

@wheredevel As maaartinus said, your workaround is not good enough because it saves virtually no boilerplate at all. @ToString(delegate = Printer.class)? This is not a win, at all, over a one-liner toString impl.

gmathias commented 4 years ago

If null needs to be skipped, why not sentinel blank values?

Who asked for filtering sentinel blank values? I didn't see any mention in the related issues. In Java "null" is a pretty specific value and still commonly used (despite Optional, that's another debate!). On the opposite sentinel values are an edge case, there's no clear definition of it (and I totally agree we don't want to open that door).

There is no progress on this issue because there is no design that is suitable, and we (@rspilker and I) have little hope that a suitable design even exists.

Probably because this ticket is trying to solve too many problems at the same time. I repeat some issues linked here should have stay separated (like "hide nulls" which is not about formatting).

We do not believe 'just add hideNulls' is the correct 80/20 balance.

not 80/20 regarding formatting? sure. But hiding nulls is a common need, without it Lombok ToString is kind of useless because most real-life objects have too many properties, making the output too verbose (especially combined with associations). I think anybody using Lombok on an enterprise project will face this issue. We end up rewriting all the toString() manually. We see less and less value in Lombok...

It's disappointing to see no solution in years. A not-perfect-solution is still better than no solution at all. You could flag it "experimental" so you could change it later.

wheredevel commented 4 years ago

@rzwitserloot,

@wheredevel As maaartinus said, your workaround is not good enough because it saves virtually no boilerplate at all. @ToString(delegate = Printer.class)? This is not a win, at all, over a one-liner toString impl.

I agree it's not a big code line saver. I can always opt for either overriding the toString() OR (usually, more useful solution to me) using interface to define the behavior to be modified, class implementing the desired behavior, and mixin to inject the modification into the existing classes.
That's where Lombok's @Delegate does particularly great job.
So, in this context, it's very troubling to me to read the following:

Current status: negative - Currently we feel this feature will not move out of experimental status anytime soon, and support for this feature may be dropped if future versions of javac or ecj make it difficult to continue to maintain the feature. Source: @Delegate

I always say that with Lombok and a Dependency Injection (i.e. Spring) I don't need to remember patterns - I can always make up the required ones from basic building blocks. @Delegate is central to that practice.

rzwitserloot commented 4 years ago

Please restrict further issue debate on either a solid plan on how to end up at a design that covers a bunch of feasible use cases in an easy to understand way. Refrain from 'but I just want X' or 'this is disappointing'. Sure. I'm disappointed too. It doesn't change my belief that it's better not to have the feature at all, than to 'eh, whatever good enough it' with an ill-thought through feature we have to then support forever.

Note that we can't stick half of ToString in experimental and the other half outside of it, either. Or at least, we don't want to.

We see less and less value in Lombok...

Is this like a threat? Add this feature or else we will stop using it?

I understand that if lombok cannot deliver something you need, that makes it less valuable to you. But you don't need to keep whining on and on about it. This feels disrespectful: I'm weighing your needs against the needs of all the other lombok users, and I have arrived at a decision which you are disappointed with.

I'm putting this thread on freeze for 3 weeks. Only comments with serious proposals will be allowed, and the rest will be deleted. If you wish to debate the policy of how we don't want to slap-dash a bunch of arbitrary features into lombok on an as-requested basis, by all means hold it, but not in this issue, maybe the forum is the right channel for such a debate. If you wish to chime in that you want more from toString but without a serious attempt at a complete proposal, please don't.

FREEZE ENDS: 2020-03-01.

jmoraleda commented 4 years ago

A possible generic solution, is to pass an optional filter to @ToString annotation that takes a String and returns a String and that would post-process the string returned by lombok.

Evidently this is not the fastest implementation, but I don't see this as a block, since toString is intended chiefly as a debugging tool, and in theory anything can be done via postprocessing (admittedly somethings would be harder than others).

As a concrete example, the following filter can be passed to remove null values from the output. The credit for this idea belongs to https://stackoverflow.com/questions/54604701/how-to-skip-null-field-with-lombok-tostring/59323411#59323411

/**
 * Removes the null values from String generated through the @ToString annotation.
 * For example:
 * - replaces:  AddressEntity(id=null, adrType=null, adrStreet=null, adrStreetNum=null, adrComplement=null, adrPoBox=null, adrNip=null, adrCity=city, adrCountry=null, adrNameCorresp=nameCorresp, adrSexCorresp=null, adrSource=null, adrSelectionReason=null, validityBegin=null, validityEnd=null, lastModification=null, dataQuality=null)
 * - by:        AddressEntity(adrCity=city, adrNameCorresp=nameCorresp)
 * Note: does not support tricky attribute content such as "when, x=null, it fails".
 * @param lombokToString a String generated by Lombok's @ToString method
 * @return a string without null values
 */
public static String removeToStringNullValues(String lombokToString) {
    //Pattern
    return lombokToString != null ? lombokToString
            .replaceAll("(?<=(, |\\())[^\\s(]+?=null(?:, )?", "")
            .replaceFirst(", \\)$", ")") : null;
}

The work around I am currently using to remove nulls in my actual implementations is to do a static import of the function above and explicitly call it every time I want a non-null representation of objects. This is evidently an issue if objects of this kind are printed or logged in many places, which is why having a way of passing a filter to the @ToString annotation would be very nice.

Maaartinus commented 4 years ago

This is error-prone (as the Lombok-generated strings are not escaped) and slow (you could do better with some static final Pattern, but still slow). Maybe filtering with a BiPrecidate (if such a thing exists) taking the field name and value could be the way to go (but there's the problem with primitive values).

There have been many related proposals, with many of them pretty good, but none of them really good enough. The project maintainers want to keep Lombok small, so it looks like none passes anytime soon.

If you really need the flexibility, I'd suggest to switch to reflection. It most probably will be faster than the string manipulation and it'll be much more flexible. You only write

@Override public String toString() {return MyReflector.toString(this);}

and can do everything with little effort.

If speed is really at premium, then you could switch to an annotation processor instead. It's much more work (but only once; no per class work) and doesn't give you access to private members, but it's full-speed.

rzwitserloot commented 2 years ago

@jmoraleda wrote:

A possible generic solution, is to pass an optional filter to @ToString annotation that takes a String and returns a String and that would post-process the string returned by lombok.

Vetoed; this is a dead-end idea. The toString output of lombok involves invoking toString()s on objects of all sorts of types, and in general toString output doesn't fully any spec (that's sort of the point), and even where it does, rarely in a 'this is easily parsable by code' way.

For example, your 'find x = null and strip it' plan can be trivially asploded into making a complete hash of the string output if one of the string fields includes the construction "this is a string with, oooh=null, how mean" - and your filter will ruin that. No filter that isn't incredibly complicated can address this.