Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify the use of extension methods #3316

Open
mizhoux opened this issue Dec 21, 2022 · 10 comments
Open

Simplify the use of extension methods #3316

mizhoux opened this issue Dec 21, 2022 · 10 comments

Comments

@mizhoux
Copy link

mizhoux commented Dec 21, 2022

Why doesn't Lombok define and use extension methods as Manifold does?

Lombok:

Define the extension class of String:

public class MyStringExtensions {

    public static boolean isNullOrEmpty(String str) {
        return str == null || str.isEmpty();
    }
}

Use the extension method:

@ExtensionMethod(MyStringExtensions.class, OtherExtensions.class)
public class SomeClass1 {

    public void method1(String input) {
        if (input.isNullOrEmpty()) {
            // do something
        }

        // do other things
    }
}
@ExtensionMethod(MyStringExtensions.class, , OtherExtensions.class)
public class SomeClass2 {

    public void method2(String input) {
        if (input.isNullOrEmpty()) {
            // do something
        }

        // do other things
    }
}

Manifold:

Define extension class of String:

@Extension
public class MyStringExtensions {

    public static boolean isNullOrEmpty(@This String str) {
        return str == null || str.isEmpty();
    }
}

Use the extension method:

public class SomeClass1 {

    public void method1(String input) {
        if (input.isNullOrEmpty()) {
            // do something
        }

        // do other things
    }
}
public class SomeClass2 {

    public void method2(String input) {
        if (input.isNullOrEmpty()) {
            // do something
        }

        // do other things
    }
}

It's really boring and tiring to add @ExtensionMethod(MyStringExtensions.class) everywhere I want to use the extension methods, and I think this behavior has gone away from Lombok's purpose of simplifying programming.

So if it's possible that Lombok providers an annotation named @ExtensionClass(String.class)? A class with @ExtensionClass(String.class) means that it is an extension class of String and it's public-static methods are extension methods of String. If I put @ExtensionClass(String.class) on my extension class, then I can use the extension methods of String everywhere in my project, without being forced to use @ExtensionMethod(MyStringExtensions.class).

What I expect of Lombok:

Define extension class of String:

@ExtensionClass(String.class)
public class MyStringExtensions {

    // fine
    public static boolean isNullOrEmpty(String str) {
        return str == null || str.isEmpty();
    }

    // Here we will get an error because MyStringExtensions is extension class for String only.
    public static boolean isNullOrEmpty(OtherClass value) {
        return false;
    }
}

Use the extension method(without @ExtensionMethod any more):

public class SomeClass1 {

    public void method1(String input) {
        if (input.isNullOrEmpty()) {
            // do something
        }

        // do other things
    }
}
public class SomeClass2 {

    public void method2(String input) {
        if (input.isNullOrEmpty()) {
            // do something
        }

        // do other things
    }
}
@Rawi01
Copy link
Collaborator

Rawi01 commented Dec 21, 2022

This is similar to #922

@zeric-rizzo
Copy link

This is similar to #922

Similar, yes. But couldn't this approach alleviate the performance problems inherent to the request in #922 ? Annotating the extensions class would effectively limit the search scope, no?

@dstango
Copy link

dstango commented Jan 10, 2023

Similar, yes. But couldn't this approach alleviate the performance problems inherent to the request in #922 ? Annotating the extensions class would effectively limit the search scope, no?

I'd guess the opposite: The project would need to be scanned for @ExtensionClass annotations first, before any further compilation steps could be done ...

@marc-guenther
Copy link

Given these weird problems described in #3261 (and some others I haven't even reported yet, which cause NPE in the compiler), I wouldn't want extension methods to be applied to my entire project all at once. Too much stuff that can go wrong. Extension methods unfortunately are not very stable.

I have defined a shortcut in Eclipse called ext, which resolves to @ExtensionMethod(LombokExtensions.class). It's very quick and easy that way to enable my extension methods on a specific class. But then, I only have one class where I define my static methods, so that might not be feasible for others.

@mizhoux
Copy link
Author

mizhoux commented Mar 7, 2023

Similar, yes. But couldn't this approach alleviate the performance problems inherent to the request in #922 ? Annotating the extensions class would effectively limit the search scope, no?

I'd guess the opposite: The project would need to be scanned for @ExtensionClass annotations first, before any further compilation steps could be done ...

Maybe we can use config to limit the scanning scope and improve the efficiency, such as restricting the package in which the extension method resides by project configuration(a global lombok.config).

@dstango
Copy link

dstango commented Mar 7, 2023

I'd propose to go forward with the discussion in #922: as the configuration system allows to have config-files for individual subdirectories (https://projectlombok.org/features/configuration), that would be a great approach to place @ExtensionMethod-Definitions that are valid for many classes.
BTW: Did anyone ever try to put @ExtensionMethod into package-info.java? That wouldn't populate to sub-packages, but might be helpful anyhow in some cases ...

@erizzo
Copy link

erizzo commented Mar 7, 2023

I've been on a project recently where the team, although familiar with Lombok and making extensive use of it, nobody knew about @ExtensionMethod and I got some sideways looks about it during PR reviews. I had to explain what it does and why (IMO) it sometimes makes code read better. To do that, I explained that I first encountered the concept when I took a 4-5 year side trip into C# Land. Even after explaining it to some pretty good developers, there was still uncertainty about it - they had trouble accepting that much magic from Lombok.

I tell that story to explain why I'm not thrilled about using config files for this. A developer who has not encountered @ExtensionMethod before, if there is no annotation to signal its usage, is likely to be hella confused about it. It's subtle and many Java developers probably haven't encountered the idea before. Hide it in a config file and you're just making that potential confusion more likely.

tl;dr Java code should be in .java with the rest of the code (including annotations), not hidden away in a config file that most developers aren't even going to know to look for.

@dstango
Copy link

dstango commented Mar 8, 2023

@erizzo: While I can totally relate to getting suspicious looks (I do, too), but that's not an argument against a config-file-possibility! If your team doesn't agree to use such an option in a config file, then simply don't do it. You can already configure lombok to reject the @ExtensionMethod-annotation ...
The annotation @ExtensionClass proposed above would suffer from quite the same problem, btw: you wouldn't see it in the class in which you're using it.

I understand you colleagues perception that @ExtensionMethod is kind of (too much) magic, but I find it soooo handy to just press "." to pop up content assist and get offered appropriate extension methods alongside regular instance methods. :-) I also like to use extension methods as an intermediate step, if I don't have a certain method in a class from a company internal dependency, and experiment with the exact details how I would that method and it's signature to look. Extension methods allow my experiments to stay local in my application, and when it's mature enough I can promote it as a contribution to the common library.

And I confess I like to use it to bypass the stream() / collect() dance for simple filter or map situations.

Also adding some useful methods to the final String class comes in quite handy ...

At first encounter I thought lombok were all about @Data and it's siblings, but my view has expanded to include @ExtensionMethod now :-P.

@erizzo
Copy link

erizzo commented Mar 8, 2023

@dstango I want to clarify that I'm not arguing against using @ExtensionMethod, just the idea of putting it in the config file. You're right that I and my teams can simply not use that; I just want to point out why it might not be a good idea in general.

@dstango
Copy link

dstango commented Mar 8, 2023

@erizzo Thanks, I got that. But do you think the proposed idea of this issue with @ExtensionClass would be preferable to a config file? I'd say it's also hard to find -- like config files. Or do you see a big difference between the two?

So I'd say having such an option in the first place is debatable, and could be decided against on the team level, yet I don't see why we shouldn't have some magic in principle (as config-file or special annotation).

BTW: Maybe it's all a variation of this classic paper: https://geo.mff.cuni.cz/~hanyk/NOFY056/prednaska/sphinx-02/_static/Clark-COME-FROM.pdf ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants