Add Detekt static analysis - #7209
Conversation
Configure Detekt across the main and included builds with adoption baselines, SARIF reporting, IDE support, CI gating, and dependency locks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20d03f8d-bfa8-4db2-a351-643d46b37c80
There was a problem hiding this comment.
Pull request overview
Adds Detekt static analysis to the Home Assistant Android Gradle build (including the included build-logic build), aiming to enforce new violations in CI while capturing existing findings in baselines. It also integrates SARIF output into GitHub code scanning and adds an Android Studio run configuration.
Changes:
- Add Detekt Gradle plugin wiring (with HTML + SARIF reporting) and configure per-project baselines.
- Add a dedicated GitHub Actions job to run Detekt and upload SARIF results to code scanning.
- Update Gradle dependency lockfiles to include Detekt and its transitive dependencies; add IDE run configuration.
Show a summary per file
| File | Description |
|---|---|
| build.gradle.kts | Applies/configures Detekt (and reporting) across all projects and sets baseline naming. |
| build-logic/convention/build.gradle.kts | Applies/configures Detekt inside the included build-logic build with per-project baselines. |
| gradle/libs.versions.toml | Adds Detekt version and plugin alias to the version catalog. |
| .github/workflows/pr.yml | Adds a Detekt CI job and wires downstream jobs to depend on it; uploads SARIF to code scanning. |
| .idea/runConfigurations/Detekt.xml | Adds an Android Studio Gradle run configuration for Detekt. |
| gradle.lockfile | Updates root dependency lock to include Detekt configurations/artifacts. |
| app/gradle.lockfile | Updates :app dependency lock for Detekt configurations/artifacts. |
| common/gradle.lockfile | Updates :common dependency lock for Detekt configurations/artifacts. |
| wear/gradle.lockfile | Updates :wear dependency lock for Detekt configurations/artifacts. |
| testing-unit/gradle.lockfile | Updates :testing-unit dependency lock for Detekt configurations/artifacts. |
| microwakeword/gradle.lockfile | Updates :microwakeword dependency lock for Detekt configurations/artifacts. |
| provides-sensor-processor/gradle.lockfile | Updates :provides-sensor-processor dependency lock for Detekt configurations/artifacts. |
| config/detekt/baseline-common.xml | Adds Detekt baseline for the :common module. |
| config/detekt/baseline-wear.xml | Adds Detekt baseline for the :wear module. |
| config/detekt/baseline-testing-unit.xml | Adds Detekt baseline for the :testing-unit module. |
| config/detekt/baseline-provides-sensor-processor.xml | Adds Detekt baseline for the :provides-sensor-processor module. |
| config/detekt/baseline-lint.xml | Adds Detekt baseline for the :lint module. |
| config/detekt/baseline-build-logic-convention.xml | Adds Detekt baseline for the build-logic:convention project. |
Review details
Files not reviewed (1)
- .idea/runConfigurations/Detekt.xml: Generated file
- Files reviewed: 17/20 changed files
- Comments generated: 1
- Review effort level: Low
TimoPtr
left a comment
There was a problem hiding this comment.
Thanks for your work. I wonder how it conflicts with the rules of ktlint, did you look a bit more to the issues and see something that KTlint accept that Detekt reject?
I would like you to add some example of what we gain from this in your PR description.
You also need to make a PR to add detekt in https://developers.home-assistant.io/docs/android/codestyle
| core-splashscreen = "1.2.0" | ||
| core-remoteviews = "1.1.0" | ||
| cronet = "143.7445.0" | ||
| detekt = "2.0.0-alpha.5" |
There was a problem hiding this comment.
I would like to wait for a stable version before merging this.
There was a problem hiding this comment.
Would you be open to reconsidering this? The Detekt maintainers have said there is no ETA for 2.0, and the milestone currently has no due date. Stable 1.23.8 targets Kotlin 2.0.21 and AGP 8.8.1, while 2.0.0-alpha.5 targets Kotlin 2.4.0 and AGP 9.2.1, which closely match this project. Since the version is pinned and Detekt is build-only tooling validated in CI, the risk should be contained. We can still leave the PR draft if stable remains a hard requirement.
There was a problem hiding this comment.
We could consider but it needs to be justified by what it brings to us in terms of rules.
There was a problem hiding this comment.
The main value beyond ktlint is detecting correctness and maintainability risks rather than formatting: 21 swallowed exceptions that can hide failures, one printStackTrace that bypasses project logging, and hundreds of complexity findings that identify methods and classes that are harder to test and safely modify. After removing rules that conflict with current policy, 641 existing findings remain, including 114 long methods, 83 cyclomatically complex methods, and 56 functions with excessive returns. New instances of those patterns would fail CI while existing code can be improved incrementally.
| } | ||
|
|
||
| detekt { | ||
| baseline = rootProject.file("config/detekt/baseline-$detektBaselineName.xml") |
There was a problem hiding this comment.
In the initial PR I did I added basic rule file https://github.com/home-assistant/android/pull/5312/changes#diff-ea29de1180303564fca0c4eddb30f7da4b596754f2c139fc252ae290e3d201e2 to match Ktlint line length constraint. Having the file is important to override some rules that we don't want to apply to the project and I think setting up this file should be part of this PR.
There was a problem hiding this comment.
Added config/detekt/detekt.yml, wired it into both Gradle builds, and enabled buildUponDefaultConfig. The initial override keeps MaxLineLength aligned with ktlint at 120 characters while retaining Detekt defaults for the other rules.
There was a problem hiding this comment.
After comparing the actual findings with ktlint and current project conventions, the shared configuration now disables MaxLineLength, FunctionNaming, and TooGenericExceptionCaught while building on the remaining Detekt defaults.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20d03f8d-bfa8-4db2-a351-643d46b37c80
Move Detekt baselines into their modules and disable rules that overlap with ktlint or conflict with current project conventions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20d03f8d-bfa8-4db2-a351-643d46b37c80
I'm generally in favour of tools like this, but liked the simplicity of ktlint with auto-fix and do hope it's not too big of a burden for new contributors. I don't have any personal experience with it so cannot comment on it. Checking their documentation I see this page about tweaks to the configuration when using Compose: https://detekt.dev/docs/introduction/compose/. Any reason not to include this? Also in their documentation there is a Ktlint rule set, should we replace the existing check in CI with it? https://detekt.dev/docs/rules/ktlint (but "Issues reported by this rule set can only be suppressed on file level " does sound quite limiting) |
Run Detekt against each production source set so rules requiring type resolution are enforced. Apply targeted Compose exclusions and baseline the existing variant-specific findings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20d03f8d-bfa8-4db2-a351-643d46b37c80
|
Thanks for pointing these out. I checked each recommendation in the Compose guide against Detekt 2.0.0-alpha.5 and type-resolved output. The configuration now ignores I kept the standalone ktlint plugin and CI check. Detekt’s ktlint rules would duplicate that path, couple formatting to the alpha Detekt version, and only support file-level suppression. Ktlint therefore remains the formatting authority, while Detekt covers correctness and maintainability rules. CI now uses Detekt’s documented type-resolved |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 20d03f8d-bfa8-4db2-a351-643d46b37c80
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- .idea/runConfigurations/Detekt.xml: Generated file
Suppressed comments (2)
config/detekt/detekt.yml:16
- The PR description says
FunctionNamingis disabled, but this config only ignores functions annotated with@Composable(so the rule still runs for everything else). Please either setactive: falsehere or update the PR description to match the intended behavior.
naming:
FunctionNaming:
ignoreAnnotated:
- Composable
.github/workflows/pr.yml:167
- This job-level
permissionsblock overrides the workflow-levelcontents: readpermission. To keep checkout working consistently (including in private forks) while still being restrictive, includecontents: readalongsidesecurity-events: write.
permissions:
security-events: write
steps:
- Files reviewed: 35/37 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 20d03f8d-bfa8-4db2-a351-643d46b37c80
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- .idea/runConfigurations/Detekt.xml: Generated file
Suppressed comments (2)
build-logic/convention/build.gradle.kts:9
- Detekt is applied twice in this build: once via
alias(libs.plugins.detekt)in theplugins {}block and again viaapply(plugin = ...)inallprojects {}. To avoid redundant application (and match the style used for other plugins), declare it withapply(false)in the plugins block.
plugins {
`kotlin-dsl`
alias(libs.plugins.detekt)
alias(libs.plugins.ktlint)
}
gradle/libs.versions.toml:45
- PR description says the repo is on AGP 9.2.1, but
androidGradlePluginin this file is 9.3.1. If the Detekt version rationale depends on matching AGP, the PR description should be updated to reflect the current toolchain (or the rationale adjusted).
coreKtx = "1.19.0"
core-splashscreen = "1.2.0"
core-remoteviews = "1.1.0"
cronet = "143.7445.0"
detekt = "2.0.0-alpha.5"
emojiJava = "5.1.1"
firebase-bom = "34.16.0"
firebaseAppdistributionGradle = "5.3.0"
- Files reviewed: 35/37 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20d03f8d-bfa8-4db2-a351-643d46b37c80
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- .idea/runConfigurations/Detekt.xml: Generated file
Suppressed comments (2)
config/detekt/detekt.yml:23
UnusedPrivateFunction.ignoreAnnotatedonly ignores@Preview, but this repo uses@PreviewLightDarkand@HAPreviewsfor preview composables. New preview helpers annotated with those will be reported as unused private functions, which is likely not intended for preview-only code.
Include the preview annotations used in this repo in the ignore list.
UnusedPrivateFunction:
ignoreAnnotated:
- Preview
config/detekt/detekt.yml:7
TooManyFunctions.ignoreAnnotatedFunctionsonly ignores@Preview, but this repo widely uses@PreviewLightDarkand@HAPreviewsfor previews. Those preview functions will still count toward theTooManyFunctionsthreshold, creating noisy failures for future preview additions.
Add the preview annotations used in this codebase to the ignore list.
This issue also appears on line 21 of the same file.
TooManyFunctions:
ignoreAnnotatedFunctions:
- Preview
- Files reviewed: 35/37 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20d03f8d-bfa8-4db2-a351-643d46b37c80
Summary
Adds Detekt static analysis across the main Gradle build and the included build logic. Module-local baselines capture the 641 existing findings, allowing CI to reject new violations without requiring unrelated cleanup in this PR. Detekt produces HTML and SARIF reports, uploads CI findings to code scanning, and includes an Android Studio run configuration.
Ktlint remains responsible for formatting and line length. The project Detekt configuration disables
MaxLineLengthandTooGenericExceptionCaught(which does not match the repository's current exception-handling convention), while applying targeted Compose exemptions toFunctionNaming,LongParameterList,TooManyFunctions, andUnusedPrivateFunction. Detekt still adds checks that ktlint does not provide. The current baseline includes 189 magic numbers, 114 long methods, 83 cyclomatically complex methods, 70 classes or objects with too many functions, 56 functions with excessive returns, 21 swallowed exceptions, and oneprintStackTracecall.The integration uses pinned Detekt 2.0.0-alpha.5 because it targets Kotlin 2.4.0 and Android Gradle Plugin 9.2.1, closely matching this repository's Kotlin 2.4.10 and AGP 9.3.1. Stable Detekt 1.23.8 instead targets Kotlin 2.0.21 and AGP 8.8.1, and Detekt has not announced a stable 2.0 release date. As build-only tooling validated in CI, the alpha does not affect the shipped application.
Closes #5135.
Checklist
Select exactly one option that describes AI usage in this contribution:
Screenshots
N/A — tooling-only change.
Link to pull request in documentation repositories
User Documentation: N/A
Developer Documentation: home-assistant/developers.home-assistant#3266
Any other notes
The setup follows the direction of the earlier closed #5312 while updating it for the current build toolchain.