mirror of
https://github.com/okhsunrog/vpnhide.git
synced 2026-08-18 04:53:47 +00:00
Merge pull request #275 from okhsunrog/refactor/diagnostic-gate-and-drop-sourcesize
refactor(diagnostics): gate-safe verdict access; drop the Kotlin source-size budget
This commit is contained in:
commit
829772425a
7 changed files with 61 additions and 97 deletions
|
|
@ -83,10 +83,6 @@ reinvent them.** `grep` for an existing helper before writing a new one.
|
|||
`@Suppress("RuleName")` + a one-line reason — visible in the code, not hidden
|
||||
in a baseline. Only disable a rule in `detekt.yml` (with a comment) when it
|
||||
doesn't fit the codebase at all.
|
||||
- **Kotlin source-size budget** — `:app:detekt` also runs
|
||||
`checkKotlinSourceSize`. New source files are capped at 700 lines; existing
|
||||
oversized files have explicit shrink-only budgets in `app/build.gradle.kts`.
|
||||
Split responsibilities instead of raising a budget.
|
||||
- **CPD** (copy-paste detector) — finds cross-file duplicated blocks that
|
||||
detekt can't (re-implemented parsers / save-builders are the classic
|
||||
AI-duplication smell). CI-enforced (`./gradlew cpdCheck`); report at
|
||||
|
|
|
|||
|
|
@ -1,43 +1,8 @@
|
|||
import java.io.FileInputStream
|
||||
import java.util.Properties
|
||||
import org.gradle.api.DefaultTask
|
||||
import org.gradle.api.file.ConfigurableFileCollection
|
||||
import org.gradle.api.provider.MapProperty
|
||||
import org.gradle.api.provider.Property
|
||||
import org.gradle.api.tasks.Exec
|
||||
import org.gradle.api.tasks.Input
|
||||
import org.gradle.api.tasks.InputFiles
|
||||
import org.gradle.api.tasks.PathSensitivity
|
||||
import org.gradle.api.tasks.PathSensitive
|
||||
import org.gradle.api.tasks.TaskAction
|
||||
import org.jetbrains.kotlin.gradle.dsl.JvmTarget
|
||||
|
||||
abstract class CheckKotlinSourceSize : DefaultTask() {
|
||||
@get:InputFiles
|
||||
@get:PathSensitive(PathSensitivity.RELATIVE)
|
||||
abstract val sources: ConfigurableFileCollection
|
||||
|
||||
@get:Input
|
||||
abstract val budgets: MapProperty<String, Int>
|
||||
|
||||
@get:Input
|
||||
abstract val defaultLimit: Property<Int>
|
||||
|
||||
@TaskAction
|
||||
fun verify() {
|
||||
val limits = budgets.get()
|
||||
val violations =
|
||||
sources.files.mapNotNull { source ->
|
||||
val lineCount = source.readLines().size
|
||||
val limit = limits[source.name] ?: defaultLimit.get()
|
||||
if (lineCount > limit) "${source.name}: $lineCount lines (limit $limit)" else null
|
||||
}
|
||||
check(violations.isEmpty()) {
|
||||
"Kotlin source-size budget exceeded:\n${violations.joinToString("\n")}"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
plugins {
|
||||
alias(libs.plugins.android.application)
|
||||
// kotlin-android removed: AGP 9+ has built-in Kotlin support.
|
||||
|
|
@ -59,33 +24,6 @@ detekt {
|
|||
source.setFrom(files("src/main/kotlin", "src/test/kotlin"))
|
||||
}
|
||||
|
||||
// Top-level Compose functions evade detekt's LargeClass rule, so keep a
|
||||
// separate file-size budget. Existing oversized files are shrink-only debt:
|
||||
// their exact current sizes are explicit here, while every new source gets the
|
||||
// default ceiling. Remove an entry once a file drops below the default.
|
||||
val kotlinSourceLineBudgets =
|
||||
mapOf(
|
||||
"DashboardData.kt" to 1730,
|
||||
"DashboardScreen.kt" to 1335,
|
||||
"HookEntry.kt" to 1276,
|
||||
"SettingsScreen.kt" to 1201,
|
||||
"StatisticsScreen.kt" to 1102,
|
||||
"AppPickerScreen.kt" to 940,
|
||||
"AgentControl.kt" to 888,
|
||||
"MainActivity.kt" to 817,
|
||||
)
|
||||
|
||||
val checkKotlinSourceSize =
|
||||
tasks.register<CheckKotlinSourceSize>("checkKotlinSourceSize") {
|
||||
group = "verification"
|
||||
description = "Rejects new Kotlin god-files and growth in existing oversized sources."
|
||||
sources.from(fileTree("src/main/kotlin") { include("**/*.kt") })
|
||||
budgets.set(kotlinSourceLineBudgets)
|
||||
defaultLimit.set(700)
|
||||
}
|
||||
|
||||
tasks.named("detekt").configure { dependsOn(checkKotlinSourceSize) }
|
||||
|
||||
tasks.withType<io.gitlab.arturbosch.detekt.Detekt>().configureEach {
|
||||
// Codegen output (IfaceLists) and UniFFI bindings aren't hand-written.
|
||||
exclude("**/generated/**")
|
||||
|
|
|
|||
|
|
@ -163,8 +163,8 @@ internal fun buildDiagnosticSummaryText(
|
|||
appendLine("Diagnostics: not run")
|
||||
} else {
|
||||
appendLine("Diagnostics gate: ${report.gate.name.lowercase()}")
|
||||
appendLine("Native verdict: ${report.native.verdictLabel()}")
|
||||
appendLine("Java verdict: ${report.java.verdictLabel()}")
|
||||
appendLine("Native verdict: ${report.native.verdictLabel(report.nativeVerdict)}")
|
||||
appendLine("Java verdict: ${report.java.verdictLabel(report.javaVerdict)}")
|
||||
appendLine("Outcomes: ${report.outcomeTally()}")
|
||||
}
|
||||
appendLine("selfNeedsRestart: $selfNeedsRestart")
|
||||
|
|
|
|||
|
|
@ -58,7 +58,10 @@ internal data class DiagnosticCheck(
|
|||
val owned: Boolean,
|
||||
)
|
||||
|
||||
/** Per-layer rollup: presence/verdict plus the classified checks that produced it. */
|
||||
/** Per-layer rollup: presence plus the classified checks that produced it. The
|
||||
* Ok/Partial/Broken verdict is deliberately NOT exposed here — it is only valid
|
||||
* for a measured run, so it is reachable only through the gate-checked
|
||||
* [DiagnosticReport.nativeVerdict] / [DiagnosticReport.javaVerdict]. */
|
||||
internal data class LayerReport(
|
||||
val layer: CheckLayer,
|
||||
val backend: NativeBackendId?,
|
||||
|
|
@ -67,12 +70,7 @@ internal data class LayerReport(
|
|||
// a warning, never counted against the tile verdict. Always 0 for the Java layer.
|
||||
val unownedLeaks: Int,
|
||||
val checks: List<DiagnosticCheck>,
|
||||
) {
|
||||
/** Ok/Partial/Broken when the layer is [LayerStatus.Active]; null otherwise.
|
||||
* Only meaningful when the report's [DiagnosticReport.gate] is
|
||||
* [DiagnosticGate.ROUTED] — a gated report carries presence only. */
|
||||
val verdict: Verdict? get() = (status as? LayerStatus.Active)?.verdict
|
||||
}
|
||||
)
|
||||
|
||||
/**
|
||||
* The single canonical diagnostic snapshot.
|
||||
|
|
@ -91,15 +89,29 @@ internal data class DiagnosticReport(
|
|||
// False after the fast core phase, true once the slow Java probes have filled in.
|
||||
val complete: Boolean,
|
||||
val schema: Int = DIAGNOSTIC_REPORT_SCHEMA,
|
||||
)
|
||||
) {
|
||||
/** Per-layer Ok/Partial/Broken — the ONLY way to read a report's verdict.
|
||||
* Null unless the run was actually measured ([DiagnosticGate.ROUTED]); a
|
||||
* gated report's layers carry a placeholder [LayerStatus.Active] with zero
|
||||
* counts, so returning its verdict would render a false "Ok". */
|
||||
val nativeVerdict: Verdict? get() = native.verdictFor(gate)
|
||||
val javaVerdict: Verdict? get() = java.verdictFor(gate)
|
||||
}
|
||||
|
||||
/** Verdict of a layer, but only for a measured run — the gate is required so no
|
||||
* caller can obtain a verdict without acknowledging whether the run measured
|
||||
* anything. */
|
||||
private fun LayerReport.verdictFor(gate: DiagnosticGate): Verdict? =
|
||||
if (gate == DiagnosticGate.ROUTED) (status as? LayerStatus.Active)?.verdict else null
|
||||
|
||||
/**
|
||||
* Fold the raw check run into the canonical [DiagnosticReport]. Pure: same inputs
|
||||
* → same report, no Android or IO dependency, so it is unit-tested directly.
|
||||
*
|
||||
* [results] is null when the gate blocked the run ([DiagnosticGate.ROUTED] is the
|
||||
* only gate that carries measurements); the layers then report presence only and
|
||||
* their [LayerReport.verdict] must not be consulted.
|
||||
* only gate that carries measurements); the layers then report presence only, and
|
||||
* [DiagnosticReport.nativeVerdict] / [DiagnosticReport.javaVerdict] return null so
|
||||
* a gated verdict can never be rendered.
|
||||
*/
|
||||
internal fun buildDiagnosticReport(
|
||||
gate: DiagnosticGate,
|
||||
|
|
|
|||
|
|
@ -17,10 +17,13 @@ private fun LayerStatus.presenceToken(): String =
|
|||
is LayerStatus.Active -> "active"
|
||||
}
|
||||
|
||||
/** One-line verdict/presence summary for a layer, e.g. `broken (hidden 0, leaks 2, unowned 1)`. */
|
||||
internal fun LayerReport.verdictLabel(): String {
|
||||
/** One-line verdict/presence summary for a layer, e.g. `broken (hidden 0, leaks 2,
|
||||
* unowned 1)`. [verdict] comes from the report's gate-checked accessor, so a gated
|
||||
* run (verdict == null) renders `not-measured`, never a placeholder `ok`. */
|
||||
internal fun LayerReport.verdictLabel(verdict: Verdict?): String {
|
||||
val active = status as? LayerStatus.Active ?: return status.presenceToken()
|
||||
return "${active.verdict.name.lowercase()} (hidden ${active.hidden}, leaks ${active.leaks}, unowned $unownedLeaks)"
|
||||
val head = verdict?.name?.lowercase() ?: "not-measured"
|
||||
return "$head (hidden ${active.hidden}, leaks ${active.leaks}, unowned $unownedLeaks)"
|
||||
}
|
||||
|
||||
/** Count of every check by outcome token across both layers — the honest headline
|
||||
|
|
@ -43,18 +46,19 @@ internal fun DiagnosticReport.toDiagnosticsText(): String =
|
|||
appendLine("complete: $complete")
|
||||
appendLine("outcomes: ${outcomeTally()}")
|
||||
appendLine()
|
||||
appendLayer(native, "Native", native.backend?.name?.lowercase() ?: "none")
|
||||
appendLayer(native, "Native", native.backend?.name?.lowercase() ?: "none", nativeVerdict)
|
||||
appendLine()
|
||||
appendLayer(java, "Java", "lsposed")
|
||||
appendLayer(java, "Java", "lsposed", javaVerdict)
|
||||
}.trimEnd()
|
||||
|
||||
private fun StringBuilder.appendLayer(
|
||||
layer: LayerReport,
|
||||
title: String,
|
||||
backendLabel: String,
|
||||
verdict: Verdict?,
|
||||
) {
|
||||
appendLine("--- $title layer ($backendLabel) ---")
|
||||
appendLine(layer.verdictLabel())
|
||||
appendLine(layer.verdictLabel(verdict))
|
||||
if (layer.checks.isEmpty()) {
|
||||
appendLine("(no checks — gated run)")
|
||||
return
|
||||
|
|
@ -113,17 +117,17 @@ private fun DiagnosticReport.toReportJson(): ReportJson =
|
|||
schema = schema,
|
||||
gate = gate.name.lowercase(),
|
||||
complete = complete,
|
||||
native = native.toLayerJson(),
|
||||
java = java.toLayerJson(),
|
||||
native = native.toLayerJson(nativeVerdict),
|
||||
java = java.toLayerJson(javaVerdict),
|
||||
)
|
||||
|
||||
private fun LayerReport.toLayerJson(): LayerJson {
|
||||
private fun LayerReport.toLayerJson(verdict: Verdict?): LayerJson {
|
||||
val active = status as? LayerStatus.Active
|
||||
return LayerJson(
|
||||
layer = layer.name.lowercase(),
|
||||
backend = backend?.name?.lowercase(),
|
||||
presence = status.presenceToken(),
|
||||
verdict = active?.verdict?.name?.lowercase(),
|
||||
verdict = verdict?.name?.lowercase(),
|
||||
hidden = active?.hidden,
|
||||
leaks = active?.leaks,
|
||||
unownedLeaks = unownedLeaks,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
package dev.okhsunrog.vpnhide
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
|
|
@ -45,19 +47,19 @@ class DiagnosticReportTest {
|
|||
@Test
|
||||
fun `native verdict is Broken when an owned vector leaks and nothing hid`() {
|
||||
val r = report(results = CheckResults(native = nativeResults("ioctl_flags" to CheckOutcome.Leak)))
|
||||
assertEquals(Verdict.Broken, r.native.verdict)
|
||||
assertEquals(Verdict.Broken, r.nativeVerdict)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `native verdict is Partial when it hides some but an owned vector leaks`() {
|
||||
val native = nativeResults("ioctl_flags" to CheckOutcome.Leak, "getifaddrs" to CheckOutcome.HiddenByBackend)
|
||||
assertEquals(Verdict.Partial, report(results = CheckResults(native = native)).native.verdict)
|
||||
assertEquals(Verdict.Partial, report(results = CheckResults(native = native)).nativeVerdict)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `native verdict is Ok when nothing owned leaks`() {
|
||||
val native = nativeResults("ioctl_flags" to CheckOutcome.HiddenByBackend)
|
||||
assertEquals(Verdict.Ok, report(results = CheckResults(native = native)).native.verdict)
|
||||
assertEquals(Verdict.Ok, report(results = CheckResults(native = native)).nativeVerdict)
|
||||
}
|
||||
|
||||
// ── unowned leaks are surfaced separately, never against the verdict ────
|
||||
|
|
@ -67,7 +69,7 @@ class DiagnosticReportTest {
|
|||
// netlink_getrule (fib_nl_fill_rule) has no zygisk hook → out of scope for
|
||||
// the zygisk tile, so the verdict stays Ok and the leak is counted as unowned.
|
||||
val r = report(results = CheckResults(native = nativeResults("netlink_getrule" to CheckOutcome.Leak)))
|
||||
assertEquals(Verdict.Ok, r.native.verdict)
|
||||
assertEquals(Verdict.Ok, r.nativeVerdict)
|
||||
assertEquals(1, r.native.unownedLeaks)
|
||||
}
|
||||
|
||||
|
|
@ -119,7 +121,7 @@ class DiagnosticReportTest {
|
|||
val r = report(results = results)
|
||||
val javaCheck = r.java.checks.single()
|
||||
assertEquals(CheckOutcome.Leak, javaCheck.outcome)
|
||||
assertEquals(Verdict.Broken, r.java.verdict)
|
||||
assertEquals(Verdict.Broken, r.javaVerdict)
|
||||
}
|
||||
|
||||
// ── gate ───────────────────────────────────────────────────────────────
|
||||
|
|
@ -132,6 +134,20 @@ class DiagnosticReportTest {
|
|||
assertTrue(r.java.checks.isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a blocked gate exposes no verdict and never renders a placeholder ok`() {
|
||||
// A gated run's active layers carry a placeholder Active(0,0). The
|
||||
// gate-checked accessors must return null and the renderers must show
|
||||
// "not-measured", never a false "ok" folded off the zero counts.
|
||||
val r = report(gate = DiagnosticGate.VPN_OFF, results = null)
|
||||
assertNull(r.nativeVerdict)
|
||||
assertNull(r.javaVerdict)
|
||||
val text = r.toDiagnosticsText()
|
||||
assertTrue("gated layers render not-measured", text.contains("not-measured"))
|
||||
assertFalse("no placeholder ok verdict", text.contains("ok (hidden 0, leaks 0"))
|
||||
assertFalse("json verdict stays null for a gated run", r.toJson().contains("\"verdict\": \"ok\""))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `gate folds the three signals worst-first`() {
|
||||
assertEquals(DiagnosticGate.VPN_OFF, resolveDiagnosticGate(vpnActive = false, selfRouted = true, selfNeedsRestart = false))
|
||||
|
|
|
|||
|
|
@ -4,8 +4,7 @@
|
|||
# Goal: catch the smell ktlint can't — god-functions, high complexity,
|
||||
# dead code, common bug patterns — without bikeshedding AI-written Compose
|
||||
# code to death. There is no baseline: inherent exceptions stay visible at the
|
||||
# call site, and a separate Gradle source-size budget covers top-level Compose
|
||||
# files that LargeClass cannot see.
|
||||
# call site.
|
||||
|
||||
build:
|
||||
# Any finding not in the baseline fails the build.
|
||||
|
|
@ -14,8 +13,7 @@ build:
|
|||
complexity:
|
||||
LongMethod:
|
||||
# The whole reason for this gate: loadDashboardState was 830 lines.
|
||||
# @Composable UI builders are exempt here; checkKotlinSourceSize bounds
|
||||
# top-level god-screens that detekt's class-oriented rules cannot see.
|
||||
# @Composable UI builders are exempt here.
|
||||
threshold: 60
|
||||
ignoreAnnotated: ['Composable']
|
||||
CyclomaticComplexMethod:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue