extend unusedfields loplugin to find fields that can be private

and apply the results in xmlscript

Change-Id: Ib126f6e1576639abfd171e99d9561be9715ece2f

2 files changed, 115 insertions, 125 deletions

diff --git a/compilerplugins/clang/unusedfields.cxx b/compilerplugins/clang/unusedfields.cxx
index 6e4da40439e1..1bf71cdea13b 100644
--- a/compilerplugins/clang/unusedfields.cxx
+++ b/compilerplugins/clang/unusedfields.cxx
@@ -47,6 +47,7 @@ struct MyFieldInfo
     std::string fieldName;
     std::string fieldType;
     std::string sourceLocation;
+    std::string access;
 };
 bool operator < (const MyFieldInfo &lhs, const MyFieldInfo &rhs)
 {
@@ -57,6 +58,7 @@ bool operator < (const MyFieldInfo &lhs, const MyFieldInfo &rhs)
 
 // try to limit the voluminous output a little
 static std::set<MyFieldInfo> touchedSet;
+static std::set<MyFieldInfo> touchedFromOutsideSet;
 static std::set<MyFieldInfo> readFromSet;
 static std::set<MyFieldInfo> definitionSet;
 
@@ -76,11 +78,13 @@ public:
         std::string output;
         for (const MyFieldInfo & s : touchedSet)
             output += "touch:\t" + s.parentClass + "\t" + s.fieldName + "\n";
+        for (const MyFieldInfo & s : touchedFromOutsideSet)
+            output += "outside:\t" + s.parentClass + "\t" + s.fieldName + "\n";
         for (const MyFieldInfo & s : readFromSet)
             output += "read:\t" + s.parentClass + "\t" + s.fieldName + "\n";
         for (const MyFieldInfo & s : definitionSet)
         {
-            output += "definition:\t" + s.parentClass + "\t" + s.fieldName + "\t" + s.fieldType + "\t" + s.sourceLocation + "\n";
+            output += "definition:\t" + s.access + "\t" + s.parentClass + "\t" + s.fieldName + "\t" + s.fieldType + "\t" + s.sourceLocation + "\n";
         }
         ofstream myfile;
         myfile.open( SRCDIR "/loplugin.unusedfields.log", ios::app | ios::out);
@@ -89,14 +93,14 @@ public:
     }
 
     bool shouldVisitTemplateInstantiations () const { return true; }
+    bool shouldVisitImplicitCode() const { return true; }
 
-    bool VisitCallExpr(CallExpr* );
     bool VisitFieldDecl( const FieldDecl* );
     bool VisitMemberExpr( const MemberExpr* );
     bool VisitDeclRefExpr( const DeclRefExpr* );
 private:
     MyFieldInfo niceName(const FieldDecl*);
-    std::string fullyQualifiedName(const FunctionDecl*);
+    void checkForTouchedFromOutside(const FieldDecl* fieldDecl, const Expr* memberExpr, const MyFieldInfo& fieldInfo);
 };
 
 MyFieldInfo UnusedFields::niceName(const FieldDecl* fieldDecl)
@@ -111,73 +115,27 @@ MyFieldInfo UnusedFields::niceName(const FieldDecl* fieldDecl)
     aInfo.sourceLocation = std::string(name.substr(strlen(SRCDIR)+1)) + ":" + std::to_string(compiler.getSourceManager().getSpellingLineNumber(expansionLoc));
     normalizeDotDotInFilePath(aInfo.sourceLocation);
 
-    return aInfo;
-}
-
-std::string UnusedFields::fullyQualifiedName(const FunctionDecl* functionDecl)
-{
-    std::string ret = compat::getReturnType(*functionDecl).getCanonicalType().getAsString();
-    ret += " ";
-    if (isa<CXXMethodDecl>(functionDecl)) {
-        const CXXRecordDecl* recordDecl = dyn_cast<CXXMethodDecl>(functionDecl)->getParent();
-        ret += recordDecl->getQualifiedNameAsString();
-        ret += "::";
-    }
-    ret += functionDecl->getNameAsString() + "(";
-    bool bFirst = true;
-    for (const ParmVarDecl *pParmVarDecl : compat::parameters(*functionDecl)) {
-        if (bFirst)
-            bFirst = false;
-        else
-            ret += ",";
-        ret += pParmVarDecl->getType().getCanonicalType().getAsString();
-    }
-    ret += ")";
-    if (isa<CXXMethodDecl>(functionDecl) && dyn_cast<CXXMethodDecl>(functionDecl)->isConst()) {
-        ret += " const";
-    }
-
-    return ret;
-}
-
-// prevent recursive templates from blowing up the stack
-static std::set<std::string> traversedFunctionSet;
-
-bool UnusedFields::VisitCallExpr(CallExpr* expr)
-{
-    // Note that I don't ignore ANYTHING here, because I want to get calls to my code that result
-    // from template instantiation deep inside the STL and other external code
-
-    FunctionDecl* calleeFunctionDecl = expr->getDirectCallee();
-    if (calleeFunctionDecl == nullptr) {
-        Expr* callee = expr->getCallee()->IgnoreParenImpCasts();
-        DeclRefExpr* dr = dyn_cast<DeclRefExpr>(callee);
-        if (dr) {
-            calleeFunctionDecl = dyn_cast<FunctionDecl>(dr->getDecl());
-            if (calleeFunctionDecl)
-                goto gotfunc;
-        }
-        return true;
+    switch (fieldDecl->getAccess())
+    {
+    case AS_public: aInfo.access = "public"; break;
+    case AS_private: aInfo.access = "private"; break;
+    case AS_protected: aInfo.access = "protected"; break;
+    default: aInfo.access = "unknown"; break;
     }
 
-gotfunc:
-    // if we see a call to a function, it may effectively create new code,
-    // if the function is templated. However, if we are inside a template function,
-    // calling another function on the same template, the same problem occurs.
-    // Rather than tracking all of that, just traverse anything we have not already traversed.
-    if (traversedFunctionSet.insert(fullyQualifiedName(calleeFunctionDecl)).second)
-        TraverseFunctionDecl(calleeFunctionDecl);
-
-    return true;
+    return aInfo;
 }
 
 bool UnusedFields::VisitFieldDecl( const FieldDecl* fieldDecl )
 {
     fieldDecl = fieldDecl->getCanonicalDecl();
-    const FieldDecl* canonicalDecl = fieldDecl;
-
-    if( ignoreLocation( fieldDecl ))
+    if (ignoreLocation( fieldDecl )) {
         return true;
+    }
+    // ignore stuff that forms part of the stable URE interface
+    if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) {
+        return true;
+    }
 
     QualType type = fieldDecl->getType();
     // unwrap array types
@@ -200,7 +158,7 @@ bool UnusedFields::VisitFieldDecl( const FieldDecl* fieldDecl )
             return true;
     }
 */
-    definitionSet.insert(niceName(canonicalDecl));
+    definitionSet.insert(niceName(fieldDecl));
     return true;
 }
 
@@ -211,20 +169,22 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
     if (!fieldDecl) {
         return true;
     }
+    fieldDecl = fieldDecl->getCanonicalDecl();
+    if (ignoreLocation(fieldDecl)) {
+        return true;
+    }
+    // ignore stuff that forms part of the stable URE interface
+    if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) {
+        return true;
+    }
 
     MyFieldInfo fieldInfo = niceName(fieldDecl);
 
-    // ignore move/copy operator, it's self->self
-    const FunctionDecl* parentFunction = parentFunctionDecl(memberExpr);
-    const CXXMethodDecl* methodDecl = dyn_cast_or_null<CXXMethodDecl>(parentFunction);
-    if (!methodDecl || !(methodDecl->isCopyAssignmentOperator() || methodDecl->isMoveAssignmentOperator())) {
-        touchedSet.insert(fieldInfo);
-    }
+  // for the touched-from-outside analysis
 
-  // for the write-only analysis
+    checkForTouchedFromOutside(fieldDecl, memberExpr, fieldInfo);
 
-    if (ignoreLocation(memberExpr))
-        return true;
+  // for the write-only analysis
 
     const Stmt* child = memberExpr;
     const Stmt* parent = parentStmt(memberExpr);
@@ -298,13 +258,48 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
 bool UnusedFields::VisitDeclRefExpr( const DeclRefExpr* declRefExpr )
 {
     const Decl* decl = declRefExpr->getDecl();
-    if (!isa<FieldDecl>(decl)) {
+    const FieldDecl* fieldDecl = dyn_cast<FieldDecl>(decl);
+    if (!fieldDecl) {
+        return true;
+    }
+    fieldDecl = fieldDecl->getCanonicalDecl();
+    if (ignoreLocation(fieldDecl)) {
         return true;
     }
-    touchedSet.insert(niceName(dyn_cast<FieldDecl>(decl)));
+    // ignore stuff that forms part of the stable URE interface
+    if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) {
+        return true;
+    }
+    MyFieldInfo fieldInfo = niceName(fieldDecl);
+    touchedSet.insert(fieldInfo);
+    checkForTouchedFromOutside(fieldDecl, declRefExpr, fieldInfo);
     return true;
 }
 
+void UnusedFields::checkForTouchedFromOutside(const FieldDecl* fieldDecl, const Expr* memberExpr, const MyFieldInfo& fieldInfo) {
+    const FunctionDecl* memberExprParentFunction = parentFunctionDecl(memberExpr);
+    const CXXMethodDecl* methodDecl = dyn_cast_or_null<CXXMethodDecl>(memberExprParentFunction);
+
+    // it's touched from somewhere outside a class
+    if (!methodDecl) {
+        touchedSet.insert(fieldInfo);
+        touchedFromOutsideSet.insert(fieldInfo);
+        return;
+    }
+
+    auto constructorDecl = dyn_cast<CXXConstructorDecl>(methodDecl);
+    if (methodDecl->isCopyAssignmentOperator() || methodDecl->isMoveAssignmentOperator()) {
+        // ignore move/copy operator, it's self->self
+    } else if (constructorDecl && (constructorDecl->isCopyConstructor() || constructorDecl->isMoveConstructor())) {
+        // ignore move/copy constructor, it's self->self
+    } else {
+        touchedSet.insert(fieldInfo);
+        if (memberExprParentFunction->getParent() != fieldDecl->getParent()) {
+           touchedFromOutsideSet.insert(fieldInfo);
+        }
+    }
+}
+
 loplugin::Plugin::Registration< UnusedFields > X("unusedfields", false);
 
 }
diff --git a/compilerplugins/clang/unusedfields.py b/compilerplugins/clang/unusedfields.py
index 785b7a9a1881..7bf910f62d80 100755
--- a/compilerplugins/clang/unusedfields.py
+++ b/compilerplugins/clang/unusedfields.py
@@ -5,11 +5,13 @@ import re
 import io
 
 definitionSet = set()
+protectedAndPublicDefinitionSet = set() # set of tuple(type, name)
 definitionToSourceLocationMap = dict()
 definitionToTypeMap = dict()
 callSet = set()
 readFromSet = set()
 sourceLocationSet = set()
+touchedFromOutsideSet = set()
 
 # clang does not always use exactly the same numbers in the type-parameter vars it generates
 # so I need to substitute them to ensure we can match correctly.
@@ -17,30 +19,43 @@ normalizeTypeParamsRegex = re.compile(r"type-parameter-\d+-\d+")
 def normalizeTypeParams( line ):
     return normalizeTypeParamsRegex.sub("type-parameter-?-?", line)
 
+def parseFieldInfo( tokens ):
+    if len(tokens) == 3:
+        return (normalizeTypeParams(tokens[1]), tokens[2])
+    else:
+        return (normalizeTypeParams(tokens[1]), "")
+
 # The parsing here is designed to avoid grabbing stuff which is mixed in from gbuild.
 # I have not yet found a way of suppressing the gbuild output.
 with io.open("loplugin.unusedfields.log", "rb", buffering=1024*1024) as txt:
     for line in txt:
         tokens = line.strip().split("\t")
         if tokens[0] == "definition:":
-            fieldInfo = (normalizeTypeParams(tokens[1]), tokens[2])
+            access = tokens[1]
+            fieldInfo = (normalizeTypeParams(tokens[2]), tokens[3])
+            srcLoc = tokens[5]
+            # ignore external source code
+            if (srcLoc.startswith("external/")):
+                continue
+            # ignore build folder
+            if (srcLoc.startswith("workdir/")):
+                continue
             definitionSet.add(fieldInfo)
-            definitionToTypeMap[fieldInfo] = tokens[3]
-            definitionToSourceLocationMap[fieldInfo] = tokens[4]
+            definitionToTypeMap[fieldInfo] = tokens[4]
+            if access == "protected" or access == "public":
+                protectedAndPublicDefinitionSet.add(fieldInfo)
+            definitionToSourceLocationMap[fieldInfo] = tokens[5]
         elif tokens[0] == "touch:":
-            if len(tokens) == 3:
-                callInfo = (normalizeTypeParams(tokens[1]), tokens[2])
-                callSet.add(callInfo)
-            else:
-                callInfo = (normalizeTypeParams(tokens[1]), "")
-                callSet.add(callInfo)
+            callSet.add(parseFieldInfo(tokens))
+        elif tokens[0] == "read:":
+            readFromSet.add(parseFieldInfo(tokens))
         elif tokens[0] == "read:":
-            if len(tokens) == 3:
-                readInfo = (normalizeTypeParams(tokens[1]), tokens[2])
-                readFromSet.add(readInfo)
-            else:
-                readInfo = (normalizeTypeParams(tokens[1]), "")
-                readFromSet.add(readInfo)
+            readFromSet.add(parseFieldInfo(tokens))
+        elif tokens[0] == "outside:":
+            touchedFromOutsideSet.add(parseFieldInfo(tokens))
+        else:
+            print( "unknown line: " + line)
+
 # Invert the definitionToSourceLocationMap
 # If we see more than one method at the same sourceLocation, it's being autogenerated as part of a template
 # and we should just ignore
@@ -58,24 +73,6 @@ for d in definitionSet:
     if d in callSet:
         continue
     srcLoc = definitionToSourceLocationMap[d];
-    # ignore external source code
-    if (srcLoc.startswith("external/")):
-        continue
-    # ignore build folder
-    if (srcLoc.startswith("workdir/")):
-        continue
-    # ignore our stable/URE/UNO api
-    if (srcLoc.startswith("include/com/")
-        or srcLoc.startswith("include/cppu/")
-        or srcLoc.startswith("include/cppuhelper/")
-        or srcLoc.startswith("include/osl/")
-        or srcLoc.startswith("include/rtl/")
-        or srcLoc.startswith("include/sal/")
-        or srcLoc.startswith("include/salhelper/")
-        or srcLoc.startswith("include/systools/")
-        or srcLoc.startswith("include/typelib/")
-        or srcLoc.startswith("include/uno/")):
-        continue
     # this is all representations of on-disk data structures
     if (srcLoc.startswith("sc/source/filter/inc/scflt.hxx")
         or srcLoc.startswith("sw/source/filter/ww8/")
@@ -108,24 +105,6 @@ for d in definitionSet:
     if d in readFromSet:
         continue
     srcLoc = definitionToSourceLocationMap[d];
-    # ignore external source code
-    if (srcLoc.startswith("external/")):
-        continue
-    # ignore build folder
-    if (srcLoc.startswith("workdir/")):
-        continue
-    # ignore our stable/URE/UNO api
-    if (srcLoc.startswith("include/com/")
-        or srcLoc.startswith("include/cppu/")
-        or srcLoc.startswith("include/cppuhelper/")
-        or srcLoc.startswith("include/osl/")
-        or srcLoc.startswith("include/rtl/")
-        or srcLoc.startswith("include/sal/")
-        or srcLoc.startswith("include/salhelper/")
-        or srcLoc.startswith("include/systools/")
-        or srcLoc.startswith("include/typelib/")
-        or srcLoc.startswith("include/uno/")):
-        continue
     # this is all representations of on-disk data structures
     if (srcLoc.startswith("sc/source/filter/inc/scflt.hxx")
         or srcLoc.startswith("sw/source/filter/ww8/")
@@ -140,6 +119,17 @@ for d in definitionSet:
 
     writeonlySet.add((clazz + " " + definitionToTypeMap[d], srcLoc))
 
+
+canBePrivateSet = set()
+for d in protectedAndPublicDefinitionSet:
+    clazz = d[0] + " " + d[1]
+    if d in touchedFromOutsideSet:
+        continue
+    srcLoc = definitionToSourceLocationMap[d];
+
+    canBePrivateSet.add((clazz + " " + definitionToTypeMap[d], srcLoc))
+
+
 # sort the results using a "natural order" so sequences like [item1,item2,item10] sort nicely
 def natural_sort_key(s, _nsre=re.compile('([0-9]+)')):
     return [int(text) if text.isdigit() else text.lower()
@@ -148,6 +138,7 @@ def natural_sort_key(s, _nsre=re.compile('([0-9]+)')):
 # sort results by name and line number
 tmp1list = sorted(untouchedSet, key=lambda v: natural_sort_key(v[1]))
 tmp2list = sorted(writeonlySet, key=lambda v: natural_sort_key(v[1]))
+tmp3list = sorted(canBePrivateSet, key=lambda v: natural_sort_key(v[1]))
 
 # print out the results
 with open("loplugin.unusedfields.report-untouched", "wt") as f:
@@ -158,5 +149,9 @@ with open("loplugin.unusedfields.report-writeonly", "wt") as f:
     for t in tmp2list:
         f.write( t[1] + "\n" )
         f.write( "    " + t[0] + "\n" )
+with open("loplugin.unusedfields.report-can-be-private", "wt") as f:
+    for t in tmp3list:
+        f.write( t[1] + "\n" )
+        f.write( "    " + t[0] + "\n" )