This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/coding_conventions in repository https://gitbox.apache.org/repos/asf/celix.git
commit c3c5772e8acc2401c990df880e1fff3f4c369ad7 Author: Pepijn Noltes <[email protected]> AuthorDate: Sat Apr 15 20:10:55 2023 +0200 Add initial coding conventions markdown document --- documents/README.md | 1 + documents/development/README.md | 479 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 480 insertions(+) diff --git a/documents/README.md b/documents/README.md index 85e2806b..ea646f41 100644 --- a/documents/README.md +++ b/documents/README.md @@ -88,3 +88,4 @@ bundles contains binaries depending on the stdlibc++ library. * [Apache Celix Patterns](patterns.md) * [Apache Celix CMake Commands](cmake_commands) * [Apache Celix Sub Projects](subprojects.md) +* [Apache Celix Coding Conventions Guide](development/README.md) diff --git a/documents/development/README.md b/documents/development/README.md new file mode 100644 index 00000000..6b12a115 --- /dev/null +++ b/documents/development/README.md @@ -0,0 +1,479 @@ +--- +title: Apache Celix Coding Conventions +--- + +<!-- +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> + +# Apache Celix Coding Conventions +Adhering to consistent and meaningful coding conventions is crucial for maintaining readable and maintainable code. +This document outlines the recommended coding conventions for Apache Celix development, including naming conventions, +formatting, comments, control structures, functions and error handling. + +Note that not all existing code adheres to these conventions. +New code should adhere to these conventions and when possible, existing code should be updated to adhere to these +conventions. + +## Naming Conventions + +### C/C++ Variables + +- Use `camelCase` for variable names. +- Use descriptive names for variables. +- Use `celix_` prefix for global variables. +- Asterisk `*` should be placed on the variable type name. + +### C Structures + +- Use `snake_case` for structure names. +- Add a typedef for the structure. +- Use a `_t` postfix for structure typedef. +- Use `celix_` prefix for structure names. + +### C Functions + +- Use `camelCase` for function names. +- Use descriptive names for functions. +- Use `celix_` prefix. +- Use a `_<obj>_` camelCase infix for the object/module name. +- Use a postfix camelCase for the function name. +- Asterisk `*` should be placed on the variable type name. +- Use verb as function names when a function has a side effect. +- Use nouns or getter/setter as function names when a function does not have a side effect. +- Use getters/setters naming convention for functions which get/set a value. + +Examples: +- `long celix_bundleContext_installBundle(celix_bundle_context_t* ctx, const char* bundleUrl, bool autoStart)` +- `bool celix_utils_stringEquals(const char* a, const char* b)` +- `celix_status_t celix_utils_createDirectory(const char* path, bool failIfPresent, const char** errorOut)` + +### C Constants + +- Use `SNAKE_CASE` for constant names. +- Use a `CELIX_` prefix for constant names. +- Use `#define` for constants. + +### C Enums + +- Use `camelCase` for enum type names. +- Use a `celix_` prefix for enum type names. +- Use `SNAKE_CASE` for enum value names. +- Use a `CELIX_` prefix for enum value names. +- Add a typedef - with a `_e` postfix - for the enum + +Example: +```c +enum celix_bundleState { + CELIX_BUNDLE_STATE_UNKNOWN = 0x00000000, + CELIX_BUNDLE_STATE_UNINSTALLED = 0x00000001 +}; +typedef enum celix_bundleState celix_bundle_state_e; +``` + +### Macros + +- Use all caps `SNAKE_CASE` for macro names. +- Use a `CELIX_` prefix for macro names. + +### C files and directories + +- Use `snake_case` for file names. +- Name header files with a `.h` extension and source files with a `.c` extension. +- Organize files in directories according to their purpose. + - Public headers files in a `include`, `api` or `spi` directory. + - Private header files in a `private` and `src` directory. + - Source files in a `src` directory. +- Google test files should be placed in a `gtest` directory with its own `CMakeLists.txt` file and `src` directory. +- Use `celix_` prefix for header file names. +- Use a header guard. +- Use a C++ "extern C" block. + +### C Libraries + +- Target name should be `snake_case`. +- There should be `celix::` prefixed aliases for the library. +- C Shared libraries should configure an output name with a `celix_` prefix. + +### C Services + +- Service headers should be made available through a CMake INTERFACE header-only api/spi library (i.e. `celix::shell_api`) +- C service should be C struct, where the first member is the service handle (`void*`) and the rest of the members are + function pointers. +- The first argument of a service function should be the service handle (`void*`). +- If memory allocation is needed or another error can occur in a service function, ensure that the return value can + be used to check for errors. This can be done by: + - Returning a `celix_status_t` and if needed using an out parameter. + - Returning a NULL pointer if the function returns a pointer. + - Returning a boolean value, where `true` indicates success and `false` indicates failure. +- In the same header as the C service struct, there should be defines for the service name and version. +- The service macro name should be all caps `SNAKE_CASE` and should be prefixed with `CELIX_`. +- The service macro version should be all caps `SNAKE_CASE` and should be prefixed with `CELIX_`. +- The value of the service macro name should be the service struct (so without a `_t` postfix +- The value of the service macro version should be the version of the service. + +Example: +```c +//celix_foo.h + +#include "celix_errno.h" + +#define CELIX_FOO_SERVICE_NAME celix_foo_service +#define CELIX_FOO_SERVICE_VERSION 1.0.0 + +typedef struct celix_foo_service { + void* handle; + celix_status_t (*doFoo)(void* handle); +} celix_foo_service_t; +``` + +### C Bundles + +- Use `snake_case` for C bundle target names. +- Do _not_ use a `celix_` prefix for C bundle target names. +- Use `celix::` prefixed aliases for C bundle targets. +- Use `snake_case` for C bundle symbolic names. +- Configure at least SYMBOLIC_NAME, NAME, FILENAME, VERSION and GROUP for C bundle targets. +- Use `apache_celix_` prefix for C bundle symbolic names. +- Use `Apache Celix ` prefix for C bundle names. +- Use a `celix_` prefix for C bundle filenames. +- Use a group name starting with `celix/` for C bundle groups. + +Examples: +```cmake +add_celix_bundle(my_bundle + SOURCES src/my_bundle.c + SYMBOLIC_NAME "apache_celix_my_bundle" + NAME "Apache Celix My Bundle" + FILENAME "celix_my_bundle" + VERSION "1.0.0" + GROUP "celix/my_bundle_group" +) +add_library(celix::my_bundle ALIAS my_bundle) +``` + +### C++ Classes + +- Use `CamelCase` (starting with a capital) for class names. +- Use descriptive names for classes. +- Classes should be part of a `celix::` namespace or sub `celix::` namespace. + +### C++ Functions + +- Use `camelCase` for function names. +- If a function is part of a class, it should be part of a `celix::` namespace or sub `celix::` namespace. +- Asterisk `*` should be placed on the variable type name. +- Use verb as function names when a function has a side effect. +- Use nouns or getter/setter as function names when a function does not have a side effect. +- Use getters/setters naming convention for functions which get/set a value. + + +### C++ Constants + +- Use `SNAKE_CASE` for constants. +- Use constexpr for constants. +- Place constants in a `celix::` namespace or sub `celix::` namespace. + +example: +```c++ +namespace celix { + constexpr long FRAMEWORK_BUNDLE_ID = 0; + constexpr const char * const SERVICE_ID = "service.id"; +} +``` + +### C++ Enums + +- Use `CamelCase` (starting with a capital) for enum types names. +- For `enum class`, Use `SNAKE_CASE` for enum values without a celix/class prefix. Note that for values names + no prefix is required because enum class values are scoped. + +Example: +```c++ +namespace celix { + enum class ServiceRegistrationState { + REGISTERING, + REGISTERED, + UNREGISTERING, + UNREGISTERED + }; +} +``` + +### C++ files and directories + +- Use `CamelCase` (starting with a capital) for file names. +- Name header files with a `.h` extension and source files with a `.cc` extension. +- Place header files in a directory based on the namespace (e.g. `celix/Bundle.h`, `celix/dm/Component.h`). +- Organize files in directories according to their purpose. + - Public headers files in a `include`, `api` or `spi` directory. + - Private header files in a `private` and `src` directory. + - Source files in a `src` directory. +- For the Apache Celix framework and utils lib only header-only C++ files are allowed. +- Prefer header-only C++ files for the Apache Celix libraries +- Use a `#pragma once` header guard. + +### C++ Libraries + +- Target name should be `CamelCase` (starting with a capital). +- There should be `celix::` prefixed aliases for the library. +- C++ Libraries should support C++14. + - Exception are `celix::Promises` and `celix::PushStreams` which requires C++17. +- C++ Libraries should prefer to be header-only. +- C++ support for `celix::framework` and `celix::utils` must be header-only. +- Header-only C++ libraries do not need an export header and do not need to configure symbol visibility. +- C++ shared libraries (lib with C++ sources), should configure an output name with a `celix_` prefix. +- C++ shared libraries (lib with C++ sources), should use an export header and configure symbol visibility. + - See the C Libraries section for more information. + +### C++ Services + +- Use `CamelCase` (starting with a capital) for service names. +- Add a 'I' prefix to the service interface name. +- Place service classes in a `celix::` namespace or sub `celix::` namespace. +- Add a `static constexpr const char * const NAME` to the service class, for the service name. +- Add a `static constexpr const char * const SERVICE_VERSION` to the service class, for the service version. + +### C++ Bundles + +- Use `CamelCase` for C++ bundle target names. +- Do _not_ use a `Celix` prefix for C++ bundle target names. +- Use `celix::` prefixed aliases for C++ bundle targets. +- Use `CamelCase` for C++ bundle symbolic names. +- Configure at least SYMBOLIC_NAME, NAME, FILENAME, VERSION and GROUP for C++ bundle targets. +- Use `Apache_Celix_` prefix for C++ bundle symbolic names. +- Use `Apache Celix ` prefix for C++ bundle names. +- Use a `celix_` prefix for C++ bundle filenames. +- Use a group name starting with `celix/` for C++ bundle groups. + +Examples: +```cmake +add_celix_bundle(MyBundle + SOURCES src/MyBundle.cc + SYMBOLIC_NAME "Apache_Celix_MyBundle" + NAME "Apache Celix My Bundle" + FILENAME "celix_MyBundle" + VERSION "1.0.0" + GROUP "celix/MyBundleGroup" +) +add_library(celix::MyBundle ALIAS MyBundle) +``` + +### Unit Tests Naming + +- The test fixture should have a`TestSuite` postfix. +- The source file should be named after the test fixture name and use a `.cc` extension. +- Testcase names should use `CamelCase` (starting with a capital) and have a `Test` postfix. +- When using error injection (one of the `error_injector` libraries) a separate test suite should be used. + - A `ErrorInjectionTestSuite` postfix should be used for the test fixture. + - The error injection setup should be reset on the `TearDown` function or destructor of the test fixture. + +## Comments and Documentation + +- Use Doxygen for code documentation. +- Write comments that explain the purpose of the code, focusing on the "why" rather than the "how." +- Apply doxygen documentation to all public API's. +- Use the javadoc style for doxygen documentation. +- Use `@` instead of `\` for doxygen commands. +- Start with a `@brief` command and a short description. +- For `@param` commands also provide in, out, or in/out information. + +## Formatting and Indentation + +- Use spaces for indentation and use 4 spaces per indentation level. +- Keep line lengths under 80 characters, if possible, to enhance readability. +- Place opening braces on the same line as the control statement or function definition, + and closing braces on a new line aligned with the control statement or function definition. +- Use a single space before and after operators and around assignment statements. + + +## Control Structures + +- Always use braces ({ }) for control structures, even for single-statement blocks, to prevent errors. +- Use `if`, `else if`, and `else` statements to handle multiple conditions. +- Use `switch` statements for multiple conditions with a default case. +- Use `while` statements for loops that may not execute. +- Use `do`/`while` statements for loops that must execute at least once. +- Use `for` statements for loops with a known number of iterations. +- The use of `goto` is not allowed, except for error handling. +- For C, try to prevent deeply nested control structures and prefer early returns or `goto` statements. + - To prevent deeply nested control structures, the `CELIX_DO_IF` and `CELIX_GOTO_IF_ERR` macros can also be used. + +## Functions and Methods + +- Limit functions to a single responsibility or purpose, following the Single Responsibility Principle (SRP). +- Keep functions short and focused, aiming for a length of fewer than 50 lines. +- Ensure const correctness. + +## Error Handling and Logging + +- For C++, throw an exception when an error occurs. +- For C, if memory allocation is needed or another error can occur, ensure that a function returns a value + than can be used to check for errors. This can be done by: + - Returning a `celix_status_t` and if needed using an out parameter. + - Returning a NULL pointer if the function returns a pointer. + - Returning a boolean value, where `true` indicates success and `false` indicates failure. +- Use consistent error handling techniques, such as returning error codes or using designated error handling functions. +- Log errors, warnings, and other important events using the Apache Celix log helper functions or - for libraries - + the `celix_err` (TBD) functionality. +- Always check for errors and log them. +- Ensure error handling is correct, using test suite with error injection. + +## Error Injection + +- Use the Apache Celix error_injector libraries to inject errors in unit tests in a controlled way. +- Create a separate test suite for error injection tests. +- Reset error injection setup on the `TearDown` function or destructor of the test fixture. +- If an - internal or external - function is missing error injection support, add it to the error_injector library. + - Try to create small error injector libraries for specific functionality. + +## Unit Test Approach + +- Use the Google Test framework for unit tests. +- Use the Google Mock framework for mocking. +- Use the Apache Celix error_injector libraries to inject errors in unit tests in a controlled way. +- Test bundles by installing them in a programmatically created framework. +- Test bundles by using their provided services and used services. +- In most cases, libraries can be tested using a white box approach and bundles can be tested using a black box approach. + +## Supported C and C++ Standards + +- C libraries should support C99. (TBD or C11)) +- C++ libraries should support C++14. + - Exception are `celix::Promises` and `celix::PushStreams` which requires C++17. +- C++ support for `celix::framework` and `celix::utils` must be header-only. +- Unit test code can be written in C++17. + +## Library target properties + +For C and C++ shared libraries, the following target properties should be set: + - `VERSION` should be set to the library version. + - `SOVERSION` should be set to the library major version. + - `OUTPUT_NAME` should be set to the library name and should contain a `celix_` prefix. + +```cmake +add_library(my_lib SHARED + src/my_lib.c) +set_target_properties(my_lib + PROPERTIES + VERSION 1.0.0 + SOVERSION 1 + OUTPUT_NAME celix_my_lib) +``` + +For C and C++ static libraries, the following target properties should be set: + - `POSITION_INDEPENDENT_CODE` should be set to `ON` for static libraries. + - `OUTPUT_NAME` should be set to the library name and should contain a `celix_` prefix. + +```cmake +add_library(my_lib STATIC + src/my_lib.c) +set_target_properties(my_lib + PROPERTIES + POSITION_INDEPENDENT_CODE ON + OUTPUT_NAME celix_my_lib) +``` + +## Symbol Visibility + +- C libraries should configure symbol visibility. +- C++ header-only (INTERFACE) libraries should not configure symbol visibility. +- C++ shared libraries (lib with C++ sources), should configure symbol visibility. +- C and C++ Bundles should configure symbol visibility. + +### Configuring Symbol Visibility for C/C++ Libraries + +For Apache Celix shared libraries, symbol visibility should be configured using the CMake target +properties `C_VISIBILITY_PRESET`, `CXX_VISIBILITY_PRESET` and `VISIBILITY_INLINES_HIDDEN` and a generated export +header. + +The `C_VISIBILITY_PRESET` and `CXX_VISIBILITY_PRESET` target properties can be used to configure the default visibility +of symbols in C and C++ code. The `VISIBILITY_INLINES_HIDDEN` property can be used to configure the visibility of +inline functions. The `VISIBILITY_INLINES_HIDDEN` property is only supported for C++ code. + +The default visibility should be configured to hidden and symbols should be explictly exported using the export +marcos from a generated export header. The export header can be generated using the CMake function +`generate_export_header`. Every library should have its own export header. + +For shared libraries, this can be done using the following CMake code: + +TODO test if this works for C/C++ shared libraries +```cmake +add_library(my_lib SHARED + src/my_lib.c) +set_target_properties(my_lib PROPERTIES + C_VISIBILITY_PRESET hidden + #For C++ shared libraries also configure CXX_VISIBILITY_PRESET + #CXX_VISIBILITY_PRESET hidden + VISIBILITY_INLINES_HIDDEN ON + OUTPUT_NAME celix_my_lib) +target_include_directories(my_lib + PUBLIC + $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include> + $<INSTALL_INTERFACE:include/$<TARGET_PROPERTY:my_lib,OUTPUT_NAME>> + PRIVATE + src) + +#generate export header +generate_export_header(my_lib + BASE_NAME "CELIX_MY_LIB" + EXPORT_FILE_NAME "${CMAKE_BINARY_DIR}/celix/gen/includes/my_lib/celix_my_lib_export.h") +target_include_directories(my_bundle PRIVATE $<BUILD_INTERFACE:${CMAKE_BINARY_DIR}/celix/gen/includes/my_lib>) + +#install +install(TARGETS my_lib EXPORT celix LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} + INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/$<TARGET_PROPERTY:my_lib,OUTPUT_NAME>) +install(DIRECTORY include/ + DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/$<TARGET_PROPERTY:my_lib,OUTPUT_NAME>) +install(DIRECTORY ${CMAKE_BINARY_DIR}/celix/gen/includes/my_lib/ + DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/$<TARGET_PROPERTY:my_lib,OUTPUT_NAME>) +``` + +For bundle, symbol visibility can be configured the same way as a shared library or the `HIDE_SYMBOLS` option in the +`add_celix_bundle` function can be used. + +The `HIDE_SYMBOLS` option in the `add_celix_bundle` will add a linker script that hides all symbols, expect for +the bundle activator symbols. Note the bundle activator symbols are needed to start and stop the bundle. + +```cmake +add_celix_bundle(my_bundle + SOURCES src/my_bundle.c + SYMBOLIC_NAME "apache_celix_my_bundle" + NAME "Apache Celix My Bundle" + FILENAME "celix_my_bundle" + VERSION "1.0.0" + GROUP "celix/my_bundle_group" + HIDE_SYMBOLS +) +add_library(celix::my_bundle ALIAS my_bundle) +``` + +## Benchmarking + +- When needed, use benchmarking to measure performance. +- Use the Google Benchmark framework for benchmarking. + +## Code Quality + +- New code should be reviewed through a pull request and no direct commits on the master branch are allowed. + - At least 1 reviewer should review the code. +- Hotfix pull request can be merged first and reviewed later, the rest is reviewed first and merged later. +- Unit tests should be written for all code. +- Code coverage should be measured and should be at least 95% for new code. +- For existing code, code coverage should be measured and should not decrease. +- Code should be checked for memory leaks using AddressSanitizer. +- Coverity scan are done on the master on a regular basis. Ideally new coverity issues should be fixed as soon as + possible.
