Skip to content

Commit

Permalink
Update Coding style documentation (project-chip#36275)
Browse files Browse the repository at this point in the history
* Update coding style guide

- removed most of the motivation sections as it was somewhat
  repetitive and made the doc quite verbose
- removed the drawing because it doesn't match the SDK implementation
  in reality
- removed C discussion - I don't think we have any
- added python versions. Didn't add the other languages because I
  dont' know but other folks can easily add a follow up
- added sections on
  + formatters
  + anonymous namespaces for internal stuff in cpp files
  + singletons
  + std containers to heap allocation sections
  + CopySpanToMutableSpan
  + std::optional
  + python section
- removed the code samples from heap allocation because it implies
  that people show re-implement these things when the preference is
  to use the provided support classes
- removed the version table - we have git history.

* Move coding style into main style dir

* Use md since that's why the doc wasn't included

* Restyled by prettier-markdown

* add new words to wordlist

* Add clarification about heap allocation

* Add clarification about code removal

* add isort ref to formatter table

* add ruff

* use links

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>

* address review comments

* Add link suffix toml to wordlist

---------

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
3 people authored Nov 18, 2024
1 parent 8b4d17f commit 83828f5
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 721 deletions.
11 changes: 11 additions & 0 deletions .github/.wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ AFL
AIDL
algs
alloc
allocator
allocators
Ambrose
Ameba
amebad
amebaiot
Expand Down Expand Up @@ -317,6 +320,7 @@ cryptographic
CSA
csg
csrrequest
cstdint
csu
csv
ctl
Expand Down Expand Up @@ -440,6 +444,7 @@ DNSStubListener
docbuild
Dockerfile
Dockerfiles
docstrings
Don'ts
DoorLock
DoorState
Expand Down Expand Up @@ -565,6 +570,8 @@ FlowMeasurement
FluorideConcentrationMeasurement
focusable
forkpty
formatter
formatters
FOTA
FreeRTOS
FreeRTOSConfig
Expand Down Expand Up @@ -731,6 +738,7 @@ IPython
ISCAN
isHexString
isLowerCase
isort
isUpperCase
itemName
iterable
Expand Down Expand Up @@ -878,6 +886,7 @@ MediaPlayback
MediaTek
MEI
mem
memcpy
memdf
MemMonitoring
menuconfig
Expand Down Expand Up @@ -933,6 +942,7 @@ mv
MX
mydir
MyPASSWORD
mypy
MySSID
NAMESERVER
NAMESPACE
Expand Down Expand Up @@ -1442,6 +1452,7 @@ toJson
tokenization
tokenized
tokenizer
toml
toolchain
toolchains
topologies
Expand Down
172 changes: 172 additions & 0 deletions docs/style/CODING_STYLE_GUIDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
# Coding Style Guide

_Revision 6_ _2024-10-28_

This guide provides a small set of code guidelines used in the SDK. This guide
reflects the currently accepted style practices for the SDK and is subject to
change.

The SDK was seeded from multiple different projects and contains contributions
from many different companies and the SDK code therefore uses several different
coding styles throughout the code base. Stylistically, code should attempt to
conform to the dominant style of the code being modified, while also adhering to
the guidelines below.

## Language standard

Code in the SDK conforms to the following standards. Changes to the C++ standard
happen relatively infrequently. Changes to the Python version are more frequent.

| Language | Version |
| -------- | ------- |
| C++ | C++17 |
| Python | 3.10 |

Product-specific software may elect to use later standards in their own work.

## Coding Guidelines

### Common

#### When in Rome

The most important convention and practice in the Matter SDK repo is "_When in
Rome..._", per the quote below.

[quote, St. Ambrose]

---

If you should be in Rome, live in the Roman manner; if you should be elsewhere,
live as they do there.

---

Your extensions or fixes to existing code should match the prevailing style of
the original code.

If you find the conventions so foreign or otherwise confusing, it may be best to
let whoever owns the file make the necessary changes or seek the counsel of
others in the group to find out what the right thing to do is. Never just start
changing code wholesale for personal reasons without consulting others first.

#### Commenting Out or Disabling Code

Unused code shall not be disabled by commenting it out with C- or C++-style
comments or with preprocessor `#if 0 ... #endif` semantics. Unused code should
be removed.

#### Auto-formatters

We use the following auto-formatters on code:

| Language | Formatter | Style File |
| ----------- | ------------------ | ------------------------------------------------------------------------------------------ |
| C++ | clang-format | [.clang-format](https://github.com/project-chip/connectedhomeip/blob/master/.clang-format) |
| Objective-C | clang-format | [.clang-format](https://github.com/project-chip/connectedhomeip/blob/master/.clang-format) |
| java | google-java-format | N/A |
| Python | pep8, isort, ruff | [.restyled.yaml][restyle_link] (command line), [isort][isort_link], [ruff][ruff_link] |
| YAML | prettier | None |
| JSON | prettier | None |
| markdown | prettier | None |

[restyle_link]:
https://github.com/project-chip/connectedhomeip/blob/master/.restyled.yaml
[isort_link]:
https://github.com/project-chip/connectedhomeip/blob/master/.isort.cfg
[ruff_link]:
https://github.com/project-chip/connectedhomeip/blob/master/ruff.toml

All pull requests run formatting checks using these tools before merge is
allowed. Generated code is not run through restyle.

### C++

#### Use C++ _cstdint_ for Plain Old Data Types

Standard, scalar data types defined in _cstdint_ should be used for basic signed
and unsigned integer types, especially when size and serialization to
non-volatile storage or across a network is concerned.

Examples of these are: `uint8_t`, `int8_t`, etc.

#### Avoid top-level `using namespace` Statements in Headers

By doing this, you are effectively forcing every other module that includes the
header to also be using the namespace. This causes namespace pollution and
generally defeats the purposes of namespaces. Fully-qualified symbols or
namespace blocks should be used instead.

#### Classes / objects not exposed in a header should be in an anonymous namespace

If a cpp class defines a class or instantiates a static object, it should be
enclosed in an anonymous namespace.

```
namespace {
// CPP internal defines go here
} // namespace
```

#### Singleton use

The decision to use a singleton class should be considered with care. Do not
default to using a singleton for ease of writing code.

If the class truly should be a singleton (ex. if it is controlling access to a
hardware resource)

- The standard function name for accessing an SDK singleton is GetInstance().
- Singleton classes should delete copy and move constructors

#### Avoid Heap-based Resource Allocation and auto-resizing std containers

Heap-based resource allocation should be avoided in the core SDK for common code
that may run on constrained embedded devices. This includes any container
element in std that automatically re-sizes itself at runtime (ex. vector, string
etc.) as these re-size operations are often large and can lead to memory
exhaustion and fragmentation on embedded systems.

Heap-based allocation is allowed for controller code and is at the discretion of
platform vendors for platform-specific code.

##### Alternatives

In either case, recommended resource allocation alternatives are:

- In-place allocation and initialization
- Pool-based allocators
- Platform-defined and -assigned allocators

[CHIPMem.h](https://github.com/project-chip/connectedhomeip/blob/master/src/lib/support/CHIPMem.h)
provides support for platform defined allocators.

[Pool.h](https://github.com/project-chip/connectedhomeip/blob/master/src/lib/support/Pool.h)
is the Matter SDK pool allocator implementation.

#### Prefer CopySpanToMutableSpan over memcpy when using spans

See
[Span.h](https://github.com/project-chip/connectedhomeip/blob/master/src/lib/support/Span.h)

#### Prefer std::optional to CHIP implementation in newer code

The Matter SDK Optional.h was implemented when the Matter SDK was C++14, but
newer code can use std::optional, which offers some benefits over the Matter SDK
implementation (ex. std::optional is trivially destructible if the underlying
type is also trivially destructible)

### Python

#### Type hints

Use type hints on function definitions for public APIs.

#### Docstrings

Docstrings should be included for all public APIs.

#### mypy

The current python code does not yet pass mypy checks, but we are working
towards this goal. The more compliant new code is to mypy, the better.
Binary file removed docs/style/coding/CODING_STYLE_GUIDE-figure1.png
Binary file not shown.
Loading

0 comments on commit 83828f5

Please sign in to comment.