[clang-format] Add SpaceBeforeBrackets

[Anteru]

Summary

Adds a new option SpaceBeforeBrackets to add spaces before brackets (i.e. int a[23]; -> int a [23];) This is present as an option in the Visual Studio C++ code formatting settings, but there was no matching setting in clang-format.

[djasper]

Sorry it took me so long to reply.

include/clang/Format/Format.h

356–357

To be consistent with the option above, this should probably be called SpaceBeforeSquareBrackets. This is to make clear that this is not considering angle brackets or curly brackets.

However, I am actually not sure that is the best name. I think you are only trying to change the behavior for array subscripts. Maybe we should name the option accordingly (SpaceBeforeArraySubscripts). We can also rename SpaceInSquareBrackets correspondingly (I'll do that in a separate follow-up patch as it is a little bit tedious to be backwards compatible).

unittests/Format/FormatTest.cpp

7992

If this is only targeting array subscripts as I presume, please add corresponding tests that it doesn't influence other behavior, specifically lambdas and objc method expressions (e.g. "f([] {});").

Also not that array subscripts can be written the other way around "1[a]" is identical to "a[1]". Add a test for that as well.

[Anteru]

Here's an updated diff which should do the trick. If not, I'll fix again :)

[MyDeveloperDay]

I realize this is an old diff, and you and I have just spoke about it on twitter https://twitter.com/NIV_Anteru/status/1193792307386081281?s=20 would you consider rebasing as the /brief style isn't used any more

It would help the approval process if you could highlight a public project with a style guide that uses this style.

I think this is a simple enough addition to consider by getting it upto date will help

include/clang/Format/Format.h

415

when you rebase you'll see examples of /codeblocks with before and after I think that would really help

[Anteru]

Updated as requested.

[MyDeveloperDay]

Thank you for this patch. I'm sorry it took so long, I think this is a perfectly reasonable idea and helps cover the functionality that visual studio also supports. (which ultimately will help visual studio users embrace clang-format even more than they are already)

This LGTM, and thank you, let us know if you need someone to land this for you.

[Anteru]

Thanks a lot! And yes, I need someone to help me land this :) -- I have no idea how to land it myself.

[sylvestre.ledru]

fyi, +x permissions was added to clang/lib/Format/Format.cpp
I reverted the change in a4a7c1259e8a8f2d11fa29686a6c2834948c1358

[sylvestre.ledru]

Would be nice to add this to the release notes too ;)

[MyDeveloperDay]

In D6920#1748764, @sylvestre.ledru wrote:

Would be nice to add this to the release notes too ;)

Addressing this and other missing release notes in D70355: [clang-format] [NFC] add recent changes to release notes, want to try and bring about a change were new clang-format changes are always release noted as I've seen done with clang-tidy

[MyDeveloperDay]

@Anteru thanks for the patch this got landed today, I got burnt by the 100755 mode change on the file, but thanks to @sylvestre.ledru it got resolved.

include/clang/Format/Format.h

   /// \brief If \c true, spaces will be inserted after '[' and before ']'.
   bool SpacesInSquareBrackets;
 
+  /// \brief If \c true, a space will be inserted before '[' in array subscripts.
+  bool SpaceBeforeSquareBrackets;
+
   /// \brief Supported language standards.
   enum LanguageStandard {
     /// Use C++03-compatible syntax.

            SpacesInCStyleCastParentheses == R.SpacesInCStyleCastParentheses &&
            SpacesInParentheses == R.SpacesInParentheses &&
            SpacesInSquareBrackets == R.SpacesInSquareBrackets &&
+           SpaceBeforeSquareBrackets == R.SpaceBeforeSquareBrackets &&
            Standard == R.Standard &&
            TabWidth == R.TabWidth &&
            UseTab == R.UseTab;

lib/Format/Format.cpp

                    Style.SpacesInCStyleCastParentheses);
     IO.mapOptional("SpacesInParentheses", Style.SpacesInParentheses);
     IO.mapOptional("SpacesInSquareBrackets", Style.SpacesInSquareBrackets);
+    IO.mapOptional("SpaceBeforeSquareBrackets", Style.SpaceBeforeSquareBrackets);
     IO.mapOptional("Standard", Style.Standard);
     IO.mapOptional("TabWidth", Style.TabWidth);
     IO.mapOptional("UseTab", Style.UseTab);

   LLVMStyle.UseTab = FormatStyle::UT_Never;
   LLVMStyle.SpacesInParentheses = false;
   LLVMStyle.SpacesInSquareBrackets = false;
+  LLVMStyle.SpaceBeforeSquareBrackets = false;
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;

lib/Format/TokenAnnotator.cpp

              Right.MatchingParen->is(TT_ArraySubscriptLSquare)));
   if (Right.is(tok::l_square) &&
       !Right.isOneOf(TT_ObjCMethodExpr, TT_LambdaLSquare) &&
-      !Left.isOneOf(tok::numeric_constant, TT_DictLiteral))
+      !Left.isOneOf(tok::numeric_constant, TT_DictLiteral) &&
+      !(!Left.is(tok::r_square) && Style.SpaceBeforeSquareBrackets &&
+        Right.is(TT_ArraySubscriptLSquare)))
     return false;
   if (Left.is(tok::colon))
     return !Left.is(TT_ObjCMethodExpr);

unittests/Format/FormatTest.cpp

   verifyFormat("X A::operator++ (T);", Space);
 }
 
+TEST_F(FormatTest, ConfigurableSpaceBeforeSquareBrackets) {
+  FormatStyle NoSpace = getLLVMStyle();
+  NoSpace.SpaceBeforeSquareBrackets = false;
+  
+  verifyFormat("int a[5];", NoSpace);
+  verifyFormat("a[3] += 42;", NoSpace);
+
+  verifyFormat("int a[1];", NoSpace);
+  verifyFormat("int 1 [a];", NoSpace);
+  verifyFormat("int a[1][2];", NoSpace);
+  verifyFormat("a[7] = 5;", NoSpace);
+  verifyFormat("int a = (f())[23];", NoSpace);
+  verifyFormat("f([] {})", NoSpace);
+
+  FormatStyle Space = getLLVMStyle();
+  Space.SpaceBeforeSquareBrackets = true;
+  verifyFormat("int c = []() -> int { return 2; }();\n", Space);
+  verifyFormat("return [i, args...] {};", Space);
+
+  verifyFormat("int a [5];", Space);
+  verifyFormat("a [3] += 42;", Space);
+  verifyFormat("constexpr char hello []{\"hello\"};", Space);
+  verifyFormat("double &operator[](int i) { return 0; }\n"
+                "int i;",
+                Space);
+  verifyFormat("std::unique_ptr<int []> foo() {}", Space);
+  verifyFormat("int i = a [a][a]->f();", Space);
+  verifyFormat("int i = (*b) [a]->f();", Space);
+
+  verifyFormat("int a [1];", Space);
+  verifyFormat("int 1 [a];", Space);
+  verifyFormat("int a [1][2];", Space);
+  verifyFormat("a [7] = 5;", Space);
+  verifyFormat("int a = (f()) [23];", Space);
+  verifyFormat("f([] {})", Space);
+}
+
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   FormatStyle Spaces = getLLVMStyle();
 

   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
   CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
+  CHECK_PARSE_BOOL(SpaceBeforeSquareBrackets);
 }
 
 #undef CHECK_PARSE_BOOL