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

2021-12-08 Thread Byoungchan Lee via Phabricator via cfe-commits
bc-lee added a comment.

Thanks for the review. Since I don't have commit access, so I want someone else 
to apply this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-18 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2e7add812eb7: [clang-format] Add a option for the position 
of Java static import (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortImportsTestJava.cpp

Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,62 @@
  "import org.c;\n"));
 }
 
+TEST_F(SortImportsTestJava, SortJavaStaticImport) {
+  FmtStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before;
+  EXPECT_EQ("import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n"
+"\n"
+"import com.test.b;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"));
+
+  FmtStyle.SortJavaStaticImport = FormatStyle::SJSIO_After;
+  EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
+TEST_F(SortImportsTestJava, SortJavaStaticImportAsGroup) {
+  FmtStyle.SortJavaStaticImport = FormatStyle::SJSIO_After;
+
+  EXPECT_EQ("import com.test.a;\n"
+"import com.test.b;\n"
+"\n"
+"import static org.a;\n"
+"import static org.b;\n",
+sort("import com.test.a;\n"
+ "import static org.a;\n"
+ "import com.test.b;\n"
+ "import static org.b;\n"));
+}
+
 TEST_F(SortImportsTestJava, DeduplicateImports) {
   EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
 "import org.a;\n"));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14328,6 +14328,12 @@
   CHECK_PARSE("BitFieldColonSpacing: After", BitFieldColonSpacing,
   FormatStyle::BFCS_After);
 
+  Style.SortJavaStaticImport = FormatStyle::SJSIO_Before;
+  CHECK_PARSE("SortJavaStaticImport: After", SortJavaStaticImport,
+  FormatStyle::SJSIO_After);
+  CHECK_PARSE("SortJavaStaticImport: Before", SortJavaStaticImport,
+  FormatStyle::SJSIO_Before);
+
   // FIXME: This is required because parsing a configuration simply overwrites
   // the first N elements of the list instead of resetting it.
   Style.ForEachMacros.clear();
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -377,6 +377,15 @@
   }
 };
 
+template <>
+struct ScalarEnumerationTraits {
+  static void enumeration(IO ,
+  FormatStyle::SortJavaStaticImportOptions ) {
+IO.enumCase(Value, "Before", FormatStyle::SJSIO_Before);
+IO.enumCase(Value, "After", FormatStyle::SJSIO_After);
+  }
+};
+
 template <> struct MappingTraits {
   static void mapping(IO , FormatStyle ) {
 // When reading, read the language first, we need it for getPredefinedStyle.
@@ -574,6 +583,7 @@
 IO.mapOptional("RawStringFormats", Style.RawStringFormats);
 IO.mapOptional("ReflowComments", Style.ReflowComments);
 IO.mapOptional("SortIncludes", Style.SortIncludes);
+IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
 IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
 IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
 IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
@@ -947,6 +957,7 @@
 
   LLVMStyle.DisableFormat = false;
   LLVMStyle.SortIncludes = true;
+  LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before;
   LLVMStyle.SortUsingDeclarations = true;
   

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

2020-09-17 Thread Byoungchan Lee via Phabricator via cfe-commits
bc-lee added a comment.

Can someone commit my changes on behalf of it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-12 Thread Byoungchan Lee via Phabricator via cfe-commits
bc-lee marked an inline comment as done.
bc-lee added a comment.

It's okay for some reviewers to make this change on my behalf. Thanks for 
reviewing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-12 Thread Jake Merdich via Phabricator via cfe-commits
JakeMerdichAMD accepted this revision.
JakeMerdichAMD added a comment.
This revision is now accepted and ready to land.

Sorry on the delay, LGTM too.

It looks like you're a first time contributor and probably don't have write 
access to the repo, do you want one of us to push this on your behalf? If you 
intend to have more commits in the future, write access to github isn't hard to 
get (just request it once you have a few changes in).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

LGTM, I'm happy if @JakeMerdichAMD  is




Comment at: clang/include/clang/Format/Format.h:1708
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+

bc-lee wrote:
> JakeMerdichAMD wrote:
> > MyDeveloperDay wrote:
> > > 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+
> > > 
> > > 
> > I'm confused, there is not currently a never option... Right now the 
> > behavior is always 'before', there is no 'after' or 'never'.
> > 
> > Making it an enum would probably be more ergonomic though, even there is no 
> > never option.
> If `SortIncludes` is `true`, it doesn't make sense to not touch Java static 
> include.
> Making it an enum is also good for me.
> 
true.. it will sort it I assume one way or the other


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-08 Thread Byoungchan Lee via Phabricator via cfe-commits
bc-lee added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:1970
+  but this behavior is changed by another option,
+  ``JavaStaticImportAfterImport``.
 

MyDeveloperDay wrote:
> 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;
> 
> ```
> 
> 
> 
> 
> 
Done.



Comment at: clang/include/clang/Format/Format.h:1708
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+

JakeMerdichAMD wrote:
> MyDeveloperDay wrote:
> > 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+
> > 
> > 
> I'm confused, there is not currently a never option... Right now the behavior 
> is always 'before', there is no 'after' or 'never'.
> 
> Making it an enum would probably be more ergonomic though, even there is no 
> never option.
If `SortIncludes` is `true`, it doesn't make sense to not touch Java static 
include.
Making it an enum is also good for me.




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

JakeMerdichAMD wrote:
> MyDeveloperDay wrote:
> > please test all options
> > 
> > You are also missing a test for checking the PARSING of the options
> Parsing test was already noted and done (unless this changes to an enum of 
> course). But testing the 'false' case here makes sense.
Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-08 Thread Byoungchan Lee via Phabricator via cfe-commits
bc-lee updated this revision to Diff 290619.
bc-lee added a comment.

Add more tests and rename options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortImportsTestJava.cpp

Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,62 @@
  "import org.c;\n"));
 }
 
+TEST_F(SortImportsTestJava, SortJavaStaticImport) {
+  FmtStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before;
+  EXPECT_EQ("import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n"
+"\n"
+"import com.test.b;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"));
+
+  FmtStyle.SortJavaStaticImport = FormatStyle::SJSIO_After;
+  EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
+TEST_F(SortImportsTestJava, SortJavaStaticImportAsGroup) {
+  FmtStyle.SortJavaStaticImport = FormatStyle::SJSIO_After;
+
+  EXPECT_EQ("import com.test.a;\n"
+"import com.test.b;\n"
+"\n"
+"import static org.a;\n"
+"import static org.b;\n",
+sort("import com.test.a;\n"
+ "import static org.a;\n"
+ "import com.test.b;\n"
+ "import static org.b;\n"));
+}
+
 TEST_F(SortImportsTestJava, DeduplicateImports) {
   EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
 "import org.a;\n"));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14276,6 +14276,12 @@
   CHECK_PARSE("BitFieldColonSpacing: After", BitFieldColonSpacing,
   FormatStyle::BFCS_After);
 
+  Style.SortJavaStaticImport = FormatStyle::SJSIO_Before;
+  CHECK_PARSE("SortJavaStaticImport: After", SortJavaStaticImport,
+  FormatStyle::SJSIO_After);
+  CHECK_PARSE("SortJavaStaticImport: Before", SortJavaStaticImport,
+  FormatStyle::SJSIO_Before);
+
   // FIXME: This is required because parsing a configuration simply overwrites
   // the first N elements of the list instead of resetting it.
   Style.ForEachMacros.clear();
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -377,6 +377,15 @@
   }
 };
 
+template <>
+struct ScalarEnumerationTraits {
+  static void enumeration(IO ,
+  FormatStyle::SortJavaStaticImportOptions ) {
+IO.enumCase(Value, "Before", FormatStyle::SJSIO_Before);
+IO.enumCase(Value, "After", FormatStyle::SJSIO_After);
+  }
+};
+
 template <> struct MappingTraits {
   static void mapping(IO , FormatStyle ) {
 // When reading, read the language first, we need it for getPredefinedStyle.
@@ -574,6 +583,7 @@
 IO.mapOptional("RawStringFormats", Style.RawStringFormats);
 IO.mapOptional("ReflowComments", Style.ReflowComments);
 IO.mapOptional("SortIncludes", Style.SortIncludes);
+IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
 IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
 IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
 IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
@@ -947,6 +957,7 @@
 
   LLVMStyle.DisableFormat = false;
   LLVMStyle.SortIncludes = true;
+  LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before;
   LLVMStyle.SortUsingDeclarations = true;
   LLVMStyle.StatementMacros.push_back("Q_UNUSED");
   LLVMStyle.StatementMacros.push_back("QT_REQUIRE_VERSION");
@@ 

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

2020-09-08 Thread Jake Merdich via Phabricator via cfe-commits
JakeMerdichAMD added inline comments.



Comment at: clang/include/clang/Format/Format.h:1708
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+

MyDeveloperDay wrote:
> 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+
> 
> 
I'm confused, there is not currently a never option... Right now the behavior 
is always 'before', there is no 'after' or 'never'.

Making it an enum would probably be more ergonomic though, even there is no 
never option.



Comment at: clang/lib/Format/Format.cpp:906
   LLVMStyle.JavaScriptWrapImports = true;
+  LLVMStyle.JavaStaticImportAfterImport = false;
   LLVMStyle.TabWidth = 8;

MyDeveloperDay wrote:
> We can't have a default that does will cause a change
Not a default change, see previous comment for discussion.



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

MyDeveloperDay wrote:
> please test all options
> 
> You are also missing a test for checking the PARSING of the options
Parsing test was already noted and done (unless this changes to an enum of 
course). But testing the 'false' case here makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-08 Thread 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-07 Thread Byoungchan Lee via Phabricator via cfe-commits
bc-lee updated this revision to Diff 290381.
bc-lee added a comment.

Fix the example to match the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortImportsTestJava.cpp

Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
  "import org.c;\n"));
 }
 
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+  FmtStyle.JavaStaticImportAfterImport = true;
+
+  EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
 TEST_F(SortImportsTestJava, DeduplicateImports) {
   EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
 "import org.a;\n"));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13929,6 +13929,7 @@
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
+  CHECK_PARSE_BOOL(JavaStaticImportAfterImport);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -544,6 +544,8 @@
 IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
 IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
+IO.mapOptional("JavaStaticImportAfterImport",
+   Style.JavaStaticImportAfterImport);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -901,6 +903,7 @@
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
+  LLVMStyle.JavaStaticImportAfterImport = false;
   LLVMStyle.TabWidth = 8;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
@@ -2312,12 +2315,15 @@
 JavaImportGroups.push_back(
 findJavaImportGroup(Style, Imports[i].Identifier));
   }
+  bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
   llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
 // Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
-   Imports[LHSI].Identifier) <
-   std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
-   Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+   std::make_tuple(!Imports[RHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[RHSI], Imports[RHSI].Identifier);
   });
 
   // Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1618,10 +1618,12 @@
 
   /// A vector of prefixes ordered by the desired groups for Java imports.
   ///
-  /// Each group is separated by a newline. Static imports will also follow the
-  /// same grouping convention above all non-static imports. One group's prefix
-  /// can be a subset of another - the longest prefix is always matched. Within
-  /// a group, the imports are ordered lexicographically.
+  /// One group's prefix can be a subset of another - the longest prefix is
+  /// always matched. Within a group, the imports 

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

2020-09-07 Thread Jake Merdich via Phabricator via cfe-commits
JakeMerdichAMD added inline comments.



Comment at: clang/include/clang/Format/Format.h:1705
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+

bc-lee wrote:
> JakeMerdichAMD wrote:
> > 3 things here:
> > 
> > 1. Did you mix up the true and false cases?
> > 2. (nit) You should also note that if this option is false, all static 
> > imports are before all non-static imports (as opposed to mixed together 
> > alphabetically). I was confused on first glance.
> > 3. Please add this config option to 
> > FormatTests.cpp:ParsesConfigurationBools.
> I understand. The description is somewhat misleading. 
New phrasing makes it clear, but true and false are still inverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-07 Thread Byoungchan Lee via Phabricator via cfe-commits
bc-lee updated this revision to Diff 290303.
bc-lee added a comment.

Some comments have been corrected and a unittest has been added in 
FormatTest.ParsesConfigurationBools


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortImportsTestJava.cpp

Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
  "import org.c;\n"));
 }
 
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+  FmtStyle.JavaStaticImportAfterImport = true;
+
+  EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
 TEST_F(SortImportsTestJava, DeduplicateImports) {
   EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
 "import org.a;\n"));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13929,6 +13929,7 @@
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
+  CHECK_PARSE_BOOL(JavaStaticImportAfterImport);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -544,6 +544,8 @@
 IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
 IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
+IO.mapOptional("JavaStaticImportAfterImport",
+   Style.JavaStaticImportAfterImport);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -901,6 +903,7 @@
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
+  LLVMStyle.JavaStaticImportAfterImport = false;
   LLVMStyle.TabWidth = 8;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
@@ -2312,12 +2315,15 @@
 JavaImportGroups.push_back(
 findJavaImportGroup(Style, Imports[i].Identifier));
   }
+  bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
   llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
 // Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
-   Imports[LHSI].Identifier) <
-   std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
-   Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+   std::make_tuple(!Imports[RHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[RHSI], Imports[RHSI].Identifier);
   });
 
   // Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1618,10 +1618,12 @@
 
   /// A vector of prefixes ordered by the desired groups for Java imports.
   ///
-  /// Each group is separated by a newline. Static imports will also follow the
-  /// same grouping convention above all non-static imports. One group's prefix
-  /// can be a subset of another - the longest prefix is always matched. Within
-  /// a group, the imports are ordered lexicographically.
+  /// One group's prefix can be a subset of another - the longest 

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

2020-09-07 Thread Byoungchan Lee via Phabricator via cfe-commits
bc-lee added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2027
+
+ .. code-block:: java
+ true:

MyDeveloperDay wrote:
> The ClangFormatStyleOptions.rst is generated using 
> doc/tools/dump_format_style.py which reads Format.h and generates this,
> 
> If this code block in not in the Format.h it will get removed the next time 
> the script is run, please don't change ClangFormatStyleOption.rst by hand use 
> the script, so add the code block to the Format.h file (see others options 
> for now to do this)
Done.



Comment at: clang/include/clang/Format/Format.h:1705
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+

JakeMerdichAMD wrote:
> 3 things here:
> 
> 1. Did you mix up the true and false cases?
> 2. (nit) You should also note that if this option is false, all static 
> imports are before all non-static imports (as opposed to mixed together 
> alphabetically). I was confused on first glance.
> 3. Please add this config option to FormatTests.cpp:ParsesConfigurationBools.
I understand. The description is somewhat misleading. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-07 Thread Jake Merdich via Phabricator via cfe-commits
JakeMerdichAMD requested changes to this revision.
JakeMerdichAMD added inline comments.
This revision now requires changes to proceed.



Comment at: clang/include/clang/Format/Format.h:1705
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+

3 things here:

1. Did you mix up the true and false cases?
2. (nit) You should also note that if this option is false, all static imports 
are before all non-static imports (as opposed to mixed together 
alphabetically). I was confused on first glance.
3. Please add this config option to FormatTests.cpp:ParsesConfigurationBools.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-07 Thread Byoungchan Lee via Phabricator via cfe-commits
bc-lee updated this revision to Diff 290246.
bc-lee added a comment.

Modify the comment of Format.h to sync ClangFormatStyleOptions.rst


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortImportsTestJava.cpp

Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
  "import org.c;\n"));
 }
 
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+  FmtStyle.JavaStaticImportAfterImport = true;
+
+  EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
 TEST_F(SortImportsTestJava, DeduplicateImports) {
   EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
 "import org.a;\n"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -544,6 +544,8 @@
 IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
 IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
+IO.mapOptional("JavaStaticImportAfterImport",
+   Style.JavaStaticImportAfterImport);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -901,6 +903,7 @@
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
+  LLVMStyle.JavaStaticImportAfterImport = false;
   LLVMStyle.TabWidth = 8;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
@@ -2312,12 +2315,15 @@
 JavaImportGroups.push_back(
 findJavaImportGroup(Style, Imports[i].Identifier));
   }
+  bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
   llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
 // Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
-   Imports[LHSI].Identifier) <
-   std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
-   Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+   std::make_tuple(!Imports[RHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[RHSI], Imports[RHSI].Identifier);
   });
 
   // Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1689,6 +1689,21 @@
   bool JavaScriptWrapImports;
   // clang-format on
 
+  /// If true, clang-format will put Java static imports after all non-static
+  /// imports.
+  /// \code{.java}
+  ///   true:
+  ///   import static org.example.function1;
+  ///
+  ///   import org.example.ClassA;
+  ///
+  ///   false:
+  ///   import org.example.ClassA;
+  ///
+  ///   import static org.example.function1;
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+
   /// If true, the empty line at the start of blocks is kept.
   /// \code
   ///true:  false:
@@ -2410,6 +2425,7 @@
JavaImportGroups == R.JavaImportGroups &&
JavaScriptQuotes == R.JavaScriptQuotes &&
JavaScriptWrapImports == R.JavaScriptWrapImports &&
+   JavaStaticImportAfterImport == R.JavaStaticImportAfterImport &&
KeepEmptyLinesAtTheStartOfBlocks ==
R.KeepEmptyLinesAtTheStartOfBlocks &&
MacroBlockBegin == R.MacroBlockBegin &&
Index: clang/docs/ClangFormatStyleOptions.rst

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

2020-09-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision.
MyDeveloperDay added a comment.
This revision now requires changes to proceed.

Thanks for the patch, You need to generate a fill context diff (see 
Contributing to LLVM)

ensure the diff is clang-formatted itself (can't quite tell if it is or not)




Comment at: clang/docs/ClangFormatStyleOptions.rst:2027
+
+ .. code-block:: java
+ true:

The ClangFormatStyleOptions.rst is generated using 
doc/tools/dump_format_style.py which reads Format.h and generates this,

If this code block in not in the Format.h it will get removed the next time the 
script is run, please don't change ClangFormatStyleOption.rst by hand use the 
script, so add the code block to the Format.h file (see others options for now 
to do this)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-09-06 Thread Byoungchan Lee via Phabricator via cfe-commits
bc-lee updated this revision to Diff 290137.
bc-lee added a comment.

Add missing initializer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortImportsTestJava.cpp

Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
  "import org.c;\n"));
 }
 
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+  FmtStyle.JavaStaticImportAfterImport = true;
+
+  EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
 TEST_F(SortImportsTestJava, DeduplicateImports) {
   EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
 "import org.a;\n"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -543,6 +543,8 @@
 IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
 IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
+IO.mapOptional("JavaStaticImportAfterImport",
+   Style.JavaStaticImportAfterImport);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -899,6 +901,7 @@
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
+  LLVMStyle.JavaStaticImportAfterImport = false;
   LLVMStyle.TabWidth = 8;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
@@ -2310,12 +2313,15 @@
 JavaImportGroups.push_back(
 findJavaImportGroup(Style, Imports[i].Identifier));
   }
+  bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
   llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
 // Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
-   Imports[LHSI].Identifier) <
-   std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
-   Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+   std::make_tuple(!Imports[RHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[RHSI], Imports[RHSI].Identifier);
   });
 
   // Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1671,6 +1671,10 @@
   bool JavaScriptWrapImports;
   // clang-format on
 
+  /// If true, clang-format will put Java Static imports after all non-static
+  /// imports.
+  bool JavaStaticImportAfterImport;
+
   /// If true, the empty line at the start of blocks is kept.
   /// \code
   ///true:  false:
@@ -2391,6 +2395,7 @@
JavaImportGroups == R.JavaImportGroups &&
JavaScriptQuotes == R.JavaScriptQuotes &&
JavaScriptWrapImports == R.JavaScriptWrapImports &&
+   JavaStaticImportAfterImport == R.JavaStaticImportAfterImport &&
KeepEmptyLinesAtTheStartOfBlocks ==
R.KeepEmptyLinesAtTheStartOfBlocks &&
MacroBlockBegin == R.MacroBlockBegin &&
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2021,6 +2021,20 @@
  false:
  import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,} from "some/module.js"
 

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

2020-09-05 Thread Byoungchan Lee via Phabricator via cfe-commits
bc-lee created this revision.
bc-lee added a reviewer: MyDeveloperDay.
bc-lee added projects: clang, clang-format.
Herald added a subscriber: cfe-commits.
bc-lee requested review of this revision.

Some Java style guides and IDEs group Java static imports after
 non-static imports. This patch allows clang-format to control
 the location of static imports.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87201

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortImportsTestJava.cpp

Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
  "import org.c;\n"));
 }
 
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+  FmtStyle.JavaStaticImportAfterImport = true;
+
+  EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
 TEST_F(SortImportsTestJava, DeduplicateImports) {
   EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
 "import org.a;\n"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -541,6 +541,8 @@
Style.IndentWrappedFunctionNames);
 IO.mapOptional("InsertTrailingCommas", Style.InsertTrailingCommas);
 IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
+IO.mapOptional("JavaStaticImportAfterImport",
+   Style.JavaStaticImportAfterImport);
 IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
@@ -2310,12 +2312,15 @@
 JavaImportGroups.push_back(
 findJavaImportGroup(Style, Imports[i].Identifier));
   }
+  bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
   llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
 // Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
-   Imports[LHSI].Identifier) <
-   std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
-   Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+   std::make_tuple(!Imports[RHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[RHSI], Imports[RHSI].Identifier);
   });
 
   // Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1671,6 +1671,10 @@
   bool JavaScriptWrapImports;
   // clang-format on
 
+  /// If true, clang-format will put Java Static imports after all non-static
+  /// imports.
+  bool JavaStaticImportAfterImport;
+
   /// If true, the empty line at the start of blocks is kept.
   /// \code
   ///true:  false:
@@ -2389,6 +2393,7 @@
IndentWidth == R.IndentWidth && Language == R.Language &&
IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
JavaImportGroups == R.JavaImportGroups &&
+   JavaStaticImportAfterImport == R.JavaStaticImportAfterImport &&
JavaScriptQuotes == R.JavaScriptQuotes &&
JavaScriptWrapImports == R.JavaScriptWrapImports &&
KeepEmptyLinesAtTheStartOfBlocks ==
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2021,6 +2021,20 @@
  false:
  import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,} from "some/module.js"
 
+**JavaStaticImportAfterImport** (``bool``)
+ If true, clang-format will put Java static imports after all non-static imports.