Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Detekt errors on releases/v80.0.0 and master #13424

Closed
sysrqb opened this issue Aug 9, 2020 · 4 comments
Closed

Detekt errors on releases/v80.0.0 and master #13424

sysrqb opened this issue Aug 9, 2020 · 4 comments
Assignees
Labels
eng:health Improve code health

Comments

@sysrqb
Copy link

sysrqb commented Aug 9, 2020

Running ./gradlew detekt there are two separate issues.

  1. The original exception message was: Value "trued" set for config parameter "complexity > LongParameterList > active" is not of required type Boolean.

This error exists on master, too, and it is fixed by

diff --git a/config/detekt.yml b/config/detekt.yml
index 56b6041c2..2568a929e 100644
--- a/config/detekt.yml
+++ b/config/detekt.yml
@@ -76,7 +76,7 @@ complexity:
     # https://github.com/mozilla-mobile/fenix/issues/4861
     threshold: 75
   LongParameterList:
-    active: trued
+    active: true
     excludes: "**/*Controller.kt, **/*Integration.kt"
     functionThreshold: 6
     constructorThreshold: 7
  1. After correcting (1), detekt complains about:
complexity - 2h debt
        ComplexMethod - 15/15 - [start] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/LeanplumMetricsService.kt:96:18
        LongMethod - 78/75 - [toEvent] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt:535:18
        LargeClass - 200/200 - [AwesomeBarView] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt:36:7
        LargeClass - 226/200 - [ToolbarGestureHandler] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/browser/ToolbarGestureHandler.kt:38:7
        TooManyFunctions - 11/11 - [ToolbarGestureHandler] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/browser/ToolbarGestureHandler.kt:38:7
        ComplexCondition - 4/4 - [onSwipeStarted] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/browser/ToolbarGestureHandler.kt:80:13
style - 15min debt
        MaxLineLength - [onLoadRequest] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt:31:17
        MayBeConst - [PREFERENCE_NAME] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/LeanplumMetricsService.kt:243:9
        MayBeConst - [DEVICE_ID_KEY] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/LeanplumMetricsService.kt:244:9

Will you accept a PR for these? Should the patch simply suppress the warnings for TooManyFunctions, LargeClass, LongMethod, ComplexCondition, and ComplexMethod?

┆Issue is synchronized with this Jira Task

@github-actions github-actions bot added the needs:triage Issue needs triage label Aug 9, 2020
@sysrqb
Copy link
Author

sysrqb commented Aug 9, 2020

Oh. After fixing (1) above on master, these are the detekt issues:

/home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt - 20min debt
        ComplexCondition - 4/4 - [performHapticIfNeeded] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt:254:13
/home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/LeanplumMetricsService.kt - 30min debt
        ComplexMethod - 15/15 - [start] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/LeanplumMetricsService.kt:96:18
        MayBeConst - [PREFERENCE_NAME] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/LeanplumMetricsService.kt:243:9
        MayBeConst - [DEVICE_ID_KEY] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/LeanplumMetricsService.kt:244:9
/home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt - 20min debt
        LongMethod - 84/75 - [toEvent] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt:143:22
/home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/browser/ToolbarGestureHandler.kt - 1h debt
        LargeClass - 226/200 - [ToolbarGestureHandler] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/browser/ToolbarGestureHandler.kt:38:7
        TooManyFunctions - 11/11 - [ToolbarGestureHandler] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/browser/ToolbarGestureHandler.kt:38:7
        ComplexCondition - 4/4 - [onSwipeStarted] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/browser/ToolbarGestureHandler.kt:80:13
/home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt - 5min debt
        MaxLineLength - [onLoadRequest] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt:31:17
/home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/tabhistory/TabHistoryViewHolder.kt - 5min debt
        MaxLineLength - [bind] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/tabhistory/TabHistoryViewHolder.kt:36:13
/home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/utils/FeatureFlagPreference.kt - 5min debt
        MaxLineLength - [featureFlagPreference] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/utils/FeatureFlagPreference.kt:11:1
/home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt - 10min debt
        MagicNumber - [<anonymous>] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt:45:27
/home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt - 10min debt
        MagicNumber - [showEngineView] at /home/androidstudio/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt:589:91
/home/androidstudio/fenix/app/src/geckoRelease/java/org/mozilla/fenix/engine/GeckoProvider.kt - 10min debt
        ForbiddenComment - [runtimeSettings] at /home/androidstudio/fenix/app/src/geckoRelease/java/org/mozilla/fenix/engine/GeckoProvider.kt:53:13 

Overall debt: 2h 55min

@sysrqb sysrqb changed the title Detekt errors on releases/v80.0.0 Detekt errors on releases/v80.0.0 and master Aug 9, 2020
@data-sync-user data-sync-user changed the title Detekt errors on releases/v80.0.0 and master FNX2-18774 ⁃ Detekt errors on releases/v80.0.0 and master Aug 10, 2020
@data-sync-user data-sync-user changed the title FNX2-18774 ⁃ Detekt errors on releases/v80.0.0 and master FNX3-23096 ⁃ Detekt errors on releases/v80.0.0 and master Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX3-23096 ⁃ Detekt errors on releases/v80.0.0 and master FNX-14605 ⁃ Detekt errors on releases/v80.0.0 and master Aug 11, 2020
@person808
Copy link
Contributor

Good catch!

@person808 person808 self-assigned this Aug 11, 2020
@person808 person808 added eng:tech-debt Engineering debt. Missing unit tests, etc. and removed needs:triage Issue needs triage labels Aug 11, 2020
@person808
Copy link
Contributor

person808 commented Aug 11, 2020

Will you accept a PR for these? Should the patch simply suppress the warnings for TooManyFunctions, LargeClass, LongMethod, ComplexCondition, and ComplexMethod?

I'll take care of it! A lot of these are probably not avoidable so suppressing the warning is what I'll do

@person808 person808 added eng:health Improve code health and removed eng:tech-debt Engineering debt. Missing unit tests, etc. labels Aug 11, 2020
person808 added a commit to person808/fenix that referenced this issue Aug 11, 2020
person808 added a commit to person808/fenix that referenced this issue Aug 11, 2020
person808 added a commit to person808/fenix that referenced this issue Aug 12, 2020
@person808
Copy link
Contributor

Fixed!

@data-sync-user data-sync-user changed the title FNX-14605 ⁃ Detekt errors on releases/v80.0.0 and master Detekt errors on releases/v80.0.0 and master May 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:health Improve code health
Projects
None yet
Development

No branches or pull requests

2 participants