Skip to content

Commit bbb7db9

Browse files
authored
docs: add 'how to add formatter' (#195)
* docs: add 'how to add formatter' * code review comments
1 parent f153e8e commit bbb7db9

File tree

4 files changed

+65
-21
lines changed

4 files changed

+65
-21
lines changed

README.md

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,19 @@ Features:
77
- **No changes needed to rulesets**. Works with the Bazel rules you already use.
88
- **No changes needed to BUILD files**. You don't need to add lint wrapper macros, and lint doesn't appear in `bazel query` output.
99
Instead, users simply lint their existing `*_library` targets.
10-
- Lint checks and fixes are run as normal Bazel actions, which means they support Remote Execution and the outputs are stored in the Remote Cache.
10+
- **Incremental**. Lint checks (including producing fixes) are run as normal Bazel actions, which means they support Remote Execution and the outputs are stored in the Remote Cache.
1111
- Lint results can be **presented in various ways**, such as Code Review comments or failing tests.
1212
See [Usage](https://github.com/aspect-build/rules_lint/blob/main/docs/linting.md#usage).
13+
- **Can lint changes only**. It's fine if your repository has a lot of existing issues.
14+
It's not necessary to fix or suppress all of them to start linting new changes.
1315
- **Can format files not known to Bazel**. Formatting just runs directly on the file tree.
1416
No need to create `sh_library` targets for your shell scripts, for example.
15-
- Honors the same configuration files you use for these tools outside Bazel (e.g. in the editor)
17+
- Honors the same **configuration files** you use for these tools outside Bazel (e.g. in the editor)
1618

1719
## Supported tools
1820

21+
New tools are being added frequently, so check this page again!
22+
1923
| Language | Formatter | Linter(s) |
2024
| ---------------------- | --------------------- | ---------------- |
2125
| C / C++ | [clang-format] | ([#112]) |
@@ -68,11 +72,10 @@ Features:
6872
1. Non-hermetic: requires that a swift toolchain is installed on the machine.
6973
See https://github.com/bazelbuild/rules_swift#1-install-swift
7074

71-
To add a linter, please follow the steps in [lint/README.md](./lint/README.md) and then send us a PR.
75+
To add a tool, please follow the steps in [lint/README.md](./lint/README.md) or [format/README.md](./format/README.md)
76+
and then send us a PR.
7277
Thanks!!
7378

74-
> We'll add documentation on adding formatters as well.
75-
7679
## Installation
7780

7881
Follow instructions from the release you wish to use:

docs/formatting.md

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,18 @@
44

55
Create a BUILD file that declares the formatter binary, typically at `tools/format/BUILD.bazel`
66

7-
Each formatter should be installed in your repository, see our `example/tools/format/BUILD.bazel` file.
8-
A formatter is just an executable target.
9-
10-
Then register them on the `formatters` attribute, for example:
7+
This file contains a `format_multirun` rule. To use the tools supplied by default in rules_lint,
8+
just make a simple call to it like so:
119

1210
```starlark
1311
load("@aspect_rules_lint//format:defs.bzl", "format_multirun")
1412

15-
format_multirun(
16-
name = "format",
17-
# register languages, e.g.
18-
# python = "//:ruff",
19-
)
13+
format_multirun(name = "format")
2014
```
2115

16+
For more details, see the `format_multirun` [API documentation](./format.md) and
17+
the `example/tools/format/BUILD.bazel` file.
18+
2219
Finally, we recommend an alias in the root BUILD file, so that developers can just type `bazel run format`:
2320

2421
```starlark
@@ -28,6 +25,27 @@ alias(
2825
)
2926
```
3027

28+
### Choosing formatter tools
29+
30+
Each formatter should be installed by Bazel. A formatter is just an executable target.
31+
32+
`rules_lint` provides some default tools at specific versions using
33+
[rules_multitool](https://github.com/theoremlp/rules_multitool).
34+
You may fetch alternate tools or versions instead.
35+
36+
To register the tools you fetch, supply them as values for that language attribute.
37+
38+
For example:
39+
40+
```starlark
41+
load("@aspect_rules_lint//format:defs.bzl", "format_multirun")
42+
43+
format_multirun(
44+
name = "format",
45+
python = ":ruff",
46+
)
47+
```
48+
3149
## Usage
3250

3351
### Configuring formatters
@@ -75,7 +93,7 @@ If you use [pre-commit.com](https://pre-commit.com/), add this in your `.pre-com
7593
files: .*
7694
```
7795
78-
> Note that pre-commit is silent while Bazel is fetching the tooling, which can make it appear hung on the first run.
96+
> Note that pre-commit is silent while Bazel is fetching the tools, which can make it appear hung on the first run.
7997
> There is no way to avoid this; see https://github.com/pre-commit/pre-commit/issues/1003
8098
8199
If you don't use pre-commit, you can just wire directly into the git hook, however

format/README.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# Adding a new formatter
2+
3+
See the "Design invariants" section in the [new linter doc](../lint/README.md).
4+
This guidance applies for formatters as well.
5+
6+
We generally avoid offering users two different formatters for the same language.
7+
It might be okay if the formatters have different strictness (like gofmt vs gofumpt)
8+
or if they format different language dialects (each Hashicorp Config Language tool has a different formatter).
9+
Note that the opposite is not true: a formatting tool like Prettier supports multiple languages.
10+
11+
Start in `format/private/formatter_binary.bzl`.
12+
This has some dictionaries that define the tool used for each language.
13+
14+
`format/private/format.sh` may also need a small change to wire up the file extensions applicable to this tool.
15+
Run `format/private/mirror_linguist_languages.sh` to get a "canonical" list of extensions used for this language.
16+
17+
In the `example` folder, add source files that demonstrate incorrect formatting and verify that the `bazel run format` command corrects it.
18+
19+
Add a new test case in `format/test/format_test.bats` so that we have some automated testing that the formatter works.
20+
21+
Update the `README.md` to include your formatter in the table.

lint/README.md

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44

55
These will be part of reviewing PRs to this repo:
66

7-
1. Avoid adding dependencies to rules_lint, they belong in the example instead. For example in adding
8-
eslint or flake8, it's up to the user to provide us the binary to run.
9-
This ensures that the user can select the versions of their tools and the toolchains used to run them
10-
rather than us baking these into rules_lint.
7+
1. Take care with dependencies. Avoid adding to `MODULE.bazel` if possible.
8+
In cases where a tool is a statically-linked binary, it can be added to the `multitool.lock.json` file
9+
to conveniently provide it to users.
10+
In other cases where a language ecosystem's package manager is involved,
11+
the tool should be setup "in userland", which means adding it to the `example` folder.
12+
Note that this distinction also determines whether users control the version of the tool.
1113

1214
2. Study the installation, CLI usage, and configuration documentation for the linter you want to add.
1315
We'll need to adapt these to Bazel's idioms. As much as possible, copy or link to this documentation
@@ -50,8 +52,8 @@ Add these three things:
5052
It should call the `my_linter_action` function.
5153
It must always return a `rules_lint_report` output group, which is easiest by using the
5254
`report_file` helper in `//lint/private:lint_aspect.bzl`.
53-
The simple lint.sh also relies on the report output file being named following the convention `*.aspect_rules_lint.report`, though this is
54-
a design smell.
55+
The simple lint.sh also relies on the report output file being named following the convention
56+
`*.aspect_rules_lint.report`, though this is a design smell.
5557

5658
3. A `my_linter_aspect` factory function. This is a higher-order function that returns an aspect.
5759
This pattern allows us to capture arguments like labels and toolchains which aren't legal

0 commit comments

Comments
 (0)