[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/docs/ClangFormatStyleOptions.rst

**Cpp11BracedListStyle** (``bool``)
   .. code-block:: c++
 
      true:                                  false:
      vector<int> x{1, 2, 3, 4};     vs.     vector<int> x{ 1, 2, 3, 4 };
      vector<T> x{{}, {}, {}, {}};           vector<T> x{ {}, {}, {}, {} };
      f(MyMap[{composite, key}]);            f(MyMap[{ composite, key }]);
      new int[3]{1, 2, 3};                   new int[3]{ 1, 2, 3 };
 
+**DeriveLineEnding** (``bool``)
+  Analyze the formatted file for the most used line ending (``\r\n``
+  or ``\n``). ``UseCRLF`` is only used as a fallback if none can be derived.
+
 **DerivePointerAlignment** (``bool``)
   If ``true``, analyze the formatted file for the most common
   alignment of ``&`` and ``*``.
   Pointer and reference alignment styles are going to be updated according
   to the preferences found in the file.
   ``PointerAlignment`` is then used only as fallback.
 
 **DisableFormat** (``bool``)
**SpaceBeforeRangeBasedForLoopColon** (``bool``)
   If ``false``, spaces will be removed before range-based for loop
   colon.
 
   .. code-block:: c++
 
      true:                                  false:
      for (auto v : values) {}       vs.     for(auto v: values) {}
 
+**SpaceBeforeSquareBrackets** (``bool``)
+  If ``true``, spaces will be before  ``[``.
+  Lambdas will not be affected. Only the first ``[`` will get a space added.
+
+  .. code-block:: c++
+
+     true:                                  false:
+     int a [5];                    vs.      int a[5];
+     int a [5][5];                 vs.      int a[5][5];
+
 **SpaceInEmptyBlock** (``bool``)
   If ``true``, spaces will be inserted into ``{}``.
 
   .. code-block:: c++
 
      true:                                false:
      void f() { }                   vs.   void f() {}
      while (true) { }                     while (true) {}
**TypenameMacros** (``std::vector<std::string>``)
   In the .clang-format configuration file, this can be configured like:
 
   .. code-block:: yaml
 
     TypenameMacros: ['STACK_OF', 'LIST']
 
   For example: OpenSSL STACK_OF, BSD LIST_ENTRY.
 
+**UseCRLF** (``bool``)
+  Use ``\r\n`` instead of ``\n`` for line breaks.
+  Also used as fallback if ``DeriveLineEnding`` is true.
+
 **UseTab** (``UseTabStyle``)
   The way to use tab characters in the resulting file.
 
   Possible values:
 
   * ``UT_Never`` (in configuration: ``Never``)
     Never use tab.
 

clang/include/clang/Format/Format.h

   /// be affected.
   /// \code
   ///    true:                                  false:
   ///    int a[ 5 ];                    vs.     int a[5];
   ///    std::unique_ptr<int[]> foo() {} // Won't be affected
   /// \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>>
   ///    c++03                          vs.     vector<set<int> >
   /// \endcode
   ///
   /// The correct way to spell a specific language version is e.g. ``c++11``.
   /// The historical aliases ``Cpp03`` and ``Cpp11`` are deprecated.
return AccessModifierOffset == R.AccessModifierOffset &&
            SpaceInEmptyBlock == R.SpaceInEmptyBlock &&
            SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
            SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&
            SpacesInAngles == R.SpacesInAngles &&
            SpacesInContainerLiterals == R.SpacesInContainerLiterals &&
            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;
 
   // Stores per-language styles. A FormatStyle instance inside has an empty
   // StyleSet. A FormatStyle instance returned by the Get method has its
   // StyleSet set to a copy of the originating StyleSet, effectively keeping the
   // internal representation of that StyleSet alive.

clang/lib/Format/Format.cpp

IO.mapOptional("SpacesBeforeTrailingComments",
                    Style.SpacesBeforeTrailingComments);
     IO.mapOptional("SpacesInAngles", Style.SpacesInAngles);
     IO.mapOptional("SpacesInContainerLiterals",
                    Style.SpacesInContainerLiterals);
     IO.mapOptional("SpacesInCStyleCastParentheses",
                    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);
     IO.mapOptional("TypenameMacros", Style.TypenameMacros);
     IO.mapOptional("UseCRLF", Style.UseCRLF);
     IO.mapOptional("UseTab", Style.UseTab);
   }
 };
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.SpaceAfterLogicalNot = false;
   LLVMStyle.SpaceAfterTemplateKeyword = true;
   LLVMStyle.SpaceBeforeCtorInitializerColon = true;
   LLVMStyle.SpaceBeforeInheritanceColon = true;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
   LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpaceBeforeCpp11BracedList = false;
+  LLVMStyle.SpaceBeforeSquareBrackets = false;
   LLVMStyle.SpacesInAngles = false;
 
   LLVMStyle.PenaltyBreakAssignment = prec::Assignment;
   LLVMStyle.PenaltyBreakComment = 300;
   LLVMStyle.PenaltyBreakFirstLessLess = 120;
   LLVMStyle.PenaltyBreakString = 1000;
   LLVMStyle.PenaltyExcessCharacter = 1000000;
   LLVMStyle.PenaltyReturnTypeOnItsOwnLine = 60;
analyze(TokenAnnotator &Annotator,
     AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
     for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
       Annotator.calculateFormattingInformation(*AnnotatedLines[i]);
     }
     Annotator.setCommentLineLevels(AnnotatedLines);
 
     WhitespaceManager Whitespaces(
         Env.getSourceManager(), Style,
-        Style.DeriveLineEnding ?
-          inputUsesCRLF(Env.getSourceManager().getBufferData(Env.getFileID()),
-                        Style.UseCRLF) :
-          Style.UseCRLF);
+        Style.DeriveLineEnding
+            ? inputUsesCRLF(
+                  Env.getSourceManager().getBufferData(Env.getFileID()),
+                  Style.UseCRLF)
+            : Style.UseCRLF);
     ContinuationIndenter Indenter(Style, Tokens.getKeywords(),
                                   Env.getSourceManager(), Whitespaces, Encoding,
                                   BinPackInconclusiveFunctions);
     unsigned Penalty =
         UnwrappedLineFormatter(&Indenter, &Whitespaces, Style,
                                Tokens.getKeywords(), Env.getSourceManager(),
                                Status)
             .format(AnnotatedLines, /*DryRun=*/false,

clang/lib/Format/TokenAnnotator.cpp

return Right.MatchingParen &&
              Right.MatchingParen->isOneOf(TT_ArraySubscriptLSquare,
                                           TT_StructuredBindingLSquare,
                                           TT_LambdaLSquare)) ||
             Right.MatchingParen->is(TT_AttributeParen));
   if (Right.is(tok::l_square) &&
       !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 "{}".
   if ((Left.is(tok::l_brace) && Left.BlockKind != BK_Block) ||
       (Right.is(tok::r_brace) && Right.MatchingParen &&
        Right.MatchingParen->BlockKind != BK_Block))
     return Style.Cpp11BracedListStyle ? Style.SpacesInParentheses : true;
   if (Left.is(TT_BlockComment))

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, ConfigurableSpacesInSquareBrackets) {
   verifyFormat("return [ i, args... ] {};", Spaces);
   verifyFormat("int foo = [ &bar ]() {};", Spaces);
   verifyFormat("int foo = [ = ]() {};", Spaces);
   verifyFormat("int foo = [ & ]() {};", Spaces);
   verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
   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;");
   verifyFormat("a or_eq 8;");
 
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpaceBeforeAssignmentOperators = false;
   verifyFormat("int a= 5;", Spaces);
TEST_F(FormatTest, ParsesConfigurationBools) {
   CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
   CHECK_PARSE_BOOL(SpaceAfterTemplateKeyword);
   CHECK_PARSE_BOOL(SpaceAfterLogicalNot);
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
   CHECK_PARSE_BOOL(SpaceBeforeCpp11BracedList);
   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);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterClass);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterEnum);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterNamespace);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterObjCDeclaration);