Simplify Qt property exporter parsing

Author: tzi4

Makefile

@@ -642,7 +642,7 @@ $(libcppdir)/pathmatch.o: lib/pathmatch.cpp lib/config.h lib/path.h lib/pathmatc
 $(libcppdir)/platform.o: lib/platform.cpp externals/tinyxml2/tinyxml2.h lib/config.h lib/mathlib.h lib/path.h lib/platform.h lib/standards.h lib/utils.h lib/xml.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/platform.cpp
 
-$(libcppdir)/preprocessor.o: lib/preprocessor.cpp externals/simplecpp/simplecpp.h lib/checkers.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/smallvector.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vfvalue.h
+$(libcppdir)/preprocessor.o: lib/preprocessor.cpp externals/simplecpp/simplecpp.h lib/checkers.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/preprocessor.cpp
 
 $(libcppdir)/programmemory.o: lib/programmemory.cpp lib/astutils.h lib/calculate.h lib/checkers.h lib/config.h lib/errortypes.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/programmemory.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h lib/vfvalue.h

lib/preprocessor.cpp

@@ -28,7 +28,6 @@
 #include "settings.h"
 #include "standards.h"
 #include "suppressions.h"
-#include "token.h"
 #include "utils.h"
 
 #include <algorithm>
@@ -902,123 +901,40 @@ simplecpp::TokenList Preprocessor::preprocess(const std::string &cfgStr, std::ve
     return tokens2;
 }
 
-static const simplecpp::Token* qtPropertyAttributes(const simplecpp::Token* tok)
-{
-    while (tok && (tok->str() == "const" || tok->str() == "volatile"))
-        tok = tok->next;
-    if (!tok)
-        return nullptr;
-    if (Token::isStandardType(tok->str())) {
-        do {
-            tok = tok->next;
-        } while (tok && Token::isStandardType(tok->str()));
-    } else {
-        if (tok->str() == "::")
-            tok = tok->next;
-        if (!tok || !tok->name)
-            return nullptr;
-        for (;;) {
-            tok = tok->next;
-            if (tok && tok->str() == "<") {
-                unsigned int depth = 1;
-                unsigned int parentheses = 0;
-                do {
-                    tok = tok->next;
-                    if (!tok)
-                        return nullptr;
-                    if (tok->str() == "(")
-                        ++parentheses;
-                    else if (tok->str() == ")" && parentheses)
-                        --parentheses;
-                    else if (!parentheses) {
-                        if (tok->str() == "<")
-                            ++depth;
-                        else if (tok->str() == ">")
-                            --depth;
-                        else if (tok->str() == ">>") {
-                            if (depth < 2)
-                                return nullptr;
-                            depth -= 2;
-                        }
-                    }
-                } while (depth);
-                tok = tok->next;
-            }
-            if (!tok || tok->str() != "::" || !tok->next || !tok->next->name)
-                break;
-            tok = tok->next;
-        }
-    }
-    while (tok && (tok->str() == "*" || tok->str() == "&" || tok->str() == "&&" ||
-                   tok->str() == "const" || tok->str() == "volatile"))
-        tok = tok->next;
-    // The property name is not a function reference.
-    return tok && tok->name ? tok->next : nullptr;
-}
-
-static std::set<std::string> qtPropertyFunctions(const simplecpp::Token* tok, const Library& library)
+static std::set<std::string> exporterFunctions(const simplecpp::Token* tok, const std::string& exporter, const Library& library)
 {
+    // Match configured exporter keywords without parsing library-specific grammar.
     std::set<std::string> functions;
-    tok = qtPropertyAttributes(tok);
-    while (tok && tok->str() != ")") {
-        const std::string& attribute = tok->str();
-        tok = tok->next;
-        if (attribute == "CONSTANT" || attribute == "FINAL" || attribute == "REQUIRED" ||
-            attribute == "VIRTUAL" || attribute == "OVERRIDE")
-            continue;
-        if (!tok)
-            return {};
-        if (attribute == "REVISION") {
-            if (tok->number)
-                tok = tok->next;
-            else if (tok->str() == "(") {
-                do {
-                    tok = tok->next;
-                } while (tok && (tok->number || tok->str() == ","));
-                if (!tok || tok->str() != ")")
-                    return {};
-                tok = tok->next;
-            } else
-                return {};
-        } else if (attribute == "MEMBER") {
-            if (!tok->name)
-                return {};
-            tok = tok->next;
-        } else if (library.isexportedprefix("Q_PROPERTY", attribute)) {
-            const bool parenthesized = tok->str() == "(";
+    unsigned int depth = 0;
+    for (; tok; tok = tok->next) {
+        if (tok->str() == "(")
+            ++depth;
+        else if (tok->str() == ")") {
+            if (depth == 0)
+                break;
+            --depth;
+        } else if (depth == 0) {
+            if (library.isexportedsuffix(exporter, tok->str()) && tok->previous && tok->previous->name)
+                functions.insert(tok->previous->str());
+            if (!library.isexportedprefix(exporter, tok->str()))
+                continue;
+            const simplecpp::Token* name = tok->next;
+            const bool parenthesized = name && name->str() == "(";
             if (parenthesized)
-                tok = tok->next;
-            if (!tok)
-                return {};
-            if (tok->str() == "::")
-                tok = tok->next;
-            if (!tok)
-                return {};
-            if (!tok->name)
-                return {};
-            std::string function = tok->str();
-            tok = tok->next;
-            while (tok && tok->str() == "::" && tok->next && tok->next->name) {
-                function = tok->next->str();
-                tok = tok->next->next;
-            }
-            if (function != "true" && function != "false" && function != "default")
-                functions.insert(function);
-            if (parenthesized) {
-                if (!tok || tok->str() != ")")
-                    return {};
-                tok = tok->next;
-            }
-            if (tok && tok->str() == "(") {
-                tok = tok->next;
-                if (!tok || tok->str() != ")")
-                    return {};
-                tok = tok->next;
-            }
-        } else
-            return {};
+                name = name->next;
+            if (name && name->str() == "::")
+                name = name->next;
+            if (!name || !name->name)
+                continue;
+            while (name->next && name->next->str() == "::" && name->next->next && name->next->next->name)
+                name = name->next->next;
+            if (parenthesized && (!name->next || name->next->str() != ")"))
+                continue;
+            if (name->str() != "true" && name->str() != "false" && name->str() != "default")
+                functions.insert(name->str());
+        }
     }
-    return tok ? functions : std::set<std::string>{};
+    return functions;
 }
 
 void Preprocessor::readQtAnnotations(simplecpp::TokenList& tokens)
@@ -1055,7 +971,7 @@ void Preprocessor::readQtAnnotations(simplecpp::TokenList& tokens)
         }
         const simplecpp::Token* type = tag->next->next;
         if (type && type->str() == "qt_property" && type->next && type->next->str() == ",") {
-            const auto functions = qtPropertyFunctions(type->next->next, mSettings.library);
+            const auto functions = exporterFunctions(type->next->next, "Q_PROPERTY", mSettings.library);
             mExportedFunctions.insert(functions.begin(), functions.end());
             mExportedLocations.insert(tok->location);
         }
@@ -1091,25 +1007,8 @@ std::set<std::string> Preprocessor::getExportedFunctions() const
             if (locations.find(tok->location) == locations.end() ||
                 !mSettings.library.isexporter(tok->str()) || !tok->next || tok->next->str() != "(")
                 continue;
-            if (tok->str() == "Q_PROPERTY") {
-                const auto accessors = qtPropertyFunctions(tok->next->next, mSettings.library);
-                functions.insert(accessors.begin(), accessors.end());
-                continue;
-            }
-            unsigned int depth = 1;
-            for (const simplecpp::Token* arg = tok->next->next; arg; arg = arg->next) {
-                if (arg->str() == "(")
-                    ++depth;
-                else if (arg->str() == ")") {
-                    if (--depth == 0)
-                        break;
-                } else if (depth == 1) {
-                    if (mSettings.library.isexportedprefix(tok->str(), arg->str()) && arg->next && arg->next->name)
-                        functions.insert(arg->next->str());
-                    if (mSettings.library.isexportedsuffix(tok->str(), arg->str()) && arg->previous && arg->previous->name)
-                        functions.insert(arg->previous->str());
-                }
-            }
+            const auto exported = exporterFunctions(tok->next->next, tok->str(), mSettings.library);
+            functions.insert(exported.begin(), exported.end());
         }
     };
     collect(mTokens);

oss-fuzz/Makefile

@@ -312,7 +312,7 @@ $(libcppdir)/pathmatch.o: ../lib/pathmatch.cpp ../lib/config.h ../lib/path.h ../
 $(libcppdir)/platform.o: ../lib/platform.cpp ../externals/tinyxml2/tinyxml2.h ../lib/config.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/standards.h ../lib/utils.h ../lib/xml.h
 	$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/platform.cpp
 
-$(libcppdir)/preprocessor.o: ../lib/preprocessor.cpp ../externals/simplecpp/simplecpp.h ../lib/checkers.h ../lib/config.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/preprocessor.h ../lib/settings.h ../lib/smallvector.h ../lib/standards.h ../lib/suppressions.h ../lib/templatesimplifier.h ../lib/token.h ../lib/utils.h ../lib/vfvalue.h
+$(libcppdir)/preprocessor.o: ../lib/preprocessor.cpp ../externals/simplecpp/simplecpp.h ../lib/checkers.h ../lib/config.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/preprocessor.h ../lib/settings.h ../lib/standards.h ../lib/suppressions.h ../lib/utils.h
 	$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/preprocessor.cpp
 
 $(libcppdir)/programmemory.o: ../lib/programmemory.cpp ../lib/astutils.h ../lib/calculate.h ../lib/checkers.h ../lib/config.h ../lib/errortypes.h ../lib/infer.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/programmemory.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/valueptr.h ../lib/vfvalue.h

test/cli/unused_function_test.py

@@ -141,13 +141,13 @@ def test_unused_functions_qt_property_configurations(tmp_path):
     assert stderr == "unusedFunction:The function 'unused' is never used.\n"
 
 
-@pytest.mark.parametrize('property_type, property_name', [
-    ('READ', 'property'),
-    ('const Names::READ*', 'property'),
-    ('unsigned long', 'READ'),
-    ('Box<Pair<::READ, ::WRITE>>', 'property'),
+@pytest.mark.parametrize('property_type, property_name, reported', [
+    ('READ', 'property', ['unrelated']),
+    ('const Names::READ*', 'property', ['property', 'unrelated']),
+    ('unsigned long', 'READ', ['property', 'unrelated']),
+    ('Box<Pair<::READ, ::WRITE>>', 'property', ['property', 'unrelated']),
 ])
-def test_unused_functions_qt_property_keyword_names(tmp_path, property_type, property_name):
+def test_unused_functions_qt_property_keyword_names(tmp_path, property_type, property_name, reported):
     source = tmp_path / 'test.cpp'
     source.write_text('''class READ {};
 class WRITE {};
@@ -162,6 +162,7 @@ class MyType {
     void valueChanged() {}
     void property() {}
     void NOTIFY() {}
+    void unrelated() {}
 };
 int main() { MyType t; }
 '''.replace('@TYPE@', property_type).replace('@PROPERTY@', property_name)
@@ -170,8 +171,11 @@ class MyType {
                                    '--enable=unusedFunction', '--library=qt',
                                    '-j1', '--no-cppcheck-build-dir', str(source)])
     assert ret == 0, stdout
-    assert stderr == ("unusedFunction:The function 'property' is never used.\n"
-                      "unusedFunction:The function 'NOTIFY' is never used.\n")
+    # Exporter keywords are matched without parsing the property's type/name.
+    # Keyword collisions can hide unused property/NOTIFY methods, while the
+    # actual accessors must stay used and unrelated methods must still warn.
+    assert stderr.splitlines() == ["unusedFunction:The function '{}' is never used.".format(name)
+                                   for name in reported]
 
 
 def test_unused_functions_qt_property_reset_keyword_name(tmp_path):
@@ -348,6 +352,46 @@ class MyType : public Base {
     assert stderr == "unusedFunction:The function 'unused' is never used.\n"
 
 
+@pytest.mark.parametrize('metadata', [
+    'GET value',
+    'GET (Base::value)',
+    'value USED',
+    'NESTED(GET unrelated) GET value',
+])
+def test_unused_functions_library_exporter(tmp_path, metadata):
+    library = tmp_path / 'exporter.cfg'
+    library.write_text('''<?xml version="1.0"?>
+<def format="2">
+    <markup ext=".meta">
+        <exported>
+            <exporter prefix="EXPORT">
+                <prefix>GET</prefix>
+                <suffix>USED</suffix>
+            </exporter>
+        </exported>
+    </markup>
+</def>
+''')
+    source = tmp_path / 'test.cpp'
+    source.write_text('''#define EXPORT(...)
+class Base {
+public:
+    int value() const { return 0; }
+};
+class MyType : public Base {
+    EXPORT(@METADATA@)
+public:
+    void unrelated() {}
+};
+int main() { MyType t; }
+'''.replace('@METADATA@', metadata))
+    ret, stdout, stderr = cppcheck(['-q', '--template={id}:{message}',
+                                   '--enable=unusedFunction', '--library=' + str(library),
+                                   '-j1', '--no-cppcheck-build-dir', str(source)])
+    assert ret == 0, stdout
+    assert stderr == "unusedFunction:The function 'unrelated' is never used.\n"
+
+
 def test_unused_functions_j():
     args = [
         '-q',