etienneb updated this revision to Diff 55242.
etienneb added a comment.
address alexfh comments.
http://reviews.llvm.org/D19547
Files:
clang-tidy/misc/StringConstructorCheck.cpp
clang-tidy/misc/SwappedArgumentsCheck.cpp
clang-tidy/utils/FixItHintUtils.cpp
clang-tidy/utils/FixItHintUtils.h
test/clang-tidy/misc-string-constructor.cpp
Index: test/clang-tidy/misc-string-constructor.cpp
===================================================================
--- test/clang-tidy/misc-string-constructor.cpp
+++ test/clang-tidy/misc-string-constructor.cpp
@@ -22,8 +22,10 @@
void Test() {
std::string str('x', 4);
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor parameters are probably swapped [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-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');
Index: clang-tidy/utils/FixItHintUtils.h
===================================================================
--- clang-tidy/utils/FixItHintUtils.h
+++ clang-tidy/utils/FixItHintUtils.h
@@ -24,6 +24,9 @@
/// \brief Creates fix to make VarDecl const qualified.
FixItHint changeVarDeclToConst(const VarDecl &Var);
+/// \brief Get a StringRef representing a SourceRange.
+StringRef getSourceRangeAsString(const ASTContext &Context, SourceRange R);
+
} // namespace create_fix_it
} // utils
} // namespace tidy
Index: clang-tidy/utils/FixItHintUtils.cpp
===================================================================
--- clang-tidy/utils/FixItHintUtils.cpp
+++ clang-tidy/utils/FixItHintUtils.cpp
@@ -30,6 +30,21 @@
return FixItHint::CreateInsertion(Var.getTypeSpecStartLoc(), "const ");
}
+StringRef getSourceRangeAsString(const ASTContext &Context, SourceRange R) {
+ const SourceManager &SM = Context.getSourceManager();
+ // 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, Context.getLangOpts()));
+
+ return StringRef(Begin, End - Begin);
+}
+
} // namespace create_fix_it
} // namespace utils
} // namespace tidy
Index: clang-tidy/misc/SwappedArgumentsCheck.cpp
===================================================================
--- clang-tidy/misc/SwappedArgumentsCheck.cpp
+++ clang-tidy/misc/SwappedArgumentsCheck.cpp
@@ -8,6 +8,7 @@
//===----------------------------------------------------------------------===//
#include "SwappedArgumentsCheck.h"
+#include "../utils/FixItHintUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/SmallPtrSet.h"
@@ -46,24 +47,8 @@
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) {
+ const ASTContext &Ctx = *Result.Context;
auto *Call = Result.Nodes.getStmtAs<CallExpr>("call");
llvm::SmallPtrSet<const Expr *, 4> UsedArgs;
@@ -108,8 +93,10 @@
<< LHS->getType() << LHSFrom->getType() << RHS->getType()
<< RHSFrom->getType() << LHSRange << RHSRange;
- StringRef RHSString = getAsString(Result, RHSRange);
- StringRef LHSString = getAsString(Result, LHSRange);
+ StringRef RHSString =
+ utils::create_fix_it::getSourceRangeAsString(Ctx, RHSRange);
+ StringRef LHSString =
+ utils::create_fix_it::getSourceRangeAsString(Ctx, LHSRange);
if (!LHSString.empty() && !RHSString.empty()) {
D << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(LHSRange), RHSString)
Index: clang-tidy/misc/StringConstructorCheck.cpp
===================================================================
--- clang-tidy/misc/StringConstructorCheck.cpp
+++ clang-tidy/misc/StringConstructorCheck.cpp
@@ -8,6 +8,7 @@
//===----------------------------------------------------------------------===//
#include "StringConstructorCheck.h"
+#include "../utils/FixItHintUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -54,7 +55,7 @@
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 @@
// 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,26 @@
}
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");
+ auto D = diag(Loc, "constructor parameters are probably swapped");
+
+ SourceRange Param1 = E->getArg(0)->getSourceRange();
+ SourceRange Param2 = E->getArg(1)->getSourceRange();
+ StringRef Param1String =
+ utils::create_fix_it::getSourceRangeAsString(Ctx, Param1);
+ StringRef Param2String =
+ utils::create_fix_it::getSourceRangeAsString(Ctx, Param2);
+ if (!Param1String.empty() && !Param2String.empty()) {
+ D << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(Param1),
+ Param2String)
+ << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(Param2),
+ Param1String);
+ }
} else if (Result.Nodes.getNodeAs<Expr>("empty-string")) {
diag(Loc, "constructor creating an empty string");
} else if (Result.Nodes.getNodeAs<Expr>("negative-length")) {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
