Skip to content

lint: enable detekt rule: NestedScopeFunctions#334

Open
AdrianLeeElder wants to merge 2 commits intodevelopfrom
fix/issue-298
Open

lint: enable detekt rule: NestedScopeFunctions#334
AdrianLeeElder wants to merge 2 commits intodevelopfrom
fix/issue-298

Conversation

@AdrianLeeElder
Copy link
Copy Markdown
Contributor

@AdrianLeeElder AdrianLeeElder commented Mar 31, 2026

Closes #298

Automated by auto-agent.

Summary by CodeRabbit

  • Refactor

    • Simplified null-handling and variable assignment across backend services for clearer control flow.
    • Restructured email assembly and retry validation flows for more straightforward processing.
    • Updated tests and token-creation support code for clearer logic.
  • Chores

    • Enabled an additional code-quality linting rule to enforce improved patterns.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 31, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8f99c2a2-b8c3-47ce-8b2d-d2a2647f8570

📥 Commits

Reviewing files that changed from the base of the PR and between 6f2e766 and 4a1695e.

📒 Files selected for processing (4)
  • backend/src/main/kotlin/com/moneat/dashboards/translation/GrafanaTranslator.kt
  • backend/src/main/kotlin/com/moneat/notifications/services/EmailService.kt
  • backend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsService.kt
  • backend/src/test/kotlin/com/moneat/services/EventServiceTest.kt
✅ Files skipped from review due to trivial changes (2)
  • backend/src/test/kotlin/com/moneat/services/EventServiceTest.kt
  • backend/src/main/kotlin/com/moneat/dashboards/translation/GrafanaTranslator.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsService.kt

📝 Walkthrough

Walkthrough

This pull request enables the Detekt NestedScopeFunctions rule and updates five Kotlin files to remove nested scope-function usage, preserving original logic and null-handling while only changing control-flow style and local variable extraction.

Changes

Cohort / File(s) Summary
Configuration
backend/detekt.yml
Enabled complexity.NestedScopeFunctions by changing active: falseactive: true.
Translation logic
backend/src/main/kotlin/com/moneat/dashboards/translation/GrafanaTranslator.kt
Replaced nested destructuring/let chains with explicit local variables and null-guards in display config extraction and legend parsing.
Email sending
backend/src/main/kotlin/com/moneat/notifications/services/EmailService.kt
Moved creation of MimeBodyPart instances out of the message-builder block into local variables, then added them to the multipart; added class KDoc.
Synthetics service
backend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsService.kt
Rewrote retry parameter validation: cache request fields in locals and use independent if-checks instead of nested lets.
Tests / Helpers
backend/src/test/kotlin/com/moneat/services/EventServiceTest.kt, backend/src/test/kotlin/com/moneat/testsupport/RouteTestSupport.kt
Removed nested scope-function usage in fingerprint construction and JWT claim assignment; preserved behavior via direct null-safe operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped through nested binds and ties,
Unwound the scopes with bright, clean eyes.
Detekt now sees the path made clear,
A tidy code-hop — give a cheer! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: enabling the NestedScopeFunctions detekt rule in the configuration and fixing all violations throughout the codebase.
Linked Issues check ✅ Passed The pull request fulfills all three coding requirements from issue #298: (1) enables NestedScopeFunctions rule in backend/detekt.yml, (2) fixes all reported violations across multiple files (GrafanaTranslator.kt, EmailService.kt, SyntheticsService.kt, EventServiceTest.kt, RouteTestSupport.kt), and (3) achieves CI compliance with maxIssues: 0.
Out of Scope Changes check ✅ Passed All changes are directly related to addressing issue #298: the detekt configuration change enables the rule, and the five modified files contain refactoring to eliminate nested scope function violations detected by the newly-enabled rule.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-298

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/src/test/kotlin/com/moneat/services/EventServiceTest.kt (1)

43-43: ⚠️ Potential issue | 🟡 Minor

Avoid wildcard imports.

Line 43 uses java.util.* which violates the coding guideline requiring explicit imports. Replace with specific imports for the classes actually used.

Proposed fix
-import java.util.*
+import java.util.UUID

As per coding guidelines: "Never use wildcard imports; always use explicit imports (e.g., import com.moneat.models.User instead of import com.moneat.models.*)"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/test/kotlin/com/moneat/services/EventServiceTest.kt` at line 43,
Replace the wildcard import `java.util.*` with explicit imports for only the
classes used in EventServiceTest (e.g., `java.util.UUID`, `java.util.Date`,
`java.util.List` or whichever specific types appear in the file); update the
import statement(s) so there are no wildcard imports and only the concrete
java.util types referenced by the file (look for usages of UUID, Date, List,
etc., to determine which explicit imports to add).
🧹 Nitpick comments (1)
backend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsService.kt (1)

142-149: Refactor is correct, with minor redundancy.

The logic correctly validates retry parameters when either retryCount or retryIntervalMs is provided, using defaults for the missing value. However, when both parameters are non-null, validateRetryParams is called twice with identical arguments.

Consider consolidating to a single validation call:

Optional: Consolidate validation
         val rc = request.retryCount
         val ri = request.retryIntervalMs
-        if (rc != null) {
-            validateRetryParams(rc, ri ?: RETRY_INTERVAL_MS_DEFAULT)
-        }
-        if (ri != null) {
-            validateRetryParams(rc ?: RETRY_COUNT_DEFAULT, ri)
+        if (rc != null || ri != null) {
+            validateRetryParams(rc ?: RETRY_COUNT_DEFAULT, ri ?: RETRY_INTERVAL_MS_DEFAULT)
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsService.kt`
around lines 142 - 149, The current code calls validateRetryParams twice when
both request.retryCount (rc) and request.retryIntervalMs (ri) are set; replace
the two conditional calls with a single validation: if either rc or ri is
non-null, call validateRetryParams(rc ?: RETRY_COUNT_DEFAULT, ri ?:
RETRY_INTERVAL_MS_DEFAULT) so you only validate once and still apply defaults
when one value is missing (refer to request.retryCount, request.retryIntervalMs,
validateRetryParams, RETRY_COUNT_DEFAULT, RETRY_INTERVAL_MS_DEFAULT).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@backend/src/test/kotlin/com/moneat/services/EventServiceTest.kt`:
- Line 43: Replace the wildcard import `java.util.*` with explicit imports for
only the classes used in EventServiceTest (e.g., `java.util.UUID`,
`java.util.Date`, `java.util.List` or whichever specific types appear in the
file); update the import statement(s) so there are no wildcard imports and only
the concrete java.util types referenced by the file (look for usages of UUID,
Date, List, etc., to determine which explicit imports to add).

---

Nitpick comments:
In `@backend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsService.kt`:
- Around line 142-149: The current code calls validateRetryParams twice when
both request.retryCount (rc) and request.retryIntervalMs (ri) are set; replace
the two conditional calls with a single validation: if either rc or ri is
non-null, call validateRetryParams(rc ?: RETRY_COUNT_DEFAULT, ri ?:
RETRY_INTERVAL_MS_DEFAULT) so you only validate once and still apply defaults
when one value is missing (refer to request.retryCount, request.retryIntervalMs,
validateRetryParams, RETRY_COUNT_DEFAULT, RETRY_INTERVAL_MS_DEFAULT).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 26c133e0-9e00-4c4b-b3e7-7ab87b1d6d7c

📥 Commits

Reviewing files that changed from the base of the PR and between fb80968 and 6f2e766.

📒 Files selected for processing (6)
  • backend/detekt.yml
  • backend/src/main/kotlin/com/moneat/dashboards/translation/GrafanaTranslator.kt
  • backend/src/main/kotlin/com/moneat/notifications/services/EmailService.kt
  • backend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsService.kt
  • backend/src/test/kotlin/com/moneat/services/EventServiceTest.kt
  • backend/src/test/kotlin/com/moneat/testsupport/RouteTestSupport.kt

@github-actions
Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable detekt rule: NestedScopeFunctions

1 participant