Skip to content

Commit

Permalink
BRAYNS-649 Add clang tidy (#1284)
Browse files Browse the repository at this point in the history
  • Loading branch information
Adrien4193 authored Sep 9, 2024
1 parent 4bc4de7 commit 91b17c9
Show file tree
Hide file tree
Showing 20 changed files with 188 additions and 49 deletions.
47 changes: 40 additions & 7 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ AlignEscapedNewlines: DontAlign
AlignOperands: DontAlign
AlignTrailingComments: false
AllowAllArgumentsOnNextLine: false
AllowAllConstructorInitializersOnNextLine: false # Deprecated
AllowAllParametersOfDeclarationOnNextLine: false
AllowShortBlocksOnASingleLine: Never
AllowShortCaseLabelsOnASingleLine: false
AllowShortCompoundRequirementOnASingleLine: true
AllowShortEnumsOnASingleLine: false
AllowShortFunctionsOnASingleLine: None
AllowShortIfStatementsOnASingleLine: Never
Expand All @@ -26,13 +26,35 @@ AlwaysBreakTemplateDeclarations: Yes
BinPackArguments: false
BinPackParameters: false
BitFieldColonSpacing: Both
BraceWrapping:
AfterCaseLabel: true
AfterClass: true
AfterControlStatement: Always
AfterEnum: true
AfterFunction: true
AfterNamespace: true
AfterStruct: true
AfterUnion: true
AfterExternBlock: true
BeforeCatch: true
BeforeElse: true
BeforeLambdaBody: true
BeforeWhile: true
IndentBraces: false
SplitEmptyRecord: true
SplitEmptyNamespace: true
BreakAdjacentStringLiterals: true
BreakAfterAttributes: Never
BreakAfterReturnType: Automatic
BreakBeforeBinaryOperators: NonAssignment
BreakBeforeBraces: Allman
BreakBeforeConceptDeclarations: true
BreakBeforeBraces: Custom
BreakBeforeConceptDeclarations: Always
BreakBeforeTernaryOperators: true
BreakConstructorInitializers: AfterColon
BreakFunctionDefinitionParameters: false
BreakInheritanceList: AfterColon
BreakStringLiterals: true
BreakStringLiterals: false
BreakTemplateDeclarations: Yes
ColumnLimit: 120
CompactNamespaces: false
ConstructorInitializerIndentWidth: 4
Expand All @@ -50,12 +72,21 @@ IndentCaseLabels: false
IndentExternBlock: true
IndentGotoLabels: false
IndentPPDirectives: BeforeHash
IndentRequires: true
IndentRequiresClause: true
IndentWidth: 4
IndentWrappedFunctionNames: true
KeepEmptyLinesAtTheStartOfBlocks: false
InsertBraces: true
IntegerLiteralSeparator:
Binary: -1
Decimal: 3
Hex: -1
KeepEmptyLines:
AtEndOfFile: false
AtStartOfBlock: false
AtStartOfFile: false
LambdaBodyIndentation: Signature
Language: Cpp
MainIncludeChar: Quote
MaxEmptyLinesToKeep: 1
NamespaceIndentation: None
PackConstructorInitializers: Never
Expand All @@ -73,7 +104,9 @@ PointerAlignment: Right
QualifierAlignment: Custom
QualifierOrder: ["static", "inline", "const", "volatile", "type"]
ReferenceAlignment: Right
ReflowComments: true
ReflowComments: false
RequiresClausePosition: WithPreceding
RequiresExpressionIndentation: OuterScope
SeparateDefinitionBlocks: Always
ShortNamespaceLines: 1
SortIncludes: true
Expand Down
57 changes: 57 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
Checks: -*
bugprone-*
cert-*
concurrency-*
cppcoreguidelines-*
misc-*
modernize-*
performance-*
portability-*
readability-*
-cppcoreguidelines-avoid-magic-numbers
-cppcoreguidelines-missing-std-forward
-cppcoreguidelines-pro-*
-misc-include-cleaner
-modernize-use-nodiscard
-modernize-use-trailing-return-type
-readability-identifier-length
-readability-magic-numbers

WarningsAsErrors: "*"
HeaderFilterRegex: "src/.*"

CheckOptions:
- key: readability-identifier-naming.ClassCase
value: CamelCase
- key: readability-identifier-naming.ClassMemberCase
value: CamelCase
- key: readability-identifier-naming.ConstexprVariableCase
value: CamelCase
- key: readability-identifier-naming.EnumCase
value: CamelCase
- key: readability-identifier-naming.EnumConstantCase
value: CamelCase
- key: readability-identifier-naming.FunctionCase
value: camelBack
- key: readability-identifier-naming.FunctionIgnoredRegexp
value: main|wmain|wWinMain|begin|end
- key: readability-identifier-naming.GlobalConstantCase
value: camelBack
- key: readability-identifier-naming.StaticConstantCase
value: camelBack
- key: readability-identifier-naming.StaticVariableCase
value: camelBack
- key: readability-identifier-naming.MacroDefinitionCase
value: UPPER_CASE
- key: readability-identifier-naming.MemberCase
value: camelBack
- key: readability-identifier-naming.PrivateMemberPrefix
value: _
- key: readability-identifier-naming.NamespaceCase
value: camelBack
- key: readability-identifier-naming.ParameterCase
value: camelBack
- key: readability-identifier-naming.TypeAliasCase
value: CamelCase
- key: readability-identifier-naming.VariableCase
value: camelBack
11 changes: 9 additions & 2 deletions .github/workflows/linter.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,15 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v4
- name: Update APT package list
run: sudo apt-get update
- name: Download and install LLVM
run: |
wget https://apt.llvm.org/llvm.sh
sudo bash llvm.sh 20
- name: Install clang-format
run: sudo apt-get -y install clang-format-20
- name: Run clang-format
run: |
apt update && apt install -y clang-format-15
SOURCES=$(find apps src tests \( -name "*.h" -or -name "*.cpp" \))
clang-format-15 --dry-run --Werror $SOURCES
clang-format-20 --dry-run --Werror $SOURCES
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,26 @@ The following cmake options (shown with their default value) can be used during
* **BRAYNS_ENABLE_CIRCUITS** (Default ON) - Activate circuit support.
* **BRAYNS_ENABLE_ATLAS** - (Default ON) Activate atlas support.

### Run linters

Run clang format as follows:

$ SOURCES=$(find apps src tests \( -name "*.h" -or -name "*.cpp" \))
$ clang-format-20 --Werror $SOURCES

Run clang tidy as follows:

Use cmake config to generate build commands:

"cacheVariables": {
"CMAKE_EXPORT_COMPILE_COMMANDS": true,
"CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES": "/usr/include/c++/11;/usr/include/x86_64-linux-gnu/c++/11"
}

Run with

$ run-clang-tidy-20 -p build/user-debug


## Running

Expand Down
12 changes: 12 additions & 0 deletions scripts/install_llvm.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright (c) 2015-2024, EPFL/Blue Brain Project
# All rights reserved. Do not distribute without permission.
# Responsible Author: Adrien Fleury <adrien.fleury@epfl.ch>
#
# This file is part of Brayns <https://github.com/BlueBrain/Brayns>

wget https://apt.llvm.org/llvm.sh
chmod +x llvm.sh
./llvm.sh 20

apt-get install clang-format-20
apt-get install clang-tidy-20
2 changes: 1 addition & 1 deletion src/brayns/core/Launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ struct ArgvSettingsReflector<ServiceSettings>
.defaultValue("localhost");
builder.option("port", [](auto &settings) { return &settings.port; })
.description("Websocket server port")
.defaultValue(5000);
.defaultValue(5'000);
builder.option("max-thread-count", [](auto &settings) { return &settings.maxThreadCount; })
.description("Maximum number of threads for the websocket server")
.defaultValue(10);
Expand Down
10 changes: 6 additions & 4 deletions src/brayns/core/cli/CommandLine.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,11 @@ template<typename T>
struct ArgvReflector;

template<typename T>
concept ReflectedArgvOption = std::same_as<std::string, decltype(ArgvReflector<T>::getType())>
&& std::same_as<std::string, decltype(ArgvReflector<T>::toString(T()))>
&& std::same_as<T, decltype(ArgvReflector<T>::parse(std::string_view()))>;
concept ReflectedArgvOption = requires(T value) {
{ ArgvReflector<T>::getType() } -> std::same_as<std::string>;
{ ArgvReflector<T>::toString(value) } -> std::same_as<std::string>;
{ ArgvReflector<T>::parse(std::string_view()) } -> std::same_as<T>;
};

template<>
struct ArgvReflector<bool>
Expand All @@ -160,7 +162,7 @@ struct ArgvReflector<bool>
};

template<typename T>
requires std::is_arithmetic_v<T>
requires std::is_arithmetic_v<T>
struct ArgvReflector<T>
{
static std::string getType()
Expand Down
2 changes: 1 addition & 1 deletion src/brayns/core/codecs/PngCodec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ std::string encodePng(const ImageView &image)
.message = {},
};

auto rowStride = PNG_IMAGE_ROW_STRIDE(png);
auto rowStride = png_int_32(PNG_IMAGE_ROW_STRIDE(png));

if (rowOrder == RowOrder::BottomUp)
{
Expand Down
6 changes: 3 additions & 3 deletions src/brayns/core/engine/Data.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ Data<T> allocateData(Device &device, std::size_t itemCount)
}

template<OsprayDataType T, std::ranges::range U>
requires(std::convertible_to<std::ranges::range_value_t<U>, T>)
requires std::convertible_to<std::ranges::range_value_t<U>, T>
Data<T> createData(Device &device, U &&items)
{
auto data = allocateData<T>(device, items.size());
Expand Down Expand Up @@ -249,15 +249,15 @@ Data3D<OutputType> createDataView3D(const Data3D<T> &data, const DataRegion3D &r
template<OsprayDataType OutputType, OsprayDataType T>
Data2D<OutputType> createDataView2D(const Data2D<T> &data, const DataRegion2D &region)
{
auto region3D = DataRegion3D{Size3(region.itemCount, 1), Size3(region.stride, 0), region.offset};
auto region3D = DataRegion3D{Size3(region.itemCount, 1), Stride3(region.stride, 0), region.offset};
auto view = createDataView3D<OutputType>(data, region3D);
return Data2D<OutputType>(std::move(view));
}

template<OsprayDataType OutputType, OsprayDataType T>
Data<OutputType> createDataView(const Data<T> &data, const DataRegion &region)
{
auto region3D = DataRegion3D{Size3(region.itemCount, 1, 1), Size3(region.stride, 0, 0), region.offset};
auto region3D = DataRegion3D{Size3(region.itemCount, 1, 1), Stride3(region.stride, 0, 0), region.offset};
auto view = createDataView3D<OutputType>(data, region3D);
return Data<OutputType>(std::move(view));
}
Expand Down
2 changes: 1 addition & 1 deletion src/brayns/core/json/JsonSchema.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,6 @@ struct JsonSchema
std::optional<std::size_t> maxItems = {};
std::map<std::string, JsonSchema> properties = {};

auto operator<=>(const JsonSchema &) const = default;
bool operator==(const JsonSchema &) const = default;
};
}
16 changes: 8 additions & 8 deletions src/brayns/core/jsonrpc/Errors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,42 +58,42 @@ const JsonValue &JsonRpcException::getData() const
}

ParseError::ParseError(const std::string &message):
JsonRpcException(-32700, message)
JsonRpcException(-32'700, message)
{
}

InvalidRequest::InvalidRequest(const std::string &message):
JsonRpcException(-32600, message)
JsonRpcException(-32'600, message)
{
}

InvalidRequest::InvalidRequest(const std::string &message, const std::vector<JsonSchemaError> &errors):
JsonRpcException(-32600, message, serializeToJson(errors))
JsonRpcException(-32'600, message, serializeToJson(errors))
{
}

MethodNotFound::MethodNotFound(const std::string &method):
JsonRpcException(-32601, fmt::format("Method not found: '{}'", method))
JsonRpcException(-32'601, fmt::format("Method not found: '{}'", method))
{
}

InvalidParams::InvalidParams(const std::string &message):
JsonRpcException(-32602, message)
JsonRpcException(-32'602, message)
{
}

InvalidParams::InvalidParams(const std::string &message, const std::vector<JsonSchemaError> &errors):
JsonRpcException(-32602, message, serializeToJson(errors))
JsonRpcException(-32'602, message, serializeToJson(errors))
{
}

InternalError::InternalError(const std::string &message):
JsonRpcException(-32603, message)
JsonRpcException(-32'603, message)
{
}

InternalError::InternalError(const std::exception &e):
JsonRpcException(-32603, e.what())
JsonRpcException(-32'603, e.what())
{
}
}
8 changes: 5 additions & 3 deletions src/brayns/core/jsonrpc/PayloadReflector.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ template<typename T>
struct PayloadReflector;

template<typename T>
concept ReflectedPayload = std::same_as<JsonSchema, decltype(PayloadReflector<T>::getSchema())>
&& std::same_as<Payload, decltype(PayloadReflector<T>::serialize(std::declval<T>()))>
&& std::same_as<T, decltype(PayloadReflector<T>::deserialize(Payload()))>;
concept ReflectedPayload = requires(T value) {
{ PayloadReflector<T>::getSchema() } -> std::same_as<JsonSchema>;
{ PayloadReflector<T>::serialize(std::move(value)) } -> std::same_as<Payload>;
{ PayloadReflector<T>::deserialize(Payload()) } -> std::same_as<T>;
};

template<>
struct PayloadReflector<Payload>
Expand Down
13 changes: 7 additions & 6 deletions src/brayns/core/objects/common/Binary.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,13 @@ Data3D<T> createData3DFromBinaryOf(Device &device, const Size3 &itemCount, std::

if (reduceMultiply(itemCount) != totalItemCount)
{
throw InvalidParams(fmt::format(
"Item count in binary {} does not match given item count {}x{}x{}",
totalItemCount,
itemCount.x,
itemCount.y,
itemCount.z));
throw InvalidParams(
fmt::format(
"Item count in binary {} does not match given item count {}x{}x{}",
totalItemCount,
itemCount.x,
itemCount.y,
itemCount.z));
}

auto data = allocateData3D<T>(device, itemCount);
Expand Down
Loading

0 comments on commit 91b17c9

Please sign in to comment.