Address detekt warnings surfaced by #378#379
Open
holodorum wants to merge 5 commits intokson-org:mainfrom
Open
Address detekt warnings surfaced by #378#379holodorum wants to merge 5 commits intokson-org:mainfrom
holodorum wants to merge 5 commits intokson-org:mainfrom
Conversation
Wires detekt 1.23.8 into the Gradle build, applied to every module that uses the Kotlin multiplatform or JVM plugin. Configuration is authored in detekt.kson and transpiled to detekt.yml via the existing TranspileKsonToYaml task, mirroring how .circleci/config.kson is handled. Pure style rules (MaxLineLength, WildcardImport, NewLineAtEndOfFile, PackageNaming) are disabled to keep the focus on code-quality findings; remaining findings are addressed in the follow-up commit.
Applies fixes for the actionable findings surfaced by the detekt setup
from the preceding commit, then tunes the config and baselines the
remaining structural debt so `./gradlew detekt` is green across every
module.
Code fixes:
- Replace unused `for (i in 1..n)` counters with `repeat(n)` in
Formatter.kt
- Replace `throw IllegalArgumentException(...)` guards with
`require(...)` / `requireNotNull(...)` in Message.kt and the schema
ref test
- Replace "should not happen" `RuntimeException` throws with the
existing `ShouldNotHappenException` across the parser, the
IntelliJ plugin highlighter / element type mapper, and LexerTest
- Convert `KsonIcons` to an `object` (it was a utility class with
a public constructor)
- Remove the empty body of the `KsonStringValidatorTest` placeholder
class; convert the constant-returning `searchExpressionSchema()`
test helper to a `val`
- Suppress `SwallowedException` at two intentional catch sites where
the exception type itself carries the full semantic
- Suppress `UnusedParameter` on the no-op native-test `validateYaml`
stub, which must match the JVM signature
Config changes in `detekt.kson`:
- Raise NestedBlockDepth to 6 and ComplexCondition to 10 to cover the
existing parser/validator code without being pathological
- Disable MatchingDeclarationName and TooGenericExceptionCaught — the
former would force a wave of file renames; the latter collides with
CLI commands that intentionally catch broadly to render
user-facing errors
Baselines:
- Per-module `detekt-baseline.xml` files capture the remaining
LongMethod / LargeClass / CyclomaticComplexMethod / ThrowsCount /
ReturnCount / LoopWithTooManyJumpStatements / SpreadOperator
findings on existing code. New code is still scrutinized against
every rule.
detekt.yml is transpiled from detekt.kson by transpileDetektConfigTask on every build, so it should not live in version control. Adds the file to .gitignore and removes the tracked copy from the index.
Adds a short section to the developer setup explaining what detekt does, that detekt.kson is the source of truth (detekt.yml is generated), and what the detekt-baseline.xml files carry: grandfathered pre-existing findings so detekt only fails the build on new issues. Also documents the "run detekt green, then detektBaseline, then diff" flow for safely refreshing the baselines after paying down debt.
The previous commit raised these two thresholds to accommodate existing code. Both rules are exactly the kind that tend to surface bugs — a stray extra boolean term or another level of nesting is precisely how parser/validator logic quietly goes wrong — so we'd rather see the warnings and fix the code than loosen the bar. ComplexCondition: extracts each offending predicate into a named `val` (or a set membership check) so the intent reads at a glance and the condition itself stays under the 4-term default. Touches Formatter, Lexer, Parser, SchemaIdLookup, TypeValidator, Ast.kt, and the JetBrains EnterHandlerDelegate. A nice side effect: lowering the EnterHandlerDelegate complexity fell off the CyclomaticComplexMethod baseline. NestedBlockDepth: mostly extracts inner branches into named helper functions (validateObjectProperties / validateListElements in the string validator, per-target helpers in IndentValidator and SelectionRangeBuilder, etc.) or inverts guards to flatten the control flow. The @deprecated `IndentFormatter.indent` and its helper `splitTokenLines`, plus the already-baselined `parseObjectSchema`, are suppressed in place rather than refactored — their depth is entwined with the same structural debt already on the baseline. Baselines regenerated to match the two methods whose signatures changed from the new @Suppress annotations. All JVM tests (root, kson-lib, kson-tooling-lib, tooling:jetbrains) pass. # Conflicts: # detekt.yml
holodorum
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow up refactor detekted by the static code analysis of #378
Detekt update two thresholds to accommodate existing code. Both rules are exactly the kind that tend to surface bugs—a stray extra boolean term or another level of nesting is precisely how parser/validator logic quietly goes wrong—so we'd rather see the warnings and fix the code than loosen the bar.
Baselines regenerated to match the two methods whose signatures changed
from the new @Suppress annotations.