[clang-format] Correctly limit formatted ranges when specifying qualifier alignment

[cogilvie]

Summary

The qualifier alignment fixer appeared to ignore any ranges specified for limiting formatting.
This change ensures that it only formats affected lines to avoid unexpected changes as reported by:
https://github.com/llvm/llvm-project/issues/54888

[owenpan]

clang/unittests/Format/QualifierFixerTest.cpp

1351

1352–1357

Please use verifyFormat instead of EXPECT_EQ here and below.

1359–1360

[MyDeveloperDay]

Thanks for the patch...this tells me people are starting to use this feature in anger!! i.e. your formatting via git-clang-format (which is brave!) ;-) which means you have the code modifying features perhaps on my default...

LGTM, if you will need someone to land this for you we'll need your name and email address, if not please wait for @owenpan

[owenpan]

Please mark review comments as done after addressing them.

clang/unittests/Format/QualifierFixerTest.cpp

1356

1366

[cogilvie]

Fixed review comments

[cogilvie]

In D149643#4316128, @MyDeveloperDay wrote:

Thanks for the patch...this tells me people are starting to use this feature in anger!! i.e. your formatting via git-clang-format (which is brave!) ;-) which means you have the code modifying features perhaps on my default...

LGTM, if you will need someone to land this for you we'll need your name and email address, if not please wait for @owenpan

Thanks for the reviews.

Yes formatting modifications by default seems to work well when working on older codebases where applying changes to the whole file hides the real intent of a change.

Please submit this for me with the details Colin Ogilvie [e-mail]

clang/lib/Format/QualifierAlignmentFixer.cpp

LeftRightQualifierAlignmentFixer::analyze(
   const AdditionalKeywords &Keywords = Tokens.getKeywords();
   const SourceManager &SourceMgr = Env.getSourceManager();
   AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
 
   tok::TokenKind QualifierToken = getTokenFromQualifier(Qualifier);
   assert(QualifierToken != tok::identifier && "Unrecognised Qualifier");
 
   for (AnnotatedLine *Line : AnnotatedLines) {
-    if (Line->InPPDirective)
+    if (!Line->Affected || Line->InPPDirective)
       continue;
     FormatToken *First = Line->First;
     assert(First);
     if (First->Finalized)
       continue;
 
     const auto *Last = Line->Last;
 

clang/unittests/Format/QualifierFixerTest.cpp

verifyFormat("template <typename T>\n"
                "  requires Concept<T const>\n"
                "volatile const Foo f();",
                Style);
   verifyFormat("TemplateType<const T> t;", "TemplateType<T const> t;", Style);
   verifyFormat("TemplateType<const Container> t;",
                "TemplateType<Container const> t;", Style);
 }
 
+TEST_F(QualifierFixerTest, Ranges) {
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"const", "volatile", "type"};
+
+  // Only the first line should be formatted; the second should remain as is.
+  verifyFormat("template <typename T> const Foo f();\n"
+               "template <typename T> Foo const f();",
+               "template <typename T> Foo const f();\n"
+               "template <typename T> Foo const f();",
+               Style, std::vector<tooling::Range>(1, tooling::Range(0, 36)));
+
+  // Only the middle line should be formatted; the first and last should remain
+  // as is.
+  verifyFormat("template <typename T> Foo const f();\n"
+               "template <typename T> const Foo f();\n"
+               "template <typename T> Foo const f();",
+               "template <typename T> Foo const f();\n"
+               "template <typename T> Foo const f();\n"
+               "template <typename T> Foo const f();",
+               Style, std::vector<tooling::Range>(1, tooling::Range(37, 36)));
+}
+
 } // namespace
 } // namespace test
 } // namespace format
 } // namespace clang