[clang-tools-extra] r269208 - [clang-tidy] Add FixIt for swapping arguments in string-constructor-checker.

[Etienne Bergeron via cfe-commits]

Author: etienneb
Date: Wed May 11 12:32:12 2016
New Revision: 269208

URL: http://llvm.org/viewvc/llvm-project?rev=269208&view=rev
Log:
[clang-tidy] Add FixIt for swapping arguments in string-constructor-checker.

Summary:
Arguments can be swapped using fixit when they are not in macros.
This is the same implementation than SwappedArguments. Some code 
got lifted to be reused.

Others checks are not safe to be fixed as they tend to be bugs or errors.
It is better to let the user manually review them.

Reviewers: alexfh

Subscribers: cfe-commits

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

Modified:
    clang-tools-extra/trunk/clang-tidy/misc/StringConstructorCheck.cpp
    clang-tools-extra/trunk/test/clang-tidy/misc-string-constructor.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/StringConstructorCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/StringConstructorCheck.cpp?rev=269208&r1=269207&r2=269208&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/StringConstructorCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/StringConstructorCheck.cpp Wed May 11 12:32:12 2016
@@ -10,6 +10,7 @@
 #include "StringConstructorCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
 
 using namespace clang::ast_matchers;
 
@@ -54,7 +55,7 @@ void StringConstructorCheck::registerMat
       isDefinition(),
       hasType(pointerType(pointee(isAnyCharacter(), isConstQualified()))),
       hasInitializer(ignoringParenImpCasts(BoundStringLiteral)));
-  auto ConstStrLiteral = expr(ignoringParenImpCasts(anyOf(
+  const auto ConstStrLiteral = expr(ignoringParenImpCasts(anyOf(
       BoundStringLiteral, declRefExpr(hasDeclaration(anyOf(
                               ConstPtrStrLiteralDecl, ConstStrLiteralDecl))))));
 
@@ -88,7 +89,7 @@ void StringConstructorCheck::registerMat
               // Detect the expression: string("...", 0);
               hasArgument(1, ZeroExpr.bind("empty-string")),
               // Detect the expression: string("...", -4);
-              hasArgument(1, NegativeExpr.bind("negative-length")),              
+              hasArgument(1, NegativeExpr.bind("negative-length")),
               // Detect the expression: string("lit", 0x1234567);
               hasArgument(1, LargeLengthExpr.bind("large-length")),
               // Detect the expression: string("lit", 5)
@@ -100,11 +101,18 @@ void StringConstructorCheck::registerMat
 }
 
 void StringConstructorCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *E = Result.Nodes.getNodeAs<Expr>("constructor");
+  const ASTContext &Ctx = *Result.Context;
+  const auto *E = Result.Nodes.getNodeAs<CXXConstructExpr>("constructor");
+  assert(E && "missing constructor expression");
   SourceLocation Loc = E->getLocStart();
 
   if (Result.Nodes.getNodeAs<Expr>("swapped-parameter")) {
-    diag(Loc, "constructor parameters are probably swapped");
+    const Expr *P0 = E->getArg(0);
+    const Expr *P1 = E->getArg(1);
+    diag(Loc, "string constructor parameters are probably swapped;"
+              " expecting string(count, character)")
+        << tooling::fixit::createReplacement(*P0, *P1, Ctx)
+        << tooling::fixit::createReplacement(*P1, *P0, Ctx);
   } else if (Result.Nodes.getNodeAs<Expr>("empty-string")) {
     diag(Loc, "constructor creating an empty string");
   } else if (Result.Nodes.getNodeAs<Expr>("negative-length")) {

Modified: clang-tools-extra/trunk/test/clang-tidy/misc-string-constructor.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-string-constructor.cpp?rev=269208&r1=269207&r2=269208&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-string-constructor.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-string-constructor.cpp Wed May 11 12:32:12 2016
@@ -21,9 +21,11 @@ extern const char kText3[];
 
 void Test() {
   std::string str('x', 4);
-  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor parameters are probably swapped [misc-string-constructor]
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: string constructor parameters are probably swapped; expecting string(count, character) [misc-string-constructor]
+  // CHECK-FIXES: std::string str(4, 'x');  
   std::wstring wstr(L'x', 4);
-  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: constructor parameters are probably swapped
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: string constructor parameters are probably swapped
+  // CHECK-FIXES: std::wstring wstr(4, L'x');    
   std::string s0(0, 'x');
   // CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor creating an empty string
   std::string s1(-4, 'x');