[-Wunsafe-buffer-usage] Preserve spelling of array size in std::array fixits

Fixits related to arrays generally change type from T [N] to
std::array<T, N>. N has to be constant but doesn't have to be an integer
literal. It can be for example a constant.

The fixits need to preserve the spelling of the array size.
The assumption is that not specifying the numerical value directly was a
conscious decision of the original author and it'll very likely still apply to
the transformed code.

E. g. for
int buffer[SIZE];

if `buffer` is unsafe it should become:
std::array<int, SIZE> buffer;

rather than:
std::array<int, 42> buffer;

Author: jkorous-apple

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -2475,10 +2475,6 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
   if (auto CAT = dyn_cast<clang::ConstantArrayType>(D->getType())) {
     const QualType &ArrayEltT = CAT->getElementType();
     assert(!ArrayEltT.isNull() && "Trying to fix a non-array type variable!");
-    // Producing fix-it for variable declaration---make `D` to be of std::array
-    // type:
-    SmallString<32> Replacement;
-    raw_svector_ostream OS(Replacement);
 
     // For most types the transformation is simple:
     //   T foo[10]; => std::array<T, 10> foo;
@@ -2496,6 +2492,21 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
       return {};
     }
 
+    // Find the '[' token.
+    std::optional<Token> NextTok = Lexer::findNextToken(getVarDeclIdentifierLoc(D), Ctx.getSourceManager(), Ctx.getLangOpts());
+    while (NextTok && !NextTok->is(tok::l_square))
+      NextTok = Lexer::findNextToken(NextTok->getLocation(), Ctx.getSourceManager(), Ctx.getLangOpts());
+    if (!NextTok)
+      return {};
+    const SourceLocation LSqBracketLoc = NextTok->getLocation();
+
+    // Get the spelling of the array size as written in the source file (including macros, etc.).
+    auto MaybeArraySizeTxt = getRangeText({LSqBracketLoc.getLocWithOffset(1), D->getTypeSpecEndLoc()}, Ctx.getSourceManager(), Ctx.getLangOpts());
+    if (!MaybeArraySizeTxt)
+      return {};
+
+    const std::string ArraySizeTxt = MaybeArraySizeTxt->str();
+
     std::optional<StringRef> IdentText =
         getVarDeclIdentifierText(D, Ctx.getSourceManager(), Ctx.getLangOpts());
 
@@ -2504,8 +2515,10 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
       return {};
     }
 
+    SmallString<32> Replacement;
+    raw_svector_ostream OS(Replacement);
     OS << "std::array<" << ArrayEltT.getAsString() << ", "
-       << getAPIntText(CAT->getSize()) << "> " << IdentText->str();
+       << ArraySizeTxt << "> " << IdentText->str();
 
     FixIts.push_back(FixItHint::CreateReplacement(
         SourceRange{D->getBeginLoc(), D->getTypeSpecEndLoc()}, OS.str()));

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp

@@ -10,6 +10,18 @@ void local_array(unsigned idx) {
   buffer[idx] = 0;
 }
 
+void weird_whitespace_in_declaration(unsigned idx) {
+  int      buffer_w   [       10 ] ;
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:35}:"std::array<int,        10 > buffer_w"
+  buffer_w[idx] = 0;
+}
+
+void weird_comments_in_declaration(unsigned idx) {
+  int   /* [ ] */   buffer_w  /* [ ] */ [ /* [ ] */ 10 /* [ ] */ ] ;
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:67}:"std::array<int,  /* [ ] */ 10 /* [ ] */ > buffer_w"
+  buffer_w[idx] = 0;
+}
+
 void unsupported_multi_decl1(unsigned idx) {
   int a, buffer[10];
   // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
@@ -55,23 +67,25 @@ template void local_array_in_template<int>(unsigned); // FIXME: expected note {{
 
 void macro_as_identifier(unsigned idx) {
 #define MY_BUFFER buffer
-  // Although fix-its include macros, the macros do not overlap with
-  // the bounds of the source range of these fix-its. So these fix-its
-  // are valid.
-
   int MY_BUFFER[10];
 // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:20}:"std::array<int, 10> MY_BUFFER"
   MY_BUFFER[idx] = 0;
-
 #undef MY_BUFFER
 }
 
-void unsupported_fixit_overlapping_macro(unsigned idx) {
-#define MY_INT int
-  MY_INT buffer[10];
+void macro_as_size(unsigned idx) {
+#define MY_TEN 10
+  int buffer[MY_TEN];
 // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
   buffer[idx] = 0;
-#undef MY_INT
+#undef MY_TEN
+}
+
+void constant_as_size(unsigned idx) {
+  const unsigned my_const = 10;
+  int buffer[my_const];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:23}:"std::array<int, my_const> buffer"
+  buffer[idx] = 0;
 }
 
 void subscript_negative() {