Expand the smartpointerderefaddress compiler plugin to more ptr types

Instead of having a hard-coded list of smart pointer types, the plugin
now checks if the called type has a method called “get” which returns a
pointer with the same type as the overall expression. The intention is
that this should be general enough to catch all smart pointers.

Change-Id: I8c6f21f34a9debb81765155c544d0564c0f8ec58
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/211848
Reviewed-by: Neil Roberts <nroberts@libreoffice.org>
Tested-by: Jenkins

compilerplugins/clang/smartpointerderefaddress.cxx

@@ -31,6 +31,9 @@
  * See tdf#173585 for an example.
  *
  * Either way x.get() is easier to understand.
+ *
+ * Whether the object is a smart pointer or not is determined by whether it has a get() method which
+ * returns the same type as the original expression.
  */
 
 namespace
@@ -52,6 +55,37 @@ public:
     bool VisitUnaryOperator(UnaryOperator const*);
 };
 
+bool cxxRecordHasGetMethod(const CXXRecordDecl* recordDecl, const QualType& expressionType)
+{
+    for (auto method : recordDecl->methods())
+    {
+        if (!method->isInstance() || method->param_size() != 0
+            || !loplugin::DeclCheck(method).Function("get"))
+        {
+            continue;
+        }
+
+        QualType returnType = method->getReturnType().getUnqualifiedType().getCanonicalType();
+
+        if (returnType == expressionType)
+            return true;
+    }
+
+    return false;
+}
+
+bool cxxRecordOrBaseHasGetMethod(const CXXRecordDecl* recordDecl, const QualType& expressionType)
+{
+    if (cxxRecordHasGetMethod(recordDecl, expressionType))
+        return true;
+
+    auto baseCallback = [&](const CXXRecordDecl* baseRecordDecl) {
+        return !cxxRecordHasGetMethod(baseRecordDecl, expressionType);
+    };
+
+    return !recordDecl->forallBases(baseCallback);
+}
+
 bool SmartPointerDerefAddress::VisitUnaryOperator(UnaryOperator const* unaryOperator)
 {
     if (ignoreLocation(unaryOperator))
@@ -65,12 +99,23 @@ bool SmartPointerDerefAddress::VisitUnaryOperator(UnaryOperator const* unaryOper
     if (!innerOp || innerOp->getOperator() != OO_Star)
         return true;
 
-    auto const tc = loplugin::TypeCheck(innerOp->getArg(0)->getType());
+    const Expr* derefedExpr = innerOp->getArg(0);
 
-    if (tc.ClassOrStruct("shared_ptr").StdNamespace()
+    // Remove any implicit casts so we can get the type of the original expression, not the one
-        || tc.ClassOrStruct("__shared_ptr_access").StdNamespace()
+    // where operator* is defined
-        || tc.ClassOrStruct("unique_ptr").StdNamespace()
+    while (const ImplicitCastExpr* castExpr = dyn_cast<ImplicitCastExpr>(derefedExpr))
-        || tc.ClassOrStruct("UnoCursorPointer").Namespace("sw").GlobalNamespace())
+        derefedExpr = castExpr->getSubExpr();
+
+    // Check if the dereferenced type is a struct or a class that also has a get method that returns
+    // the same type as the whole expression
+    const CXXRecordDecl* recordDecl = derefedExpr->getType()->getAsCXXRecordDecl();
+
+    if (!recordDecl)
+        return true;
+
+    QualType expressionType = unaryOperator->getType().getUnqualifiedType().getCanonicalType();
+
+    if (cxxRecordOrBaseHasGetMethod(recordDecl, expressionType))
     {
         report(DiagnosticsEngine::Warning,
                "'*' followed by '&' operating on %0, rather use '.get()'",

compilerplugins/clang/test/smartpointerderefaddress.cxx

@@ -18,6 +18,7 @@ class UnoCursorPointer
 {
 public:
     int& operator*() { return *m_pInteger; };
+    int* get() { return m_pInteger; };
 
 private:
     int* m_pInteger;