range-loop: Add fixit for adding qAsConst()

9 files changed, 117 insertions, 8 deletions

diff --git a/Changelog b/Changelog
index 71134a14..b8a6474f 100644
--- a/Changelog
+++ b/Changelog
@@ -100,6 +100,7 @@
     lowercase-qml-type-name
   - New Fixits:
     range-loop now supports adding missing refs or const-ref
+    range-loop now supports adding qAsConst()
     function-args-by-ref now adding missing refs or const-ref (experimental)
     Introduced CLAZY_FIXIT_SUFFIX env variable
   - Removed support for the obscure -DCLAZY_BUILD_UTILS_LIB to simplify the CMakeLists.txt
diff --git a/README.md b/README.md
index 198cbfd8..1c0ddc44 100644
--- a/README.md
+++ b/README.md
@@ -274,7 +274,7 @@ clazy runs all checks from level1 by default.
     - [qlatin1string-non-ascii](docs/checks/README-qlatin1string-non-ascii.md)
     - [qproperty-without-notify](docs/checks/README-qproperty-without-notify.md)
     - [qstring-left](docs/checks/README-qstring-left.md)
-    - [range-loop](docs/checks/README-range-loop.md)    (fix-range-loop-add-ref)
+    - [range-loop](docs/checks/README-range-loop.md)    (fix-range-loop-add-ref,fix-range-loop-add-qasconst)
     - [returning-data-from-temporary](docs/checks/README-returning-data-from-temporary.md)
     - [rule-of-two-soft](docs/checks/README-rule-of-two-soft.md)
     - [skipped-base-method](docs/checks/README-skipped-base-method.md)
diff --git a/checks.json b/checks.json
index 240d4f79..16afd44a 100644
--- a/checks.json
+++ b/checks.json
@@ -367,6 +367,9 @@
             "fixits" : [
                 {
                     "name" : "range-loop-add-ref"
+                },
+                {
+                    "name" : "range-loop-add-qasconst"
                 }
             ]
         },
diff --git a/src/Checks.h b/src/Checks.h
index 63e656c0..84a20f72 100644
--- a/src/Checks.h
+++ b/src/Checks.h
@@ -180,6 +180,7 @@ void CheckManager::registerChecks()
     registerCheck(check<QStringLeft>("qstring-left", CheckLevel1,  RegisteredCheck::Option_VisitsStmts));
     registerCheck(check<RangeLoop>("range-loop", CheckLevel1,  RegisteredCheck::Option_VisitsStmts));
     registerFixIt(1, "fix-range-loop-add-ref", "range-loop");
+    registerFixIt(2, "fix-range-loop-add-qasconst", "range-loop");
     registerCheck(check<ReturningDataFromTemporary>("returning-data-from-temporary", CheckLevel1,  RegisteredCheck::Option_VisitsStmts));
     registerCheck(check<RuleOfTwoSoft>("rule-of-two-soft", CheckLevel1,  RegisteredCheck::Option_VisitsStmts));
     registerCheck(check<SkippedBaseMethod>("skipped-base-method", CheckLevel1,  RegisteredCheck::Option_VisitsStmts));
diff --git a/src/checks/level1/range-loop.cpp b/src/checks/level1/range-loop.cpp
index 64636e5b..a126294a 100644
--- a/src/checks/level1/range-loop.cpp
+++ b/src/checks/level1/range-loop.cpp
@@ -31,6 +31,8 @@
 #include "StmtBodyRange.h"
 #include "SourceCompatibilityHelpers.h"
 #include "FixItUtils.h"
+#include "ClazyContext.h"
+#include "PreProcessorVisitor.h"
 
 #include <clang/AST/Decl.h>
 #include <clang/AST/DeclCXX.h>
@@ -46,9 +48,17 @@ class ClazyContext;
 using namespace clang;
 using namespace std;
 
+enum Fixit {
+    Fixit_AddRef = 1,
+    Fixit_AddqAsConst = 2
+};
+
 RangeLoop::RangeLoop(const std::string &name, ClazyContext *context)
     : CheckBase(name, context, Option_CanIgnoreIncludes)
 {
+    if (isFixitEnabled(Fixit_AddqAsConst)) {
+        context->enablePreprocessorVisitor();
+    }
 }
 
 void RangeLoop::VisitStmt(clang::Stmt *stmt)
@@ -58,6 +68,25 @@ void RangeLoop::VisitStmt(clang::Stmt *stmt)
     }
 }
 
+bool RangeLoop::islvalue(Expr *exp, SourceLocation &endLoc)
+{
+     if (isa<DeclRefExpr>(exp)) {
+         endLoc = clazy::locForEndOfToken(&m_astContext, exp->getLocStart());
+         return true;
+     }
+
+     if (auto me = dyn_cast<MemberExpr>(exp)) {
+         auto decl = me->getMemberDecl();
+         if (!decl || isa<FunctionDecl>(decl))
+             return false;
+
+         endLoc = clazy::locForEndOfToken(&m_astContext, me->getMemberLoc());
+         return true;
+     }
+
+     return false;
+}
+
 void RangeLoop::processForRangeLoop(CXXForRangeStmt *rangeLoop)
 {
     Expr *containerExpr = rangeLoop->getRangeInit();
@@ -86,7 +115,20 @@ void RangeLoop::processForRangeLoop(CXXForRangeStmt *rangeLoop)
     if (clazy::containerNeverDetaches(clazy::containerDeclForLoop(rangeLoop), bodyRange))
         return;
 
-    emitWarning(getLocStart(rangeLoop), "c++11 range-loop might detach Qt container (" + record->getQualifiedNameAsString() + ')');
+    std::vector<FixItHint> fixits;
+
+    SourceLocation end;
+    if (isFixitEnabled(Fixit_AddqAsConst) && islvalue(containerExpr, end)) {
+        PreProcessorVisitor *preProcessorVisitor = m_context->preprocessorVisitor;
+        if (!preProcessorVisitor || preProcessorVisitor->qtVersion() >= 50700) { // qAsConst() was added to 5.7
+            SourceLocation start = getLocStart(containerExpr);
+            fixits.push_back(clazy::createInsertion(start, "qAsConst("));
+            //SourceLocation end = getLocEnd(containerExpr);
+            fixits.push_back(clazy::createInsertion(end, ")"));
+        }
+    }
+
+    emitWarning(getLocStart(rangeLoop), "c++11 range-loop might detach Qt container (" + record->getQualifiedNameAsString() + ')', fixits);
 }
 
 void RangeLoop::checkPassByConstRefCorrectness(CXXForRangeStmt *rangeLoop)
@@ -103,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()) {
+        if (isFixitEnabled(Fixit_AddRef)) {
             const bool isConst = varDecl->getType().isConstQualified();
 
             if (!isConst) {
diff --git a/src/checks/level1/range-loop.h b/src/checks/level1/range-loop.h
index 3941012d..b039f94e 100644
--- a/src/checks/level1/range-loop.h
+++ b/src/checks/level1/range-loop.h
@@ -47,6 +47,7 @@ public:
     RangeLoop(const std::string &name, ClazyContext *context);
     void VisitStmt(clang::Stmt *stmt) override;
 private:
+    bool islvalue(clang::Expr *exp, clang::SourceLocation &endLoc);
     void processForRangeLoop(clang::CXXForRangeStmt *rangeLoop);
     void checkPassByConstRefCorrectness(clang::CXXForRangeStmt *rangeLoop);
 };
diff --git a/tests/range-loop/main.cpp b/tests/range-loop/main.cpp
index e3eadd14..39520039 100644
--- a/tests/range-loop/main.cpp
+++ b/tests/range-loop/main.cpp
@@ -209,3 +209,31 @@ void test_add_ref_fixits()
         s.clear();
     }
 }
+
+struct SomeStruct
+{
+    QStringList m_list;
+    void test_add_qasconst_fixits()
+    {
+         for (const auto &s : m_list) {} // Warn
+    }
+
+    QStringList getList();
+};
+
+
+void test_add_qasconst_fixits()
+{
+    SomeStruct f;
+    for (const auto &s : f.m_list) {}  // Warn
+
+    SomeStruct *f2;
+    for (const auto &s : f2->m_list) {}  // Warn
+
+    QStringList locallist = f.getList();
+    for (const auto &s : locallist) {} // Warn
+
+    for (const auto &s : getQtList()) {} // Warn
+
+    for (const auto &s : f.getList()) {} // Warn
+}
diff --git a/tests/range-loop/main.cpp.expected b/tests/range-loop/main.cpp.expected
index 8985dfcb..94e1959b 100644
--- a/tests/range-loop/main.cpp.expected
+++ b/tests/range-loop/main.cpp.expected
@@ -7,3 +7,8 @@ range-loop/main.cpp:123:10: warning: Missing reference in range-for with non tri
 range-loop/main.cpp:169:5: warning: c++11 range-loop might detach Qt container (QList) [-Wclazy-range-loop]
 range-loop/main.cpp:201:10: warning: Missing reference in range-for with non trivial type (QString) [-Wclazy-range-loop]
 range-loop/main.cpp:202:10: warning: Missing reference in range-for with non trivial type (QString) [-Wclazy-range-loop]
+range-loop/main.cpp:218:10: warning: c++11 range-loop might detach Qt container (QStringList) [-Wclazy-range-loop]
+range-loop/main.cpp:228:5: warning: c++11 range-loop might detach Qt container (QStringList) [-Wclazy-range-loop]
+range-loop/main.cpp:231:5: warning: c++11 range-loop might detach Qt container (QStringList) [-Wclazy-range-loop]
+range-loop/main.cpp:236:5: warning: c++11 range-loop might detach Qt container (QList) [-Wclazy-range-loop]
+range-loop/main.cpp:238:5: warning: c++11 range-loop might detach Qt container (QStringList) [-Wclazy-range-loop]
diff --git a/tests/range-loop/main.cpp_fixed.cpp.expected b/tests/range-loop/main.cpp_fixed.cpp.expected
index d18b397c..09e9a255 100644
--- a/tests/range-loop/main.cpp_fixed.cpp.expected
+++ b/tests/range-loop/main.cpp_fixed.cpp.expected
@@ -34,7 +34,7 @@ void testQtContainer()
 {
     QList<int> qt_container;
     receivingList(qt_container);
-    for (int i : qt_container) { // Warning
+    for (int i : qAsConst(qt_container)) { // Warning
     }
 
     const QList<int> const_qt_container;
@@ -44,8 +44,8 @@ void testQtContainer()
     for (int i : getQtList()) { // Warning
     }
 
-    for (int i : qt_container) { }  // Warning
-    for (const int &i : qt_container) { } // Warning
+    for (int i : qAsConst(qt_container)) { }  // Warning
+    for (const int &i : qAsConst(qt_container)) { } // Warning
     for (int &i : qt_container) { } // OK
 
 
@@ -76,7 +76,7 @@ void testQMultiMapDetach()
 {
     QMultiMap<int,int> m;
     receivingMap(m);
-    for (int i : m) {
+    for (int i : qAsConst(m)) {
     }
 }
 
@@ -166,7 +166,7 @@ void testBug367485()
 
     QList<int> list2;
     receivingList(list2);
-    for (auto a : list2) {} // Warning
+    for (auto a : qAsConst(list2)) {} // Warning
 
     QList<int> list3;
     for (auto a : list3) {} // OK
@@ -209,3 +209,31 @@ void test_add_ref_fixits()
         s.clear();
     }
 }
+
+struct SomeStruct
+{
+    QStringList m_list;
+    void test_add_qasconst_fixits()
+    {
+         for (const auto &s : qAsConst(m_list)) {} // Warn
+    }
+
+    QStringList getList();
+};
+
+
+void test_add_qasconst_fixits()
+{
+    SomeStruct f;
+    for (const auto &s : qAsConst(f.m_list)) {}  // Warn
+
+    SomeStruct *f2;
+    for (const auto &s : qAsConst(f2->m_list)) {}  // Warn
+
+    QStringList locallist = f.getList();
+    for (const auto &s : locallist) {} // Warn
+
+    for (const auto &s : getQtList()) {} // Warn
+
+    for (const auto &s : f.getList()) {} // Warn
+}