diff --git a/.github/workflows/_detekt.yml b/.github/workflows/_detekt.yml new file mode 100644 index 0000000..98a9058 --- /dev/null +++ b/.github/workflows/_detekt.yml @@ -0,0 +1,38 @@ +name: Detekt Static Analysis + +on: + workflow_call: + +permissions: + contents: read + +jobs: + detekt: + name: Run Detekt Analysis + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up JDK 17 + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: 17 + + - name: Cache Gradle + uses: actions/cache@v4 + with: + path: | + ~/.gradle/caches + ~/.gradle/wrapper + key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} + restore-keys: | + ${{ runner.os }}-gradle- + + - name: Grant execute permission for Gradle wrapper + run: chmod +x gradlew + + - name: Run Detekt + run: ./gradlew detekt diff --git a/.github/workflows/_docs.yml b/.github/workflows/_docs.yml index 393165f..76e79f7 100644 --- a/.github/workflows/_docs.yml +++ b/.github/workflows/_docs.yml @@ -47,6 +47,14 @@ jobs: mkdir -p openmapview/build/dokka/html/licenses cp openmapview/build/reports/licenses/licenses.html openmapview/build/dokka/html/licenses/index.html + - name: Run Detekt Analysis + run: ./gradlew detekt --no-daemon + + - name: Copy Detekt Report to Docs + run: | + mkdir -p openmapview/build/dokka/html/detekt + cp openmapview/build/reports/detekt/detekt.html openmapview/build/dokka/html/detekt/index.html + - name: Upload documentation artifact uses: actions/upload-artifact@v4 with: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7d38e1a..ef30def 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,29 +18,33 @@ jobs: name: Check Copyright Headers uses: ./.github/workflows/_copyright.yml + detekt: + name: Static Analysis + uses: ./.github/workflows/_detekt.yml + test: name: Unit Tests - needs: [format, copyright] + needs: [format, copyright, detekt] uses: ./.github/workflows/_test.yml coverage: name: Test Coverage - needs: [format, copyright] + needs: [format, copyright, detekt] uses: ./.github/workflows/_coverage.yml docs: name: Documentation - needs: [format, copyright] + needs: [format, copyright, detekt] uses: ./.github/workflows/_docs.yml build-library: name: Build Library - needs: [format, copyright, test, coverage, docs] + needs: [format, copyright, detekt, test, coverage, docs] uses: ./.github/workflows/_build-library.yml build-examples: name: Build Examples - needs: [format, copyright, test, coverage, docs] + needs: [format, copyright, detekt, test, coverage, docs] uses: ./.github/workflows/_build-examples.yml instrumented-test: diff --git a/.github/workflows/docs-deploy.yml b/.github/workflows/docs-deploy.yml index 0f40e95..acc10f7 100644 --- a/.github/workflows/docs-deploy.yml +++ b/.github/workflows/docs-deploy.yml @@ -6,6 +6,7 @@ on: paths: - 'openmapview/src/**/*.kt' - 'README.md' + - 'config/detekt/detekt.yml' - '.github/workflows/_docs.yml' - '.github/workflows/docs-deploy.yml' diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1423453..b684b2f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -13,25 +13,29 @@ jobs: name: Check Formatting uses: ./.github/workflows/_format.yml + detekt: + name: Static Analysis + uses: ./.github/workflows/_detekt.yml + test: name: Unit Tests - needs: format + needs: [format, detekt] uses: ./.github/workflows/_test.yml build-library: name: Build Library - needs: [format, test] + needs: [format, detekt, test] uses: ./.github/workflows/_build-library.yml build-examples: name: Build Examples - needs: [format, test] + needs: [format, detekt, test] uses: ./.github/workflows/_build-examples.yml publish: name: Publish to Maven Central runs-on: ubuntu-latest - needs: [format, test, build-library, build-examples] + needs: [format, detekt, test, build-library, build-examples] steps: - name: Checkout repository diff --git a/build.gradle.kts b/build.gradle.kts index 46c7cd9..e3b286f 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -24,6 +24,7 @@ plugins { alias(libs.plugins.kotlin.compose) apply false alias(libs.plugins.spotless) apply false alias(libs.plugins.dokka) apply false + alias(libs.plugins.detekt) apply false alias(libs.plugins.nmcp) } @@ -37,3 +38,24 @@ nmcpAggregation { publishAllProjectsProbablyBreakingProjectIsolation() } +subprojects { + afterEvaluate { + if (plugins.hasPlugin("org.jetbrains.kotlin.android")) { + apply(plugin = "io.gitlab.arturbosch.detekt") + + extensions.configure { + buildUponDefaultConfig = true + config.setFrom(files("$rootDir/config/detekt/detekt.yml")) + } + + tasks.withType().configureEach { + reports { + html.required.set(true) + xml.required.set(true) + sarif.required.set(true) + } + } + } + } +} + diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml new file mode 100644 index 0000000..1a4e77c --- /dev/null +++ b/config/detekt/detekt.yml @@ -0,0 +1,63 @@ +build: + maxIssues: 0 + +complexity: + active: true + LongMethod: + threshold: 200 + LongParameterList: + functionThreshold: 10 + constructorThreshold: 12 + CyclomaticComplexMethod: + threshold: 50 + TooManyFunctions: + thresholdInClasses: 100 + thresholdInObjects: 20 + LargeClass: + threshold: 3000 + ComplexCondition: + threshold: 5 + NestedBlockDepth: + threshold: 8 + +style: + active: true + MagicNumber: + active: false + MaxLineLength: + maxLineLength: 140 + ReturnCount: + max: 15 + FunctionOnlyReturningConstant: + active: false + LoopWithTooManyJumpStatements: + active: false + UseRequire: + active: true + UnusedPrivateMember: + active: false + +naming: + active: true + FunctionNaming: + ignoreAnnotated: + - Composable + +empty-blocks: + active: true + +comments: + active: false + +performance: + active: true + +coroutines: + active: true + +exceptions: + active: true + SwallowedException: + active: false + TooGenericExceptionCaught: + active: false diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index c0cf398..6667970 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -51,7 +51,28 @@ Running `./gradlew spotlessApply` automatically adds this header to any files mi The CI pipeline includes a copyright check that will fail if any `.kt` files are missing the required header. -### 3. Dependency Management +### 3. Static Code Analysis + +All code must pass Detekt static analysis checks. Detekt analyzes code for: +- Code complexity and potential bugs +- Naming conventions +- Performance issues +- Code smells + +**Run static analysis:** + +```bash +./gradlew detekt +``` + +The CI pipeline will fail if Detekt finds any issues. View the HTML report at: +``` +openmapview/build/reports/detekt/detekt.html +``` + +The Detekt report is also published at https://afarber.github.io/OpenMapView/detekt/ + +### 4. Dependency Management Before adding new dependencies to OpenMapView, verify the following: @@ -98,8 +119,8 @@ From the repository root, run: This installs a pre-commit hook that automatically: 1. Checks code formatting with `./scripts/check-format.sh` 2. Checks copyright headers with `./scripts/check-copyright.sh` -3. Blocks the commit if any issues are found -4. Provides instructions to run `./gradlew spotlessApply` to fix issues +3. Runs Detekt static analysis with `./scripts/check-detekt.sh` +4. Blocks the commit if any issues are found ### What Happens on Commit @@ -112,6 +133,8 @@ Running pre-commit checks... Code formatting: OK 2. Checking copyright headers... Copyright headers: OK +3. Running Detekt static analysis... + Static analysis: OK All pre-commit checks passed! ``` @@ -190,10 +213,11 @@ The CI pipeline runs automatically on all pull requests and includes: 1. **Format Check** - Verifies Spotless formatting 2. **Copyright Check** - Verifies MIT license headers -3. **Unit Tests** - Runs all JVM unit tests -4. **Test Coverage** - Ensures minimum 20% code coverage -5. **Build Library** - Builds the OpenMapView AAR -6. **Build Examples** - Builds all example applications +3. **Static Analysis** - Runs Detekt code analysis +4. **Unit Tests** - Runs all JVM unit tests +5. **Test Coverage** - Ensures minimum 20% code coverage +6. **Build Library** - Builds the OpenMapView AAR +7. **Build Examples** - Builds all example applications All checks must pass before merging. @@ -206,6 +230,9 @@ All checks must pass before merging. # Check formatting ./gradlew spotlessCheck +# Run static analysis +./gradlew detekt + # Run unit tests ./gradlew :openmapview:test diff --git a/docs/GITHUB_WORKFLOWS.md b/docs/GITHUB_WORKFLOWS.md index 44ffb50..d878414 100644 --- a/docs/GITHUB_WORKFLOWS.md +++ b/docs/GITHUB_WORKFLOWS.md @@ -38,6 +38,13 @@ Located in `.github/workflows/`: - **Usage**: Called by CI and Release workflows - **Fast**: ~30 seconds +#### `_detekt.yml` +- **Purpose**: Run Detekt static code analysis +- **Runs**: `./gradlew detekt` +- **Usage**: Called by CI and Release workflows +- **Duration**: ~1-2 minutes +- **Reports**: HTML, XML, and SARIF reports generated + #### `_coverage.yml` - **Purpose**: Check test coverage meets minimum threshold - **Runs**: `./scripts/check-coverage.sh` (minimum threshold defined in [Contributing Guide](CONTRIBUTING.md#test-coverage)) @@ -74,11 +81,15 @@ Located in `.github/workflows/`: - **Duration**: ~10-15 minutes (may vary due to emulator startup) #### `_docs.yml` -- **Purpose**: Build API documentation with Dokka -- **Runs**: `./gradlew dokkaGenerate` -- **Artifacts**: Uploads generated HTML documentation +- **Purpose**: Build API documentation with Dokka, license report, and Detekt report +- **Runs**: `./gradlew dokkaGenerate`, `./gradlew generateLicenseReport`, `./gradlew detekt` +- **Artifacts**: Uploads generated HTML documentation with license and Detekt reports - **Usage**: Called by CI for validation and docs-deploy for publishing -- **Duration**: ~1-2 minutes +- **Duration**: ~2-3 minutes +- **Published URLs**: + - API docs: https://afarber.github.io/OpenMapView/ + - Licenses: https://afarber.github.io/OpenMapView/licenses/ + - Detekt: https://afarber.github.io/OpenMapView/detekt/ ### Main Workflows @@ -89,41 +100,26 @@ Located in `.github/workflows/`: **Execution Flow**: ``` -format (Check formatting) - | - v -copyright (Check headers) - | - v - +----------------+----------------+----------------+ - | | | - v v v -test docs coverage -(Unit tests) (Build API docs) (Test coverage) - | | | - +----------------+----------------+----------------+ - | - +--------------------------+-------------------------+ - | | - v v -build-library build-examples -(Build AAR) (Build 3 APKs) +format ────┐ + │ +copyright ─┼──→ test/coverage/docs ──→ build-library + │ │ +detekt ────┘ └──→ build-examples For Pull Requests only: - v -instrumented-test (Phone + Automotive) -(Runs in parallel, not blocking) +instrumented-test (Phone + Automotive) runs in parallel ``` **Jobs**: 1. **format** - Runs Spotless formatting check 2. **copyright** - Verifies MIT license headers -3. **test** - Runs unit tests (depends on format + copyright) -4. **docs** - Builds API documentation with Dokka (depends on format + copyright) -5. **coverage** - Checks test coverage meets 20% minimum (depends on format + copyright) -6. **build-library** - Builds library AAR (depends on format + copyright + test + docs + coverage, runs in parallel with build-examples) -7. **build-examples** - Builds example APKs (depends on format + copyright + test + docs + coverage, runs in parallel with build-library) -8. **instrumented-test** - Runs instrumentation tests on phone and automotive emulators (PR only, required status check) +3. **detekt** - Runs Detekt static code analysis +4. **test** - Runs unit tests (depends on format + copyright + detekt) +5. **docs** - Builds API documentation with Dokka (depends on format + copyright + detekt) +6. **coverage** - Checks test coverage meets 20% minimum (depends on format + copyright + detekt) +7. **build-library** - Builds library AAR (depends on all above, runs in parallel with build-examples) +8. **build-examples** - Builds example APKs (depends on all above, runs in parallel with build-library) +9. **instrumented-test** - Runs instrumentation tests on phone and automotive emulators (PR only) **Total Duration**: - **Maintainer pushes to main**: ~3-4 minutes (no instrumentation tests) @@ -143,30 +139,19 @@ instrumented-test (Phone + Automotive) **Execution Flow**: ``` -format (Check formatting) - | - v -test (Run unit tests) - | - v - +----------------+----------------+ - | | - v v -build-library build-examples - | | - +-----------------+---------------+ - | - v - publish - (Maven Central + GitHub Release) +format ────┐ + ├──→ test ──→ build-library ──┐ +detekt ────┘ │ ├──→ publish + └──→ build-examples ──┘ ``` **Jobs**: 1. **format** - Runs Spotless formatting check -2. **test** - Runs unit tests (depends on format) -3. **build-library** - Builds library AAR (depends on format + test) -4. **build-examples** - Builds example APKs (depends on format + test) -5. **publish** - Publishes to Maven Central and creates GitHub Release (depends on all above) +2. **detekt** - Runs Detekt static code analysis +3. **test** - Runs unit tests (depends on format + detekt) +4. **build-library** - Builds library AAR (depends on format + detekt + test) +5. **build-examples** - Builds example APKs (depends on format + detekt + test) +6. **publish** - Publishes to Maven Central and creates GitHub Release (depends on all above) **Publish Job Details**: - Validates tag format (must be `vMAJOR.MINOR.PATCH`) @@ -310,6 +295,9 @@ Before pushing, developers can run the same checks locally: # Auto-fix formatting ./gradlew spotlessApply +# Static analysis +./gradlew detekt + # Run unit tests ./gradlew :openmapview:test @@ -375,6 +363,7 @@ Artifacts are retained for 30 days. **Common Failures**: - **Format check fails**: Run `./gradlew spotlessApply` locally and commit +- **Detekt fails**: Run `./gradlew detekt` locally to see issues - **Test fails**: Run `./gradlew :openmapview:test` locally to debug - **Build fails**: Check for compilation errors in logs - **Publish fails**: Verify GitHub Secrets are configured correctly @@ -390,34 +379,19 @@ Developer creates PR with code changes GitHub triggers ci.yml workflow | v -format job: Check Spotless formatting (30 sec) - PASS +format/copyright/detekt jobs run in parallel (30-60 sec each) - PASS | v -copyright job: Check license headers (30 sec) - PASS +test/coverage/docs jobs run (1-2 min each) - PASS + | + v +build-library + build-examples run in parallel (2-3 min) - PASS | v - +-------------+--------------+ - | | - v v -test job: Unit tests coverage job: Test coverage - (1 min) - PASS (1 min) - PASS (>20%) - | | - +-------------+--------------+ - | - +-------------+--------------+ - | | - v v -build-library: Build AAR build-examples: Build 3 APKs - (2 min) - PASS (3 min) - PASS - | | - +-------------+--------------+ - | - v All core checks pass (3-4 min) Meanwhile (in parallel, PR only): -instrumented-test: Phone + Automotive tests - (10-15 min) - PASS +instrumented-test: Phone + Automotive tests (10-15 min) - PASS | v All checks complete -> PR is ready to merge @@ -432,21 +406,15 @@ Developer tags v0.2.0 and pushes GitHub triggers release.yml workflow | v -format job: Check Spotless formatting - PASS +format + detekt jobs run in parallel - PASS | v test job: Run unit tests - PASS | v - +-------------+--------------+ - | | - v v -build-library: Build AAR build-examples: Build 3 APKs - - PASS - PASS - | | - +-------------+--------------+ - | - v +build-library + build-examples run in parallel - PASS + | + v publish job: - Validate tag format - PASS - Sign artifacts with GPG - PASS diff --git a/examples/Example11MapTypes/src/main/kotlin/de/afarber/openmapview/example11maptypes/MainActivity.kt b/examples/Example11MapTypes/src/main/kotlin/de/afarber/openmapview/example11maptypes/MainActivity.kt index 4d6cc21..591cf62 100644 --- a/examples/Example11MapTypes/src/main/kotlin/de/afarber/openmapview/example11maptypes/MainActivity.kt +++ b/examples/Example11MapTypes/src/main/kotlin/de/afarber/openmapview/example11maptypes/MainActivity.kt @@ -30,7 +30,6 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalConfiguration -import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp @@ -57,7 +56,6 @@ class MainActivity : ComponentActivity() { @Composable fun MapViewScreen() { - val context = LocalContext.current val lifecycleOwner = androidx.lifecycle.compose.LocalLifecycleOwner.current val configuration = LocalConfiguration.current var currentMapType by remember { mutableStateOf(MapType.STANDARD) } @@ -287,7 +285,8 @@ fun MapTypeControls( // API Key Info Text( - text = "API key required: Cycle Map, Transport, Transport Dark, Tracestrack Topo\n\nConfigure in AndroidManifest.xml\nSee docs/API_KEYS.md", + text = "API key required: Cycle Map, Transport, Transport Dark, " + + "Tracestrack Topo\n\nConfigure in AndroidManifest.xml\nSee docs/API_KEYS.md", style = MaterialTheme.typography.bodySmall, fontSize = 9.sp, lineHeight = 11.sp, diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 6b6c129..758026a 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -36,6 +36,7 @@ kotlinx-coroutines = "1.10.2" # Code Quality ktlint = "1.3.1" license-report = "3.0.1" +detekt = "1.23.7" [libraries] # AndroidX Core @@ -80,6 +81,7 @@ spotless = { id = "com.diffplug.spotless", version.ref = "spotless" } dokka = { id = "org.jetbrains.dokka", version.ref = "dokka" } nmcp = { id = "com.gradleup.nmcp.aggregation", version.ref = "nmcp" } license-report = { id = "com.github.jk1.dependency-license-report", version.ref = "license-report" } +detekt = { id = "io.gitlab.arturbosch.detekt", version.ref = "detekt" } [bundles] # Common dependencies for example apps diff --git a/openmapview/src/main/kotlin/de/afarber/openmapview/MapController.kt b/openmapview/src/main/kotlin/de/afarber/openmapview/MapController.kt index 4c47a32..ba56c82 100644 --- a/openmapview/src/main/kotlin/de/afarber/openmapview/MapController.kt +++ b/openmapview/src/main/kotlin/de/afarber/openmapview/MapController.kt @@ -213,13 +213,6 @@ class MapController( strokeWidth = 2f } - private val tileTextPaint = - Paint().apply { - color = Color.BLACK - textSize = 24f - textAlign = Paint.Align.CENTER - } - private val tilePlaceholderPaint = Paint().apply { style = Paint.Style.FILL diff --git a/openmapview/src/main/kotlin/de/afarber/openmapview/OpenMapView.kt b/openmapview/src/main/kotlin/de/afarber/openmapview/OpenMapView.kt index 7fd59c5..69cbc61 100644 --- a/openmapview/src/main/kotlin/de/afarber/openmapview/OpenMapView.kt +++ b/openmapview/src/main/kotlin/de/afarber/openmapview/OpenMapView.kt @@ -305,7 +305,7 @@ class OpenMapView } } - val consumed = controller.onMarkerClickListener?.onMarkerClick(touchedMarker) ?: false + controller.onMarkerClickListener?.onMarkerClick(touchedMarker) controller.commitPan() invalidate() return true @@ -500,10 +500,7 @@ class OpenMapView * @see getMapType */ fun setMapType(type: Int) { - // Validate map type - if (type !in 0..14) { - throw IllegalArgumentException("Unknown map type: $type. Must be a valid MapType constant (0-14).") - } + require(type in 0..14) { "Unknown map type: $type. Must be a valid MapType constant (0-14)." } currentMapType = type controller.setMapType(type) diff --git a/openmapview/src/test/kotlin/de/afarber/openmapview/MapControllerTest.kt b/openmapview/src/test/kotlin/de/afarber/openmapview/MapControllerTest.kt index 78490da..59cf063 100644 --- a/openmapview/src/test/kotlin/de/afarber/openmapview/MapControllerTest.kt +++ b/openmapview/src/test/kotlin/de/afarber/openmapview/MapControllerTest.kt @@ -86,7 +86,6 @@ class MapControllerTest { @Test fun testZoom_BeyondMaxLimit() { controller.setZoom(18.0f) - val oldZoom = controller.getZoom() controller.zoom(5.0f, 540f, 960f) assertEquals(MapController.DEFAULT_MAX_ZOOM, controller.getZoom(), DELTA) } diff --git a/scripts/check-detekt.sh b/scripts/check-detekt.sh new file mode 100755 index 0000000..5f5d974 --- /dev/null +++ b/scripts/check-detekt.sh @@ -0,0 +1,19 @@ +#!/bin/bash + +# Check code quality with Detekt static analysis +# Returns 0 if no issues found, 1 if issues are found + +echo "Running Detekt static analysis..." + +./gradlew detekt + +if [ $? -ne 0 ]; then + echo "" + echo "Detekt static analysis failed!" + echo "Run './gradlew detekt' to see details." + echo "" + exit 1 +fi + +echo "Static analysis passed." +exit 0 diff --git a/scripts/setup-git-hooks.sh b/scripts/setup-git-hooks.sh index af5c22c..2f332e6 100755 --- a/scripts/setup-git-hooks.sh +++ b/scripts/setup-git-hooks.sh @@ -49,6 +49,20 @@ if [ $? -ne 0 ]; then fi echo " Copyright headers: OK" + +# Check detekt static analysis +echo "3. Running Detekt static analysis..." +./scripts/check-detekt.sh > /dev/null 2>&1 + +if [ $? -ne 0 ]; then + echo "" + echo "Detekt static analysis failed!" + echo "Run './gradlew detekt' to see issues." + echo "" + exit 1 +fi + +echo " Static analysis: OK" echo "" echo "All pre-commit checks passed!" EOF @@ -61,6 +75,6 @@ echo "" echo "Pre-commit hook will now:" echo " - Check code formatting before each commit" echo " - Check copyright headers on all Kotlin files" +echo " - Run Detekt static analysis" echo " - Block commits if issues are found" -echo " - Prompt to run './gradlew spotlessApply' to fix issues" echo ""