Configure clang-tidy CI check (#1028)

This commit is contained in:
Siarhei Fedartsou
2023-04-20 09:57:25 +02:00
committed by GitHub
parent f03deb7032
commit 2a7a6f01a2
27 changed files with 200 additions and 31 deletions

116
.clang-tidy Normal file
View File

@@ -0,0 +1,116 @@
---
Checks: >
android-*,
boost-*,
bugprone-*,
clang-analyzer-core*,
clang-analyzer-cplusplus*,
clang-analyzer-deadcode*,
clang-analyzer-optin.cplusplus*,
clang-analyzer-optin.performance.Padding,
clang-analyzer-security*,
clang-diagnostic-*,
cppcoreguidelines-avoid-goto,
cppcoreguidelines-no-malloc,
google-*,
llvm-*,
misc-*,
modernize-*,
performance-*,
portability-*,
readability-*,
-bugprone-branch-clone,
-bugprone-easily-swappable-parameters,
-bugprone-exception-escape,
-bugprone-forward-declaration-namespace,
-bugprone-forwarding-reference-overload,
-bugprone-implicit-widening-of-multiplication-result,
-bugprone-macro-parentheses,
-bugprone-narrowing-conversions,
-bugprone-reserved-identifier,
-bugprone-signed-char-misuse,
-bugprone-sizeof-expression,
-bugprone-unchecked-optional-access,
-bugprone-unhandled-exception-at-new,
-bugprone-unhandled-self-assignment,
-clang-analyzer-optin.cplusplus.UninitializedObject,
-clang-analyzer-core.NonNullParamChecker,
-clang-analyzer-core.NullDereference,
-clang-analyzer-core.uninitialized.Assign,
-clang-analyzer-core.UndefinedBinaryOperatorResult,
-clang-analyzer-security.FloatLoopCounter,
-google-build-explicit-make-pair,
-google-build-namespaces,
-google-build-using-namespace,
-google-default-arguments,
-google-explicit-constructor,
-google-objc-global-variable-declaration,
-google-readability-braces-around-statements,
-google-readability-casting,
-google-readability-function-size,
-google-readability-function,
-google-readability-namespace-comments,
-google-readability-todo,
-google-runtime-int,
-google-runtime-references,
-llvm-else-after-return,
-llvm-header-guard,
-llvm-include-order,
-llvm-namespace-comment,
-llvm-qualified-auto,
-llvm-twine-local,
-misc-confusable-identifiers,
-misc-const-correctness,
-misc-no-recursion,
-misc-non-private-member-variables-in-classes,
-misc-static-assert,
-modernize-avoid-bind,
-modernize-avoid-c-arrays,
-modernize-concat-nested-namespaces,
-modernize-macro-to-enum,
-modernize-pass-by-value,
-modernize-return-braced-init-list,
-modernize-use-auto,
-modernize-use-default-member-init,
-modernize-use-emplace,
-modernize-use-equals-default,
-modernize-use-nodiscard,
-modernize-use-nullptr,
-modernize-use-trailing-return-type,
-performance-move-const-arg,
-performance-noexcept-move-constructor,
-performance-no-int-to-ptr,
-performance-unnecessary-value-param,
-readability-avoid-const-params-in-decls,
-readability-braces-around-statements,
-readability-const-return-type,
-readability-container-size-empty,
-readability-convert-member-functions-to-static,
-readability-else-after-return,
-readability-function-cognitive-complexity,
-readability-function-size,
-readability-identifier-length,
-readability-identifier-naming,
-readability-implicit-bool-conversion,
-readability-inconsistent-declaration-parameter-name,
-readability-isolate-declaration,
-readability-magic-numbers,
-readability-make-member-function-const,
-readability-named-parameter,
-readability-non-const-parameter,
-readability-qualified-auto,
-readability-redundant-access-specifiers,
-readability-redundant-declaration,
-readability-redundant-member-init,
-readability-redundant-string-init,
-readability-simplify-boolean-expr,
-readability-static-accessed-through-instance
-readability-static-definition-in-anonymous-namespace,
-readability-suspicious-call-argument,
-readability-uppercase-literal-suffix,
-readability-use-anyofallof
WarningsAsErrors: '*'
HeaderFilterRegex: '.*'
CheckOptions:
- key: performance-unnecessary-value-param.AllowedTypes
value: 'exception_ptr'

View File

@@ -53,5 +53,5 @@ jobs:
- name: Build Maplibre GL Core
run: |
cmake --version
cmake -B build -DCMAKE_BUILD_TYPE=Debug -DMBGL_WITH_CORE_ONLY=ON -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DMBGL_WITH_COVERAGE=ON
cmake -B build -GNinja -DCMAKE_BUILD_TYPE=Debug -DMBGL_WITH_CLANG_TIDY=ON -DMBGL_WITH_CORE_ONLY=ON -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DMBGL_WITH_COVERAGE=ON
cmake --build build

1
.gitignore vendored
View File

@@ -20,6 +20,7 @@
*.code-workspace
.DS_Store
.cache
/build
**/.idea
/platform/android/MapboxGLAndroidSDKTestApp/local.properties

View File

@@ -1,6 +1,7 @@
cmake_minimum_required(VERSION 3.10 FATAL_ERROR)
option(MBGL_WITH_CORE_ONLY "Build only the core bits, no platform code" OFF)
option(MBGL_WITH_CLANG_TIDY "Build with clang-tidy checks enabled" OFF)
option(MBGL_WITH_COVERAGE "Enable code coverage collection" OFF)
option(MBGL_WITH_QT "Build Mapbox GL Qt bindings" OFF)
option(MBGL_WITH_SANITIZER "Use [address|thread|undefined] here" OFF)
@@ -10,6 +11,19 @@ option(MBGL_WITH_EGL "Build with EGL renderer" OFF)
option(MBGL_WITH_OSMESA "Build with OSMesa (Software) renderer" OFF)
option(MBGL_WITH_WERROR "Make all compilation warnings errors" ON)
if (MBGL_WITH_CLANG_TIDY)
find_program(CLANG_TIDY_COMMAND NAMES clang-tidy)
if(NOT CLANG_TIDY_COMMAND)
message(FATAL_ERROR "ENABLE_CLANG_TIDY is ON but clang-tidy is not found!")
else()
message(STATUS "Found clang-tidy at ${CLANG_TIDY_COMMAND}")
endif()
# TODO: there are options which are only available on GCC(e.g. -Werror=maybe-uninitialized),
# that's why we need to disable this `unknown-warning-option` here.
# We could check if current compiler supports particular flag before enabling it.
set(CLANG_TIDY_COMMAND "${CLANG_TIDY_COMMAND};--extra-arg=-Wno-error=unknown-warning-option")
endif()
if (MBGL_WITH_QT AND NOT CMAKE_OSX_DEPLOYMENT_TARGET)
set(CMAKE_OSX_DEPLOYMENT_TARGET 13.0)
endif()
@@ -967,6 +981,11 @@ target_sources(
${INCLUDE_FILES}
${SRC_FILES}
)
if (MBGL_WITH_CLANG_TIDY)
set_target_properties(mbgl-core PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}")
endif()
source_group(TREE ${PROJECT_SOURCE_DIR}/include FILES ${INCLUDE_FILES})
source_group(TREE ${PROJECT_SOURCE_DIR}/src FILES ${SRC_FILES})

View File

@@ -75,6 +75,10 @@ or any of the platform make files:
* Android developers can use [Android Studio](https://developer.android.com/studio/debug) or [`ndk-gdb`](https://developer.android.com/ndk/guides/ndk-gdb)
* iOS developers can use [Xcode](https://developer.apple.com/support/debugging/). See also [Advanced Debugging with Xcode and LLDB](https://developer.apple.com/videos/play/wwdc2018/412/).
## Static Analysis
We use [`clang-tidy`](https://clang.llvm.org/extra/clang-tidy/) for static analysis and run it on CI for each pull request. If you want to run it locally use `-DMBGL_WITH_CLANG_TIDY=ON` CMake option and just run regular build. For the list of enabled checks please see:
[`.clang-tidy`](.clang-tidy) and [`test/.clang-tidy`](test/.clang-tidy)(for tests we are less strict and use different set of checks).
## Logging in C++
* Android developers can review NDK logging at [developer.android.com/ndk/reference/group/logging](https://developer.android.com/ndk/reference/group/logging).

View File

@@ -67,7 +67,7 @@ public:
CGLPixelFormatObj pixelFormat = nullptr;
};
class CGLBackendImpl : public HeadlessBackend::Impl {
class CGLBackendImpl final : public HeadlessBackend::Impl {
public:
CGLBackendImpl() {
CGLError error = CGLCreateContext(cglDisplay->pixelFormat, nullptr, &glContext);

View File

@@ -37,6 +37,7 @@ CGImageRef CGImageCreateWithMLNPremultipliedImage(mbgl::PremultipliedImage&& src
return CGImageCreate(src.size.width, src.size.height, bitsPerComponent, bitsPerPixel,
bytesPerRow, *colorSpace,
// NOLINTNEXTLINE(bugprone-suspicious-enum-usage)
kCGBitmapByteOrderDefault | kCGImageAlphaPremultipliedLast, *provider,
NULL, false, kCGRenderingIntentDefault);
}
@@ -58,6 +59,7 @@ mbgl::PremultipliedImage MLNPremultipliedImageFromCGImage(CGImageRef src) {
CGContextHandle context(CGBitmapContextCreate(
image.data.get(), width, height, bitsPerComponent, bytesPerRow, *colorSpace,
// NOLINTNEXTLINE(bugprone-suspicious-enum-usage)
kCGBitmapByteOrderDefault | kCGImageAlphaPremultipliedLast));
if (!context) {
throw std::runtime_error("CGBitmapContextCreate failed");

View File

@@ -242,6 +242,7 @@ PremultipliedImage drawGlyphBitmap(GlyphID glyphID, CTFontRef font, GlyphMetrics
bitsPerComponent,
bytesPerRow,
*colorSpace,
// NOLINTNEXTLINE(bugprone-suspicious-enum-usage)
kCGBitmapByteOrderDefault | kCGImageAlphaPremultipliedLast));
if (!context) {
throw std::runtime_error("CGBitmapContextCreate failed");

View File

@@ -11,7 +11,7 @@ std::string uppercase(const std::string &string) {
encoding:NSUTF8StringEncoding
freeWhenDone:NO];
NSString *uppercase = [original uppercaseString];
const std::string result{[uppercase cStringUsingEncoding : NSUTF8StringEncoding],
std::string result{[uppercase cStringUsingEncoding : NSUTF8StringEncoding],
[uppercase lengthOfBytesUsingEncoding:NSUTF8StringEncoding]};
return result;
}
@@ -22,8 +22,8 @@ std::string lowercase(const std::string &string) {
encoding:NSUTF8StringEncoding
freeWhenDone:NO];
NSString *lowercase = [original lowercaseString];
const std::string result{[lowercase cStringUsingEncoding : NSUTF8StringEncoding],
[lowercase lengthOfBytesUsingEncoding:NSUTF8StringEncoding]};
std::string result{[lowercase cStringUsingEncoding : NSUTF8StringEncoding],
[lowercase lengthOfBytesUsingEncoding:NSUTF8StringEncoding]};
return result;
}

View File

@@ -90,7 +90,7 @@ public:
auto &allocator = doc.GetAllocator();
std::map<std::string, std::string> values;
auto db = mapbox::sqlite::Database::open(path.c_str(), mapbox::sqlite::ReadOnly);
auto db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly);
mapbox::sqlite::Statement meta(db, "SELECT * from metadata");
for (mapbox::sqlite::Query q(meta); q.run();) {
@@ -268,7 +268,7 @@ private:
return ptr->second;
};
auto ptr2 = db_cache.insert(std::pair<std::string, mapbox::sqlite::Database>(path, mapbox::sqlite::Database::open(path.c_str(), mapbox::sqlite::ReadOnly)));
auto ptr2 = db_cache.insert(std::pair<std::string, mapbox::sqlite::Database>(path, mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly)));
return ptr2.first->second;
}

View File

@@ -42,7 +42,7 @@ std::u16string applyArabicShaping(const std::u16string& input) {
std::u16string outputText(outputLength, 0);
u_shapeArabic(mbgl::utf16char_cast<const UChar*>(input.c_str()), static_cast<int32_t>(input.size()), mbgl::utf16char_cast<UChar*>(&outputText[0]), outputLength,
u_shapeArabic(mbgl::utf16char_cast<const UChar*>(input.c_str()), static_cast<int32_t>(input.size()), mbgl::utf16char_cast<UChar*>(outputText.data()), outputLength,
(U_SHAPE_LETTERS_SHAPE & U_SHAPE_LETTERS_MASK) |
(U_SHAPE_TEXT_DIRECTION_LOGICAL & U_SHAPE_TEXT_DIRECTION_MASK),
&errorCode);
@@ -191,7 +191,7 @@ std::u16string BiDi::writeReverse(const std::u16string& input, std::size_t logic
int32_t outputLength =
ubidi_writeReverse(mbgl::utf16char_cast<const UChar*>(&input[logicalStart]),
logicalLength,
mbgl::utf16char_cast<UChar*>(&outputText[0]),
mbgl::utf16char_cast<UChar*>(outputText.data()),
logicalLength + 1, // Extra room for null terminator, although we don't really need to have ICU write it for us
UBIDI_DO_MIRRORING | UBIDI_REMOVE_BIDI_CONTROLS,
&errorCode);
@@ -223,7 +223,7 @@ std::u16string BiDi::getLine(std::size_t start, std::size_t end) {
// UBIDI_REMOVE_BIDI_CONTROLS: Now that all the lines are set, remove control characters so that
// they don't show up on screen (some fonts have glyphs representing them)
int32_t finalLength = ubidi_writeReordered(impl->bidiLine,
mbgl::utf16char_cast<UChar*>(&outputText[0]),
mbgl::utf16char_cast<UChar*>(outputText.data()),
outputLength,
UBIDI_DO_MIRRORING | UBIDI_REMOVE_BIDI_CONTROLS,
&errorCode);

View File

@@ -1,4 +0,0 @@
Checks: '-*,bugprone-*,clang-analyzer-*,google-*,modernize-*,performance-*,-clang-analyzer-osx.*,-google-readability-braces-around-statements,-google-readability-todo,-google-runtime-int,-google-runtime-references'
WarningsAsErrors: 'bugprone-use-after-move,performance-move-const-arg'
HeaderFilterRegex: '.*'
FormatStyle: file

View File

@@ -17,7 +17,7 @@ std::optional<std::string> LayerFactory::getSource(const style::conversion::Conv
return std::nullopt;
}
auto const source = toString(*sourceValue);
auto source = toString(*sourceValue);
if (!source) {
return std::nullopt;
}

View File

@@ -32,7 +32,7 @@ public:
std::optional<TaggedString> formattedText;
std::optional<style::expression::Image> icon;
float sortKey = 0.0f;
std::size_t index;
std::size_t index = 0;
bool allowsVerticalWritingMode = false;
};

View File

@@ -20,7 +20,7 @@ class ResourceTransform;
namespace gfx {
class ShaderRegistry;
}
} // namespace gfx
struct StillImageRequest {
StillImageRequest(Map::StillImageCallback&& callback_)

View File

@@ -28,12 +28,10 @@
#include <mbgl/util/mat4.hpp>
#include <mapbox/eternal.hpp>
#include <mbgl/gl/context.hpp>
#include <mbgl/gl/defines.hpp>
#include <mbgl/gl/texture.hpp>
#include <mbgl/gl/texture_resource.hpp>
#include <mbgl/gl/types.hpp>
#include <mbgl/platform/gl_functions.hpp>
#include <mbgl/renderer/image_manager.hpp>
using namespace mbgl::platform;

View File

@@ -17,7 +17,6 @@
#include <mbgl/style/layers/symbol_layer_impl.hpp>
#include <mbgl/gfx/cull_face_mode.hpp>
#include <mbgl/layout/symbol_layout.hpp>
#include <mbgl/layout/symbol_layout.hpp>
#include <mbgl/util/math.hpp>
#include <cmath>

View File

@@ -403,7 +403,7 @@ public:
const float possiblyRoundedZoom = expression.useIntegerZoom ? std::floor(currentZoom) : currentZoom;
return std::tuple<float>{
::fmax(0.0f, ::fmin(1.0f, expression.interpolationFactor(zoomRange, possiblyRoundedZoom)))};
std::fmax(0.0f, std::fmin(1.0f, expression.interpolationFactor(zoomRange, possiblyRoundedZoom)))};
}
std::tuple<T> uniformValue(const PossiblyEvaluatedPropertyValue<T>& currentValue) const override {

View File

@@ -303,6 +303,7 @@ ParseResult parseMatch(const Convertible& value, ParsingContext& ctx) {
assert(inputType && outputType);
std::optional<std::string> err;
// NOLINTNEXTLINE(bugprone-assignment-in-if-condition)
if ((*input)->getType() != type::Value && (err = type::checkSubtype(*inputType, (*input)->getType()))) {
ctx.error(*err, 1);
return ParseResult();

View File

@@ -5,7 +5,6 @@
#include <mbgl/renderer/transition_parameters.hpp>
#include <mbgl/renderer/possibly_evaluated_property_value.hpp>
#include <mbgl/renderer/property_evaluation_parameters.hpp>
#include <mbgl/renderer/transition_parameters.hpp>
#include <mbgl/util/indexed_tuple.hpp>
#include <mbgl/util/ignore.hpp>

View File

@@ -19,7 +19,7 @@ OpacityState::OpacityState(bool placed_, bool skipFade)
}
OpacityState::OpacityState(const OpacityState& prevState, float increment, bool placed_) :
opacity(::fmax(0.0f, ::fmin(1.0f, prevState.opacity + (prevState.placed ? increment : -increment)))),
opacity(std::fmax(0.0f, std::fmin(1.0f, prevState.opacity + (prevState.placed ? increment : -increment)))),
placed(placed_) {}
bool OpacityState::isHidden() const {

View File

@@ -16,7 +16,7 @@ double vec4Dot(const vec4& a, const vec4& b) {
}
template <size_t N>
static Point<double> ProjectPointsToAxis(const std::array<vec3, N>& points, const vec3& origin, const vec3& axis) {
Point<double> ProjectPointsToAxis(const std::array<vec3, N>& points, const vec3& origin, const vec3& axis) {
double min = std::numeric_limits<double>::max();
double max = -std::numeric_limits<double>::max();

View File

@@ -64,7 +64,7 @@ std::map<std::string, std::string> createTokenMap(const std::string& urlTemplate
i != std::sregex_iterator();
++i)
{
std::smatch m = *i;
const std::smatch& m = *i;
if (m.size() > 0) {
auto captureGroupValue = m[1].str();
if (!captureGroupValue.empty()){
@@ -86,7 +86,7 @@ std::map<std::string, std::string> createTokenMap(const std::string& urlTemplate
i != std::sregex_iterator();
++i)
{
std::smatch m = *i;
const std::smatch& m = *i;
if (m.size() > 0) {
auto captureGroupValue = m[1].str();
if (!captureGroupValue.empty()){
@@ -152,7 +152,7 @@ bool isNormalizedURL(const TileServerOptions& tileServerOptions, const std::stri
}
std::string makeQueryString(const TileServerOptions& tileServerOptions, const std::string& apiKey) {
const std::string queryString = (!tileServerOptions.requiresApiKey() || tileServerOptions.apiKeyParameterName().empty() || apiKey.empty()) ?
std::string queryString = (!tileServerOptions.requiresApiKey() || tileServerOptions.apiKeyParameterName().empty() || apiKey.empty()) ?
"" : "?" + tileServerOptions.apiKeyParameterName() + "=" + apiKey;
return queryString;
}

View File

@@ -1,2 +1,31 @@
Checks: 'modernize-*,-modernize-use-equals-delete,-modernize-use-equals-default,misc-static-assert,llvm-namespace-comment,-clang-analyzer-security.insecureAPI.rand,-clang-analyzer-core.uninitialized.UndefReturn,-clang-analyzer-core.StackAddressEscape,-clang-analyzer-core.CallAndMessage,-clang-diagnostic-unused-command-line-argument,-clang-analyzer-core.uninitialized.*,-clang-analyzer-core.NullDereference,-clang-analyzer-core.NonNullParamChecker'
HeaderFilterRegex: '\/mbgl\/'
---
Checks: >
modernize-*,
-modernize-avoid-bind,
-modernize-avoid-c-arrays,
-modernize-concat-nested-namespaces,
-modernize-macro-to-enum,
-modernize-pass-by-value,
-modernize-return-braced-init-list,
-modernize-use-auto,
-modernize-use-default-member-init,
-modernize-use-emplace,
-modernize-use-equals-default,
-modernize-use-equals-delete,
-modernize-use-nodiscard,
-modernize-use-nullptr,
-modernize-use-override,
-modernize-use-trailing-return-type,
misc-static-assert,
-clang-analyzer-security.insecureAPI.rand,
-clang-analyzer-core.uninitialized.UndefReturn,
-clang-analyzer-core.StackAddressEscape,
-clang-analyzer-core.CallAndMessage,
-clang-analyzer-deadcode.DeadStores,
-clang-diagnostic-unused-command-line-argument,
-clang-analyzer-core.uninitialized.*,
-clang-analyzer-core.NullDereference,
-clang-analyzer-core.NonNullParamChecker,
-clang-analyzer-optin.cplusplus.UninitializedObject
WarningsAsErrors: '*'
HeaderFilterRegex: '.*'

View File

@@ -237,3 +237,7 @@ if(WIN32)
endif()
set_property(TARGET mbgl-test PROPERTY FOLDER Core)
if (MBGL_WITH_CLANG_TIDY)
set_target_properties(mbgl-test PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}")
endif()

View File

@@ -53,7 +53,7 @@
namespace httplib {
class Server;
}
} // namespace httplib
namespace mbgl {
namespace test {

View File

@@ -1,7 +1,7 @@
#include <mbgl/util/bounding_volumes.hpp>
#include <gtest/gtest.h>
#include <math.h>
#include <cmath>
#include <algorithm>
using namespace mbgl;