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

[BUG] Organize imports remove val import #2972

Closed
Rawi01 opened this issue Sep 23, 2021 · 8 comments · Fixed by #2975
Closed

[BUG] Organize imports remove val import #2972

Rawi01 opened this issue Sep 23, 2021 · 8 comments · Fixed by #2975
Assignees
Labels
Milestone

Comments

@Rawi01
Copy link
Collaborator

Rawi01 commented Sep 23, 2021

Describe the bug
Using the "Organize imports" command in eclipse removes the lombok.val import.

To Reproduce

  • Create a java file and use val
  • Use "Organize imports" (Ctrl + Shift + O, Right click -> Source -> Organize Imports)

Expected behavior
Eclipse does not remove the used import.

Version info (please complete the following information):

  • Lombok version: edge
  • Platform: Eclipse 2021-09

Additional context
I have already started to work on this

@Rawi01 Rawi01 self-assigned this Sep 23, 2021
@rzwitserloot
Copy link
Collaborator

I recall that when I wrote the original val support stuff I turned val into @lombok.val String, keeping that annotation around partly to avoid this scenario. Just in case it helps :)

@andi-huber
Copy link

Might be a related issue:

Given following snipped ...

// choosing LocalDateTime is just exemplary, any other type would work as well

public static LocalDateTime now() {
    val now = LocalDateTime.now();
    // now.  <-- trigger code completion
    return now;
}

Eclipse (2021-09) fails to fully populate code completion suggestions, that is, in the above case it believes now to be of type Object.class and does not provide any methods specific to the LocalDateTime type.

I'm using my own lombok build from the 'master' with latest commit ba68962

Side Note: I'm contributer to an ASF project with 160k+ lines of code and we use lombok as much as we can. Apart from code-completion and organize-imports, I'm happy to say that the new snapshot version works pretty solid.

@Rawi01
Copy link
Collaborator Author

Rawi01 commented Sep 24, 2021

@rzwitserloot thats exatly what I did too but I used the general addAnnotation method and that one always generates a QualifiedTypeReference even if you only pass a single type e.g. val. As this is a qualified reference eclipse ignores it while searching for used imports. Using an existing helper method in addAnnotation fixes the problem.

@andi-huber I noticed that too while fixing this issue and still search for the reason. The resolved type is Annotation so lombok does not get called to modify the tree during completion. I hope that I can finish this today or tomorrow.

@andi-huber
Copy link

Thanks @Rawi01, much appreciated!

@Rawi01
Copy link
Collaborator Author

Rawi01 commented Sep 27, 2021

@andi-huber Can you test if my changes fix the problem?

@andi-huber
Copy link

At a brief glance, looks good to me, thanks! Let me use this version for a day and report back how it goes.

Found a different issue though, (rather minor for me):

@Log4j2
public class AnyClass {
    void action() {
        // log.  <-- trigger code completion
    }
}

Eclipse (2021-09) fails to fully populate code completion suggestions.

@rzwitserloot
Copy link
Collaborator

@andi-huber interesting. This sounds sufficiently unrelated to this issue that I think it'll be easier to track the @Log4j2 problem with a separate issue. Would you mind filing that?

@rzwitserloot rzwitserloot added awaiting-fix-confirmation We believe this bug has been fixed, the issue may be left open for a little while for confirmation. bug labels Sep 27, 2021
@rzwitserloot rzwitserloot added this to the next-version milestone Sep 27, 2021
@andi-huber
Copy link

I am counting 9k+ occurrences of the keyword val in our code base. Everything compiles with ECJ on Eclipse (2021-09).

Great stuff!

Just for reference, and this is rather minor to me ...

As we build our project with a CI pipeline utilizing Maven, I can only test the latest lombok-edge snapshot there. I was seeing some corner cases, where javac would fail with

 -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] Lombok visitor handler class lombok.javac.handlers.HandleVal failed: java.lang.NullPointerException: Cannot invoke "com.sun.tools.javac.code.Type.hasTag(com.sun.tools.javac.code.TypeTag)" because "tree.type" is null

These very briefly are:
recursive generics ... I believe already a known issue
interface compounds ... val model = (HasCommonContext & IModel<ManagedObject>)getModel();
enums with body ... enum { ANY{ val ... }, ...}

@rzwitserloot rzwitserloot removed the awaiting-fix-confirmation We believe this bug has been fixed, the issue may be left open for a little while for confirmation. label Sep 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants