aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSergio Martins <smartins@kde.org>2019-05-05 13:44:44 +0100
committerSergio Martins <sergio.martins@kdab.com>2019-05-05 18:19:23 +0100
commit05c9ccade074fb7468f7983990b9697accd07fe1 (patch)
tree52ebcf1c725bf1e0762394e160c21c9de7d21ceb
parenta95d6e93bdf7f6d70de6c53a0c2d698a6a104269 (diff)
Make the clazy plugin also export fixes, not only clazy-standalone
Now fixits are always enabled (printed on screen, like clang and clang-tidy). What the user can change is if they are exported to yaml files or not. This makes the code much simpler and more similar to other tools. The env variables to choose fixits are gone, you instead choose checks, and pass --export-fixes= instead. (Or -Xclang -plugin-arg-clazy -Xclang export-fixes for the plugin)
-rw-r--r--src/Clazy.cpp15
-rw-r--r--src/Clazy.h4
-rw-r--r--src/ClazyContext.cpp23
-rw-r--r--src/ClazyContext.h14
-rw-r--r--src/ClazyStandaloneMain.cpp11
-rw-r--r--src/checkbase.cpp21
-rw-r--r--src/checkbase.h9
-rw-r--r--src/checkmanager.cpp20
-rw-r--r--src/checks/level0/qdatetime-utc.cpp2
-rw-r--r--src/checks/level0/qgetenv.cpp2
-rw-r--r--src/checks/level0/qstring-ref.cpp4
-rw-r--r--src/checks/level1/auto-unexpected-qstringbuilder.cpp2
-rw-r--r--src/checks/level1/range-loop.cpp6
-rw-r--r--src/checks/level2/function-args-by-ref.cpp4
-rw-r--r--src/checks/level2/function-args-by-value.cpp2
-rw-r--r--src/checks/level2/old-style-connect.cpp2
-rw-r--r--src/checks/level2/qstring-allocations.cpp40
-rw-r--r--src/checks/level2/qstring-allocations.h6
-rw-r--r--src/checks/manuallevel/qt-keywords.cpp2
-rw-r--r--tests/range-loop/bug370609.cpp.fixed.expected18
-rw-r--r--tests/range-loop/config.json3
-rwxr-xr-xtests/run_tests.py120
22 files changed, 167 insertions, 163 deletions
diff --git a/src/Clazy.cpp b/src/Clazy.cpp
index ba75bb93..b651d7be 100644
--- a/src/Clazy.cpp
+++ b/src/Clazy.cpp
@@ -242,10 +242,8 @@ bool ClazyASTAction::ParseArgs(const CompilerInstance &ci, const std::vector<std
return true;
}
- if (parseArgument("enable-all-fixits", args)) {
- // This is useful for unit-tests, where we also want to run fixits. Don't use it otherwise.
- m_options |= ClazyContext::ClazyOption_AllFixitsEnabled;
- }
+ if (parseArgument("export-fixes", args))
+ m_options |= ClazyContext::ClazyOption_ExportFixes;
if (parseArgument("qt4-compat", args))
m_options |= ClazyContext::ClazyOption_Qt4Compat;
@@ -370,20 +368,23 @@ void ClazyASTAction::PrintHelp(llvm::raw_ostream &ros) const
ros << "FixIts are experimental and rewrite your code therefore only one FixIt is allowed per build.\nSpecifying a list of different FixIts is not supported.\nBackup your code before running them.\n";
}
-ClazyStandaloneASTAction::ClazyStandaloneASTAction(const string &checkList, const string &headerFilter, const string &ignoreDirs, const string &exportFixes,
+ClazyStandaloneASTAction::ClazyStandaloneASTAction(const string &checkList,
+ const string &headerFilter,
+ const string &ignoreDirs,
+ const string &exportFixesFilename,
ClazyContext::ClazyOptions options)
: clang::ASTFrontendAction()
, m_checkList(checkList.empty() ? "level1" : checkList)
, m_headerFilter(headerFilter.empty() ? getEnvVariable("CLAZY_HEADER_FILTER") : headerFilter)
, m_ignoreDirs(ignoreDirs.empty() ? getEnvVariable("CLAZY_IGNORE_DIRS") : ignoreDirs)
- , m_exportFixes(exportFixes)
+ , m_exportFixesFilename(exportFixesFilename)
, m_options(options)
{
}
unique_ptr<ASTConsumer> ClazyStandaloneASTAction::CreateASTConsumer(CompilerInstance &ci, llvm::StringRef)
{
- auto context = new ClazyContext(ci, m_headerFilter, m_ignoreDirs, m_exportFixes, m_options);
+ auto context = new ClazyContext(ci, m_headerFilter, m_ignoreDirs, m_exportFixesFilename, m_options);
auto astConsumer = new ClazyASTConsumer(context);
auto cm = CheckManager::instance();
diff --git a/src/Clazy.h b/src/Clazy.h
index 846e694b..bdb75688 100644
--- a/src/Clazy.h
+++ b/src/Clazy.h
@@ -86,7 +86,7 @@ public:
explicit ClazyStandaloneASTAction(const std::string &checkList,
const std::string &headerFilter,
const std::string &ignoreDirs,
- const std::string &exportFixes,
+ const std::string &exportFixesFilename,
ClazyContext::ClazyOptions = ClazyContext::ClazyOption_None);
protected:
std::unique_ptr<clang::ASTConsumer> CreateASTConsumer(clang::CompilerInstance &ci, llvm::StringRef) override;
@@ -94,7 +94,7 @@ private:
const std::string m_checkList;
const std::string m_headerFilter;
const std::string m_ignoreDirs;
- const std::string m_exportFixes;
+ const std::string m_exportFixesFilename;
const ClazyContext::ClazyOptions m_options;
};
diff --git a/src/ClazyContext.cpp b/src/ClazyContext.cpp
index 8b185352..e29d4ad0 100644
--- a/src/ClazyContext.cpp
+++ b/src/ClazyContext.cpp
@@ -62,14 +62,14 @@ public:
};
ClazyContext::ClazyContext(const clang::CompilerInstance &compiler,
- const string &headerFilter, const string &ignoreDirs, const string &exportFixes, ClazyOptions opts)
+ const string &headerFilter, const string &ignoreDirs,
+ string exportFixesFilename, ClazyOptions opts)
: ci(compiler)
, astContext(ci.getASTContext())
, sm(ci.getSourceManager())
, m_noWerror(getenv("CLAZY_NO_WERROR") != nullptr) // Allows user to make clazy ignore -Werror
, options(opts)
, extraOptions(clazy::splitString(getenv("CLAZY_EXTRA_OPTIONS"), ','))
- , exportFixes(exportFixes)
{
if (!headerFilter.empty())
headerFilterRegex = std::unique_ptr<llvm::Regex>(new llvm::Regex(headerFilter));
@@ -77,19 +77,16 @@ ClazyContext::ClazyContext(const clang::CompilerInstance &compiler,
if (!ignoreDirs.empty())
ignoreDirsRegex = std::unique_ptr<llvm::Regex>(new llvm::Regex(ignoreDirs));
- const char *fixitsEnv = getenv("CLAZY_FIXIT");
- allFixitsEnabled = (options & ClazyOption_AllFixitsEnabled);
- if (!allFixitsEnabled && fixitsEnv) {
- const string fixitsEnvStr = clazy::unquoteString(fixitsEnv);
- if (fixitsEnvStr == "all_fixits") {
- allFixitsEnabled = true;
- } else {
- requestedFixitName = fixitsEnvStr;
+ if (exportFixesEnabled()) {
+ if (exportFixesFilename.empty()) {
+ // Only clazy-standalone sets the filename by argument.
+ // clazy plugin sets it automatically here:
+ const FileEntry *fileEntry = sm.getFileEntryForID(sm.getMainFileID());
+ exportFixesFilename = fileEntry->getName().str() + ".clazy.yaml";
}
- }
- if (fixitsEnabled() && exportFixesEnabled()) // TODO: A single one is enough
- exporter = new FixItExporter(ci.getDiagnostics(), sm, ci.getLangOpts(), exportFixes);
+ exporter = new FixItExporter(ci.getDiagnostics(), sm, ci.getLangOpts(), exportFixesFilename);
+ }
}
ClazyContext::~ClazyContext()
diff --git a/src/ClazyContext.h b/src/ClazyContext.h
index 511f3cd9..c6af007d 100644
--- a/src/ClazyContext.h
+++ b/src/ClazyContext.h
@@ -59,7 +59,7 @@ public:
enum ClazyOption {
ClazyOption_None = 0,
- ClazyOption_AllFixitsEnabled = 4,
+ ClazyOption_ExportFixes = 1,
ClazyOption_Qt4Compat = 8,
ClazyOption_OnlyQt = 16, // Ignore non-Qt files. This is done by bailing out if QT_CORE_LIB is not set.
ClazyOption_QtDeveloper = 32, // For running clazy on Qt itself, optional, but honours specific guidelines
@@ -71,7 +71,7 @@ public:
explicit ClazyContext(const clang::CompilerInstance &ci,
const std::string &headerFilter,
const std::string &ignoreDirs,
- const std::string &exportFixes,
+ std::string exportFixesFilename,
ClazyOptions = ClazyOption_None);
~ClazyContext();
@@ -85,14 +85,9 @@ public:
return m_noWerror;
}
- bool fixitsEnabled() const
- {
- return allFixitsEnabled || !requestedFixitName.empty();
- }
-
bool exportFixesEnabled() const
{
- return !exportFixes.empty();
+ return options & ClazyOption_ExportFixes;
}
bool isQtDeveloper() const
@@ -180,9 +175,6 @@ public:
const ClazyOptions options;
const std::vector<std::string> extraOptions;
FixItExporter *exporter = nullptr;
- bool allFixitsEnabled = false;
- std::string requestedFixitName;
- std::string exportFixes;
clang::CXXMethodDecl *lastMethodDecl = nullptr;
clang::FunctionDecl *lastFunctionDecl = nullptr;
clang::Decl *lastDecl = nullptr;
diff --git a/src/ClazyStandaloneMain.cpp b/src/ClazyStandaloneMain.cpp
index 93f0176b..7d454daa 100644
--- a/src/ClazyStandaloneMain.cpp
+++ b/src/ClazyStandaloneMain.cpp
@@ -44,9 +44,6 @@ static llvm::cl::OptionCategory s_clazyCategory("clazy options");
static cl::opt<std::string> s_checks("checks", cl::desc("Comma-separated list of clazy checks. Default is level1"),
cl::init(""), cl::cat(s_clazyCategory));
-static cl::opt<bool> s_enableAllFixits("enable-all-fixits", cl::desc("Enables all fixits"),
- cl::init(false), cl::cat(s_clazyCategory));
-
static cl::opt<std::string> s_exportFixes("export-fixes", cl::desc("YAML file to store suggested fixes in. The stored fixes can be applied to the input source code with clang-apply-replacements."),
cl::init(""), cl::cat(s_clazyCategory));
@@ -88,8 +85,8 @@ public:
{
ClazyContext::ClazyOptions options = ClazyContext::ClazyOption_None;
- if (s_enableAllFixits.getValue())
- options |= ClazyContext::ClazyOption_AllFixitsEnabled;
+ if (!s_exportFixes.getValue().empty())
+ options |= ClazyContext::ClazyOption_ExportFixes;
if (s_qt4Compat.getValue())
options |= ClazyContext::ClazyOption_Qt4Compat;
@@ -107,7 +104,9 @@ public:
options |= ClazyContext::ClazyOption_IgnoreIncludedFiles;
// TODO: We need to agregate the fixes with previous run
- return new ClazyStandaloneASTAction(s_checks.getValue(), s_headerFilter.getValue(), s_ignoreDirs.getValue(), s_exportFixes.getValue(), options);
+ return new ClazyStandaloneASTAction(s_checks.getValue(), s_headerFilter.getValue(),
+ s_ignoreDirs.getValue(), s_exportFixes.getValue(),
+ options);
}
};
diff --git a/src/checkbase.cpp b/src/checkbase.cpp
index de88ff01..8b40e19d 100644
--- a/src/checkbase.cpp
+++ b/src/checkbase.cpp
@@ -261,9 +261,9 @@ void CheckBase::reallyEmitWarning(clang::SourceLocation loc, const std::string &
}
}
-void CheckBase::queueManualFixitWarning(clang::SourceLocation loc, const string &message, int fixitType)
+void CheckBase::queueManualFixitWarning(clang::SourceLocation loc, const string &message)
{
- if (isFixitEnabled(fixitType) && !manualFixitAlreadyQueued(loc)) {
+ if (fixitsEnabled() && !manualFixitAlreadyQueued(loc)) {
m_queuedManualInterventionWarnings.push_back({loc, message});
m_emittedManualFixItsWarningsInMacro.push_back(loc.getRawEncoding());
}
@@ -301,25 +301,8 @@ bool CheckBase::isOptionSet(const std::string &optionName) const
return m_context->isOptionSet(qualifiedName);
}
-void CheckBase::setEnabledFixits(int fixits)
-{
- m_enabledFixits = fixits;
-}
-
-bool CheckBase::isFixitEnabled(int fixit) const
-{
- return (m_enabledFixits & fixit) || (m_context->options & ClazyContext::ClazyOption_AllFixitsEnabled);
-}
-
-bool CheckBase::isFixitEnabled() const
-{
- // Checks with only 1 fixit (which is most of them) don't need to pass fixit id
- return isFixitEnabled(1);
-}
-
ClazyAstMatcherCallback::ClazyAstMatcherCallback(CheckBase *check)
: MatchFinder::MatchCallback()
, m_check(check)
{
-
}
diff --git a/src/checkbase.h b/src/checkbase.h
index 5a4a363d..485d826b 100644
--- a/src/checkbase.h
+++ b/src/checkbase.h
@@ -122,10 +122,6 @@ public:
std::string name() const { return m_name; }
- void setEnabledFixits(int);
- bool isFixitEnabled(int fixit) const;
- bool isFixitEnabled() const;
-
void emitWarning(const clang::Decl *, const std::string &error, bool printWarningTag = true);
void emitWarning(const clang::Stmt *, const std::string &error, bool printWarningTag = true);
void emitWarning(clang::SourceLocation loc, const std::string &error, bool printWarningTag = true);
@@ -158,11 +154,13 @@ protected:
bool shouldIgnoreFile(clang::SourceLocation) const;
void reallyEmitWarning(clang::SourceLocation loc, const std::string &error, const std::vector<clang::FixItHint> &fixits);
- void queueManualFixitWarning(clang::SourceLocation loc, const std::string &message = {}, int fixitType = 1);
+ void queueManualFixitWarning(clang::SourceLocation loc, const std::string &message = {});
bool warningAlreadyEmitted(clang::SourceLocation loc) const;
bool manualFixitAlreadyQueued(clang::SourceLocation loc) const;
bool isOptionSet(const std::string &optionName) const;
+ bool fixitsEnabled() const { return true; } // Fixits are always shown
+
// 3 shortcuts for stuff that litter the codebase all over.
const clang::SourceManager &sm() const { return m_sm; }
const clang::LangOptions &lo() const { return m_astContext.getLangOpts(); }
@@ -179,7 +177,6 @@ private:
std::vector<unsigned int> m_emittedWarningsInMacro;
std::vector<unsigned int> m_emittedManualFixItsWarningsInMacro;
std::vector<std::pair<clang::SourceLocation, std::string>> m_queuedManualInterventionWarnings;
- int m_enabledFixits = 0;
const Options m_options;
const std::string m_tag;
};
diff --git a/src/checkmanager.cpp b/src/checkmanager.cpp
index 85f4d519..eae5bed5 100644
--- a/src/checkmanager.cpp
+++ b/src/checkmanager.cpp
@@ -139,11 +139,6 @@ RegisteredCheck::List CheckManager::requestedChecksThroughEnv(const ClazyContext
requestedChecksThroughEnv = checksEnvStr == "all_checks" ? availableChecks(CheckLevel2)
: checksForCommaSeparatedString(checksEnvStr, /*by-ref=*/ disabledChecksThroughEnv);
}
-
- const string checkName = checkNameForFixIt(context->requestedFixitName);
- if (!checkName.empty() && checkForName(requestedChecksThroughEnv, checkName) == requestedChecksThroughEnv.cend()) {
- requestedChecksThroughEnv.push_back(*checkForName(m_registeredChecks, checkName));
- }
}
std::copy(disabledChecksThroughEnv.begin(), disabledChecksThroughEnv.end(),
@@ -244,26 +239,11 @@ std::vector<std::pair<CheckBase*, RegisteredCheck>> CheckManager::createChecks(c
ClazyContext *context)
{
assert(context);
- const string fixitCheckName = checkNameForFixIt(context->requestedFixitName);
- RegisteredFixIt fixit = m_fixitByName[context->requestedFixitName];
std::vector<std::pair<CheckBase*, RegisteredCheck>> checks;
checks.reserve(requestedChecks.size() + 1);
for (const auto& check : requestedChecks) {
checks.push_back({createCheck(check.name, context), check });
- if (check.name == fixitCheckName) {
- checks.back().first->setEnabledFixits(fixit.id);
- }
- }
-
- if (!context->requestedFixitName.empty()) {
- // We have one fixit enabled, we better have the check instance too.
- if (!fixitCheckName.empty()) {
- if (checkForName(requestedChecks, fixitCheckName) == requestedChecks.cend()) {
- checks.push_back({createCheck(fixitCheckName, context), {} });
- checks.back().first->setEnabledFixits(fixit.id);
- }
- }
}
return checks;
diff --git a/src/checks/level0/qdatetime-utc.cpp b/src/checks/level0/qdatetime-utc.cpp
index b7e19cfc..0cabbe04 100644
--- a/src/checks/level0/qdatetime-utc.cpp
+++ b/src/checks/level0/qdatetime-utc.cpp
@@ -78,7 +78,7 @@ void QDateTimeUtc::VisitStmt(clang::Stmt *stmt)
}
std::vector<FixItHint> fixits;
- if (isFixitEnabled()) {
+ if (fixitsEnabled()) {
const bool success = clazy::transformTwoCallsIntoOneV2(&m_astContext, secondCall, replacement, fixits);
if (!success) {
queueManualFixitWarning(clazy::getLocStart(secondCall));
diff --git a/src/checks/level0/qgetenv.cpp b/src/checks/level0/qgetenv.cpp
index 7f5a46fa..143ac969 100644
--- a/src/checks/level0/qgetenv.cpp
+++ b/src/checks/level0/qgetenv.cpp
@@ -95,7 +95,7 @@ void QGetEnv::VisitStmt(clang::Stmt *stmt)
if (!errorMsg.empty()) {
std::vector<FixItHint> fixits;
- if (isFixitEnabled()) {
+ if (fixitsEnabled()) {
const bool success = clazy::transformTwoCallsIntoOne(&m_astContext, qgetEnvCall, memberCall, replacement, fixits);
if (!success) {
queueManualFixitWarning(clazy::getLocStart(memberCall));
diff --git a/src/checks/level0/qstring-ref.cpp b/src/checks/level0/qstring-ref.cpp
index 1495b5bc..d1d8a4ea 100644
--- a/src/checks/level0/qstring-ref.cpp
+++ b/src/checks/level0/qstring-ref.cpp
@@ -175,7 +175,7 @@ bool StringRefCandidates::processCase1(CXXMemberCallExpr *memberCall)
const string firstMethodName = firstMemberCall->getMethodDecl()->getNameAsString();
std::vector<FixItHint> fixits;
- if (isFixitEnabled())
+ if (fixitsEnabled())
fixits = fixit(firstMemberCall);
emitWarning(clazy::getLocEnd(firstMemberCall), "Use " + firstMethodName + "Ref() instead", fixits);
@@ -218,7 +218,7 @@ bool StringRefCandidates::processCase2(CallExpr *call)
return false;
std::vector<FixItHint> fixits;
- if (isFixitEnabled()) {
+ if (fixitsEnabled()) {
fixits = fixit(innerMemberCall);
}
diff --git a/src/checks/level1/auto-unexpected-qstringbuilder.cpp b/src/checks/level1/auto-unexpected-qstringbuilder.cpp
index 22a559f7..b07d5bb4 100644
--- a/src/checks/level1/auto-unexpected-qstringbuilder.cpp
+++ b/src/checks/level1/auto-unexpected-qstringbuilder.cpp
@@ -68,7 +68,7 @@ void AutoUnexpectedQStringBuilder::VisitDecl(Decl *decl)
return;
std::vector<FixItHint> fixits;
- if (isFixitEnabled()) {
+ if (fixitsEnabled()) {
std::string replacement = "QString " + clazy::name(varDecl).str();
if (qualtype.isConstQualified())
diff --git a/src/checks/level1/range-loop.cpp b/src/checks/level1/range-loop.cpp
index 7d2a88b8..19695b89 100644
--- a/src/checks/level1/range-loop.cpp
+++ b/src/checks/level1/range-loop.cpp
@@ -56,7 +56,7 @@ enum Fixit {
RangeLoop::RangeLoop(const std::string &name, ClazyContext *context)
: CheckBase(name, context, Option_CanIgnoreIncludes)
{
- if (isFixitEnabled(Fixit_AddqAsConst)) {
+ if (fixitsEnabled()) {
context->enablePreprocessorVisitor();
}
}
@@ -118,7 +118,7 @@ void RangeLoop::processForRangeLoop(CXXForRangeStmt *rangeLoop)
std::vector<FixItHint> fixits;
SourceLocation end;
- if (isFixitEnabled(Fixit_AddqAsConst) && islvalue(containerExpr, end)) {
+ if (fixitsEnabled() && islvalue(containerExpr, end)) {
PreProcessorVisitor *preProcessorVisitor = m_context->preprocessorVisitor;
if (!preProcessorVisitor || preProcessorVisitor->qtVersion() >= 50700) { // qAsConst() was added to 5.7
SourceLocation start = clazy::getLocStart(containerExpr);
@@ -145,7 +145,7 @@ void RangeLoop::checkPassByConstRefCorrectness(CXXForRangeStmt *rangeLoop)
msg = "Missing reference in range-for with non trivial type (" + paramStr + ')';
std::vector<FixItHint> fixits;
- if (isFixitEnabled(Fixit_AddRef)) {
+ if (fixitsEnabled()) {
const bool isConst = varDecl->getType().isConstQualified();
if (!isConst) {
diff --git a/src/checks/level2/function-args-by-ref.cpp b/src/checks/level2/function-args-by-ref.cpp
index cf49faa7..6b7b5cb3 100644
--- a/src/checks/level2/function-args-by-ref.cpp
+++ b/src/checks/level2/function-args-by-ref.cpp
@@ -114,7 +114,7 @@ void FunctionArgsByRef::processFunction(FunctionDecl *func)
if (m_context->isQtDeveloper() && shouldIgnoreFunction(func))
return;
- const bool warnForOverriddenMethods = isOptionSet("warn-for-overridden-methods") || isFixitEnabled();
+ const bool warnForOverriddenMethods = isOptionSet("warn-for-overridden-methods") || fixitsEnabled();
if (!warnForOverriddenMethods && Utils::methodOverrides(dyn_cast<CXXMethodDecl>(func))) {
// When overriding you can't change the signature. You should fix the base classes first
return;
@@ -160,7 +160,7 @@ void FunctionArgsByRef::processFunction(FunctionDecl *func)
void FunctionArgsByRef::addFixits(std::vector<FixItHint> &fixits, FunctionDecl *func, unsigned int parmIndex)
{
- if (isFixitEnabled()) {
+ if (fixitsEnabled()) {
for (auto funcRedecl : func->redecls()) {
auto funcParams = Utils::functionParameters(funcRedecl);
if (funcParams.size() <= parmIndex)
diff --git a/src/checks/level2/function-args-by-value.cpp b/src/checks/level2/function-args-by-value.cpp
index d62fa862..0bd270ad 100644
--- a/src/checks/level2/function-args-by-value.cpp
+++ b/src/checks/level2/function-args-by-value.cpp
@@ -183,7 +183,7 @@ void FunctionArgsByValue::processFunction(FunctionDecl *func)
std::vector<FixItHint> fixits;
auto method = dyn_cast<CXXMethodDecl>(func);
const bool isVirtualMethod = method && method->isVirtual();
- if ((!isVirtualMethod || warnForOverriddenMethods) && isFixitEnabled()) { // Don't try to fix virtual methods, as build can fail
+ if ((!isVirtualMethod || warnForOverriddenMethods) && fixitsEnabled()) { // Don't try to fix virtual methods, as build can fail
for (auto redecl : func->redecls()) { // Fix in both header and .cpp
auto fdecl = dyn_cast<FunctionDecl>(redecl);
const ParmVarDecl *param = fdecl->getParamDecl(i);
diff --git a/src/checks/level2/old-style-connect.cpp b/src/checks/level2/old-style-connect.cpp
index 6a149318..0fe68c13 100644
--- a/src/checks/level2/old-style-connect.cpp
+++ b/src/checks/level2/old-style-connect.cpp
@@ -322,7 +322,7 @@ bool OldStyleConnect::isSignalOrSlot(SourceLocation loc, string &macroName) cons
template <typename T>
vector<FixItHint> OldStyleConnect::fixits(int classification, T *callOrCtor)
{
- if (!isFixitEnabled())
+ if (!fixitsEnabled())
return {};
if (!callOrCtor) {
diff --git a/src/checks/level2/qstring-allocations.cpp b/src/checks/level2/qstring-allocations.cpp
index 6ea42975..999ee1e9 100644
--- a/src/checks/level2/qstring-allocations.cpp
+++ b/src/checks/level2/qstring-allocations.cpp
@@ -229,16 +229,16 @@ void QStringAllocations::VisitCtor(Stmt *stm)
}
vector<FixItHint> fixits;
- if (qlatin1expr.enableFixit && isFixitEnabled(QLatin1StringAllocations)) {
+ if (qlatin1expr.enableFixit && fixitsEnabled()) {
if (!clazy::getLocStart(qlatin1Ctor).isMacroID()) {
if (!ternary) {
- fixits = fixItReplaceWordWithWord(qlatin1Ctor, "QStringLiteral", "QLatin1String", QLatin1StringAllocations);
+ fixits = fixItReplaceWordWithWord(qlatin1Ctor, "QStringLiteral", "QLatin1String");
bool shouldRemoveQString = clazy::getLocStart(qlatin1Ctor).getRawEncoding() != clazy::getLocStart(stm).getRawEncoding() && dyn_cast_or_null<CXXBindTemporaryExpr>(clazy::parent(m_context->parentMap, ctorExpr));
if (shouldRemoveQString) {
// This is the case of QString(QLatin1String("foo")), which we just fixed to be QString(QStringLiteral("foo)), so now remove QString
auto removalFixits = clazy::fixItRemoveToken(&m_astContext, ctorExpr, true);
if (removalFixits.empty()) {
- queueManualFixitWarning(clazy::getLocStart(ctorExpr), "Internal error: invalid start or end location", QLatin1StringAllocations);
+ queueManualFixitWarning(clazy::getLocStart(ctorExpr), "Internal error: invalid start or end location");
} else {
clazy::append(removalFixits, fixits);
}
@@ -247,7 +247,7 @@ void QStringAllocations::VisitCtor(Stmt *stm)
fixits = fixItReplaceWordWithWordInTernary(ternary);
}
} else {
- queueManualFixitWarning(clazy::getLocStart(qlatin1Ctor), "Can't use QStringLiteral in macro", QLatin1StringAllocations);
+ queueManualFixitWarning(clazy::getLocStart(qlatin1Ctor), "Can't use QStringLiteral in macro");
}
}
@@ -258,7 +258,7 @@ void QStringAllocations::VisitCtor(Stmt *stm)
auto pointerDecay = dyn_cast<ImplicitCastExpr>(*(ctorExpr->child_begin()));
if (clazy::hasChildren(pointerDecay)) {
auto lt = dyn_cast<StringLiteral>(*pointerDecay->child_begin());
- if (lt && isFixitEnabled(CharPtrAllocations)) {
+ if (lt && fixitsEnabled()) {
Stmt *grandParent = clazy::parent(m_context->parentMap, lt, 2);
Stmt *grandGrandParent = clazy::parent(m_context->parentMap, lt, 3);
Stmt *grandGrandGrandParent = clazy::parent(m_context->parentMap, lt, 4);
@@ -267,11 +267,11 @@ void QStringAllocations::VisitCtor(Stmt *stm)
const bool literalIsEmpty = lt->getLength() == 0;
if (literalIsEmpty && clazy::getFirstParentOfType<MemberExpr>(m_context->parentMap, ctorExpr) == nullptr)
- fixits = fixItReplaceWordWithWord(ctorExpr, "QLatin1String", "QString", CharPtrAllocations);
+ fixits = fixItReplaceWordWithWord(ctorExpr, "QLatin1String", "QString");
else if (!clazy::getLocStart(ctorExpr).isMacroID())
- fixits = fixItReplaceWordWithWord(ctorExpr, "QStringLiteral", "QString", CharPtrAllocations);
+ fixits = fixItReplaceWordWithWord(ctorExpr, "QStringLiteral", "QString");
else
- queueManualFixitWarning(clazy::getLocStart(ctorExpr), "Can't use QStringLiteral in macro.", CharPtrAllocations);
+ queueManualFixitWarning(clazy::getLocStart(ctorExpr), "Can't use QStringLiteral in macro.");
} else {
auto parentMemberCallExpr = clazy::getFirstParentOfType<CXXMemberCallExpr>(m_context->parentMap, lt, /*maxDepth=*/ 6); // 6 seems like a nice max from the ASTs I've seen
@@ -297,7 +297,7 @@ void QStringAllocations::VisitCtor(Stmt *stm)
}
}
-vector<FixItHint> QStringAllocations::fixItReplaceWordWithWord(clang::Stmt *begin, const string &replacement, const string &replacee, int fixitType)
+vector<FixItHint> QStringAllocations::fixItReplaceWordWithWord(clang::Stmt *begin, const string &replacement, const string &replacee)
{
StringLiteral *lt = stringLiteralForCall(begin);
if (replacee == "QLatin1String") {
@@ -313,7 +313,7 @@ vector<FixItHint> QStringAllocations::fixItReplaceWordWithWord(clang::Stmt *begi
vector<FixItHint> fixits;
FixItHint fixit = clazy::fixItReplaceWordWithWord(&m_astContext, begin, replacement, replacee);
if (fixit.isNull()) {
- queueManualFixitWarning(clazy::getLocStart(begin), "", fixitType);
+ queueManualFixitWarning(clazy::getLocStart(begin), "");
} else {
fixits.push_back(fixit);
}
@@ -416,7 +416,7 @@ std::vector<FixItHint> QStringAllocations::fixItReplaceFromLatin1OrFromUtf8(Call
std::string replacement = isQStringLiteralCandidate(callExpr, m_context->parentMap, lo(), sm()) ? "QStringLiteral"
: "QLatin1String";
if (replacement == "QStringLiteral" && clazy::getLocStart(callExpr).isMacroID()) {
- queueManualFixitWarning(clazy::getLocStart(callExpr), "Can't use QStringLiteral in macro!", FromLatin1_FromUtf8Allocations);
+ queueManualFixitWarning(clazy::getLocStart(callExpr), "Can't use QStringLiteral in macro!");
return {};
}
@@ -441,7 +441,7 @@ std::vector<FixItHint> QStringAllocations::fixItReplaceFromLatin1OrFromUtf8(Call
SourceRange range(clazy::getLocStart(callExpr), methodNameLoc);
fixits.push_back(FixItHint::CreateReplacement(range, replacement));
} else {
- queueManualFixitWarning(clazy::getLocStart(callExpr), "Internal error: literal is null", FromLatin1_FromUtf8Allocations);
+ queueManualFixitWarning(clazy::getLocStart(callExpr), "Internal error: literal is null");
}
return fixits;
@@ -454,21 +454,21 @@ std::vector<FixItHint> QStringAllocations::fixItRawLiteral(clang::StringLiteral
SourceRange range = clazy::rangeForLiteral(&m_astContext, lt);
if (range.isInvalid()) {
if (lt) {
- queueManualFixitWarning(clazy::getLocStart(lt), "Internal error: Can't calculate source location", CharPtrAllocations);
+ queueManualFixitWarning(clazy::getLocStart(lt), "Internal error: Can't calculate source location");
}
return {};
}
SourceLocation start = clazy::getLocStart(lt);
if (start.isMacroID()) {
- queueManualFixitWarning(start, "Can't use QStringLiteral in macro", CharPtrAllocations);
+ queueManualFixitWarning(start, "Can't use QStringLiteral in macro");
} else {
if (Utils::literalContainsEscapedBytes(lt, sm(), lo()))
return {};
string revisedReplacement = lt->getLength() == 0 ? "QLatin1String" : replacement; // QLatin1String("") is better than QStringLiteral("")
if (revisedReplacement == "QStringLiteral" && clazy::getLocStart(lt).isMacroID()) {
- queueManualFixitWarning(clazy::getLocStart(lt), "Can't use QStringLiteral in macro...", CharPtrAllocations);
+ queueManualFixitWarning(clazy::getLocStart(lt), "Can't use QStringLiteral in macro...");
return {};
}
@@ -519,9 +519,9 @@ void QStringAllocations::VisitOperatorCall(Stmt *stm)
}
}
- if (isFixitEnabled(CharPtrAllocations)) {
+ if (fixitsEnabled()) {
if (literals.empty()) {
- queueManualFixitWarning(clazy::getLocStart(stm), "Couldn't find literal", CharPtrAllocations);
+ queueManualFixitWarning(clazy::getLocStart(stm), "Couldn't find literal");
} else {
const string replacement = Utils::isAscii(literals[0]) ? "QLatin1String" : "QStringLiteral";
fixits = fixItRawLiteral(literals[0], replacement);
@@ -572,7 +572,7 @@ void QStringAllocations::VisitFromLatin1OrUtf8(Stmt *stmt)
std::vector<FixItHint> fixits;
- if (isFixitEnabled(FromLatin1_FromUtf8Allocations)) {
+ if (fixitsEnabled()) {
const FromFunction fromFunction = clazy::name(functionDecl) == "fromLatin1" ? FromLatin1 : FromUtf8;
fixits = fixItReplaceFromLatin1OrFromUtf8(callExpr, fromFunction);
}
@@ -601,8 +601,8 @@ void QStringAllocations::VisitAssignOperatorQLatin1String(Stmt *stmt)
vector<FixItHint> fixits;
- if (isFixitEnabled(QLatin1StringAllocations)) {
- fixits = ternary == nullptr ? fixItReplaceWordWithWord(begin, "QStringLiteral", "QLatin1String", QLatin1StringAllocations)
+ if (fixitsEnabled()) {
+ fixits = ternary == nullptr ? fixItReplaceWordWithWord(begin, "QStringLiteral", "QLatin1String")
: fixItReplaceWordWithWordInTernary(ternary);
}
diff --git a/src/checks/level2/qstring-allocations.h b/src/checks/level2/qstring-allocations.h
index a71f0007..560be7b6 100644
--- a/src/checks/level2/qstring-allocations.h
+++ b/src/checks/level2/qstring-allocations.h
@@ -72,10 +72,8 @@ private:
void VisitFromLatin1OrUtf8(clang::Stmt *);
void VisitAssignOperatorQLatin1String(clang::Stmt *);
- void maybeEmitWarning(clang::SourceLocation loc, std::string error,
- std::vector<clang::FixItHint> fixits = {});
-
- std::vector<clang::FixItHint> fixItReplaceWordWithWord(clang::Stmt *begin, const std::string &replacement, const std::string &replacee, int fixitType);
+ void maybeEmitWarning(clang::SourceLocation loc, std::string error, std::vector<clang::FixItHint> fixits = {});
+ std::vector<clang::FixItHint> fixItReplaceWordWithWord(clang::Stmt *begin, const std::string &replacement, const std::string &replacee);
std::vector<clang::FixItHint> fixItReplaceWordWithWordInTernary(clang::ConditionalOperator *);
std::vector<clang::FixItHint> fixItReplaceFromLatin1OrFromUtf8(clang::CallExpr *callExpr, FromFunction);
std::vector<clang::FixItHint> fixItRawLiteral(clang::StringLiteral *stmt, const std::string &replacement);
diff --git a/src/checks/manuallevel/qt-keywords.cpp b/src/checks/manuallevel/qt-keywords.cpp
index dfe6a141..e792e95a 100644
--- a/src/checks/manuallevel/qt-keywords.cpp
+++ b/src/checks/manuallevel/qt-keywords.cpp
@@ -69,7 +69,7 @@ void QtKeywords::VisitMacroExpands(const Token &macroNameTok, const SourceRange
return;
std::vector<FixItHint> fixits;
- if (isFixitEnabled()) {
+ if (fixitsEnabled()) {
std::string replacement = "Q_" + name;
std::transform(replacement.begin(), replacement.end(), replacement.begin(), ::toupper);
fixits.push_back(clazy::createReplacement(range, replacement));
diff --git a/tests/range-loop/bug370609.cpp.fixed.expected b/tests/range-loop/bug370609.cpp.fixed.expected
new file mode 100644
index 00000000..505d1bdc
--- /dev/null
+++ b/tests/range-loop/bug370609.cpp.fixed.expected
@@ -0,0 +1,18 @@
+// bug 370609, Simply tests if clazy crashes
+
+#include <QtCore/QVector>
+
+template <typename T>
+struct Example
+{
+ Example()
+ {
+ for (auto sample : qAsConst(m_sampleCache)) { }
+ }
+ QVector<int> m_sampleCache;
+};
+
+void CreateExample()
+{
+ new Example<int>();
+}
diff --git a/tests/range-loop/config.json b/tests/range-loop/config.json
index a5f3648d..0184a574 100644
--- a/tests/range-loop/config.json
+++ b/tests/range-loop/config.json
@@ -5,7 +5,8 @@
"has_fixits" : true
},
{
- "filename" : "bug370609.cpp"
+ "filename" : "bug370609.cpp",
+ "has_fixits" : true
}
]
}
diff --git a/tests/run_tests.py b/tests/run_tests.py
index 09fe9d2d..387269c6 100755
--- a/tests/run_tests.py
+++ b/tests/run_tests.py
@@ -49,14 +49,22 @@ class Test:
return ""
def relativeFilename(self):
+ # example: "auto-unexpected-qstringbuilder/main.cpp"
return self.check.name + "/" + self.filename()
- def yamlFilename(self):
- # In case clazy-standalone generates a yaml file with fixits, this is what it will be called
- return self.relativeFilename() + ".yaml"
+ def yamlFilename(self, is_standalone):
+ # The name of the yaml file with fixits
+ # example: "auto-unexpected-qstringbuilder/main.cpp.clazy.yaml"
+ if is_standalone:
+ return self.relativeFilename() + ".clazy-standalone.yaml"
+ else:
+ return self.relativeFilename() + ".clazy.yaml"
- def fixedFilename(self):
- return self.relativeFilename() + ".fixed"
+ def fixedFilename(self, is_standalone):
+ if is_standalone:
+ return self.relativeFilename() + ".clazy-standalone.fixed"
+ else:
+ return self.relativeFilename() + ".clazy.fixed"
def expectedFixedFilename(self):
return self.relativeFilename() + ".fixed.expected"
@@ -89,12 +97,21 @@ class Test:
name = self.check.name
if len(self.check.tests) > 1:
name += "/" + self.filename()
- if is_fixits:
- name += " (fixits)"
+ if is_fixits and is_standalone:
+ name += " (standalone, fixits)"
elif is_standalone:
name += " (standalone)"
+ elif is_fixits:
+ name += " (plugin, fixits)"
+ else:
+ name += " (plugin)"
return name
+ def removeYamlFiles(self):
+ for f in [test.yamlFilename(False), test.yamlFilename(True)]:
+ if os.path.exists(f):
+ os.remove(f)
+
class Check:
def __init__(self, name):
self.name = name
@@ -264,7 +281,7 @@ def clazy_standalone_command(test, qt):
result = " -checks=" + string.join(test.checks, ',') + " " + result
if test.has_fixits:
- result = " -enable-all-fixits -export-fixes=" + test.yamlFilename() + result
+ result = " -export-fixes=" + test.yamlFilename(is_standalone=True) + result
if test.qt4compat:
result = " -qt4-compat " + result
@@ -308,7 +325,8 @@ def clazy_command(qt, test, filename):
result = result + " -c "
result = result + test.flags + " -Xclang -plugin-arg-clazy -Xclang " + string.join(test.checks, ',') + " "
- result += _enable_fixits_argument + " "
+ if test.has_fixits:
+ result += _export_fixes_argument + " "
result += filename
return result
@@ -340,7 +358,7 @@ if args.only_standalone and args.no_standalone:
#-------------------------------------------------------------------------------
# Global variables
-_enable_fixits_argument = "-Xclang -plugin-arg-clazy -Xclang enable-all-fixits"
+_export_fixes_argument = "-Xclang -plugin-arg-clazy -Xclang export-fixes"
_dump_ast = args.dump_ast
_verbose = args.verbose
_no_standalone = args.no_standalone
@@ -446,29 +464,32 @@ def get_check_names():
# The yaml file references the test file in our git repo, but we don't want
# to rewrite that one, as we would need to discard git changes afterwards,
# so patch the yaml file and add a ".fixed" suffix to those files
-def patch_fixit_yaml_file(test):
+def patch_fixit_yaml_file(test, is_standalone):
+
+ yamlfilename = test.yamlFilename(is_standalone)
+ fixedfilename = test.fixedFilename(is_standalone)
- f = open(test.yamlFilename(), 'r')
+ f = open(yamlfilename, 'r')
lines = f.readlines()
f.close()
- f = open(test.yamlFilename(), 'w')
+ f = open(yamlfilename, 'w')
possible_headerfile = test.relativeFilename().replace(".cpp", ".h")
for line in lines:
stripped = line.strip()
if stripped.startswith('MainSourceFile') or stripped.startswith("FilePath") or stripped.startswith("- FilePath"):
- line = line.replace(test.relativeFilename(), test.fixedFilename())
+ line = line.replace(test.relativeFilename(), fixedfilename)
# Some tests also apply fix their to their headers:
- line = line.replace(possible_headerfile, test.fixedFilename().replace(".cpp", ".h"))
+ line = line.replace(possible_headerfile, fixedfilename.replace(".cpp", ".h"))
f.write(line)
f.close()
- shutil.copyfile(test.relativeFilename(), test.fixedFilename())
+ shutil.copyfile(test.relativeFilename(), fixedfilename)
if os.path.exists(possible_headerfile):
- shutil.copyfile(possible_headerfile, test.fixedFilename().replace(".cpp", ".h"))
+ shutil.copyfile(possible_headerfile, fixedfilename.replace(".cpp", ".h"))
return True
@@ -571,12 +592,10 @@ def run_unit_test(test, is_standalone):
# Check that it printed the expected warnings
if not compare_files(test.expects_failure, expected_file, result_file, test.printableName(is_standalone, False)):
- if os.path.exists(test.yamlFilename()):
- os.remove(test.yamlFilename())
-
+ test.removeYamlFiles();
return False
- if is_standalone and test.has_fixits:
+ if test.has_fixits:
# The normal tests succeeded, we can run the respective fixits then
test.should_run_fixits_test = True
@@ -595,25 +614,51 @@ def run_unit_tests(tests):
with _lock:
_was_successful = _was_successful and result
-# This is run sequentially, due to races. As clang-apply-replacements just applies all .yaml files it can find.
-# We run a single clang-apply-replacements invocation, which changes all files in the tests/ directory.
-def run_fixit_tests(requested_checks):
- if _no_standalone:
- # Only clazy-standalone supports fixits
+def patch_yaml_files(requested_checks, is_standalone):
+ if (is_standalone and _no_standalone) or (not is_standalone and _only_standalone):
+ # Nothing to do
return True
+ success = True
for check in requested_checks:
for test in check.tests:
if test.should_run_fixits_test:
- if not os.path.exists(test.yamlFilename()):
- print "[FAIL] " + test.yamlFilename() + " is missing!!"
+ yamlfilename = test.yamlFilename(is_standalone)
+ if not os.path.exists(yamlfilename):
+ print "[FAIL] " + yamlfilename + " is missing!!"
success = False
continue
- if not patch_fixit_yaml_file(test):
- print "[FAIL] Could not patch " + test.yamlFilename()
+ if not patch_fixit_yaml_file(test, is_standalone):
+ print "[FAIL] Could not patch " + yamlfilename
success = False
continue
+ return success
+
+def compare_fixit_results(test, is_standalone):
+ if (is_standalone and _no_standalone) or (not is_standalone and _only_standalone):
+ # Nothing to do
+ return True
+
+ # Check that the rewritten file is identical to the expected one
+ if not compare_files(False, test.expectedFixedFilename(), test.fixedFilename(is_standalone), test.printableName(is_standalone, True)):
+ return False
+
+ # Some fixed cpp files have an header that was also fixed. Compare it here too.
+ possible_headerfile_expected = test.expectedFixedFilename().replace('.cpp', '.h')
+ if os.path.exists(possible_headerfile_expected):
+ possible_headerfile = test.fixedFilename(is_standalone).replace('.cpp', '.h')
+ if not compare_files(False, possible_headerfile_expected, possible_headerfile, test.printableName(is_standalone, True).replace('.cpp', '.h')):
+ return False
+
+ return True
+
+# This is run sequentially, due to races. As clang-apply-replacements just applies all .yaml files it can find.
+# We run a single clang-apply-replacements invocation, which changes all files in the tests/ directory.
+def run_fixit_tests(requested_checks):
+
+ success = patch_yaml_files(requested_checks, is_standalone=False)
+ success = patch_yaml_files(requested_checks, is_standalone=True) and success
# Call clazy-apply-replacements[.exe]
if not run_clang_apply_replacements():
@@ -621,24 +666,17 @@ def run_fixit_tests(requested_checks):
# Now compare all the *.fixed files with the *.fixed.expected counterparts
- success = True
-
for check in requested_checks:
for test in check.tests:
if test.should_run_fixits_test:
# Check that the rewritten file is identical to the expected one
- if not compare_files(False, test.expectedFixedFilename(), test.fixedFilename(), test.printableName(True, True)):
+ if not compare_fixit_results(test, is_standalone=False):
success = False
continue
-
- # Some fixed cpp files have an header that was also fixed. Compare it here too.
- possible_headerfile_expected = test.expectedFixedFilename().replace('.cpp', '.h')
- if os.path.exists(possible_headerfile_expected):
- possible_headerfile = test.fixedFilename().replace('.cpp', '.h')
- if not compare_files(False, possible_headerfile_expected, possible_headerfile, test.printableName(True, True).replace('.cpp', '.h')):
- success = False
- continue
+ if not compare_fixit_results(test, is_standalone=True):
+ success = False
+ continue
return success