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

Comment section moved between two import declarations #372

Open
MSaguer opened this issue Mar 11, 2020 · 10 comments
Open

Comment section moved between two import declarations #372

MSaguer opened this issue Mar 11, 2020 · 10 comments
Labels
area: bug 🐛 Something isn't working theme: Re-Writer

Comments

@MSaguer
Copy link

MSaguer commented Mar 11, 2020

Prettier-Java 0.7.0

Launched through prettier-maven-plugin 0.7.0 without any option.
In the following case, the comment section is moved between two import lines.
This bugs depends on import declaration order. If "com.me.Util" is declared first, the generated file is OK.

Input:

package org.mypackage;

/*-
 */

import java.lang.Util;
import java.util.Map;
import com.me.Util;

public class Prettier {}

Output:

package org.mypackage;

import com.me.Util;
/*-
 */
import java.lang.Util;
import java.util.Map;

public class Prettier {}

Expected behavior:

package org.mypackage;

/*-
 */
import java.lang.Util;
import java.util.Map;

import com.me.Util;

public class Prettier {}
@MSaguer
Copy link
Author

MSaguer commented Mar 14, 2020

When the java code is parsed, the comment section is added as leading comment to the java.lang.Util import line. When the import are sorted by printer-utils.sortImports, this import with its leading comment line is set at the second place.

In my use case, the comment contains the license section generated by the license-maven-plugin. That's why, setting this comment as child of the import element could be seen as an error.

So here are my questions:

  • Is it a common way to add comment between imports? How should it be managed?
  • If it's not a common way, could the parser be modified? The comment could be associated to the package or module declaration if it exists or else to the first import line (after sorting).
  • if it is, I can't see how to manage it because. This code formating action would be not compliant with the license-maven-plugin that generates the header between the package and the import lines

According to you, what solution would be the best solution?

@clementdessoude
Copy link
Contributor

I'll have a look this week !

@MSaguer
Copy link
Author

MSaguer commented Mar 15, 2020

Thanks Clément.
Tell me if you need a hand. I would be happy to help.

@Shaolans
Copy link
Member

This issue is due on how we process the comments in prettier-java, the comments are always attached to the closest node. We tried multiple ways of attaching comments but we have not figured out the best one.
Unfortunately, this edge case of sorting imports and comments results of this issue. Since, most people use IDE and manage imports automatically, I think prettier-java should not sort imports.
@clementdessoude WDYT ?

@MSaguer For the time being, I think you can either remove the sort function or force your comment to be attached to the right expression (Since it attach to the closest node, you can add newline to create a gap, but I don't recommend this one since if you prettify twice, it won't be stable)

@clementdessoude
Copy link
Contributor

Sorry, I was very busy last week because of work. I am also a bit sceptic with the sorting of imports, I noticed it caused some issues with Intellij.

@jhipster/developers WDYT ?

@jhaber
Copy link
Contributor

jhaber commented Mar 27, 2020

My $0.02 is that sorting imports feels within scope for prettier-java, and we really like having it

@pascalgrimaud
Copy link
Member

is it really usefull ? import package are generated by our IDE and we never look them... putting comments inside is strange

@MSaguer
Copy link
Author

MSaguer commented Mar 27, 2020

I agree with jahber that sorting is required. Some devs may add it manually even if it is strange.
I also agree with Pascal. I don't see any use case where adding comment between imports is required.

I propose a quick solution that I could implement next week

  • sorting is still performed on import lines
  • all comments attached to import line are moved to the first comment
    WDYT?

@murdos
Copy link

murdos commented Mar 27, 2020

I also think prettier-java should sort imports.
The issue with relying on IDE for sorting is that it's very hard to share the same sorting configuration, moreover when people use different IDE (Eclipse, IntelliJ, ...).

@MathieuAA
Copy link
Member

MathieuAA commented Mar 27, 2020

I agree with @murdos on sorting imports.
Further more, in my opinion, prettier's goal is to standardize code across an organization by sharing a common set of rules so that code made by each contributor looks the same (or at least aims at looking the same)., and imports too are a part of a codebase (maybe not as important as the rest, but still...).

The example is a bit weird too, and may not happen very often (I may not fully understand the motivation behind putting a comment in such a place though).
IMO, I wouldn't say this issue has a "high priority". I'm sure there must be a more important task to do.

MSaguer added a commit to MSaguer/prettier-java that referenced this issue Mar 29, 2020
@Shaolans Shaolans added area: bug 🐛 Something isn't working theme: Re-Writer labels Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: bug 🐛 Something isn't working theme: Re-Writer
Projects
None yet
Development

No branches or pull requests

7 participants