Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReturnCount 1.22.0 crashes on valid 1.21.0 config property excludedFunctions when using --all-rules cli flag #5323

Closed
arturbosch opened this issue Sep 20, 2022 · 5 comments · Fixed by #5347
Labels
Milestone

Comments

@arturbosch
Copy link
Member

arturbosch commented Sep 20, 2022

Expected Behavior

Configuration without migration entry in changelog should work in next minor version.

Observed Behavior

Caused by: java.lang.IllegalStateException: Value "[equals]" set for config parameter "style > ReturnCount > excludedFunctions" is not of required type String.

When using --all-rules cli flag.

Unexpected error while running detekt analysis

java.lang.IllegalStateException: Analyzing <file.kt> led to an exception.
Location: io.gitlab.arturbosch.detekt.core.config.BaseConfigKt.valueOrDefaultInternal(BaseConfig.kt:30)
The original exception message was: Value "[equals]" set for config parameter "style > ReturnCount > excludedFunctions" is not of required type String.
Running detekt '1.22.0-RC1' on Java '17.0.4+7-b469.53' on OS 'Linux'
If the exception message does not help, please feel free to create an issue on our GitHub page.
	at io.gitlab.arturbosch.detekt.core.AnalyzerKt.throwIllegalStateException(Analyzer.kt:184)
	at io.gitlab.arturbosch.detekt.core.AnalyzerKt.access$throwIllegalStateException(Analyzer.kt:1)
	at io.gitlab.arturbosch.detekt.core.Analyzer.runSync(Analyzer.kt:74)
	at io.gitlab.arturbosch.detekt.core.Analyzer.run(Analyzer.kt:53)
	at io.gitlab.arturbosch.detekt.core.tooling.Lifecycle$analyze$result$1.invoke(Lifecycle.kt:47)
	at io.gitlab.arturbosch.detekt.core.tooling.Lifecycle$analyze$result$1.invoke(Lifecycle.kt:44)
	at io.gitlab.arturbosch.detekt.core.util.PerformanceMonitor.measure(PerformanceMonitor.kt:42)
	at io.gitlab.arturbosch.detekt.core.tooling.Lifecycle$DefaultImpls.measure(Lifecycle.kt:34)
	at io.gitlab.arturbosch.detekt.core.tooling.Lifecycle$DefaultImpls.analyze(Lifecycle.kt:44)
	at io.gitlab.arturbosch.detekt.core.tooling.DefaultLifecycle.analyze(Lifecycle.kt:61)
	at io.gitlab.arturbosch.detekt.core.tooling.AnalysisFacade$runAnalysis$1.invoke(AnalysisFacade.kt:48)
	at io.gitlab.arturbosch.detekt.core.tooling.AnalysisFacade$runAnalysis$1.invoke(AnalysisFacade.kt:47)
	at io.gitlab.arturbosch.detekt.core.tooling.ProcessingSpecSettingsBridgeKt.withSettings(ProcessingSpecSettingsBridge.kt:26)
	at io.gitlab.arturbosch.detekt.core.tooling.AnalysisFacade.runAnalysis$detekt_core(AnalysisFacade.kt:47)
	at io.gitlab.arturbosch.detekt.core.tooling.AnalysisFacade.run(AnalysisFacade.kt:33)
	at io.gitlab.arturbosch.detekt.idea.ConfiguredService.execute(ConfiguredService.kt:140)
	at io.gitlab.arturbosch.detekt.idea.ConfiguredService.execute(ConfiguredService.kt:117)
	at io.gitlab.arturbosch.detekt.idea.DetektAnnotator.doAnnotate(DetektAnnotator.kt:39)
	at io.gitlab.arturbosch.detekt.idea.DetektAnnotator.doAnnotate(DetektAnnotator.kt:19)
	at com.intellij.codeInsight.daemon.impl.ExternalToolPass.doAnnotate(ExternalToolPass.java:220)
	at com.intellij.codeInsight.daemon.impl.ExternalToolPass.doAnnotate(ExternalToolPass.java:214)
	at com.intellij.codeInsight.daemon.impl.ExternalToolPass$1.lambda$run$0(ExternalToolPass.java:192)
	at com.intellij.codeInsight.daemon.impl.ExternalToolPass.runChangeAware(ExternalToolPass.java:289)
	at com.intellij.codeInsight.daemon.impl.ExternalToolPass$1.lambda$run$2(ExternalToolPass.java:192)
	at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$runProcess$2(CoreProgressManager.java:188)
	at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$executeProcessUnderProgress$12(CoreProgressManager.java:608)
	at com.intellij.openapi.progress.impl.CoreProgressManager.registerIndicatorAndRun(CoreProgressManager.java:683)
	at com.intellij.openapi.progress.impl.CoreProgressManager.computeUnderProgress(CoreProgressManager.java:639)
	at com.intellij.openapi.progress.impl.CoreProgressManager.executeProcessUnderProgress(CoreProgressManager.java:607)
	at com.intellij.openapi.progress.impl.ProgressManagerImpl.executeProcessUnderProgress(ProgressManagerImpl.java:60)
	at com.intellij.openapi.progress.impl.CoreProgressManager.runProcess(CoreProgressManager.java:175)
	at com.intellij.openapi.progress.util.BackgroundTaskUtil.runUnderDisposeAwareIndicator(BackgroundTaskUtil.java:365)
	at com.intellij.openapi.progress.util.BackgroundTaskUtil.runUnderDisposeAwareIndicator(BackgroundTaskUtil.java:343)
	at com.intellij.codeInsight.daemon.impl.ExternalToolPass$1.run(ExternalToolPass.java:191)
	at com.intellij.util.ui.update.MergingUpdateQueue.execute(MergingUpdateQueue.java:332)
	at com.intellij.util.ui.update.MergingUpdateQueue.execute(MergingUpdateQueue.java:322)
	at com.intellij.util.ui.update.MergingUpdateQueue.lambda$flush$1(MergingUpdateQueue.java:271)
	at com.intellij.util.ui.update.MergingUpdateQueue.flush(MergingUpdateQueue.java:285)
	at com.intellij.util.ui.update.MergingUpdateQueue.run(MergingUpdateQueue.java:240)
	at com.intellij.util.concurrency.QueueProcessor.runSafely(QueueProcessor.java:241)
	at com.intellij.util.Alarm$Request.runSafely(Alarm.java:388)
	at com.intellij.util.Alarm$Request.run(Alarm.java:377)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at com.intellij.util.concurrency.SchedulingWrapper$MyScheduledFutureTask.run(SchedulingWrapper.java:223)
	at com.intellij.util.concurrency.BoundedTaskExecutor.doRun(BoundedTaskExecutor.java:241)
	at com.intellij.util.concurrency.BoundedTaskExecutor.access$200(BoundedTaskExecutor.java:31)
	at com.intellij.util.concurrency.BoundedTaskExecutor$1.execute(BoundedTaskExecutor.java:214)
	at com.intellij.util.ConcurrencyUtil.runUnderThreadName(ConcurrencyUtil.java:212)
	at com.intellij.util.concurrency.BoundedTaskExecutor$1.run(BoundedTaskExecutor.java:203)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(Executors.java:702)
	at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(Executors.java:699)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
	at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1.run(Executors.java:699)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.IllegalStateException: Value "[equals]" set for config parameter "style > ReturnCount > excludedFunctions" is not of required type String.
	at io.gitlab.arturbosch.detekt.core.config.BaseConfigKt.valueOrDefaultInternal(BaseConfig.kt:30)
	at io.gitlab.arturbosch.detekt.core.config.BaseConfigKt.valueOrDefaultInternal$default(BaseConfig.kt:11)
	at io.gitlab.arturbosch.detekt.core.config.YamlConfig.valueOrDefault(YamlConfig.kt:33)
	at io.gitlab.arturbosch.detekt.core.config.AllRulesConfig.valueOrDefault(AllRulesConfig.kt:19)
	at io.gitlab.arturbosch.detekt.core.config.DisabledAutoCorrectConfig.valueOrDefault(DisabledAutoCorrectConfig.kt:15)
	at io.gitlab.arturbosch.detekt.api.ConfigAware$DefaultImpls.valueOrDefault(ConfigAware.kt:65)
	at io.gitlab.arturbosch.detekt.api.Rule.valueOrDefault(Rule.kt:20)
	at io.gitlab.arturbosch.detekt.api.internal.PathFiltersKt.valueOrDefaultCommaSeparated$fallBack(PathFilters.kt:69)
	at io.gitlab.arturbosch.detekt.api.internal.PathFiltersKt.valueOrDefaultCommaSeparated(PathFilters.kt:77)
	at io.gitlab.arturbosch.detekt.api.ConfigPropertyKt.getListOrDefault(ConfigProperty.kt:130)
	at io.gitlab.arturbosch.detekt.api.ConfigPropertyKt.getValueOrDefault(ConfigProperty.kt:115)
	at io.gitlab.arturbosch.detekt.api.ConfigPropertyKt.access$getValueOrDefault(ConfigProperty.kt:1)
	at io.gitlab.arturbosch.detekt.api.TransformedConfigProperty.doGetValue(ConfigProperty.kt:192)
	at io.gitlab.arturbosch.detekt.api.MemoizedConfigProperty.getValue(ConfigProperty.kt:169)
	at io.gitlab.arturbosch.detekt.api.MemoizedConfigProperty.getValue(ConfigProperty.kt:165)
	at io.gitlab.arturbosch.detekt.rules.style.ReturnCount.getExcludedFunctions(ReturnCount.kt:63)
	at io.gitlab.arturbosch.detekt.rules.style.ReturnCount.shouldBeIgnored(ReturnCount.kt:93)
	at io.gitlab.arturbosch.detekt.rules.style.ReturnCount.visitNamedFunction(ReturnCount.kt:77)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitNamedFunction(KtVisitorVoid.java:491)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitNamedFunction(KtVisitorVoid.java:21)
	at org.jetbrains.kotlin.psi.KtNamedFunction.accept(KtNamedFunction.java:51)
	at org.jetbrains.kotlin.psi.KtElementImplStub.accept(KtElementImplStub.java:49)
	at org.jetbrains.kotlin.com.intellij.psi.impl.PsiElementBase.acceptChildren(PsiElementBase.java:69)
	at org.jetbrains.kotlin.psi.KtTreeVisitorVoid.visitElement(KtTreeVisitorVoid.java:25)
	at org.jetbrains.kotlin.psi.KtVisitor.visitKtElement(KtVisitor.java:24)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitKtElement(KtVisitorVoid.java:25)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitKtElement(KtVisitorVoid.java:455)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitKtElement(KtVisitorVoid.java:21)
	at org.jetbrains.kotlin.psi.KtVisitor.visitClassBody(KtVisitor.java:98)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClassBody(KtVisitorVoid.java:89)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClassBody(KtVisitorVoid.java:545)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClassBody(KtVisitorVoid.java:21)
	at org.jetbrains.kotlin.psi.KtClassBody.accept(KtClassBody.kt:38)
	at org.jetbrains.kotlin.psi.KtElementImplStub.accept(KtElementImplStub.java:49)
	at org.jetbrains.kotlin.com.intellij.psi.impl.PsiElementBase.acceptChildren(PsiElementBase.java:69)
	at org.jetbrains.kotlin.psi.KtTreeVisitorVoid.visitElement(KtTreeVisitorVoid.java:25)
	at org.jetbrains.kotlin.psi.KtVisitor.visitKtElement(KtVisitor.java:24)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitKtElement(KtVisitorVoid.java:25)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitKtElement(KtVisitorVoid.java:455)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitKtElement(KtVisitorVoid.java:21)
	at org.jetbrains.kotlin.psi.KtVisitor.visitExpression(KtVisitor.java:186)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitExpression(KtVisitorVoid.java:173)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitExpression(KtVisitorVoid.java:667)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitExpression(KtVisitorVoid.java:21)
	at org.jetbrains.kotlin.psi.KtVisitor.visitDeclaration(KtVisitor.java:29)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitDeclaration(KtVisitorVoid.java:29)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitDeclaration(KtVisitorVoid.java:461)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitDeclaration(KtVisitorVoid.java:21)
	at org.jetbrains.kotlin.psi.KtVisitor.visitNamedDeclaration(KtVisitor.java:406)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitNamedDeclaration(KtVisitorVoid.java:385)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitNamedDeclaration(KtVisitorVoid.java:973)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitNamedDeclaration(KtVisitorVoid.java:21)
	at org.jetbrains.kotlin.psi.KtVisitor.visitClassOrObject(KtVisitor.java:41)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClassOrObject(KtVisitorVoid.java:37)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClassOrObject(KtVisitorVoid.java:473)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClassOrObject(KtVisitorVoid.java:21)
	at org.jetbrains.kotlin.psi.KtVisitor.visitClass(KtVisitor.java:33)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClass(KtVisitorVoid.java:33)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClass(KtVisitorVoid.java:467)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitClass(KtVisitorVoid.java:21)
	at org.jetbrains.kotlin.psi.KtClass.accept(KtClass.kt:20)
	at org.jetbrains.kotlin.psi.KtElementImplStub.accept(KtElementImplStub.java:49)
	at org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.SharedImplUtil.acceptChildren(SharedImplUtil.java:185)
	at org.jetbrains.kotlin.com.intellij.psi.impl.source.PsiFileImpl.acceptChildren(PsiFileImpl.java:754)
	at org.jetbrains.kotlin.psi.KtTreeVisitorVoid.visitElement(KtTreeVisitorVoid.java:25)
	at org.jetbrains.kotlin.com.intellij.psi.PsiElementVisitor.visitFile(PsiElementVisitor.java:35)
	at org.jetbrains.kotlin.psi.KtVisitor.visitKtFile(KtVisitor.java:73)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitKtFile(KtVisitorVoid.java:69)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitKtFile(KtVisitorVoid.java:521)
	at org.jetbrains.kotlin.psi.KtVisitorVoid.visitKtFile(KtVisitorVoid.java:21)
	at org.jetbrains.kotlin.psi.KtFile.accept(KtFile.kt:249)
	at org.jetbrains.kotlin.psi.KtFile.accept(KtFile.kt:236)
	at io.gitlab.arturbosch.detekt.api.BaseRule.visit(BaseRule.kt:56)
	at io.gitlab.arturbosch.detekt.api.BaseRule.visitFile(BaseRule.kt:46)
	at io.gitlab.arturbosch.detekt.core.Analyzer.analyze$executeRules(Analyzer.kt:130)
	at io.gitlab.arturbosch.detekt.core.Analyzer.analyze(Analyzer.kt:142)
	at io.gitlab.arturbosch.detekt.core.Analyzer.runSync(Analyzer.kt:73)
	... 54 more

Steps to Reproduce

# config.yaml
  ReturnCount:
    active: true
    max: 2
    excludedFunctions: 'equals'
    excludeLabeled: false
    excludeReturnFromLambda: true
    excludeGuardClauses: false

CLI

java -jar detekt-cli/build/libs/detekt-cli-1.22.0-RC1-all.jar -c ~/dotfiles/detekt/detekt.yml -i detekt-cli/build/resources/test/cases/Poko.kt --all-rules

IJ plugin

  1. Build current IJ plugin main branch.
  2. Install plugin, enable all-rules and set the config
  3. Go to any file

Context

Testing 1.22.0-RC1

Your Environment

  • Version of detekt used: 1.22.0-RC1
  • Version of Gradle used (if applicable): 7.5.1
  • Operating System and version: Fedora Linux 37
@arturbosch arturbosch added the bug label Sep 20, 2022
@cortinico
Copy link
Member

Yup that's "expected" I guess as we updated the type of that property. I omitted it from the "notable changes" but it feels like we want to make it more prominent:

#5120 (comment)

@arturbosch
Copy link
Member Author

Refere

Hm, as far as I see in the linked PR there is a testcase to verify the rule still works with a string property.
Shouldn't both string and list work for the next releases and trigger a deprecation warning?

@cortinico
Copy link
Member

verify the rule still works with a string property.

Nope there is no such test. All the tests are checking against a listOf(...) values.
So if a user is using a string type, they will crash.

The alternative approach here would be to deprecate excludedFunctions and have a excludedFunctionsList instead

@arturbosch
Copy link
Member Author

arturbosch commented Sep 20, 2022

Hm, I see

fun `should not get flagged`() {
val findings = ReturnCount(
TestConfig(
mapOf(
MAX to "2",
EXCLUDED_FUNCTIONS to "test"
)
)
).compileAndLint(code)
assertThat(findings).isEmpty()
}
}

which takes "test" as string literal and converts it to a list via

private fun ConfigAware.getListOrDefault(propertyName: String, defaultValue: List<*>): List<String> {
return if (defaultValue.all { it is String }) {
@Suppress("UNCHECKED_CAST")
val defaultValueAsListOfStrings = defaultValue as List<String>
valueOrDefaultCommaSeparated(propertyName, defaultValueAsListOfStrings)
} else {
error("Only lists of strings are supported. '$propertyName' is invalid. ")
}
}

Edit:
Okay, running the CLI I get
Property 'style>ReturnCount>excludedFunctions' should be a YAML array instead of a comma-separated String. which should be expecting. The property is deprecate.
So I need to debug why the IJ plugin does not like it

Edit2:
I can reproduce the issue with following cli command: java -jar detekt-cli/build/libs/detekt-cli-1.22.0-RC1-all.jar -c ~/dotfiles/detekt/detekt.yml -i detekt-cli/build/resources/test/cases/Poko.kt --all-rules

@arturbosch arturbosch changed the title ReturnCount 1.22.0 crashes on valid 1.21.0 config property excludedFunctions ReturnCount 1.22.0 crashes on valid 1.21.0 config property excludedFunctions when using --all-rules Sep 20, 2022
@arturbosch arturbosch changed the title ReturnCount 1.22.0 crashes on valid 1.21.0 config property excludedFunctions when using --all-rules ReturnCount 1.22.0 crashes on valid 1.21.0 config property excludedFunctions when using --all-rules cli flag Sep 20, 2022
@BraisGabin
Copy link
Collaborator

Yes, this should be a deprecation warning but it should work. I think that were we stop suportting Strings as list were on the rules that have a "reason". For example ForbiddenImport.

schalkms pushed a commit that referenced this issue Sep 25, 2022
…ue (#5347)

* Convert previously known string property to list based on default value - #5323

* Improve test name
@arturbosch arturbosch added this to the 1.22.0 milestone Sep 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants