[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 ↗

(On Diff #17989)

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 ↗

(On Diff #17989)

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 ↗

(On Diff #35810)

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.

clang/include/clang/Format/Format.h

   /// \endcode
   bool SpacesInSquareBrackets;
 
+  /// If ``true``, spaces will be before  ``[``.
+  /// Lambdas will not be affected. Only the first ``[`` will get a space added.
+  /// \code
+  ///    true:                                  false:
+  ///    int a [5];                    vs.      int a[5];
+  ///    int a [5][5];                 vs.      int a[5][5];
+  /// \endcode
+  bool SpaceBeforeSquareBrackets;
+
   /// Supported language standards for parsing and formatting C++ constructs.
   /// \code
   ///    Latest:                                vector<set<int>>

            SpacesInCStyleCastParentheses == R.SpacesInCStyleCastParentheses &&
            SpacesInParentheses == R.SpacesInParentheses &&
            SpacesInSquareBrackets == R.SpacesInSquareBrackets &&
+           SpaceBeforeSquareBrackets == R.SpaceBeforeSquareBrackets &&
            Standard == R.Standard && TabWidth == R.TabWidth &&
            StatementMacros == R.StatementMacros && UseTab == R.UseTab &&
-           UseCRLF == R.UseCRLF &&
-           TypenameMacros == R.TypenameMacros;
+           UseCRLF == R.UseCRLF && TypenameMacros == R.TypenameMacros;
   }
 
   llvm::Optional<FormatStyle> GetLanguageStyle(LanguageKind Language) const;

clang/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("StatementMacros", Style.StatementMacros);
     IO.mapOptional("TabWidth", Style.TabWidth);

   LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpaceBeforeCpp11BracedList = false;
+  LLVMStyle.SpaceBeforeSquareBrackets = false;
   LLVMStyle.SpacesInAngles = false;
 
   LLVMStyle.PenaltyBreakAssignment = prec::Assignment;

clang/lib/Format/TokenAnnotator.cpp

       !Right.isOneOf(TT_ObjCMethodExpr, TT_LambdaLSquare,
                      TT_DesignatedInitializerLSquare,
                      TT_StructuredBindingLSquare, TT_AttributeSquare) &&
-      !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::l_brace) && Right.is(tok::r_brace))
     return !Left.Children.empty(); // No spaces in "{}".

clang/unittests/Format/FormatTest.cpp

   verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
 
+TEST_F(FormatTest, ConfigurableSpaceBeforeBrackets) {
+  FormatStyle NoSpaceStyle = getLLVMStyle();
+  verifyFormat("int a[5];", NoSpaceStyle);
+  verifyFormat("a[3] += 42;", NoSpaceStyle);
+
+  verifyFormat("int a[1];", NoSpaceStyle);
+  verifyFormat("int 1 [a];", NoSpaceStyle);
+  verifyFormat("int a[1][2];", NoSpaceStyle);
+  verifyFormat("a[7] = 5;", NoSpaceStyle);
+  verifyFormat("int a = (f())[23];", NoSpaceStyle);
+  verifyFormat("f([] {})", NoSpaceStyle);
+
+  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, ConfigurableSpaceBeforeAssignmentOperators) {
   verifyFormat("int a = 5;");
   verifyFormat("a += 42;");

   CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
   CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
   CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
+  CHECK_PARSE_BOOL(SpaceBeforeSquareBrackets);
   CHECK_PARSE_BOOL(UseCRLF);
 
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterCaseLabel);