[clang-format] Adding style option for absolute formatting

[acoomans]

Summary

When formatting only certain lines for a file (clang-format -lines=x:x), two types of indentation could be expected:

• absolute: the indentation as if the whole file was formatted
• relative: the indentation is relative to the previous line, even if those are not indented correctly and are not in the range of lines to format

The default behavior for clang-format is relative formatting; e.g: running clang-format -lines=3:3 on (note the extra white space in front of @optional):

@protocol A
 @optional
// comment
- (void)f;
@end
MACRO

would give:

@protocol A
 @optional
 // comment
 - (void)f;
 @end
 MACRO

This diff adds a style option to specify absolute indentation; continuing the example above, with the option set, running clang-format -lines=3:3 would give:

@protocol A
 @optional
// comment
- (void)f;
@end
MACRO

Initial bug report / feature request: https://bugs.llvm.org/show_bug.cgi?id=37975

[acoomans]

An alternative to changing the whole behavior would be to make it a parameter / configuration option.

[djasper]

Could you explain what problem this is fixing?

[acoomans]

@djasper I updated the description of the diff.

This fixes the issue of clang-format -lines=x:x not returning the same results as clang-format, while keeping the current behavior as default.

[acoomans]

Also ping @jolesiak since he initially filed the report

[djasper]

In my opinion, this only addresses one edge case where clang-format -lines output is not identical with a full reformatting. I believe they cannot all usefully be avoided. As such, I am unsure that this option carries its weight of making the implementation more complex.
How often does this happen for you? Why can't you just format the additional incorrect line?

[acoomans]

I don't know; I just picked a random bug from the Bugzilla to get myself familiarized with the LLVM codebase :)

@jolesiak?

[jolesiak]

Sorry for the delay, I'll comment on that tomorrow.

[jolesiak]

First of all, thanks, Arnaud, for looking into this.

Let's address the mentioned bug first.
Let's assume that we use IndentReference = Relative. I think that this particular formatting bug is local (i.e. can be solved while processing a @protocol block). Look at this example:

if (true) {
  int a = 42;
    int b = 42;
  int c = 42;
}
int d = 42;

When we run clang-format -lines=4:4 we get:

if (true) {
  int a = 42;
    int b = 42;
    int c = 42;
}
int d = 42;

Please note that neither } nor int d = 42 is indented.
But this is a different behavior from what we see after running clang-format -lines=3:3 on:

@protocol A
 @optional
// comment
- (void)f;
@end
MACRO

the output is:

@protocol A
 @optional
 // comment
 - (void)f;
 @end
 MACRO

Please note that @end is indented (has different indentation than @protocol; MACRO is indented as well).
To clarify why this is undesired, @end ends @protocol, not @optional. By the way, clang-format doesn't think that @end should match @optional, check the output of an extended example:

@protocol A
 @optional
 // comment
 - (void)f;
 @end
 MACRO
 @end

Second @end still doesn't match the @protocol.

I think it's perfectly fine to allow users to have custom indentations if they so choose. In this particular example, though, clang-format should stop indenting when it reaches @end, giving the following output:

@protocol A
 @optional
 // comment
 - (void)f;
@end
MACRO

Moving to the general case, I must say that introducing IndentReference = Relative/Absolute is a very interesting approach. I can imagine situations when somebody actually want to use IndentReference = Absolute. However, adding an additional option comes at quite big cost and I think that in this case it outweighs the benefits.
I think that a very good outcome could be to comment what is happening next to IndentTracker.adjustToUnmodifiedLine(TheLine); line. E.g. "We adjust an indentation to match the existing code to give users flexibility (instead of ignoring it). It's their responsibility to provide a correct formatting of lines they didn't initially change if these lines break formatting.".

[djasper]

Ok, so IIUC, understanding that @end effective ends a section much like "}" would address the currently observed problems?

[jolesiak]

In D49580#1179924, @djasper wrote:

Ok, so IIUC, understanding that @end effective ends a section much like "}" would address the currently observed problems?

I think so.

[acoomans]

Thanks for the review. I’ll update when I’m back from vacation

Arnaud

[jolesiak]

Thanks!

[acoomans]

Changed approach based on feedback. New changeset: https://reviews.llvm.org/D50535

docs/ClangFormatStyleOptions.rst

.. code-block:: c++
 
        #if FOO
        #  if BAR
        #    include <foo>
        #  endif
        #endif
 
 
+ **IndentReference** (``IndentReferenceStyle``)
+   Indent relative to the previous line or in absolute.
+
+   Only useful for when formatting only certain lines.
+
+   Possible values:
+
+   * ``IRS_Relative`` (in configuration: ``Relative``)
+     Indent relative to previous line, e.g.:
+
+     .. code-block:: c
+
+       int a = 0;
+        int f(void) {
+          int i = 0;
+          return i;
+       }
+
+   * ``IRS_Absolute`` (in configuration: ``Absolute``)
+     Indent as if the whole file was formatted, e.g.:
+
+     .. code-block:: c
+
+       int a = 0;
+        int f(void) {
+         int i = 0;
+         return i;
+       }
+
 
 **IndentWidth** (``unsigned``)
   The number of columns to use for indentation.
 
   .. code-block:: c++
 
      IndentWidth: 3
 

include/clang/Format/Format.h

enum PPDirectiveIndentStyle {
     ///    #endif
     /// \endcode
     PPDIS_AfterHash
   };
 
   /// The preprocessor directive indenting style to use.
   PPDirectiveIndentStyle IndentPPDirectives;
 
+  /// Different styles for indenting, used for partial formatting.
+  enum IndentReferenceStyle {
+    /// Indent relative to previous line, e.g.:
+    /// \code
+    ///   int a = 0;
+    ///    int f(void) { // mis-indented by one space
+    ///      int i = 0; <- those lines indented relative to
+    ///      return i;  <- the indentation of f(), including
+    ///    }            <- the one space mis-indentation
+    /// \endcode
+    IRS_Relative,
+    /// Indent as if the whole file was formatted, e.g.:
+    /// \code
+    ///   int a = 0;
+    ///    int f(void) { // mis-indented by one space
+    ///     int i = 0; <- those lines indented absolutely;
+    ///     return i;  <- i.e. ignoring f()'s one space
+    ///   }            <- mis-indentation
+    /// \endcode
+    IRS_Absolute,
+  };
+
+  IndentReferenceStyle IndentReference;
+
   /// The number of columns to use for indentation.
   /// \code
   ///    IndentWidth: 3
   ///
   ///    void f() {
   ///       someFunction();
   ///       if (true, false) {
   ///          f();
return AccessModifierOffset == R.AccessModifierOffset &&
            ExperimentalAutoDetectBinPacking ==
                R.ExperimentalAutoDetectBinPacking &&
            FixNamespaceComments == R.FixNamespaceComments &&
            ForEachMacros == R.ForEachMacros &&
            IncludeStyle.IncludeBlocks == R.IncludeStyle.IncludeBlocks &&
            IncludeStyle.IncludeCategories == R.IncludeStyle.IncludeCategories &&
            IndentCaseLabels == R.IndentCaseLabels &&
            IndentPPDirectives == R.IndentPPDirectives &&
+           IndentReference == R.IndentReference &&
            IndentWidth == R.IndentWidth && Language == R.Language &&
            IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
            JavaScriptQuotes == R.JavaScriptQuotes &&
            JavaScriptWrapImports == R.JavaScriptWrapImports &&
            KeepEmptyLinesAtTheStartOfBlocks ==
                R.KeepEmptyLinesAtTheStartOfBlocks &&
            MacroBlockBegin == R.MacroBlockBegin &&
            MacroBlockEnd == R.MacroBlockEnd &&

lib/Format/Format.cpp

static void enumeration(IO &IO,
     IO.enumCase(Value, "Right", FormatStyle::ENAS_Right);
 
     // For backward compatibility.
     IO.enumCase(Value, "true", FormatStyle::ENAS_Left);
     IO.enumCase(Value, "false", FormatStyle::ENAS_Right);
   }
 };
 
+template <> struct ScalarEnumerationTraits<FormatStyle::IndentReferenceStyle> {
+  static void enumeration(IO &IO, FormatStyle::IndentReferenceStyle &Value) {
+    IO.enumCase(Value, "Relative", FormatStyle::IRS_Relative);
+    IO.enumCase(Value, "Absolute", FormatStyle::IRS_Absolute);
+  }
+};
+
 template <> struct ScalarEnumerationTraits<FormatStyle::PointerAlignmentStyle> {
   static void enumeration(IO &IO, FormatStyle::PointerAlignmentStyle &Value) {
     IO.enumCase(Value, "Middle", FormatStyle::PAS_Middle);
     IO.enumCase(Value, "Left", FormatStyle::PAS_Left);
     IO.enumCase(Value, "Right", FormatStyle::PAS_Right);
 
     // For backward compatibility.
     IO.enumCase(Value, "true", FormatStyle::PAS_Left);
IO.mapOptional("ExperimentalAutoDetectBinPacking",
                    Style.ExperimentalAutoDetectBinPacking);
     IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);
     IO.mapOptional("ForEachMacros", Style.ForEachMacros);
     IO.mapOptional("IncludeBlocks", Style.IncludeStyle.IncludeBlocks);
     IO.mapOptional("IncludeCategories", Style.IncludeStyle.IncludeCategories);
     IO.mapOptional("IncludeIsMainRegex", Style.IncludeStyle.IncludeIsMainRegex);
     IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
     IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
+    IO.mapOptional("IndentReference", Style.IndentReference);
     IO.mapOptional("IndentWidth", Style.IndentWidth);
     IO.mapOptional("IndentWrappedFunctionNames",
                    Style.IndentWrappedFunctionNames);
     IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
     IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
     IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
                    Style.KeepEmptyLinesAtTheStartOfBlocks);
     IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
FormatStyle getLLVMStyle() {
   LLVMStyle.IncludeStyle.IncludeCategories = {
       {"^\"(llvm|llvm-c|clang|clang-c)/", 2},
       {"^(<|\"(gtest|gmock|isl|json)/)", 3},
       {".*", 1}};
   LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$";
   LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve;
   LLVMStyle.IndentCaseLabels = false;
   LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
+  LLVMStyle.IndentReference = FormatStyle::IRS_Relative;
   LLVMStyle.IndentWrappedFunctionNames = false;
   LLVMStyle.IndentWidth = 2;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
   LLVMStyle.TabWidth = 8;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;

lib/Format/UnwrappedLineFormatter.cpp

if (ShouldFormat && TheLine.Type != LT_Invalid) {
       // If no token in the current line is affected, we still need to format
       // affected children.
       if (TheLine.ChildrenAffected)
         for (const FormatToken *Tok = TheLine.First; Tok; Tok = Tok->Next)
           if (!Tok->Children.empty())
             format(Tok->Children, DryRun);
 
       // Adapt following lines on the current indent level to the same level
-      // unless the current \c AnnotatedLine is not at the beginning of a line.
+      // if indentation should be relative and unless the
+      // current \c AnnotatedLine is not at the beginning of a line.
+      bool RelativeIndentation =
+          Style.IndentReference != FormatStyle::IRS_Absolute;
       bool StartsNewLine =
           TheLine.First->NewlinesBefore > 0 || TheLine.First->IsFirst;
-      if (StartsNewLine)
+      if (RelativeIndentation && StartsNewLine)
         IndentTracker.adjustToUnmodifiedLine(TheLine);
       if (!DryRun) {
         bool ReformatLeadingWhitespace =
             StartsNewLine && ((PreviousLine && PreviousLine->Affected) ||
                               TheLine.LeadingEmptyLinesAffected);
         // Format the first token.
         if (ReformatLeadingWhitespace)
           formatFirstToken(TheLine, PreviousLine, Lines,

test/Format/line-ranges-indent-abs.c

+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -style="{BasedOnStyle: LLVM, IndentReference: Absolute}" -lines=2:2 \
+// RUN:   | FileCheck -strict-whitespace %s
+// CHECK: {{^\ int f\(void\) \{$}}
+ int f(void) {
+// CHECK: {{^\ \ int i = 0;$}}
+  int i = 0;
+// CHECK: {{^\ \ return i;$}}
+  return i;
+}

test/Format/line-ranges-indent-abs.m

+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -style="{BasedOnStyle: LLVM, IndentReference: Absolute}" -lines=2:2 \
+// RUN:   | FileCheck -strict-whitespace %s
+// CHECK: {{^\ \@protocol\ A$}}
+ @protocol A
+// CHECK: {{^-\ \(void\)f;$}}
+- (void)f;
+// CHECK: {{^\@end$}}
+@end

test/Format/line-ranges-indent-rel.c

+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -style=LLVM -lines=2:2 \
+// RUN:   | FileCheck -strict-whitespace %s
+// CHECK: {{^\ int f\(void\) \{$}}
+ int f(void) {
+// CHECK: {{^\ \ \ int i = 0;$}}
+  int i = 0;
+// CHECK: {{^\ \ \ return i;$}}
+  return i;
+}