diff --git a/.githooks/pre-push b/.githooks/pre-push new file mode 100755 index 0000000..b2445da --- /dev/null +++ b/.githooks/pre-push @@ -0,0 +1,13 @@ +#!/usr/bin/env bash +# +# The same gate CI runs, minus the release build, before a push leaves the machine. +# +# Install once per clone (hooks are not committed as active hooks on purpose — a repository should +# not be able to run code on your machine without you saying so): +# +# git config core.hooksPath .githooks +# +# In an emergency: git push --no-verify +set -euo pipefail + +exec "$(git rev-parse --show-toplevel)/tools/verify.sh" diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml new file mode 100644 index 0000000..2ca0c1d --- /dev/null +++ b/.github/workflows/verify.yml @@ -0,0 +1,54 @@ +name: verify + +on: + push: + branches: [main] + pull_request: + schedule: + # Nightly, because the release build is the one check nothing else performs: R8 has already + # hidden a real defect here once (a video decoder reached only through a ServiceLoader entry), + # and it was invisible to the unit tests, to lint and to the debug APK. + - cron: '17 3 * * *' + workflow_dispatch: + +concurrency: + group: verify-${{ github.ref }} + cancel-in-progress: true + +jobs: + gate: + if: github.event_name != 'schedule' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 + with: + distribution: temurin + # detekt 1.23 cannot start on JDK 25 (it fails to parse the version string), so the whole + # job stays on 21. The sources still target Java 17, which is what compileOptions asks for. + java-version: '21' + - uses: android-actions/setup-android@v3 + - uses: gradle/actions/setup-gradle@v4 + - run: ./tools/verify.sh + + release: + if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: '21' + - uses: android-actions/setup-android@v3 + - uses: gradle/actions/setup-gradle@v4 + - run: ./tools/verify.sh --release + # The mapping is what turns a stack trace from a release crash into a line of Kotlin. It is + # only worth keeping if it belongs to the build that shipped, which is why it is uploaded + # rather than committed. + - uses: actions/upload-artifact@v4 + if: always() + with: + name: r8-mapping + path: app/build/outputs/mapping/release/mapping.txt + if-no-files-found: ignore diff --git a/AGENTS.md b/AGENTS.md index 691157b..6b9ad3e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,17 +22,22 @@ feature name once you know what you are changing. ## Commands ```bash -./gradlew :app:testDebugUnitTest # the gate: 355 JVM tests -./gradlew :app:lintDebug # the gate: Android Lint -./gradlew :app:assembleDebug # then: adb install -r app/build/outputs/apk/debug/app-debug.apk -./gradlew :app:assembleRelease # R8 is on here, and it has hidden real bugs — see Traps +./tools/verify.sh # the gate: map check, detekt, tests, lint +./tools/verify.sh --release # ...and an R8 release build (what CI runs nightly) +GRADLE_FLAGS=--offline ./tools/verify.sh # when the network is unavailable -python3 tools/make-code-map.py # regenerate docs/CODE_MAP.md after touching source -python3 tools/make-code-map.py --check # fails when the map is stale +./gradlew :app:assembleDebug # then: adb install -r app/build/outputs/apk/debug/app-debug.apk +./gradlew :app:detektBaseline # re-freezes the findings that exist now — think before running it +python3 tools/make-code-map.py # regenerate docs/CODE_MAP.md after touching source ``` -There is **no CI**: those two Gradle tasks are the gate, and whoever changes the code runs them. -Add `--offline` when the network is unavailable — the dependency cache is already warm. +CI (`.github/workflows/verify.yml`) runs the same script on every push and pull request, plus a nightly +`--release` build, because R8 is the one check nothing else performs and it has hidden a real defect +here before. `git config core.hooksPath .githooks` runs it before every push as well. + +The detekt gap it leaves — the findings in `config/detekt/baseline.xml` — is a **ratchet**: new +findings fail the build, existing ones are parked until the file they live in is touched for another +reason. `tools/` holds the gate and the generators; `config/detekt/` holds the rules and that baseline. ## Where things live, and the one rule about direction @@ -92,6 +97,13 @@ comment. `app/src/test/java/…/ProjectInvariantsTest.kt` holds the source-level - **The debug build is a separate package** (`…filemanager.debug`), so it installs beside a release build and the two can be compared directly. +- **detekt 1.23 cannot run on JDK 25.** It parses the version string of the JVM it runs on and + throws `25.0.4.1` as the entire error message, which is why `:app:detekt` drives the CLI on a 21 + launcher. A bare version number as a build failure is this, not the code — and a **JDK 21 has to be + installed** for the toolchain to find: Gradle does not download one here. +- **A detekt baseline has to exist before it can be read.** It starts as an empty document on + purpose; `--create-baseline` cannot conjure the path it was told to consult. + ## Documentation map — read on demand | file | what it is for | @@ -106,7 +118,7 @@ comment. `app/src/test/java/…/ProjectInvariantsTest.kt` holds the source-level ## Definition of done -1. `:app:testDebugUnitTest` and `:app:lintDebug` green. +1. `./tools/verify.sh` green — the map current, detekt, the unit tests and lint in one command. 2. `python3 tools/make-code-map.py` run, so that `--check` passes. 3. A `CHANGELOG.md` entry in the house voice: what changed, why, what was rejected, and what was verified rather than assumed. diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f858ff..5dc8c63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -125,6 +125,39 @@ upload needs a **higher `versionCode`** than every previous upload. home screen lists it above the libraries; the row opens it with the star already filled; and removing it from the row takes the whole section away with a snackbar. +- **One command is now the gate, and Kotlin static analysis is part of it.** `tools/verify.sh` runs the + code-map check, detekt, the unit tests and lint — cheapest first, so a stale document fails in a + second rather than after a release build — with `--release` adding the R8 build. The same script + runs in `.github/workflows/verify.yml` on every push and pull request, nightly with `--release`, and + in a pre-push hook once `git config core.hooksPath .githooks` has been run. Until now every + guarantee was voluntary: 371 tests, lint and the map ran only if someone remembered, which was the + largest maintenance risk this repository had. + + **Detekt is adopted with a baseline, and that is the whole point**: everything it finds today is + frozen in `config/detekt/baseline.xml`, so the gate fails on a *new* finding rather than on a + backlog nobody is going to clear this week. Entries are keyed by rule, file and enclosing + declaration, so a new long function is still reported inside a file that already has one — proved + by writing a `TODO` into a file, watching the build fail on that line, and removing it. What it + found first is what the churn numbers had already said: `ArchiveEngine` 16 findings, + `BrowserViewModel` 11, `SelectionBar` 9 — 86 over 13 rules, the largest groups being the complexity + ones this was added for (cyclomatic complexity 10, nested depth 7, too many functions 4, long + methods 3, two classes over 700 lines) and 30 over-long lines. Three findings were fixed rather + than parked: dead `menuOpen` state in `BrowserScreen`, a missing trailing newline, and two `db` + parameters that are the platform's signature rather than an oversight (suppressed, with the + reason). Four rules were configured rather than obeyed: `PackageNaming`, because the + applicationId's underscore is an identity this app cannot change; `FunctionNaming` for `@Composable` + (PascalCase by convention) and for backticked test names; `MagicNumber`, because sizes and limits + are named constants here; and `MatchingDeclarationName`, because a file is named after its concept. + + Two tooling facts are recorded because they cost time and are in neither tool's documentation. + Detekt runs through its **CLI** rather than its Gradle plugin: the plugin performs the analysis + inside the build's own JVM, and detekt 1.23 cannot parse JDK 25's version string — it fails with + `25.0.4.1` as the entire error message — so `:app:detekt` drives `detekt-cli` on a 21 launcher, + which is also the only way to pin the JVM the analyser runs on. And a baseline has to *exist* + before it can be read, which is why it starts as an empty document instead of being generated into + place. **371 JVM tests** still pass, lint is clean, the map is current, and the gate is green in + 17 seconds. + ### Changed - **File thumbnails are bigger.** The leading visual in a list row went from 28 dp to 48 dp and diff --git a/README.md b/README.md index ba9e7d2..193ec22 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,8 @@ before spending an hour on them. Prerequisites: - JDK 17+ (JDK 25 verified) +- A **JDK 21** as well: detekt 1.23 cannot run on 25, so the analyser is pinned to a 21 toolchain and + Gradle will not download one. - Android SDK with `platforms;android-36` and `build-tools;36.0.0` ```bash @@ -88,8 +90,17 @@ echo "sdk.dir=$HOME/Android/Sdk" > local.properties ./gradlew :app:testDebugUnitTest # JVM unit tests for the engine ./gradlew :app:lintDebug # Android Lint ./gradlew :app:bundleRelease # Play upload bundle (.aab) + +# Everything the gate requires, in one command — what CI and the pre-push hook run: +./tools/verify.sh ``` +The gate is the code-map check, [detekt](https://detekt.dev) against a frozen baseline, the unit tests +and Android Lint, cheapest first; `--release` adds an R8 build. CI +(`.github/workflows/verify.yml`) runs it on every push and pull request and nightly with `--release`, +because R8 is the one check that has caught a defect nothing else could. To run it before pushing: +`git config core.hooksPath .githooks`. + Release signing is optional and driven by a git-ignored `keystore.properties` (see `keystore.properties.example`); without it the release build still runs R8/minification but produces an unsigned artefact. diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 69cbcd0..4086988 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -1,4 +1,6 @@ import java.util.Properties +import org.gradle.api.artifacts.Configuration +import org.gradle.api.tasks.JavaExec import org.gradle.api.tasks.testing.Test plugins { @@ -6,6 +8,11 @@ plugins { alias(libs.plugins.kotlin.compose) } +/* + * Static analysis for the Kotlin side of the app (Android Lint covers the rest) — see the two tasks + * at the bottom of this file. Nothing to configure here: the CLI is driven by them. + */ + // Optional release signing. // // Create a `keystore.properties` file in the project ROOT (it is git-ignored; see @@ -197,3 +204,72 @@ dependencies { // Lets a ViewModel test drive `viewModelScope` on a deterministic dispatcher. testImplementation(libs.kotlinx.coroutines.test) } + +// ---------------------------------------------------------------- static analysis + +/** The detekt CLI, on a configuration of its own so it never becomes an app dependency. */ +val detektCli: Configuration by configurations.creating { + isCanBeConsumed = false +} + +dependencies { + detektCli(libs.detekt.cli) +} + +/** + * Command line shared by [detekt] and [detektBaseline]. + * + * The CLI rather than the Gradle plugin, for one concrete reason: the plugin runs the analysis + * inside the build's own JVM, and detekt 1.23 cannot make sense of the version string of the JDK 25 + * this project builds with — it fails with `25.0.4.1` as the entire error message. Running the CLI + * through [JavaExec] also lets the analyser run on a 21 launcher, which it does understand. + */ +fun detektArguments(vararg extra: String): List = listOf( + "--input", "src/main/java,src/test/java", + "--config", rootProject.file("config/detekt/detekt.yml").absolutePath, + // The sources use Java 17 language features, which is what compileOptions asks for as well. + "--jvm-target", "17", + // config/detekt/detekt.yml only lists overrides; everything else is detekt's default. + "--build-upon-default-config", + "--parallel", +) + extra + +val detektJava = javaToolchains.launcherFor { + languageVersion.set(org.gradle.jvm.toolchain.JavaLanguageVersion.of(21)) +} + +val detektReports = layout.buildDirectory.dir("reports/detekt") + +/** + * Kotlin static analysis, failing on any finding not frozen in `config/detekt/baseline.xml`. + * + * The baseline is the whole adoption strategy: everything detekt finds today is parked there, so the + * gate fails on a *new* finding rather than on a backlog nobody is going to clear this week. Entries + * are keyed by rule, file and enclosing declaration, so a new long function is still reported even in + * a file that already has one. + */ +val detekt by tasks.registering(JavaExec::class) { + group = "verification" + description = "Kotlin static analysis against config/detekt, minus the baseline." + classpath = detektCli + mainClass.set("io.gitlab.arturbosch.detekt.cli.Main") + javaLauncher.set(detektJava) + args = detektArguments( + "--baseline", rootProject.file("config/detekt/baseline.xml").absolutePath, + "--report", "html:" + detektReports.get().file("detekt.html").asFile.absolutePath, + "--report", "sarif:" + detektReports.get().file("detekt.sarif").asFile.absolutePath, + ) +} + +/** Rewrites the baseline with whatever detekt finds right now. Run it deliberately, not to go green. */ +val detektBaseline by tasks.registering(JavaExec::class) { + group = "verification" + description = "Rewrites config/detekt/baseline.xml with the findings that exist now." + classpath = detektCli + mainClass.set("io.gitlab.arturbosch.detekt.cli.Main") + javaLauncher.set(detektJava) + args = detektArguments( + "--baseline", rootProject.file("config/detekt/baseline.xml").absolutePath, + "--create-baseline", + ) +} diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/data/SqliteBookmarkStore.kt b/app/src/main/java/de/softwareapp_hb/filemanager/data/SqliteBookmarkStore.kt index bec93b2..fe3da1b 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/data/SqliteBookmarkStore.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/data/SqliteBookmarkStore.kt @@ -104,6 +104,7 @@ class SqliteBookmarkStore(context: Context) : BookmarkStore { * is not a migration — which is why a version with no step reports itself instead of * recreating the table. */ + @Suppress("UnusedParameter") // `db` is SQLiteOpenHelper's signature; a migration will use it. fun upgrade(db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { for (version in oldVersion until newVersion) { error( diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/data/SqliteTrashIndex.kt b/app/src/main/java/de/softwareapp_hb/filemanager/data/SqliteTrashIndex.kt index 269d569..9701472 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/data/SqliteTrashIndex.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/data/SqliteTrashIndex.kt @@ -149,6 +149,7 @@ class SqliteTrashIndex(context: Context) : TrashIndex { * A version with no step is therefore a bug to be reported, not an excuse to wipe the * table, which is why this ends in an `error` rather than in [create]. */ + @Suppress("UnusedParameter") // `db` is SQLiteOpenHelper's signature; a migration will use it. fun upgrade(db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { for (version in oldVersion until newVersion) { error( diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt index fb99c04..5df4b7a 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt @@ -102,7 +102,6 @@ fun BrowserScreen( var renameAllTargets by remember { mutableStateOf?>(null) } var confirmDelete by remember { mutableStateOf?>(null) } var searchOpen by remember { mutableStateOf(false) } - var menuOpen by remember { mutableStateOf(false) } /* * Hoisted above the overlay branch below, and deliberately so: a viewer, the editor, the diff --git a/app/src/test/java/de/softwareapp_hb/filemanager/ProjectInvariantsTest.kt b/app/src/test/java/de/softwareapp_hb/filemanager/ProjectInvariantsTest.kt index 005f491..fcc9f09 100644 --- a/app/src/test/java/de/softwareapp_hb/filemanager/ProjectInvariantsTest.kt +++ b/app/src/test/java/de/softwareapp_hb/filemanager/ProjectInvariantsTest.kt @@ -154,4 +154,4 @@ class ProjectInvariantsTest { private fun imports(file: File): List = codeLines(file).map { it.trim() }.filter { it.startsWith("import ") } -} \ No newline at end of file +} diff --git a/config/detekt/baseline.xml b/config/detekt/baseline.xml new file mode 100644 index 0000000..60e3f52 --- /dev/null +++ b/config/detekt/baseline.xml @@ -0,0 +1,82 @@ + + + + + ComplexCondition:BrowserContent.kt$wide && state.isRootView && state.searchRoot == null && state.focus == null + ComplexCondition:BrowserScreen.kt$state.path != null && !state.isSelecting && !state.loading && !state.needsTreeGrant + CyclomaticComplexMethod:ArchiveEngine.kt$ArchiveEngine$private suspend fun readSevenZip( channel: SeekableByteChannel, destination: StoragePath, state: ArchiveState, ) + CyclomaticComplexMethod:ArchiveEngine.kt$ArchiveEngine$private suspend fun withEntry( archive: StoragePath, name: String, sink: suspend (InputStream) -> Unit, ): Boolean + CyclomaticComplexMethod:ArchiveFormats.kt$ArchiveFormats$fun formatOf(fileName: String): ArchiveFormat + CyclomaticComplexMethod:BrowserContent.kt$@Composable fun BrowserContent( state: BrowserUiState, contentPadding: PaddingValues, crumbLabel: (StoragePath) -> String, onOpenEntry: (FileEntry) -> Unit, onFocusEntry: (FileEntry) -> Unit, onFocusRoot: (RootItem) -> Unit, onToggleSelection: (StoragePath) -> Unit, onOpenRoot: (StoragePath) -> Unit, onNavigate: (StoragePath) -> Unit, onOpenCategory: (FileCategory) -> Unit, onOpenBookmark: (StoragePath) -> Unit, onRemoveBookmark: (StoragePath) -> Unit, onGrantAllFiles: () -> Unit, onPickFolder: () -> Unit, onRebuildIndex: () -> Unit, /** Hoisted by the screen so the scroll position outlives a viewer taking over. */ listState: LazyListState = rememberLazyListState(), gridState: LazyGridState = rememberLazyGridState(), ) + CyclomaticComplexMethod:BrowserContent.kt$@Composable private fun DirectoryView( state: BrowserUiState, crumbLabel: (StoragePath) -> String, wide: Boolean, listState: LazyListState, gridState: LazyGridState, onOpenEntry: (FileEntry) -> Unit, onFocusEntry: (FileEntry) -> Unit, onToggleSelection: (StoragePath) -> Unit, onNavigate: (StoragePath) -> Unit, onGrantAllFiles: () -> Unit, onRebuildIndex: () -> Unit, modifier: Modifier = Modifier, ) + CyclomaticComplexMethod:BrowserScreen.kt$@OptIn(ExperimentalMaterial3Api::class) @Composable fun BrowserScreen( incoming: IncomingRequest? = null, onDeliver: (FileEntry) -> Unit = {}, onDone: () -> Unit = {}, viewModel: BrowserViewModel = viewModel(factory = BrowserViewModel.Factory), ) + CyclomaticComplexMethod:BrowserViewModel.kt$BrowserViewModel$fun open(entry: FileEntry) + CyclomaticComplexMethod:BrowserViewModel.kt$BrowserViewModel$fun previewArchiveEntry(entry: ArchiveEntry) + CyclomaticComplexMethod:FileOperations.kt$FileOperations$private suspend fun transfer( from: StoragePath, to: StoragePath, move: Boolean, state: OperationState, ) + CyclomaticComplexMethod:SearchIndexer.kt$SearchIndexer$suspend fun refresh( volume: StoragePath, showHidden: Boolean, onProgress: (Int) -> Unit = {}, ): Int + LargeClass:BrowserViewModel.kt$BrowserViewModel : ViewModel + LargeClass:BrowserViewModelTest.kt$BrowserViewModelTest + LongMethod:BrowserContent.kt$@Composable fun BrowserContent( state: BrowserUiState, contentPadding: PaddingValues, crumbLabel: (StoragePath) -> String, onOpenEntry: (FileEntry) -> Unit, onFocusEntry: (FileEntry) -> Unit, onFocusRoot: (RootItem) -> Unit, onToggleSelection: (StoragePath) -> Unit, onOpenRoot: (StoragePath) -> Unit, onNavigate: (StoragePath) -> Unit, onOpenCategory: (FileCategory) -> Unit, onOpenBookmark: (StoragePath) -> Unit, onRemoveBookmark: (StoragePath) -> Unit, onGrantAllFiles: () -> Unit, onPickFolder: () -> Unit, onRebuildIndex: () -> Unit, /** Hoisted by the screen so the scroll position outlives a viewer taking over. */ listState: LazyListState = rememberLazyListState(), gridState: LazyGridState = rememberLazyGridState(), ) + LongMethod:BrowserScreen.kt$@OptIn(ExperimentalMaterial3Api::class) @Composable fun BrowserScreen( incoming: IncomingRequest? = null, onDeliver: (FileEntry) -> Unit = {}, onDone: () -> Unit = {}, viewModel: BrowserViewModel = viewModel(factory = BrowserViewModel.Factory), ) + LongMethod:BrowserTopBar.kt$@OptIn(ExperimentalMaterial3Api::class) @Composable fun BrowserTopBar( title: String, canNavigateUp: Boolean, /** Whether this view is a folder, and so can be starred at all. */ canBookmark: Boolean, bookmarked: Boolean, query: String, searchOpen: Boolean, searchScope: SearchScope, /** Whether the scope is the user's to choose at all; see the note beside the toggle. */ scopeChoosable: Boolean, canPaste: Boolean, onUp: () -> Unit, onQuery: (String) -> Unit, onSearchOpen: (Boolean) -> Unit, onSearchScope: () -> Unit, onSort: () -> Unit, onPaste: () -> Unit, onToggleBookmark: () -> Unit, onRefresh: () -> Unit, onUsage: () -> Unit, onDuplicates: () -> Unit, onTrash: () -> Unit, ) + LongParameterList:BrowserContent.kt$( state: BrowserUiState, contentPadding: PaddingValues, crumbLabel: (StoragePath) -> String, onOpenEntry: (FileEntry) -> Unit, onFocusEntry: (FileEntry) -> Unit, onFocusRoot: (RootItem) -> Unit, onToggleSelection: (StoragePath) -> Unit, onOpenRoot: (StoragePath) -> Unit, onNavigate: (StoragePath) -> Unit, onOpenCategory: (FileCategory) -> Unit, onOpenBookmark: (StoragePath) -> Unit, onRemoveBookmark: (StoragePath) -> Unit, onGrantAllFiles: () -> Unit, onPickFolder: () -> Unit, onRebuildIndex: () -> Unit, /** Hoisted by the screen so the scroll position outlives a viewer taking over. */ listState: LazyListState = rememberLazyListState(), gridState: LazyGridState = rememberLazyGridState(), ) + LongParameterList:BrowserContent.kt$( state: BrowserUiState, crumbLabel: (StoragePath) -> String, wide: Boolean, listState: LazyListState, gridState: LazyGridState, onOpenEntry: (FileEntry) -> Unit, onFocusEntry: (FileEntry) -> Unit, onToggleSelection: (StoragePath) -> Unit, onNavigate: (StoragePath) -> Unit, onGrantAllFiles: () -> Unit, onRebuildIndex: () -> Unit, modifier: Modifier = Modifier, ) + LongParameterList:BrowserTopBar.kt$( title: String, canNavigateUp: Boolean, /** Whether this view is a folder, and so can be starred at all. */ canBookmark: Boolean, bookmarked: Boolean, query: String, searchOpen: Boolean, searchScope: SearchScope, /** Whether the scope is the user's to choose at all; see the note beside the toggle. */ scopeChoosable: Boolean, canPaste: Boolean, onUp: () -> Unit, onQuery: (String) -> Unit, onSearchOpen: (Boolean) -> Unit, onSearchScope: () -> Unit, onSort: () -> Unit, onPaste: () -> Unit, onToggleBookmark: () -> Unit, onRefresh: () -> Unit, onUsage: () -> Unit, onDuplicates: () -> Unit, onTrash: () -> Unit, ) + LongParameterList:SelectionBar.kt$( state: BrowserUiState, onCopy: () -> Unit, onCut: () -> Unit, onDelete: () -> Unit, onDetails: () -> Unit, onRename: () -> Unit, onOpenWith: () -> Unit, onShare: () -> Unit, onCompress: () -> Unit, onExtract: () -> Unit, onSelectAll: () -> Unit, onClear: () -> Unit, ) + LoopWithTooManyJumpStatements:ArchiveEngine.kt$ArchiveEngine$while + LoopWithTooManyJumpStatements:BatchRename.kt$BatchRename$for + LoopWithTooManyJumpStatements:DuplicateFinder.kt$DuplicateFinder$while + LoopWithTooManyJumpStatements:SearchIndexer.kt$SearchIndexer$for + LoopWithTooManyJumpStatements:TransferPlanner.kt$TransferPlanner$for + LoopWithTooManyJumpStatements:Trash.kt$Trash$for + MaxLineLength:AllFilesSourceTest.kt$AllFilesSourceTest$roots = roots.map { FileEntry(StoragePath.of(it.absolutePath), isDirectory = true, sizeBytes = 0, lastModifiedMillis = 0) } + MaxLineLength:ArchiveEngineTest.kt$ArchiveEngineTest$assertEquals("hello gzip", ArchiveEngine(storage).readEntry(archive, entries.single())?.decodeToString()) + MaxLineLength:ArchiveEngineTest.kt$ArchiveEngineTest$assertFalse("nothing was written — that is the point of a preview", storage.contains(out.child("b.txt"))) + MaxLineLength:ArchiveEngineTest.kt$ArchiveEngineTest$val bytes = ArchiveEngine(storage).readEntry(zip, ArchiveEngine(storage).list(zip).single { it.name == "b.txt" }) + MaxLineLength:BrowserViewModel.kt$BrowserViewModel$done.isNotEmpty() -> messageUndoable(R.string.message_renamed_some, done.size, plan.moves.size) + MaxLineLength:BrowserViewModelTest.kt$BrowserViewModelTest$assertTrue("a folder that is there is not marked missing", model.state.value.bookmarks.single().exists) + MaxLineLength:FileOperations.kt$FileOperations$) + MaxLineLength:FileOperationsTest.kt$FileOperationsTest$paths = listOf(src.child("f0.txt"), src.child("f1.txt"), src.child("f2.txt"), src.child("f3.txt")) + MaxLineLength:InMemoryStorageSource.kt$InMemoryStorageSource$entries[path] = FileEntry(path, isDirectory = false, sizeBytes = bytes.size.toLong(), lastModifiedMillis = 0L) + MaxLineLength:IncomingRequest.kt$IncomingRequest.Companion$receive(IntentCompat.getParcelableArrayListExtra(intent, Intent.EXTRA_STREAM, Uri::class.java).orEmpty()) + MaxLineLength:IncomingRequest.kt$IncomingRequest.Companion$receive(listOfNotNull(IntentCompat.getParcelableExtra(intent, Intent.EXTRA_STREAM, Uri::class.java))) + MaxLineLength:MediaStoreDeleter.kt$MediaStoreDeleter$if (cursor.moveToFirst()) collection.buildUpon().appendPath(cursor.getLong(0).toString()).build() else null + MaxLineLength:SafSourceTest.kt$SafSourceTest$provider.add("primary:Download", doc("primary:Download/a.txt", "a.txt", size = 12, modified = 1_700_000_000_000)) + MaxLineLength:SearchIndexerTest.kt$SearchIndexerTest$assertEquals(listOf(volume, volume.child("Documents")).sortedBy { it.value }, storage.namesCalls.sortedBy { it.value }) + MaxLineLength:SearchIndexerTest.kt$SearchIndexerTest$assertEquals(listOf(volume.child("DCIM/new-photo.jpg")), storage.statCalls.filter { it.name == "new-photo.jpg" }) + MaxLineLength:SelectionBar.kt$Icon(Icons.Filled.Archive, contentDescription = stringResource(R.string.browser_action_compress)) + MaxLineLength:SelectionBar.kt$Icon(Icons.Filled.Close, contentDescription = stringResource(R.string.browser_action_clear_selection)) + MaxLineLength:SelectionBar.kt$Icon(Icons.Filled.ContentCopy, contentDescription = stringResource(R.string.browser_action_copy)) + MaxLineLength:SelectionBar.kt$Icon(Icons.Filled.ContentCut, contentDescription = stringResource(R.string.browser_action_cut)) + MaxLineLength:SelectionBar.kt$Icon(Icons.Filled.Info, contentDescription = stringResource(R.string.browser_action_details)) + MaxLineLength:SelectionBar.kt$Icon(Icons.Filled.SelectAll, contentDescription = stringResource(R.string.browser_action_select_all)) + MaxLineLength:SelectionBar.kt$Icon(Icons.Filled.Share, contentDescription = stringResource(R.string.browser_action_share)) + MaxLineLength:SelectionBar.kt$Icon(Icons.Filled.Unarchive, contentDescription = stringResource(R.string.browser_action_extract)) + MaxLineLength:SqliteSearchIndex.kt$SqliteSearchIndex$. + MaxLineLength:SqliteSearchIndex.kt$SqliteSearchIndex.Companion$"ALTER TABLE $TABLE_VOLUMES ADD COLUMN $COLUMN_FULL_AT INTEGER NOT NULL DEFAULT 0" + MaxLineLength:SqliteSearchIndexTest.kt$SqliteSearchIndexTest$"(${SqliteSearchIndex.COLUMN_VOLUME}, ${SqliteSearchIndex.COLUMN_INDEXED_AT}) VALUES ('$volume', 1234)" + MaxLineLength:SqliteSearchIndexTest.kt$SqliteSearchIndexTest$db.rawQuery("SELECT ${SqliteSearchIndex.COLUMN_FULL_AT}, ${SqliteSearchIndex.COLUMN_HIDDEN_INCLUDED} FROM ${SqliteSearchIndex.TABLE_VOLUMES}", null) + MaxLineLength:SqliteSearchIndexTest.kt$SqliteSearchIndexTest$val failure = runCatching { SqliteSearchIndex.upgrade(db, SqliteSearchIndex.DATABASE_VERSION, SqliteSearchIndex.DATABASE_VERSION + 1) } + MaxLineLength:Trash.kt$Trash$onProgress(OperationProgress(OperationKind.MOVE, index + 1, batch.entries.size, entry.original)) + NestedBlockDepth:ArchiveEngine.kt$ArchiveEngine$private suspend fun readArchive( archive: ArchiveInputStream<*>, destination: StoragePath, state: ArchiveState, ) + NestedBlockDepth:ArchiveEngine.kt$ArchiveEngine$private suspend fun readSevenZip( channel: SeekableByteChannel, destination: StoragePath, state: ArchiveState, ) + NestedBlockDepth:ArchiveEngine.kt$ArchiveEngine$private suspend fun withEntry( archive: StoragePath, name: String, sink: suspend (InputStream) -> Unit, ): Boolean + NestedBlockDepth:ArchiveEngine.kt$ArchiveEngine$suspend fun createZip( entries: List<StoragePath>, destination: StoragePath, onProgress: (OperationProgress) -> Unit = {}, ): OperationResult + NestedBlockDepth:ArchiveEngine.kt$ArchiveEngine$suspend fun extract( archive: StoragePath, destination: StoragePath, onProgress: (OperationProgress) -> Unit = {}, ): OperationResult + NestedBlockDepth:ArchiveEngine.kt$ArchiveEngine$suspend fun list(archive: StoragePath): List<ArchiveEntry> + NestedBlockDepth:ArchiveEngineTest.kt$ArchiveEngineTest$private fun sevenZip(vararg entries: Pair<String, String>): ByteArray + RethrowCaughtException:BrowserViewModel.kt$BrowserViewModel$throw cancellation + ReturnCount:ArchiveEngine.kt$ArchiveEngine$private suspend fun withEntry( archive: StoragePath, name: String, sink: suspend (InputStream) -> Unit, ): Boolean + ReturnCount:BrowserScreen.kt$@OptIn(ExperimentalMaterial3Api::class) @Composable fun BrowserScreen( incoming: IncomingRequest? = null, onDeliver: (FileEntry) -> Unit = {}, onDone: () -> Unit = {}, viewModel: BrowserViewModel = viewModel(factory = BrowserViewModel.Factory), ) + SpreadOperator:BrowserScreen.kt$(it.textRes, *it.args.toTypedArray()) + SpreadOperator:BrowserScreen.kt$(it.textRes, quantity, *it.args.toTypedArray()) + TooGenericExceptionCaught:ArchiveEngine.kt$ArchiveEngine$failure: Exception + TooGenericExceptionCaught:BrowserViewModel.kt$BrowserViewModel$failure: Exception + TooGenericExceptionCaught:FileOperations.kt$FileOperations$failure: Exception + TooGenericExceptionCaught:Trash.kt$Trash$failure: Exception + TooManyFunctions:BrowserViewModel.kt$BrowserViewModel : ViewModel + TooManyFunctions:SearchIndex.kt$SearchIndex + TooManyFunctions:SearchIndex.kt$SearchIndex$None : SearchIndex + TooManyFunctions:StorageSource.kt$StorageSource + + diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml new file mode 100644 index 0000000..940782a --- /dev/null +++ b/config/detekt/detekt.yml @@ -0,0 +1,61 @@ +# Detekt: the rules this project deliberately overrides. +# +# Everything else is detekt's default (buildUponDefaultConfig is set in app/build.gradle.kts), and +# findings that exist today are frozen in baseline.xml. Overrides are here for one of two reasons: +# the threshold has to match this codebase, or the rule disagrees with a deliberate choice. +build: + # The gate is "no new findings", which the baseline answers; a hard zero would only re-report the + # backlog the baseline exists to park. + maxIssues: 0 + +complexity: + # The size rules are the point of adding detekt: BrowserViewModel is 2 100 lines and was touched + # in 2 300 changed lines of history, which is the one place this codebase could rot. + LongMethod: + threshold: 100 + LargeClass: + threshold: 700 + TooManyFunctions: + thresholdInFiles: 30 + thresholdInClasses: 30 + CognitiveComplexMethod: + threshold: 25 + NestedBlockDepth: + threshold: 4 + LongParameterList: + functionThreshold: 10 + constructorThreshold: 14 + +style: + # Comments are prose here and wrap where the sentence ends, not at a column. + MaxLineLength: + maxLineLength: 110 + excludeCommentStatements: true + # A TODO that nobody is reminded of is a TODO that stays forever. + ForbiddenComment: + comments: + - 'TODO:' + - 'FIXME:' + - 'STOPSHIP:' + # Sizes, timeouts and buffer lengths are named constants in this codebase; the rule would argue + # with every one of them. + MagicNumber: + active: false + ReturnCount: + max: 5 + +naming: + # `de.softwareapp_hb.filemanager` is the applicationId this app is locked to; the underscore is + # part of it and an identity change is not a lint fix. + PackageNaming: + active: false + # Test names are sentences in backticks, which is the project's specification style, and a + # `@Composable` is a type in PascalCase — that is Compose's convention, not this project's choice. + FunctionNaming: + ignoreAnnotated: ['Composable'] + excludes: + - '**/src/test/**' + # Files are named after the concept they hold (BrowserRoots.kt is a group of composables), not after + # a declaration inside them. + MatchingDeclarationName: + active: false \ No newline at end of file diff --git a/docs/CODE_MAP.md b/docs/CODE_MAP.md index 1976b3a..6293cee 100644 --- a/docs/CODE_MAP.md +++ b/docs/CODE_MAP.md @@ -2,7 +2,7 @@ Generated from the source by `tools/make-code-map.py` - do not edit by hand. Regenerate it after a change (`python3 tools/make-code-map.py`); `--check` exits non-zero when this file is stale. -76 source files, 12462 lines - 29 test files, 371 tests. The behaviour index at the end lists every test name, and in this project a test name is a sentence about what the app does, so it doubles as the specification. +76 source files, 12463 lines - 29 test files, 371 tests. The behaviour index at the end lists every test name, and in this project a test name is a sentence about what the app does, so it doubles as the specification. Start at `AGENTS.md`. What is done, and why: `docs/ROADMAP.md` and `CHANGELOG.md`. What was verified by hand rather than by a test: `docs/EDGE_CASE_TESTING.md`. @@ -43,9 +43,9 @@ Storage backends (all-files access and SAF), volumes, incoming shares, the SQLit | `SafGrant.kt` | 41 | Picking and keeping a folder tree for the SAF fallback. SafGrant | - | | `SafSource.kt` | 187 | The Storage Access Framework fallback backend. SafSource | `SafSourceTest` (5) | | `SearchIndex.kt` | 196 | A by-name index of what is on the volumes, so a search does not have to walk them. IndexedFile, VolumeStamp, SearchIndex | - | -| `SqliteBookmarkStore.kt` | 117 | The starred folders, on SQLite. SqliteBookmarkStore | `SqliteBookmarkStoreTest` (9) | +| `SqliteBookmarkStore.kt` | 118 | The starred folders, on SQLite. SqliteBookmarkStore | `SqliteBookmarkStoreTest` (9) | | `SqliteSearchIndex.kt` | 438 | The search index, on SQLite. SqliteSearchIndex | `SqliteSearchIndexTest` (27) | -| `SqliteTrashIndex.kt` | 162 | The trash index, on SQLite. SqliteTrashIndex | `SqliteTrashIndexTest` (7) | +| `SqliteTrashIndex.kt` | 163 | The trash index, on SQLite. SqliteTrashIndex | `SqliteTrashIndexTest` (7) | | `StorageAccess.kt` | 80 | The Android-facing side of the storage-access decision. StorageAccess | - | | `StorageAccessPolicy.kt` | 68 | The decision rules behind StorageAccess. StorageAccessMode, StorageAccessPolicy | `StorageAccessPolicyTest` (6) | | `StorageLocation.kt` | 87 | Everything the browser needs to know about where files live and what it may touch. StorageLocation, StorageProvider | - | @@ -98,7 +98,7 @@ The browser screen: view model, state, list and grid, dialogs, selection bar | `BrowserDetailPane.kt` | 131 | The second pane of the wide layout: what is inside the folder the user picked, or a preview and the details of the file they picked. DetailPane(), FolderPreview(), ImagePreview() | - | | `BrowserDialogs.kt` | 279 | Renaming several files at once, with the numbering worked out before anything moves. BatchRenameDialog(), PREVIEW_NAMES, batchRenameNote(), NameDialog(), ConfirmDialog(), ConflictDialog() | - | | `BrowserRoots.kt` | 270 | A row on the root screen: a volume, with its free space when the platform reports it. RootRow(), itemCountLabel(), EmptyState(), LibrarySectionHeader(), BookmarkSectionHeader(), SectionHeader(), BookmarkRow(), LibraryRow(), Breadcrumbs() | - | -| `BrowserScreen.kt` | 592 | BrowserScreen() | - | +| `BrowserScreen.kt` | 591 | BrowserScreen() | - | | `BrowserTopBar.kt` | 238 | The browser's app bar: title, up, the favourite star, and the search / sort / overflow actions. BrowserTopBar(), SEARCH_FIELD_WIDTH | - | | `BrowserUiState.kt` | 311 | One immutable description of everything the browser shows. ViewMode, SearchScope, Clipboard, UiMessage, ConflictPrompt, RootItem, Bookmark, DetailFocus, Undo, UndoMove, Overlay, BrowserUiState | - | | `BrowserViewModel.kt` | 2100 | Drives the browser screen. BrowserViewModel, ArchivePreview | `BrowserViewModelTest` (89) | diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 0aa6891..aa31f75 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -34,6 +34,7 @@ Legend: ✅ done · 🚧 in progress · ⏳ not started · 💡 candidate (propo | Storage access | `MANAGE_EXTERNAL_STORAGE` first, SAF fallback | Google Play explicitly permits "All files access" for file managers; SAF keeps the app usable if the user declines | | Connectivity | Local-first (no `INTERNET` in v1) | Preserves the "nothing leaves the device" claim and keeps the Data Safety form trivial | | File sharing | Receives `SEND`/`SEND_MULTIPLE`, serves `GET_CONTENT`/`OPEN_DOCUMENT`, and deliberately does **not** register `ACTION_VIEW` | A file manager should be able to receive a file and to supply one, and neither costs a permission. Registering as a viewer as well would put Plain Files in the "open with" chooser the app itself uses, offering it back to itself | +| Quality gate | `tools/verify.sh` — map check, detekt, tests, lint; `--release` adds R8 — run by CI, by the pre-push hook and by hand | One command that means the same thing in all three places. The detekt baseline is what makes adopting static analysis possible without a backlog-clearing week, while still failing on anything new | | Monetisation | One-time paid (€4.49; €2.29 launch price), set in the Console | No Billing client, no `BILLING` permission, no purchase code — which is what keeps "nothing leaves the device" true in the shipped APK. Free → paid is impossible after the first upload, so the price must be set beforehand. See [MONETISATION](MONETISATION.md) | ## Deliberately deferred (post-1.0) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index a281a2a..cdf9464 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,6 +1,12 @@ [versions] agp = "9.1.0" kotlin = "2.4.10" +# Static analysis for Kotlin, driven by tools/verify.sh. 1.23.8 is the newest release on the plugin +# portal (there is no 2.x there yet). It is used through its CLI rather than its Gradle plugin: the +# plugin runs the analysis inside the build's own JVM, and detekt 1.23 cannot parse JDK 25's version +# string — see the task in app/build.gradle.kts. The findings that exist today are frozen in +# config/detekt/baseline.xml, so the gate fails on a new one rather than on the backlog. +detekt = "1.23.8" # AndroidX / Compose coreKtx = "1.18.0" @@ -69,6 +75,9 @@ junit = { group = "junit", name = "junit", version.ref = "junit" } robolectric = { group = "org.robolectric", name = "robolectric", version.ref = "robolectric" } kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "coroutines" } +# Static analysis, run by the :app:detekt task from its own configuration — never an app dependency. +detekt-cli = { group = "io.gitlab.arturbosch.detekt", name = "detekt-cli", version.ref = "detekt" } + [plugins] android-application = { id = "com.android.application", version.ref = "agp" } # NOTE: `org.jetbrains.kotlin.android` is intentionally absent — AGP 9.x provides diff --git a/tools/verify.sh b/tools/verify.sh new file mode 100755 index 0000000..335b188 --- /dev/null +++ b/tools/verify.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# +# The gate. Everything that has to be true before a change is finished, in one command: +# +# ./tools/verify.sh # what every change has to pass +# ./tools/verify.sh --release # ...and an R8 release build, which is what the nightly runs +# GRADLE_FLAGS=--offline ./tools/verify.sh # keeps working without a network +# +# Cheapest first, so a stale document fails in a second rather than after a release build. Each step +# is here because it has caught a real defect in this project at least once; nothing is on the list +# because it is fashionable. +set -euo pipefail + +cd "$(dirname "$0")/.." + +release=false +for arg in "$@"; do + case "$arg" in + --release) release=true ;; + *) echo "verify: unknown option '$arg' (only --release is understood)" >&2; exit 2 ;; + esac +done + +# Word splitting is intended here: GRADLE_FLAGS is a place to put several flags. +# shellcheck disable=SC2086 +gradle() { ./gradlew ${GRADLE_FLAGS:-} "$@" --console=plain; } + +step() { printf '\n\033[1m== %s\033[0m\n' "$1"; } + +step "Code map is current" +python3 tools/make-code-map.py --check + +step "Kotlin static analysis (detekt, against the baseline)" +gradle :app:detekt + +step "JVM unit tests" +gradle :app:testDebugUnitTest + +step "Android Lint" +gradle :app:lintDebug + +if $release; then + step "Release build (R8)" + # Signed when keystore.properties happens to be present, unsigned otherwise — either way R8 + # runs, and a release-only defect (a class reached only through a resource, for instance) is + # exactly what this step is for. + gradle :app:assembleRelease +fi + +printf '\n\033[1mverify: all checks passed\033[0m\n'