Clazy 1.4 released

presenting 10 new Qt compile-time checks

Sérgio Martins

11 October 2018

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.

9 Comments

Sorry, left this comment on an older post by accident. So now here where it actually fits. Clazy is a must-have in our company. Thanks for giving birth to such a useful tool. :) If you are looking for inspiration for new checks, maybe ask on the kde-i18n-doc mailing list. A few years ago, there were two or three things programmers got wrong that only broke translations. I think i18n("foo %1").arg(bar) was one of them. It had to be i18n("foo %1", bar) to work as expected. But well, I am away from i18n for some time and my knowledge is growing old. You might want to talk to Albert (aacid) about it. Cheers

This is a must-have tool for any Qt project - thanks for the update!

I always use QString() instead of QStringLiteral(""). Never thought about using QString("") instead. What would be the advantage?

It's just a difference, rather than an advantage. QString().isNull() is true, while QStringLiteral("").isNull() is false. If you have a big codebase and are replacing QStringLiteral("") with QString() then you might be introducing bugs if somewhere someone is using isNull() instead of isEmpty(). I would go directly to QString() if isNull() isn't an issue. Furthermore, it seems even QStringLiteral().isNull() is false (unlike QString() and QLatin1String), so I better update this check's readme.

Have a look: https://github.com/KDE/clazy/blob/master/docs/checks/README-empty-qstringliteral.md Should be more explicit now.

I'm having trouble building 1.4 on Ubuntu 16.04. I've tried building with clang 3.8 (default), clang 5.0 and clang 7.0 as well as gcc 5.4. I'm always getting an error on:

clazy-1.4/src/SourceCompatibilityHelpers.h:52:12: error: no viable conversion from returned value
      of type 'std::pair' to function return type 'clang::CharSourceRange'
    return sm.getImmediateExpansionRange(macroLoc);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Try clang 5.0 again, but make sure cmake says it's using the headers from 5.0, and not from 3.8. They are not compatible. Also check which llvm-config you have in PATH.

I keep getting this error

/opt/jdx/tools//usr/local/bin/clazy    --sysroot=/home/hm68841/workspace/guidance/jdx/wr8-baytrail_64/buildbox -std=c++14 -fno-omit-frame-pointer -gsplit-dwarf -fdiagnostics-color=always --target=x86_64-wrs-linux -o CMakeFiles/cmTC_c0901.dir/testCXXCompiler.cxx.o -c testCXXCompiler.cxx
    : CommandLine Error: Option 'static-func-full-module-prefix' registered more than once!
    fatal error: error in backend: inconsistency in registered CommandLine options```

I see the same error with clang-6. Does it only work with clang-5, or am I having some other problem. (there doesn't seem to be a good place to ask support questions)

Hi, This is usually because your clang and/or llvm was built statically, which isn't supported very well. If you build clang and llvm yourself, do pass -DBUILD_SHARED_LIBS=ON to cmake, or use a distro that is known to ship with shared libraries (Debian, Ubuntu, Archlinux, etc.) We're working on an AppImage for clazy, so you can just use that without having to compile it yourself, so it should just work on any distro.

Sérgio Martins

Senior Software Engineer

Sérgio has been with KDAB for 13 years. He loves performance profiling, debugging, static-analysis, and tooling. You won’t see him writing nice-looking GUIs or 3D. He’d rather enjoy tackling a hard problem, frequently embracing the pain of bugs nobody wants, such as HDPI, fonts, QDockWidget, and even working on Windows when needed.