Clazy 1.4 released presenting 10 new Qt compile-time checks

11.10.2018 Sérgio Martins 9 comments

Clazy 1.4 has been released and brings 10 new checks. Clazy is a clang compiler plugin which emits warnings related to Qt best practices. We’ll be showing Clazy at Qt World Summit in Boston, Oct 29-30, where we are a main Sponsor. You can read more about it in our previous blog posts:

• How to use static analysis to improve performance
• Uncovering 32 Qt Best Practices
• Nailing 13 signal and slot mistakes with clazy 1.3

Today I’ll go through all 10 new warnings, one by one. For other changes, check the complete 1.4 Changelog. So let’s start. I’ve ordered them according to my personal preference, starting with the ones that have helped me uncover the most bugs and finishing with some exotic ones which you’ll rarely need.

1. skipped-base-method

Warns when calling a method from the grand-base class instead of the direct base class one. Example:

class MyFrame : public QFrame
{
    Q_OBJECT
public:
    bool event(QEvent *ev) override
    {
        (...)
        // warning: Maybe you meant to call QFrame::event() instead [-Wclazy-skipped-base-method]
        return QWidget::event(ev);
    }
};

The motivation came after hitting bugs when overriding QObject::event() and QObject::eventFilter(). I would suggest always using this check and annotating your legit cases with // clazy:exclude=skipped-base-method, to make your intention clear. The same way you would comment your fallthroughs in switch statements. This check might get removed in the future, as in the end it’s not specific to Qt and clang-tidy recently got a similar feature.

2. wrong-qevent-cast

Warns when a QEvent is cast to the wrong derived class via static_cast. Example:

switch (ev->type()) {
    case QEvent::MouseMove:
        auto e = static_cast<QKeyEvent*>(ev);
}

Only casts inside switch statements are verified.

3. qhash-with-char-pointer-key

Finds cases of QHash<const char *, T>. It’s error-prone as the key is just compared by the address of the string literal, and not the value itself. This check is disabled by default as there are valid use cases. But again, I would suggest always using it and adding a // clazy:exclude= comment.

4. fully-qualified-moc-types

Warns when a signal, slot or invokable declaration is not using fully-qualified type names, which will break old-style connects and interaction with QML. Also warns if a Q_PROPERTY of type gadget is not fully-qualified (Enums and QObjects in Q_PROPERTY don’t need to be fully qualified). Example:

namespace MyNameSpace {
    struct MyType { (...) };
    class MyObject : public QObject
    {
        Q_OBJECT
        Q_PROPERTY(MyGadget myprop READ myprop); // Wrong, needs namespace
    Q_SIGNALS:
        void mySignal(MyType); // Wrong
        void mySignal(MyNameSpace::MyType); // OK
    };
}

Beware that fixing these type names might break user code if they are connecting to them via old-style connects, since the users might have worked around your bug and not included the namespace in their connect statement.

5. connect-by-name

Warns when auto-connection slots are used. They’re also known as “connect by name“, an old and unpopular feature which isn’t recommended. Consult the official documentation for more information. These types of connections are very brittle, as a simple object rename would break your code. In Qt 5 the pointer-to-member-function connect syntax is recommended as it catches errors at compile time.

6. empty-qstringliteral

Suggests to use an empty QString instead of an empty QStringLiteral. The later causes unneeded code bloat. You should use QString() instead of QStringLiteral() and QString("") instead of QStringLiteral("").

7. qt-keywords (with fixit)

Warns when using Qt keywords such as emit, slots, signals or foreach. This check is disabled by default. Using the above Qt keywords is fine unless you’re using 3rdparty headers that also define them, in which case you’ll want to use Q_EMIT, Q_SLOTS, Q_SIGNALS or Q_FOREACH instead. This check is mainly useful due to its fixit to automatically convert the keywords to their Q_ variants. Once you’ve converted all usages, then simply enforce them via CONFIG += no_keywords (qmake) or ADD_DEFINITIONS(-DQT_NO_KEYWORDS) (CMake).

8. raw-environment-function

Warns when putenv() or qputenv() are being used and suggests the Qt thread-safe equivalents instead. This check is disabled by default and should be enabled manually if thread-safety is important for you.

9. qstring-varargs

This implements the equivalent of -Wnon-pod-varargs but only for QString. This check is only useful in cases where you don’t want to enable -Wnon-pod-varargs. For example on projects with thousands of benign warnings (like with MFC’s CString), where you might only want to fix the QString cases.

10. static-pmf

Warns when storing a pointer to a QObject member function and passing it to a connect statement. Passing such variable to a connect() is known to fail at runtime when using MingW. You can check if you’re affected by this problem with the following snippet:

static auto pmf = &QObject::destroyed;
if (pmf == &QObject::destroyed) // Should be false for MingW

Conclusion and thoughts for version 1.5

Clazy has matured and it’s getting increasingly difficult to come up with new ideas for checks. For version 1.5 I won’t be focusing in writing new warnings, but instead figuring out how to organize the existing ones. This project has come a long way, there’s now 77 checks and I feel the current classification by false-positive probability (levels 0, 1, 2, 3) is not scaling anymore. I will try to organize them by categories (bug, performance, readability, containers, etc), which would be orthogonal to levels and hopefully also answer the following questions:

• What’s the absolute sub-set of checks that every project should use ?
• Which ones should abort the build if triggered (-Werror style) ?
• How to make clazy useful in CI without getting in the way with annoying false-positives ?
• How to let the user configure all this in an easy way ?

But of course, if you know of any interesting check that wouldn’t cost me many days of work please file a suggestion or catch me at #kde-clazy (freenode) and it might make it to the next release.