[clang-tools-extra] r269210 - [clang-tidy] Refactoring of FixHintUtils

Etienne Bergeron via cfe-commits cfe-commits at lists.llvm.org
Wed May 11 10:38:23 PDT 2016


Author: etienneb
Date: Wed May 11 12:38:22 2016
New Revision: 269210

URL: http://llvm.org/viewvc/llvm-project?rev=269210&view=rev
Log:
[clang-tidy] Refactoring of FixHintUtils

Summary:
Refactor some checkers to use the tooling re-writing API.
see: http://reviews.llvm.org/D19941

Reviewers: klimek, alexfh

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D19807

Modified:
    clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp
    clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
    clang-tools-extra/trunk/test/clang-tidy/misc-swapped-arguments.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp?rev=269210&r1=269209&r2=269210&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp Wed May 11 12:38:22 2016
@@ -10,6 +10,7 @@
 #include "SwappedArgumentsCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
 #include "llvm/ADT/SmallPtrSet.h"
 
 using namespace clang::ast_matchers;
@@ -46,25 +47,9 @@ static bool isImplicitCastCandidate(cons
          Cast->getCastKind() == CK_PointerToBoolean;
 }
 
-/// \brief Get a StringRef representing a SourceRange.
-static StringRef getAsString(const MatchFinder::MatchResult &Result,
-                             SourceRange R) {
-  const SourceManager &SM = *Result.SourceManager;
-  // Don't even try to resolve macro or include contraptions. Not worth emitting
-  // a fixit for.
-  if (R.getBegin().isMacroID() ||
-      !SM.isWrittenInSameFile(R.getBegin(), R.getEnd()))
-    return StringRef();
-
-  const char *Begin = SM.getCharacterData(R.getBegin());
-  const char *End = SM.getCharacterData(Lexer::getLocForEndOfToken(
-      R.getEnd(), 0, SM, Result.Context->getLangOpts()));
-
-  return StringRef(Begin, End - Begin);
-}
-
 void SwappedArgumentsCheck::check(const MatchFinder::MatchResult &Result) {
-  auto *Call = Result.Nodes.getStmtAs<CallExpr>("call");
+  const ASTContext &Ctx = *Result.Context;
+  const auto *Call = Result.Nodes.getStmtAs<CallExpr>("call");
 
   llvm::SmallPtrSet<const Expr *, 4> UsedArgs;
   for (unsigned I = 1, E = Call->getNumArgs(); I < E; ++I) {
@@ -99,24 +84,14 @@ void SwappedArgumentsCheck::check(const
       continue;
 
     // Emit a warning and fix-its that swap the arguments.
-    SourceRange LHSRange = LHS->getSourceRange(),
-                RHSRange = RHS->getSourceRange();
-    auto D =
-        diag(Call->getLocStart(), "argument with implicit conversion from %0 "
-                                  "to %1 followed by argument converted from "
-                                  "%2 to %3, potentially swapped arguments.")
+    diag(Call->getLocStart(), "argument with implicit conversion from %0 "
+                              "to %1 followed by argument converted from "
+                              "%2 to %3, potentially swapped arguments.")
         << LHS->getType() << LHSFrom->getType() << RHS->getType()
-        << RHSFrom->getType() << LHSRange << RHSRange;
-
-    StringRef RHSString = getAsString(Result, RHSRange);
-    StringRef LHSString = getAsString(Result, LHSRange);
-    if (!LHSString.empty() && !RHSString.empty()) {
-      D << FixItHint::CreateReplacement(
-               CharSourceRange::getTokenRange(LHSRange), RHSString)
-        << FixItHint::CreateReplacement(
-               CharSourceRange::getTokenRange(RHSRange), LHSString);
-    }
-
+        << RHSFrom->getType()
+        << tooling::fixit::createReplacement(*LHS, *RHS, Ctx)
+        << tooling::fixit::createReplacement(*RHS, *LHS, Ctx);
+    
     // Remember that we emitted a warning for this argument.
     UsedArgs.insert(RHSCast);
   }

Modified: clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp?rev=269210&r1=269209&r2=269210&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp Wed May 11 12:38:22 2016
@@ -10,6 +10,7 @@
 #include "ElseAfterReturnCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
 
 using namespace clang::ast_matchers;
 
@@ -28,20 +29,17 @@ void ElseAfterReturnCheck::registerMatch
       this);
 }
 
-static FixItHint removeToken(SourceLocation Loc) {
-  return FixItHint::CreateRemoval(CharSourceRange::getTokenRange(Loc, Loc));
-}
-
 void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *If = Result.Nodes.getNodeAs<IfStmt>("if");
   SourceLocation ElseLoc = If->getElseLoc();
   DiagnosticBuilder Diag = diag(ElseLoc, "don't use else after return");
-  Diag << removeToken(ElseLoc);
+  Diag << tooling::fixit::createRemoval(ElseLoc);
 
   // FIXME: Removing the braces isn't always safe. Do a more careful analysis.
   // FIXME: Change clang-format to correctly un-indent the code.
   if (const auto *CS = Result.Nodes.getNodeAs<CompoundStmt>("else"))
-    Diag << removeToken(CS->getLBracLoc()) << removeToken(CS->getRBracLoc());
+    Diag << tooling::fixit::createRemoval(CS->getLBracLoc())
+         << tooling::fixit::createRemoval(CS->getRBracLoc());
 }
 
 } // namespace readability

Modified: clang-tools-extra/trunk/test/clang-tidy/misc-swapped-arguments.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-swapped-arguments.cpp?rev=269210&r1=269209&r2=269210&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-swapped-arguments.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-swapped-arguments.cpp Wed May 11 12:38:22 2016
@@ -22,8 +22,7 @@ void foo() {
   double b = 1.0;
   F(b, M(Some, Function));
 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: argument with implicit conversion from 'int' to 'double' followed by argument converted from 'double' to 'int', potentially swapped arguments.
-// In macro, don't emit fixits.
-// CHECK-FIXES: F(b, M(Some, Function));
+// CHECK-FIXES: F(M(Some, Function), b);
 
 #define N F(b, SomeFunction())
 
@@ -40,4 +39,15 @@ void foo() {
   F(3, 1.0);      // no-warning
   F(true, false); // no-warning
   F(0, 'c');      // no-warning
+
+#define APPLY(f, x, y) f(x, y)
+  APPLY(F, 1.0, 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: argument with implicit conversion from 'int' to 'double' followed by argument converted from 'double' to 'int', potentially swapped arguments.
+// CHECK-FIXES: APPLY(F, 3, 1.0);
+
+#define PARAMS 1.0, 3
+#define CALL(P) F(P)
+  CALL(PARAMS);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: argument with implicit conversion from 'int' to 'double' followed by argument converted from 'double' to 'int', potentially swapped arguments.  
+// In macro, don't emit fixits.  
 }




More information about the cfe-commits mailing list