[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 between identifer and '['.
+  bool SpaceBeforeBrackets;
+
   /// \brief If \c true, spaces may be inserted into '()'.
   bool SpaceInEmptyParentheses;
 

            SpacesInCStyleCastParentheses == R.SpacesInCStyleCastParentheses &&
            SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
            SpaceBeforeParens == R.SpaceBeforeParens &&
+           SpaceBeforeBrackets == R.SpaceBeforeBrackets &&
            SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
            ContinuationIndentWidth == R.ContinuationIndentWidth &&
            CommentPragmas == R.CommentPragmas &&

lib/Format/Format.cpp

                    Style.SpacesInContainerLiterals);
     IO.mapOptional("SpaceBeforeAssignmentOperators",
                    Style.SpaceBeforeAssignmentOperators);
+    IO.mapOptional("SpaceBeforeBrackets", Style.SpaceBeforeBrackets);
     IO.mapOptional("ContinuationIndentWidth", Style.ContinuationIndentWidth);
     IO.mapOptional("CommentPragmas", Style.CommentPragmas);
     IO.mapOptional("ForEachMacros", Style.ForEachMacros);

   LLVMStyle.SpaceAfterCStyleCast = false;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
+  LLVMStyle.SpaceBeforeBrackets = false;
   LLVMStyle.SpacesInAngles = false;
 
   LLVMStyle.PenaltyBreakComment = 300;

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.SpaceBeforeBrackets))
     return false;
   if (Left.is(tok::colon))
     return !Left.is(TT_ObjCMethodExpr);

unittests/Format/FormatTest.cpp

   verifyFormat("int f () throw (Deprecated);", Space);
 }
 
+TEST_F(FormatTest, ConfigurableSpaceBeforeBrackets) {
+    FormatStyle NoSpace = getLLVMStyle();
+    NoSpace.SpaceBeforeBrackets = false;
+
+    verifyFormat("int a[1];", NoSpace);
+    verifyFormat("int a[1][2];", NoSpace);
+    verifyFormat("a[7] = 5;", NoSpace);
+    verifyFormat("int a = (f())[23];", NoSpace);
+
+    FormatStyle Space = getLLVMStyle();
+    Space.SpaceBeforeBrackets = true;
+
+    verifyFormat("int a [1];", Space);
+    verifyFormat("int a [1][2];", Space);
+    verifyFormat("a [7] = 5;", Space);
+    verifyFormat("int a = (f()) [23];", Space);
+}
+
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   FormatStyle Spaces = getLLVMStyle();
 

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