etienneb created this revision.
etienneb added a reviewer: alexfh.
etienneb added a subscriber: cfe-commits.
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.
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,27 @@
}
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");
+ assert(E->getNumArgs() == 2 && "Invalid number of arguments");
+ 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
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
