blob: fdd8c5a2372801e794fdcaf1bab6c3bfdc1291da [file] [log] [blame] [view]
# Overview
There are various tools at the disposal of Fuchsia developers for linting and formatting code. This
is a general overview of those tools for each language, as well as a description of how additional
lint checks should be added and the accuracy standards to which they should be held.
Note that this doesn't try to explain the specific language configurations for each linter and
formatter. Though the purpose of linting and formatting is to encourage and enforce recommendations
around style and best practices, each relevant language has its own guides that explain the
decisions made and the configurations enabled.
# Tooling Integration
The Fuchsia team provides two ways to format and lint code: subcommands in the developer-side fx
tool, and integrated Tricium analysis on uploaded changes. In addition, a subset of formatting and
linting is eligible to be directly included in the build, with strict limitations around accuracy.
## Developer tooling (IDEs and fx)
The primary developer tooling suite is the fx command and its subcommands. It provides two
subcommands relevant here: fx format-code and fx lint. Each runs the relevant tooling on a list of
files and prints the output to the terminal’s stdout/stderr. Running fx lint assumes that the
developer has already run fx build; if not, many of the linters will produce errors related to
missing files that are created by the build.
The list of files can be specified in one of three ways:
- The list of files changed since the second-to-last Git commit, including committed, modified,
and cached files (this is the default behavior)
- A list of files passed in a comma-separated list to the --files flag
- The list of files in the sources of the GN target passed to the --target flag
Formatting is done in-place. Linting is by default warn-only, but users can pass the --fix flag to
fx lint to automatically fix the errors for which the tools provide fixes.
Most editors will also integrate formatters and linters to allow developers to automatically
format-on-save or format-on-keybinding. In most cases, setup (if any) consists of pointing the IDE
at the relevant configuration file and Fuchsia-distributed tool binary.
## Integrated tooling (Tricium)
Tricium is a service that integrates with the Gerrit code review system to surface relevant
warnings in a way that does not block commits. It triggers on each patchset uploaded by a user with
commit access to the Fuchsia repository and runs two suites of tooling analysis.
The formatter analysis does a minimal checkout (no third_party, no prebuilts) and extracts the list
of changed files from the patch commit. It runs the relevant formatter based on file extension on
each file. If the produced formatted file differs from the file content in the uploaded patch,
Tricium posts a comment on the patch explaining how to run the appropriate formatter on the file.
The linter analysis does a full checkout and does a minimal build (to produce the necessary
configuration files and headers). It extracts the list of changed files from the patch commit and
then runs the relevant linter based on file extension. Machine-readable outputs are requested from
the linters, and if warnings are produced the output is then parsed and collected into comment
form. Tricium then comments on the appropriate line with the linter warning.
Tricium, where possible, only runs the tools on the changed lines in a commit, though not all
linters support this behavior. For the ones that do, this is so that existing but irrelevant lint
errors do not distract from the change itself and only directly relevant lints are surfaced.
Analysis results are often based on heuristics. As a result, they do from time to time produce
false positives. Fuchsia aims to support a high bar for these analyzers, with any analyzer with
greater than 10% error rates as measured by the metrics produced by the Tricium service being disabled.
New linters should generally be added to the existing Tricium recipes. Since checkout/build times
are by far the most costly in these builds (the analysis itself takes at best a few seconds, and at
worst a few minutes, while checkouts and/o builds can take much longer), it is more efficient from
both time and infrastructure resource perspectives to simply extend the existing builders. The
selection of which recipe to extend should be based on the amount of information needed, e.g. if
prebuilts/third_party code are not needed to run the analysis, the minimal checkout recipe should
be used.
## Build Integration
An alternative for linter checks that provide zero false-positive rates is to include them in the
build.
Adding additional checks to this category is not encouraged unless it is certain that they do not
fire on false positives.
These checks are directly implemented in the build (generally as actions that run the relevant
script), and so will cause the whole build to fail if they catch errors. They also extend the build
time, and so should only be used in cases where they provide valuable and correct information to
the developer.
# Standards
## Formatters
Formatters should adhere to the relevant style guides, but whether the formatter’s output is the
source-of-truth for the style guide is left up to languages and their style arbiters. When a
formatter is changed in the upstream community (e.g. when the Rust community changes `rustfmt`),
the updated formatter will roll into Fuchsia with the toolchain. This doesn't happen often, but can
be the cause of conflicting formats between Tricium and local tooling until developers update to
use the new toolchain.
Generally, Fuchsia’s support for formatters is dependent on developers running the formatting
commands. The only automation is from Tricium, which will warn if a file differs from the
formatter’s output, but will not block the CL’s commit.
## Linters
Linters should generally provide useful and actionable comments to developers. Since they are often
heuristics-based, they can produce false positives, but any linter exceeding the 10% false positive
rate should be disabled. The process for adding a linter check is to file a bug requesting the new
check, outlining its value and the expected false positive rate. Removing a linter check can either
be done by filing a bug or submitting a patch with the requested configuration change.
Only linters that are guaranteed to not produce false positives should be implemented in the build
itself. These should be enforced by both local builds and by CQ, so that there are no surprises
when developers attempt to submit their code.
# Language Tools
Each supported language provides a formatter and optionally linters. This section describes the
integration of these tools into the Fuchsia workflow. While the formatters tend to be
straightforward, the tooling is a bit complex in how the linters are integrated. In most cases,
developers do not need to understand the internals of `fx` and Tricium.
All commands are assumed to be run from the root of a Fuchsia checkout.
## C/C++
C/C++ code uses [`clang-format`](https://clang.llvm.org/docs/ClangFormat.html) and [`clang-tidy`](https://clang.llvm.org/extra/clang-tidy/). These are distributed as prebuilts from the Clang
toolchain. Both use root-level configuration files (`.clang-format` and `.clang-tidy`,
respectively). Developers should not create additional configuration files at a lower level, as
this will cause disagreements in the tree.
`clang-format` is run on source files as follows:
```sh
prebuilt/third_party/clang/$HOST_PLATFORM/bin/clang-format \
-i \
-style=file \
-fallback-style=Google \
-sort-includes \
$FILES
```
Before you run `clang-tidy`, you must:
* Build the set of generated headers.
The `clang-tidy` tool partially compiles the source code and most C and C++ code in Fuchsia
includes headers generated as part of the build.
Once the compilation database and generated headers are present, you can run the `run-clang-tidy.py`
script to start the `clang-tidy` tool. The script handles handles parallelization and deduplication
of errors, which is necessary when the same header is included in multiple source files. When you
use this script, you must also pass the `clang-tidy` and `clang-apply-replacements` binaries from
the distributed Fuchsia toolchain to make sure the correct ones are used.
```sh
export CLANG_TOOLCHAIN_PREFIX=prebuilt/third_party/clang/$HOST_PLATFORM
$CLANG_TOOLCHAIN_PREFIX/share/clang/run-clang-tidy.py \
-clang-tidy-binary $CLANG_TOOLCHAIN_PREFIX/bin/clang-tidy \
-clang-apply-replacements-binary $CLANG_TOOLCHAIN_PREFIX/bin/clang-apply-replacements \
$FILES
```
An optional `-fix` flag can be added to automatically apply fixes. This is available in the
developer-side tooling.
## Rust
Rust code uses [`rustfmt`](https://github.com/rust-lang/rustfmt) and [`clippy`](https://github.com/rust-lang/rust-clippy). These are distributed as prebuilts from the Rust toolchain. The
formatter has a root-level configuration file (`rustfmt.toml`).
`rustfmt` runs on source files as follows:
```sh
prebuilt/third_party/rust/${HOST_PLATFORM}/bin/rustfmt \
--config-path=rustfmt.toml \
--unstable-features \
--skip-children \
$FILES
```
`clippy` is run by `fx build` on all Rust targets by default. `clippy` warnings
are treated as build errors during presubmits.
If you want to run only the linter it can also be invoked locally on GN targets
with our fx helper script:
```sh
fx clippy TARGET
fx clippy --files FILE1 FILE2 # Filters lints for a list of specific files
fx lint # Same as --files but implicitly runs on locally changed files
```
### Enabling clippy lints
To see lints for your target you'll need to enable them in one of the following ways:
- Enable the default set of clippy lints on a specific target by adding a config: `configs += [ "//build/config/rust/lints:clippy_warn_all" ]`.
- Locally enable clippy lints as warnings for every target with the gn arg: `fx set core.x64 --args clippy_warn_all=true`. This is useful for figuring out which lints are common before enabling them for your crate, or gathering statistics like lint frequency across the project.
[Here is the list](https://rust-lang.github.io/rust-clippy/stable/) of all available clippy lints and their names.
### Disabling clippy lints for specific code
Add `#[allow(clippy::LINT_NAME)]` to any Rust code where you'd like to ignore a
particular lint. Note that attributes [cannot be applied to all expressions][rust-attr-expr]
and it may be necessary to apply the attribute to a containing syntax element.
[rust-attr-expr]: https://doc.rust-lang.org/reference/expressions.html#expression-attributes
## Go
Go code uses [`gofmt`](https://golang.org/cmd/gofmt/) and [`go vet`](https://golang.org/cmd/vet/). These are built as part of the Go toolchain build, and also
distributed in the Go host toolchain prebuilts.
`gofmt` runs on source files as follows:
```sh
prebuilt/third_party/go/$HOST_PLATFORM/bin/gofmt -s -w $FILES
```
TODO(https://fxbug.dev/42101826): Document go vet once implementation details are finalized.
## FIDL
FIDL code uses the `fidl-format` and `fidl-lint` tools. These are built as host tools from in-tree.
Before running either the `zircon/tools` target must be built so that the binaries exist.
`fidl-format` runs on source files as follows:
```sh
$ZIRCON_BUILD_DIR/tools/fidl-format -i $FILES
```
`fidl-lint` runs on source files as follows:
```sh
$ZIRCON_BUILD_DIR/tools/fidl-lint $FILES
```
## GN
GN files use the [`gn format`](https://gn.googlesource.com/gn/+/HEAD/docs/reference.md#cmd_format) subcommand. There is not a linter. This is distributed as part of the GN
prebuilt.
It runs on source files as follows:
```sh
prebuilt/third_party/gn/$HOST_PLATFORM/gn format <files>
```