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

Spotless for imports #5741

Open
MartinWitt opened this issue Apr 6, 2024 · 8 comments · May be fixed by #5984
Open

Spotless for imports #5741

MartinWitt opened this issue Apr 6, 2024 · 8 comments · May be fixed by #5984

Comments

@MartinWitt
Copy link
Collaborator

MartinWitt commented Apr 6, 2024

Currently, our import ordering in spoon is a bit messy. This leads to an unpleasant time with IntelliJ trying to sort the imports in a more normal order. Why don't we use spotless-maven-plugin to sort all our imports in a fixed sorting? This also helps to reduce diffs in the future for PRs. The benefit compared to checkstyle is that we can do the sorting automatic. See https://github.com/diffplug/spotless/tree/main/plugin-maven#java for an example.

@MartinWitt
Copy link
Collaborator Author

We need to add

          <java>
            <importOrder />
          </java>

Any wishes for the order?
<order>java|javax,org,com,com.diffplug,,\#com.diffplug,\#</order>
<semanticSort>false</semanticSort>
<wildcardsLast>false</wildcardsLast>

@I-Al-Istannen
Copy link
Collaborator

I don't care as long as IJ can follow the same order and I don't need to run spotless :^)

@algomaster99
Copy link
Contributor

Can we please add a good-first-issue label here? We can then ask new contributors to work on this :)

@monperrus
Copy link
Collaborator

done

@micka-lama
Copy link

micka-lama commented Sep 30, 2024

Hello, I would like to try to contribute on this issue.

I noticed that Spotless is applied to the files included in src/test/java/spoon/testing/assertions/**/*.java. I plan to accept all the Java files excepted the resources (example: **/resources/**). But, there will be a lot of changes due to the initial Plantir format usage on src/test/java/spoon/testing/assertions/**/*.java (around 1550 Java files impacted). May I remove this particular format?

I suggest also to add <removeUnusedImports />. There are 29 files impacted.
By using only <importOrder />, there are 735 files impacted.

I guess it is better to do a PR only about the pom.xml change and another one about the spotless:apply.

@algomaster99
Copy link
Contributor

Hi @micka-lama ! We would want to be similar to IJ style guide. @I-Al-Istannen do you use the formatted in IJ with default configurations? We could then configure spotless accordingly.

@I-Al-Istannen
Copy link
Collaborator

Yea, just the defaults (except forbidding star imports). static imports are split and placed after the normal imports:

non-static
<blank>
static javax
static java
<blank>
static *

@algomaster99
Copy link
Contributor

@micka-lama we look forward to your PR that configures spotless following two rules:

  1. Sticking to IJ style guide.
  2. Configure the style guide for imports like @I-Al-Istannen said. You can also include configuration for IntelliJ workspace so that they don't contradict with spotless.

@micka-lama micka-lama linked a pull request Oct 6, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants