Skip to content

Pull some high level options up into the toolchain - #237

Open
Bencodes wants to merge 2 commits into
masterfrom
pull-some-high-level-options-up-into-the-toolchain
Open

Pull some high level options up into the toolchain#237
Bencodes wants to merge 2 commits into
masterfrom
pull-some-high-level-options-up-into-the-toolchain

Conversation

@Bencodes

@Bencodes Bencodes commented Jul 4, 2026

Copy link
Copy Markdown
Member

Summary

  • Moves reusable Detekt defaults into detekt_toolchain: config files, plugins, configuration booleans, JVM and Kotlin targets, failure policy, parallel execution, and the wrapper executable.
  • Lets each rule select a toolchain implementation directly while preserving registered-toolchain fallback, so multiple profiles can coexist in one repository and build configuration.
  • Makes explicit rule values replace inherited defaults, including False, empty lists, empty strings, and max_issues = -1. An explicit rule failure policy replaces the inherited failure-policy pair.
  • Removes Detekt-specific JVM process flags; the exec-configured Java wrapper uses native Bazel --host_jvmopt configuration instead.
  • Supports both Detekt 1.23.8 and Detekt 2.0.0-alpha.6 and updates README guidance and generated attribute documentation.

Compatibility notes

Toolchain profiles configure the selected Detekt runtime; they do not select a Detekt version. Use max_issues with Detekt 1.x, fail_on_severity with Detekt 2.x, and plugins built for the matching major version. Target-specific inputs, baselines, filters, Android context, autocorrection, and report outputs remain rule-level options.

Windows support remains separate in #250.

Validation

  • pre-commit run --all-files
  • bazel mod deps --lockfile_mode=error
  • Root analysis, integration, and unit suites under local and clean worker Detekt strategies
  • Detekt 2 alpha success and expected-failure gates under local and worker strategies
  • Detekt 1.23.8 success and expected-failure gates under local and worker strategies
  • bazel build //docs:docs and generated documentation comparison

Copilot AI review requested due to automatic review settings July 4, 2026 06:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR moves several Detekt invocation options (configs, plugins, JVM/language settings, and flags) into the Detekt toolchain so they can be set once and inherited by targets, and updates analysis tests and documentation accordingly.

Changes:

  • Extends detekt_toolchain to carry defaults for cfgs/plugins and several CLI flags (and wires the rule implementation to inherit them).
  • Updates analysis tests to validate toolchain-default forwarding and removal of JVM flag forwarding.
  • Updates README and generated attributes docs to describe toolchain default behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
detekt/toolchain.bzl Adds toolchain attributes for cfgs/plugins and CLI default flags, and exposes the detekt wrapper through the toolchain.
detekt/defs.bzl Inherits cfgs/plugins and several flags from the toolchain when the target leaves attributes at rule defaults.
tests/analysis/tests.bzl Updates existing analysis tests and adds a new test for toolchain-default inheritance.
tests/analysis/BUILD Adds a custom toolchain instance used by analysis tests to validate toolchain defaults.
README.md Documents toolchain defaults and updates JVM target / language version inheritance wording.
docs/attrs.md Updates generated attribute documentation to mention inheritance from the toolchain.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/analysis/tests.bzl Outdated
assert_argv_contains(env, action, "--input")
assert_argv_contains(env, action, _expand_path(ctx, "{{source_dir}}/path A.kt,{{source_dir}}/path B.kt,{{source_dir}}/path C.kt"))

# These values are supplied by custom_defaults_toolchain, not the target.
Comment thread detekt/defs.bzl Outdated
Comment on lines +179 to +180
build_upon_default_config = ctx.attr.build_upon_default_config or detekt_toolchain.build_upon_default_config
if build_upon_default_config:
Comment thread README.md Outdated
Comment on lines 212 to 215
`cfgs`, `plugins`, `build_upon_default_config`, `disable_default_rulesets`, `jvm_target`,
`language_version`, `max_issues`, and `parallel` can be set once on a custom
`detekt_toolchain`. Target attributes use those values when left at their rule defaults:

Comment thread docs/attrs.md Outdated
Comment on lines +13 to +16
| `build_upon_default_config` | [`bool`](https://docs.bazel.build/versions/master/skylark/lib/bool.html) | `False` | Preconfigures detekt with a bunch of rules and some opinionated defaults for you. Defaults to the detekt toolchain value. |
| `cfgs` | [`[Label]`](https://docs.bazel.build/versions/master/skylark/lib/list.html) | `[]` | Path to the config file (path/to/config.yml). Multiple configuration files can be specified. Empty list inherits from the detekt toolchain. |
| `config_resource` | [`string`](https://docs.bazel.build/versions/master/skylark/lib/string.html) | `""` | Path to the config resource on detekt's classpath (path/to/config.yml). |
| `disable_default_rulesets` | [`bool`](https://docs.bazel.build/versions/master/skylark/lib/bool.html) | `False` | Disables default rule sets. |
| `disable_default_rulesets` | [`bool`](https://docs.bazel.build/versions/master/skylark/lib/bool.html) | `False` | Disables default rule sets. Defaults to the detekt toolchain value. |
@Bencodes
Bencodes force-pushed the pull-some-high-level-options-up-into-the-toolchain branch from 7657d67 to 88819be Compare July 22, 2026 14:43
@Bencodes
Bencodes force-pushed the pull-some-high-level-options-up-into-the-toolchain branch 3 times, most recently from 84ffeda to 94b5a27 Compare August 26, 2026 19:16
@Bencodes
Bencodes force-pushed the pull-some-high-level-options-up-into-the-toolchain branch from 94b5a27 to 72ddd79 Compare August 26, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants