[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)

[Richard Li via cfe-commits]

https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568

>From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001
From: chomosuke <a13323600 at gmail.com>
Date: Tue, 3 Dec 2024 07:10:33 +0000
Subject: [PATCH 1/2] fixed removeFunctionArgs don't remove comma

---
 .../clang-tidy/utils/UseRangesCheck.cpp       | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
index aba4d17ccd035e..88cba70b931d5d 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
@@ -28,6 +28,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
+#include <iostream>
 #include <optional>
 #include <string>
 
@@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call,
   for (unsigned Index : Sorted) {
     const Expr *Arg = Call.getArg(Index);
     if (Commas[Index]) {
-      if (Index >= Commas.size()) {
-        Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
-      } else {
+      if (Index + 1 < Call.getNumArgs()) {
         // Remove the next comma
         Commas[Index + 1] = true;
+        const Expr *NextArg = Call.getArg(Index + 1);
         Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
-            {Arg->getBeginLoc(),
-             Lexer::getLocForEndOfToken(
-                 Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts())
-                 .getLocWithOffset(1)}));
+            {Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)}));
+      } else {
+        Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
       }
     } else {
-      Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
-          Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc()));
+      // At this point we know Index > 0 because `Commas[0] = true` earlier
       Commas[Index] = true;
+      const Expr *PrevArg = Call.getArg(Index - 1);
+      Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+          PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc()));
     }
   }
 }

>From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323600 at gmail.com>
Date: Mon, 23 Dec 2024 05:17:08 +1100
Subject: [PATCH 2/2] find , and remove only ,

---
 .../clang-tidy/utils/UseRangesCheck.cpp       | 49 +++++++++++++++----
 1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
index 88cba70b931d5d..8b8e44a9898fdd 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
@@ -28,7 +28,6 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
-#include <iostream>
 #include <optional>
 #include <string>
 
@@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) {
 static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call,
                                ArrayRef<unsigned> Indexes,
                                const ASTContext &Ctx) {
+  auto GetCommaLoc =
+      [&](SourceLocation BeginLoc,
+          SourceLocation EndLoc) -> std::optional<CharSourceRange> {
+    auto Invalid = false;
+    auto SourceText = Lexer::getSourceText(
+        CharSourceRange::getCharRange({BeginLoc, EndLoc}),
+        Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid);
+    assert(!Invalid);
+
+    size_t I = 0;
+    while (I < SourceText.size() && SourceText[I] != ',') {
+      I++;
+    }
+
+    if (I < SourceText.size()) {
+      // also remove space after ,
+      size_t J = I + 1;
+      while (J < SourceText.size() && SourceText[J] == ' ') {
+        J++;
+      }
+
+      return std::make_optional(CharSourceRange::getCharRange(
+          {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)}));
+    }
+    return std::nullopt;
+  };
+
   llvm::SmallVector<unsigned> Sorted(Indexes);
   llvm::sort(Sorted);
   // Keep track of commas removed
@@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call,
     if (Commas[Index]) {
       if (Index + 1 < Call.getNumArgs()) {
         // Remove the next comma
-        Commas[Index + 1] = true;
         const Expr *NextArg = Call.getArg(Index + 1);
-        Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
-            {Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)}));
-      } else {
-        Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
+        auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1),
+                                    NextArg->getBeginLoc());
+        if (CommaLoc) {
+          Commas[Index + 1] = true;
+          Diag << FixItHint::CreateRemoval(*CommaLoc);
+        }
       }
     } else {
       // At this point we know Index > 0 because `Commas[0] = true` earlier
-      Commas[Index] = true;
       const Expr *PrevArg = Call.getArg(Index - 1);
-      Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
-          PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc()));
+      auto CommaLoc = GetCommaLoc(PrevArg->getEndLoc().getLocWithOffset(1),
+                                  Arg->getBeginLoc());
+      if (CommaLoc) {
+        Commas[Index] = true;
+        Diag << FixItHint::CreateRemoval(*CommaLoc);
+      }
     }
+    Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
   }
 }