Skip to content

SortImports option is ignored #42

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

Closed
sherter opened this issue Apr 19, 2016 · 9 comments
Closed

SortImports option is ignored #42

sherter opened this issue Apr 19, 2016 · 9 comments

Comments

@sherter
Copy link
Contributor

sherter commented Apr 19, 2016

No matter which SortImports option I set (ONLY, ALSO or NO), the imports are never sorted and it always returns the same result. Am I missing something here?

public static void main(String[] args) throws FormatterException {
  JavaFormatterOptions options = new JavaFormatterOptions(JavadocFormatter.NONE, Style.GOOGLE, SortImports.ONLY);
  Formatter formatter = new Formatter(options);
  String result = formatter.formatSource("package com.google.example;\n" +
                "\n" +
                "import com.google.common.base.Preconditions;\n" +
                "\n" +
                "import org.junit.runner.RunWith;\n" +
                "import org.junit.runners.JUnit4;\n" +
                "\n" +
                "import java.util.List;\n" +
                "\n" +
                "import javax.annotations.Nullable;\n" +
                "\n" +
                "import static org.junit.Assert.fail;\n" +
                "import static com.google.truth.Truth.assertThat;\n" +
                "\n" +
                "@RunWith( JUnit4.class ) public class SomeTest  {}");
  System.out.println(result);
}
@sherter
Copy link
Contributor Author

sherter commented Apr 19, 2016

Seems like the Formatter can't do imports sorting at all. This is all done in FormatFileCallable with a different SortImports enum, which is not part of the public API. The principle of most astonishment definitely applies here 😉

@cushon
Copy link
Collaborator

cushon commented Apr 23, 2016

Yep, currently import ordering only works with the cli. It's work in progress.

Adding it to the methods in the api that return strings would be easy, but some details need to be sorted out for the methods that do partial formatting or return replacement ranges.

@talios
Copy link

talios commented May 6, 2016

Look forward to seeing this in a release, so I can also plug it into my https://github.com/talios/googleformatter-maven-plugin Maven plugin.

@sherter
Copy link
Contributor Author

sherter commented May 6, 2016

The same goes for the JavadocFormatter option. JavaDoc is always formatted, even if the formatter is set to JavadocFormatter.NONE. I'm wondering why you make an API public although it's not providing any functionality.

@cushon
Copy link
Collaborator

cushon commented May 6, 2016

JavaDoc is always formatted, even if the formatter is set to JavadocFormatter.NONE.

Do you have a repro?

@sherter
Copy link
Contributor Author

sherter commented May 6, 2016

I wouldn't expect the text to change in any way in the example below. Or is this not how it works?

public static void main(String[] args) throws FormatterException {
    JavaFormatterOptions options = new JavaFormatterOptions(
            JavaFormatterOptions.JavadocFormatter.NONE,
            JavaFormatterOptions.Style.GOOGLE,
            JavaFormatterOptions.SortImports.ALSO);
    Formatter formatter = new Formatter(options);
    String result = formatter.formatSource(
            "/**\n" +
            "foo\n" +
            "*/\n");
    System.out.println(result); // /**
                                //  * foo
                                //  */
}

@cushon
Copy link
Collaborator

cushon commented May 6, 2016

The implementation makes a distinction between normalizing leading * and other javadoc formatting, like wrapping lines.

Part of the reason that the configuration is rough around the edges is that non-configurability is a stated goal of the formatter. Javadoc formatting and import sorting are off by default because they're work in progress. Once they're done, they'll be on by default.

@sherter
Copy link
Contributor Author

sherter commented May 6, 2016

Ah okay, I didn't know that. And the style option also works as expected 😉
Does "on by default" mean that the configuration option will be removed from the API eventually?

@cushon
Copy link
Collaborator

cushon commented Jul 13, 2016

There's an update on import sorting in #37. I'm still not sure how it should look in the API, let me know if you have ideas.

And the javadoc formatting is now more polished, and has been enabled by default.

@cushon cushon closed this as completed Jul 13, 2016
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

3 participants