qstring-allocations: turn s == "" into s.isEmpty()

5 files changed, 60 insertions, 4 deletions

diff --git a/src/checks/level2/qstring-allocations.cpp b/src/checks/level2/qstring-allocations.cpp
index 900b4442..cd1385d1 100644
--- a/src/checks/level2/qstring-allocations.cpp
+++ b/src/checks/level2/qstring-allocations.cpp
@@ -347,7 +347,7 @@ void QStringAllocations::VisitCtor(CXXConstructExpr *ctorExpr)
                             }
                         }
 
-                        fixits = fixItRawLiteral(lt, replacement);
+                        fixits = fixItRawLiteral(lt, replacement, nullptr);
                     }
                 }
             }
@@ -526,7 +526,25 @@ std::vector<FixItHint> QStringAllocations::fixItReplaceFromLatin1OrFromUtf8(Call
     return fixits;
 }
 
-std::vector<FixItHint> QStringAllocations::fixItRawLiteral(clang::StringLiteral *lt, const std::string &replacement)
+namespace
+{
+// Start at <loc> and go left as long as there's whitespace, stopping at <start> in the worst case
+SourceLocation eatLeadingWhitespace(SourceLocation start, SourceLocation loc, const SourceManager &sm, const LangOptions &lo)
+{
+    const SourceRange range(start, loc);
+    const CharSourceRange cr = Lexer::getAsCharRange(range, sm, lo);
+    const StringRef str = Lexer::getSourceText(cr, sm, lo);
+    const int initialPos = sm.getDecomposedLoc(loc).second - sm.getDecomposedLoc(start).second;
+    int i = initialPos;
+    while (--i >= 0) {
+        if (!isHorizontalWhitespace(str[i]))
+            return loc.getLocWithOffset(i - initialPos + 1);
+    }
+    return loc;
+}
+}
+
+std::vector<FixItHint> QStringAllocations::fixItRawLiteral(StringLiteral *lt, const std::string &replacement, CXXOperatorCallExpr *operatorCall)
 {
     std::vector<FixItHint> fixits;
 
@@ -546,6 +564,27 @@ std::vector<FixItHint> QStringAllocations::fixItRawLiteral(clang::StringLiteral
             return {};
         }
 
+        // Turn str == "" into str.isEmpty()
+        if (operatorCall && operatorCall->getOperator() == OO_EqualEqual && lt->getLength() == 0) {
+            // For some reason this returns the same as getStartLoc
+            // SourceLocation start = operatorCall->getArg(0)->getEndLoc();
+            // So instead, we have to start from the "==" sign and eat whitespace to the left
+            SourceLocation start = eatLeadingWhitespace(operatorCall->getBeginLoc(), operatorCall->getExprLoc(), sm(), lo());
+            fixits.push_back(clazy::createReplacement({start, range.getEnd()}, ".isEmpty()"));
+            return fixits;
+        }
+
+        // Turn str != "" into !str.isEmpty()
+        if (operatorCall && operatorCall->getOperator() == OO_ExclaimEqual && lt->getLength() == 0) {
+            // For some reason this returns the same as getStartLoc
+            // SourceLocation start = operatorCall->getArg(0)->getEndLoc();
+            // So instead, we have to start from the "==" sign and eat whitespace to the left
+            SourceLocation start = eatLeadingWhitespace(operatorCall->getBeginLoc(), operatorCall->getExprLoc(), sm(), lo());
+            fixits.push_back(clazy::createReplacement({start, range.getEnd()}, ".isEmpty()"));
+            fixits.push_back(clazy::createInsertion(operatorCall->getBeginLoc(), "!"));
+            return fixits;
+        }
+
         std::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...");
@@ -610,7 +649,7 @@ void QStringAllocations::VisitOperatorCall(Stmt *stm)
         queueManualFixitWarning(clazy::getLocStart(stm), "Couldn't find literal");
     } else {
         const std::string replacement = Utils::isAscii(literals[0]) ? "QLatin1String" : "QStringLiteral";
-        fixits = fixItRawLiteral(literals[0], replacement);
+        fixits = fixItRawLiteral(literals[0], replacement, operatorCall);
     }
 
     std::string msg("QString(const char*) being called");
diff --git a/src/checks/level2/qstring-allocations.h b/src/checks/level2/qstring-allocations.h
index 416fa26d..1f44550d 100644
--- a/src/checks/level2/qstring-allocations.h
+++ b/src/checks/level2/qstring-allocations.h
@@ -57,7 +57,7 @@ private:
     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);
+    std::vector<clang::FixItHint> fixItRawLiteral(clang::StringLiteral *stmt, const std::string &replacement, clang::CXXOperatorCallExpr *operatorCall);
 
     Latin1Expr qlatin1CtorExpr(clang::Stmt *stm, clang::ConditionalOperator *&ternary);
 };
diff --git a/tests/qstring-allocations/main.cpp b/tests/qstring-allocations/main.cpp
index c0a435a4..56f3b0d3 100644
--- a/tests/qstring-allocations/main.cpp
+++ b/tests/qstring-allocations/main.cpp
@@ -252,3 +252,10 @@ QString s3 = true ? "ö" : "\xc3\xb6";
 
 // bug #391807
 Q_GLOBAL_STATIC_WITH_ARGS(const QString, strUnit, (QLatin1String("unit"))) // OK, since QStringLiteral doesn't work inside Q_GLOBAL_STATIC_WITH_ARGS
+
+void testEmptyString()
+{
+    QString s1, s2, s3;
+    if (s1 == "" && s2 != "" && s3 == "foo")
+        s1 = s3;
+}
diff --git a/tests/qstring-allocations/main.cpp.expected b/tests/qstring-allocations/main.cpp.expected
index ba6f7bc7..399bbcf4 100644
--- a/tests/qstring-allocations/main.cpp.expected
+++ b/tests/qstring-allocations/main.cpp.expected
@@ -78,3 +78,6 @@ qstring-allocations/main.cpp:245:18: warning: QString(QLatin1String) being calle
 qstring-allocations/main.cpp:248:13: warning: QString::fromUtf8() being passed a literal [-Wclazy-qstring-allocations]
 qstring-allocations/main.cpp:249:14: warning: QString::fromUtf8() being passed a literal [-Wclazy-qstring-allocations]
 qstring-allocations/main.cpp:250:14: warning: QString(const char*) being called [-Wclazy-qstring-allocations]
+qstring-allocations/main.cpp:259:9: warning: QString(const char*) being called [-Wclazy-qstring-allocations]
+qstring-allocations/main.cpp:259:21: warning: QString(const char*) being called [-Wclazy-qstring-allocations]
+qstring-allocations/main.cpp:259:33: warning: QString(const char*) being called [-Wclazy-qstring-allocations]
diff --git a/tests/qstring-allocations/main.cpp.fixed.expected b/tests/qstring-allocations/main.cpp.fixed.expected
index 6cab2a1d..4373fbef 100644
--- a/tests/qstring-allocations/main.cpp.fixed.expected
+++ b/tests/qstring-allocations/main.cpp.fixed.expected
@@ -252,3 +252,10 @@ QString s3 = true ? "ö" : "\xc3\xb6";
 
 // bug #391807
 Q_GLOBAL_STATIC_WITH_ARGS(const QString, strUnit, (QLatin1String("unit"))) // OK, since QStringLiteral doesn't work inside Q_GLOBAL_STATIC_WITH_ARGS
+
+void testEmptyString()
+{
+    QString s1, s2, s3;
+    if (s1.isEmpty() && !s2.isEmpty() && s3 == QLatin1String("foo"))
+        s1 = s3;
+}