Earlier this year we launched the support for C and C++ in SonarLint for CLion to address quality and security issues for your C/C++ projects. Since then, the team has continued to bring even greater value to the C and C++ users, continuing our mission to empower the community to deliver code that meets the highest quality and security standards.
In this post, we want to demonstrate the powerful capabilities of the C++ analyzer with SonarLint (a free, in-IDE static analysis plugin) and highlight some unique and interesting rules that you might find useful. Through that lens, we want to show how you can leverage them to elevate your CLion’s inbuilt static analysis capabilities.
Rules that build on Checks and Inspections
CLion has a great set of what it calls Inspections already built in - many of which it integrates directly from clang-tidy Checks. Some of what SonarLint calls Rules are expanded forms of Inspections already found in CLion in a more limited form (often coming from clang-tidy).
A great example of this is S995, which detects if parameters, taken by pointer or reference, could be made const. clang-tidy has the readability-non-const-parameter check, which has the same goal, but, at time of writing, only works for pointers to numeric types - so for:
Within CLion, clang-tidy reports:
But for:
clang-tidy is silent, but SonarLint reports:
That makes this a much more useful rule. And note the link to a more in-depth rule description.
But the benefits of using const, here, are well known, and it’s often easy to spot them by eye. So let’s take a look at something less obvious. Imagine you have some code, like:
Looks harmless enough. And when you run that, it will work as you might expect and produce the results you intended. So what’s the problem?
The problem occurs in the call to find. We are passing a string literal, but find takes its argument by the key type of the container - in this case std::string. It’s a const-ref to a string, but it will still have to create a temporary string and copy the characters in.
If that is an issue for you it’s hard to spot that just by looking at it.
That all seems a shame, given that we’re only using the string to compare against each element - and comparisons already work across these string types.
So C++14 introduced transparent comparators. The associative containers take a template argument for a comparator, which defaults to std::less<KeyType>. It’s that KeyType that’s causing our issue. So std::less<void> was specified to be a specialization where all its members are templates - so they use whatever types they are called with, rather than baking in the KeyType - hence transparent. Cool! That’s exactly what we need. In fact, void is the default for std::less, so the common idiom is to just use std::less<>. Less is more, as they say.
Here’s how that looks:
Again, clang-tidy has a check in this area, modernize-use-transparent-functors, but it has several problems. It may actually make things worse, for some types, and it only works if a comparator is explicitly provided. Using the default comparator is by far the more common case (such as in our example), so S6045 detects these cases, too, and avoids those pessimizations, whereas S6021 actively warns against using transparent comparators for types that lack heterogeneous comparisons.
This is also a good example to point out the extensive description for the rule that can be found on the rule page - but is also surfaced right within the IDE. If you weren’t already familiar with transparent comparators you will receive a mini-lesson on them - helping you understand why the rule is triggering and how you might address it (or choose not to). Many of our rule descriptions turn out to be great learning resources - and having them right in your IDE at the point that you need to understand them makes them highly relevant to your code, as well.
Modern, New, Rules
Of course SonarLint has many rules that are not, currently at least, implemented at all by clang-tidy or CLion. For example, S3608 suggests that “Default capture should not be used”. This corresponds to Item #31 of Scott Meyers’ “Effective Modern C++”, and is partially captured (if you’ll excuse the pun) by the Core Guidelines and Google’s C++ Style Guide.
There are several potential problems with default capture modes, which are highlighted in the rule description. Most notably, even default value capture ([=]) captures the this pointer, which may lead to surprising results if a member variable is referenced. It might look like it should be captured by value but, in fact it is not captured at all - it’s being accessed through the captured this pointer. Oh C++!
As well as S3608, we also trigger S5019, “Lambdas that capture "this" should capture everything explicitly” if the this pointer is being used. If all variables are captured explicitly it is much easier to verify that there are no lifetime issues. A notable exception to this rule is when using Immediately Invoked Lambda Expressions (i.e. calling the lambda straight away) - and this exception, along with a couple of others, is allowed for by the rule.
Lambdas have been around since C++11, so may not be considered “modern” anymore. But CLion and clang-tidy have many inspections and checks that help us modernize our code. New language features are introduced for good reasons - usually leading to safer, easier-to-read code. So while it may not be worth rewriting all our existing code, we should take advantage of this evolution with any new code we write. So it’s great that our tools will help us there, too. SonarLint adds even more modernizing rules, such as S6004, “"if","switch", and range-based for loop initializer should be used to reduce scope of variables”.
C++17 introduced a way to initialize a variable from right within a control flow statement, such as if, in much the same way we’ve always been able to do with classic for statements. This allows us to scope our variables to only the parts of our code that need them. For objects employing the RAII pattern, this is especially valuable. C++20 expanded this feature for range-based-for loops - very useful for avoiding the trap of trying to iterate a temporary object!
S6004 reports when an init-statement could be used but isn’t - along with the extensive rule description explaining the details.
Unforgettable Security
Some issues are more serious than others - especially when they relate to the security of your running application. Sometimes a well-intentioned attempt to secure your code backfires due to subtleties in what the compiler is allowed to do.
For example, when dealing with sensitive data, such as passwords, we may feel that removing them from memory as soon as possible is a good idea - and it is! But if we use memset to do this (and don’t otherwise access that memory before it is released) the compiler is allowed to (and very often will) assume that it has no effect and remove it!
For this reason, memset_s was introduced (in annex K of the C standard, so you may have to opt in for it) to perform this role as expected. But knowing about all of this, or spotting it in existing code, is hard!
So S5798 detects this usage and gives us all of that background information in the rule description (including how to enable memset_s). Very useful!
And while we’re talking about security, we even have something to say about the strength of encryption algorithms we might use.
We detect many outdated or otherwise weak algorithms being used from several commonly used security and cryptography libraries, including OpenSSL, crypto++ and Botan.
For example, use of algorithms with less than 128 bit block sizes will trigger vulnerability warnings. The description for rule S5547 recommends what to use instead.
When it comes to security features, especially things like algorithm and block size, these recommendations change over time - so it’s very useful to be able to run these over existing code as well as new code.
A naturally great pairing
They say the best camera is the one you have with you. Perhaps that is true of static analysis tools, too. A substantial fraction of developers never look beyond what comes built into their IDE. For them, CLion’s inspections, and the bundled clang-tidy checks, are a great step forward. The way CLion surfaces this information right alongside your code, usually as you write it, makes it more accessible and useful than it’s ever been. So enhancing that with the rich ruleset and insightful rule descriptions of SonarLint is the natural and logical next step.
CLion naturally enhances SonarLint, too, in many ways - such as seamlessly running analysis even when developing with remote toolchains. And SonarLint takes things even further when running in connected mode with SonarQube or SonarCloud - enabling quality gates, clean-as-you-code, pull-request gates and much more - all with a browser-based dashboard for all your reporting.
So why not try it for yourself and take the quality of your code to the next level, today?