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

Source printer import ordering strategy #3807

Conversation

4everTheOne
Copy link
Contributor

Hello!

This PR adds support for custom import ordering.
Previously, the user could not define the "template" to be used for imports, only having the option to have imports like this:

import java.util.List;
import com.github.javaparser.ast.NodeList;
import com.github.javaparser.ast.ImportDeclaration;

What if the user wants to separate the static imports from the others? What if the user wants to group all java imports into a single group?

Example:

import com.github.javaparser.ast.NodeList;
import com.github.javaparser.ast.ImportDeclaration;

import java.util.List;

With this PR this can now be achieve Included in this PR, there are 3 implementation of import ordering.

  • DefaultImportOrderingStrategy: The logic that existed previously in JavaParser.
  • IntelliJImportOrderingStrategy: Implements the ordering like IntelliJ does;
  • EclipseImportOrderingStrategy: Implements the ordering like Eclipse does;

An example of code that makes the printer use a custom formatitng can be seen below:

CompilationUnit cu = ...;

IntelliJImportOrderingStrategy strategy = new IntelliJImportOrderingStrategy();
DefaultConfigurationOption option = new DefaultConfigurationOption(ConfigOption.SORT_IMPORTS_STRATEGY, strategy);
printerConfiguration.addOption(option);

Printer printer = new DefaultPrettyPrinter(printerConfiguration);
String actualCode = printer.print(cu);

This design is flexible, so if the user wants to create their own implementation they can also do it.

Feedback is welcome! :)

@codecov
Copy link

codecov bot commented Dec 18, 2022

Codecov Report

Merging #3807 (4d2d1be) into master (b9301ef) will increase coverage by 0.083%.
The diff coverage is 94.318%.

Impacted file tree graph

@@               Coverage Diff               @@
##              master     #3807       +/-   ##
===============================================
+ Coverage     57.490%   57.574%   +0.083%     
  Complexity      2182      2182               
===============================================
  Files            635       638        +3     
  Lines          33802     33878       +76     
  Branches        5837      5850       +13     
===============================================
+ Hits           19433     19505       +72     
- Misses         12290     12293        +3     
- Partials        2079      2080        +1     
Flag Coverage Δ
AlsoSlowTests 57.574% <94.318%> (+0.083%) ⬆️
javaparser-core 50.334% <94.318%> (+0.127%) ⬆️
javaparser-symbol-solver 36.610% <1.136%> (-0.080%) ⬇️
jdk-10 57.570% <94.318%> (+0.086%) ⬆️
jdk-11 57.567% <94.318%> (+0.080%) ⬆️
jdk-12 57.570% <94.318%> (+0.083%) ⬆️
jdk-13 57.570% <94.318%> (+0.083%) ⬆️
jdk-14 57.564% <94.318%> (+0.077%) ⬆️
jdk-15 57.570% <94.318%> (+0.083%) ⬆️
jdk-16 57.570% <94.318%> (+0.089%) ⬆️
jdk-8 57.571% <94.318%> (+0.089%) ⬆️
jdk-9 57.570% <94.318%> (+0.086%) ⬆️
macos-latest 57.565% <94.318%> (+0.083%) ⬆️
ubuntu-latest 57.559% <94.318%> (+0.083%) ⬆️
windows-latest 57.556% <94.318%> (+0.083%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...uration/imports/DefaultImportOrderingStrategy.java 90.000% <90.000%> (ø)
...ration/imports/IntelliJImportOrderingStrategy.java 91.304% <91.304%> (ø)
...avaparser/printer/DefaultPrettyPrinterVisitor.java 92.897% <94.736%> (+0.040%) ⬆️
...uration/imports/EclipseImportOrderingStrategy.java 97.142% <97.142%> (ø)
...ter/configuration/DefaultPrinterConfiguration.java 87.500% <100.000%> (+0.403%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9301ef...4d2d1be. Read the comment docs.

@jlerbsc
Copy link
Collaborator

jlerbsc commented Dec 18, 2022

As usual it is a very nice contribution.
Nevertheless, would it be possible to rename the "strategy" classes so as not to refer to a solution (eclipse, intelliJ) but rather to an order. If it's too complex to describe the order other than by the name of the solution, that's okay.

@4everTheOne
Copy link
Contributor Author

4everTheOne commented Dec 18, 2022

Hi @jlerbsc. Thanks for your feedback!
I'm not sure how to name it, because they are a little complex.

For example, in intellij the order is:

  • Other Imports;
  • Java and Javax:
  • Static Imports

On eclipse the order is:

  • Static Imports
  • Java
  • JavaX
  • org
  • com
  • Other

If you have any suggestion, please let me know! I would be happy to integrate it.

@jlerbsc
Copy link
Collaborator

jlerbsc commented Dec 18, 2022

No I have no idea that's why I asked you for yours. Have you finished this PR?

@4everTheOne
Copy link
Contributor Author

Yes I have finished it :)
Thanks

@jlerbsc jlerbsc merged commit c66b6d6 into javaparser:master Dec 18, 2022
@jlerbsc jlerbsc added this to the Next release milestone Dec 18, 2022
@jlerbsc jlerbsc added the PR: Added A PR that introduces new behaviour (e.g. functionality, tests) label Dec 18, 2022
@4everTheOne 4everTheOne deleted the improvement/printer-import-ordering-strategy branch December 18, 2022 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Added A PR that introduces new behaviour (e.g. functionality, tests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants