[PATCH] D87201: [clang-format] Add a option for the position of Java static import

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:1970
+  but this behavior is changed by another option,
+  ``JavaStaticImportAfterImport``.
 
----------------
Can you add a test that shows if the sorting is still in the groups, i.e. I can't tell if

```
import com.test.a;
import static org.a;

import com.test.b;
import static org.b;
```

becomes

```
import static org.a;
import static org.b;
import com.test.a;

import com.test.b;
```

or

```
import static org.a;
import static org.b;

import com.test.a;

import com.test.b;
```

or

```
import static org.a;
import com.test.a;

import static org.b;
import com.test.b;

```







================
Comment at: clang/include/clang/Format/Format.h:1708
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+
----------------
Can we consider changing the name or the option to make it clearer what its for?

`SortJavaStaticImport`

and make it an enumeration rather than true/false

`Before,After,Never`

There need to be a "don't touch it option (.e.g. Never)" that does what it does today (and that should be the default, otherwise we end up causing clang-format to change ALL java code" which could be 100's of millions of lines+




================
Comment at: clang/lib/Format/Format.cpp:906
   LLVMStyle.JavaScriptWrapImports = true;
+  LLVMStyle.JavaStaticImportAfterImport = false;
   LLVMStyle.TabWidth = 8;
----------------
We can't have a default that does will cause a change


================
Comment at: clang/unittests/Format/SortImportsTestJava.cpp:253
 
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+  FmtStyle.JavaStaticImportAfterImport = true;
----------------
please test all options

You are also missing a test for checking the PARSING of the options


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87201/new/

https://reviews.llvm.org/D87201