Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flake8 #5

Merged
merged 4 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,19 @@ We have a separate project for formatting, see <https://github.com/aspect-build/
[tricorder]: https://static.googleusercontent.com/media/research.google.com/en//pubs/archive/43322.pdf
[reviewdog]: https://github.com/reviewdog/reviewdog

## Adding a linter
## Available linters

TODO: step-by-step instructions to add a linter in this repo, and maybe how to add one in your repo.
- Protocol Buffers
- `buf lint`: <https://buf.build/docs/lint/overview>
- JavaScript
- `eslint`: https://eslint.org/
- Python
- `flake8`: <https://flake8.pycqa.org/en/latest/index.html>

### Adding a linter

Please follow the steps in [lint/README.md](./lint/README.md) and then send us a PR.
Thanks!!

## Installation

Expand Down
5 changes: 5 additions & 0 deletions docs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,9 @@ stardoc_with_diff_test(
bzl_library_target = "//lint:eslint",
)

stardoc_with_diff_test(
name = "flake8",
bzl_library_target = "//lint:flake8",
)

update_docs(name = "update")
74 changes: 74 additions & 0 deletions docs/flake8.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions example/.aspect/cli/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ plugins:
lint_aspects:
- //:lint.bzl%eslint
- //:lint.bzl%buf
- //:lint.bzl%flake8
5 changes: 5 additions & 0 deletions example/.flake8
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[flake8]
extend-ignore = E203
exclude = external,bazel-out
max-complexity = 10
max-line-length = 100
22 changes: 14 additions & 8 deletions example/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
load("@aspect_rules_js//js:defs.bzl", "js_library")
load("@npm//:defs.bzl", "npm_link_all_packages")
load("@npm//:eslint/package_json.bzl", eslint_bin = "bin")
load("@bazel_skylib//rules:build_test.bzl", "build_test")
load("@rules_python//python:pip.bzl", "compile_pip_requirements")
load("@rules_python//python/entry_points:py_console_script_binary.bzl", "py_console_script_binary")

package(default_visibility = ["//visibility:public"])

compile_pip_requirements(
name = "requirements",
)

npm_link_all_packages(name = "node_modules")

exports_files(["buf.yaml"], visibility = ["//visibility:public"])
exports_files(["buf.yaml", ".flake8"], visibility = ["//visibility:public"])

# We can test that it works with:
# bazel run :flake8 -- --help
py_console_script_binary(
name = "flake8",
pkg = "@pip//flake8:pkg",
)

eslint_bin.eslint_binary(name = "eslint")

Expand All @@ -19,9 +31,3 @@ js_library(
":node_modules/@typescript-eslint/parser",
],
)

# Just so there's at least one test target in the example
build_test(
name = "test",
targets = [":eslint"],
)
17 changes: 16 additions & 1 deletion example/MODULE.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
"Bazel dependencies"
bazel_dep(name = "aspect_rules_lint", dev_dependency = True, version = "0.0.0")
bazel_dep(name = "aspect_bazel_lib", version = "1.31.2")
bazel_dep(name = "aspect_bazel_lib", version = "1.31.2", dev_dependency = True)
bazel_dep(name = "aspect_rules_js", version = "1.32.2", dev_dependency = True)
bazel_dep(name = "aspect_rules_ts", version = "1.3.3", dev_dependency = True)
bazel_dep(name = "rules_python", version = "0.26.0", dev_dependency = True)
bazel_dep(name = "bazel_skylib", dev_dependency = True, version = "1.4.1")
# Needed due to rules_proto leaking the dependency
bazel_dep(name = "protobuf", version = "21.7", repo_name = "com_google_protobuf")
Expand Down Expand Up @@ -35,3 +36,17 @@ npm.npm_translate_lock(
)

use_repo(npm, "npm")

python_version = "3.9"
python = use_extension("@rules_python//python/extensions:python.bzl", "python")
python.toolchain(
python_version = python_version,
)

pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip")
pip.parse(
python_version = python_version,
hub_name = "pip",
requirements_lock = "//:requirements.txt",
)
use_repo(pip, "pip")
6 changes: 6 additions & 0 deletions example/lint.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

load("@aspect_rules_lint//lint:eslint.bzl", "eslint_aspect")
load("@aspect_rules_lint//lint:buf.bzl", "buf_lint_aspect")
load("@aspect_rules_lint//lint:flake8.bzl", "flake8_aspect")

buf = buf_lint_aspect(
config = "@@//:buf.yaml",
Expand All @@ -11,3 +12,8 @@ eslint = eslint_aspect(
binary = "@@//:eslint",
config = "@@//:eslintrc",
)

flake8 = flake8_aspect(
binary = "@@//:flake8",
config = "@@//:.flake8",
)
2 changes: 1 addition & 1 deletion example/lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ if [ "$#" -eq 0 ]; then
fi

# Produce report files
bazel build --aspects //:lint.bzl%eslint,//:lint.bzl%buf --output_groups=report $@
bazel build --aspects //:lint.bzl%eslint,//:lint.bzl%buf,//:lint.bzl%flake8 --output_groups=report $@

# Show the results.
# `-mtime -1`: only look at files modified in the last day, to mitigate showing stale results of old bazel runs.
Expand Down
1 change: 1 addition & 0 deletions example/requirements.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
flake8
22 changes: 22 additions & 0 deletions example/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#
# This file is autogenerated by pip-compile with Python 3.9
# by the following command:
#
# bazel run //:requirements.update
#
flake8==6.1.0 \
--hash=sha256:d5b3857f07c030bdb5bf41c7f53799571d75c4491748a3adcd47de929e34cd23 \
--hash=sha256:ffdfce58ea94c6580c77888a86506937f9a1a227dfcd15f245d694ae20a6b6e5
# via -r requirements.in
mccabe==0.7.0 \
--hash=sha256:348e0240c33b60bbdf4e523192ef919f28cb2c3d7d5c7794f74009290f236325 \
--hash=sha256:6c2d30ab6be0e4a46919781807b4f0d834ebdd6c6e3dca0bda5a15f863427b6e
# via flake8
pycodestyle==2.11.0 \
--hash=sha256:259bcc17857d8a8b3b4a2327324b79e5f020a13c16074670f9c8c8f872ea76d0 \
--hash=sha256:5d1013ba8dc7895b548be5afb05740ca82454fd899971563d2ef625d090326f8
# via flake8
pyflakes==3.1.0 \
--hash=sha256:4132f6d49cb4dae6819e5379898f2b8cce3c5f23994194c24b77d5da2e36f774 \
--hash=sha256:a0aae034c444db0071aa077972ba4768d40c830d9539fd45bf4cd3f8f6992efc
# via flake8
5 changes: 5 additions & 0 deletions example/src/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,8 @@ proto_library(
deps = [":unused"],
visibility = ["//visibility:public"],
)

py_library(
name = "unused_import",
srcs = ["unused_import.py"],
)
6 changes: 6 additions & 0 deletions example/src/unused_import.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Demo with just running flake8:
# $ bazel run --run_under="cd $PWD &&" -- :flake8 --config=.flake8 --exit-zero src/*.py
# INFO: Build completed successfully, 1 total action
# src/unused_import.py:6:1: F401 'os' imported but unused
# Error: bazel exited with exit code: 1
import os
6 changes: 6 additions & 0 deletions lint/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,9 @@ bzl_library(
"@aspect_rules_js//js:libs",
],
)

bzl_library(
name = "flake8",
srcs = ["flake8.bzl"],
visibility = ["//visibility:public"],
)
80 changes: 80 additions & 0 deletions lint/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# Adding a new linter

## Design invariants

These will be part of reviewing PRs to this repo:

1. Avoid adding dependencies to rules_lint, they belong in the example instead. For example in adding
eslint or flake8, it's up to the user to provide us the binary to run.
This ensures that the user can select the versions of their tools and the toolchains used to run them
rather than us baking these into rules_lint.

2. Study the installation, CLI usage, and configuration documentation for the linter you want to add.
We'll need to adapt these to Bazel's idioms. As much as possible, copy or link to this documentation
in your code so that maintainers can understand it.
Usage of the linter under Bazel should be as similar as possible to how it's used outside Bazel,
so we don't end up with a unique bug "surface area" that only Bazel users encounter.

## Step 1: Run linter in the example

In the example folder, install the tool the same way a user would. We need an executable target
(typically a `*_binary`) that can be passed to our aspect factory function.

For example, to install flake8, we need to do the normal install for rules_python,
create a `requirements_lock.txt` file, call `pip_parse` to install it, and find the `entry_point`
function that gives us a `py_binary` for it.

Add a config file for the linter following their documentation. We want users to be able to continue
running the linter outside Bazel (e.g. using their editor plugin) and they should be assured that
they get the same result.

Add a file under `src/` that violates one of the linter checks, and add a comment describing how we
can see the linter running. This should basically be a "demo" of how the linter works, without
rules_lint getting involved yet.

Take a look at this commit as an example of step 1:
https://github.com/aspect-build/rules_lint/commit/7365b82957dd60898ef1051f5ae94539714a38f4

Please check in a commit at this point to make your PR easier for us to review.

## Step 2: create linter

Add the new_linter.bzl file in this folder.

Add these three things:

1. A `my_linter_action` function which declares one or more actions with `ctx.actions.run` or `ctx.actions.run_shell`.
It should accept the report output as a parameter.
You can use the "demo" from step 1 to guide you in setting the command-line options for the linter CLI.
Note: this function provides a reusable API for developers who want to use flake8 in a different way than we prescribe with aspects.

2. A `_my_linter_aspect_impl` function, following the https://bazel.build/extending/aspects#aspect_implementation API.
This is responsible for selecting which rule types in the graph it "knows how to lint".
It should call the `my_linter_action` function.
It must always return a `report` output group.
Currently we also rely on the report output file being named following the convention `*-report.txt`, though this is
a design smell.

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

Then wire this up into the example and confirm that you can get the same linter result as you did in
step 1.

Take a look at this commit as an example of step 2:
https://github.com/aspect-build/rules_lint/commit/29d275bcf7ecf5b99c6bff6913322fe1909302eb

## Step 3: docs

Add a rule in the `docs/` folder matching the existing ones, so that the API docs are auto-generated.
Run `bazel run docs:update` to create the Markdown file.

Also add your new linter to the README.

Here's a commit showing what it should look like:
https://github.com/aspect-build/rules_lint/commit/c3bf01a39c2e68b0b37918620aeafbf8ef0b2d85

## Step 4: Send the PR!

We'd love to make your linter available to everyone.
alexeagle marked this conversation as resolved.
Show resolved Hide resolved
Loading