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.

Sign up for GitHub

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

Jump to bottom

Add list of functions to skip in IgnoredReturnValue rule #4434

Merged
merged 9 commits into from Jan 28, 2022
Merged

Add list of functions to skip in IgnoredReturnValue rule #4434

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d3bd193
Add list of functions to skip in IgnoredReturnValue rule
artem-zinnatullin Jan 1, 2022
f11ccbf
Fix formatting
artem-zinnatullin Jan 1, 2022
8537603
Regenerate default detekt config
artem-zinnatullin Jan 1, 2022
35e851c
Switch to FunctionMatcher, rename property to ignoreFunctionCall
artem-zinnatullin Jan 3, 2022
2749f47
Update test
artem-zinnatullin Jan 3, 2022
ec8c643
style
artem-zinnatullin Jan 3, 2022
eb912c8
Damn it IJ
artem-zinnatullin Jan 3, 2022
da96af0
remove testing version
artem-zinnatullin Jan 3, 2022
d575313
Update detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/d…
BraisGabin Jan 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
View file
Open in desktop
Expand Up @@ -552,6 +552,7 @@ potential-bugs:
- '*.CheckReturnValue'
ignoreReturnValueAnnotations:
- '*.CanIgnoreReturnValue'
skipFunctions: []
ImplicitDefaultLocale:
active: true
ImplicitUnitReturnType:
Expand Down
9 changes: 9 additions & 0 deletions ...s-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/IgnoredReturnValue.kt
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@ import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression
import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
import org.jetbrains.kotlin.types.typeUtil.isUnit

/**
Expand Down Expand Up @@ -58,6 +59,12 @@ class IgnoredReturnValue(config: Config = Config.empty) : Rule(config) {
it.map(String::simplePatternToRegex)
}

@Configuration(
"List of fully-qualified function names that should be skipped by this check. " +
"Example: 'package.class.fun1'"
)
private val skipFunctions: List<String> by config(emptyList())
Copy link
Member

Choose a reason for hiding this comment

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

I think that you should use FunctionMatcher. It provides more porwerful matching (you can match only by name or name + param types. Here a rule using it:

detekt/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt

Lines 46 to 58 in ff7824d

@Configuration(
"List of fully qualified method signatures which are forbidden. " +
"Methods can be defined without full signature (i.e. `java.time.LocalDate.now`) which will report " +
"calls of all methods with this name or with full signature " +
"(i.e. `java.time.LocalDate(java.time.Clock)`) which would report only call " +
"with this concrete signature."
)
private val methods: List<FunctionMatcher> by config(
listOf(
"kotlin.io.print",
"kotlin.io.println",
)
) { it.map(FunctionMatcher::fromFunctionSignature) }


@Suppress("ReturnCount")
override fun visitCallExpression(expression: KtCallExpression) {
super.visitCallExpression(expression)
Expand All @@ -68,6 +75,8 @@ class IgnoredReturnValue(config: Config = Config.empty) : Rule(config) {
val resultingDescriptor = expression.getResolvedCall(bindingContext)?.resultingDescriptor ?: return
if (resultingDescriptor.returnType?.isUnit() == true) return

if (resultingDescriptor.fqNameSafe.asString() in skipFunctions) return

val annotations = resultingDescriptor.annotations
if (annotations.any { it in ignoreReturnValueAnnotations }) return
if (restrictToAnnotatedMethods &&
Expand Down
23 changes: 23 additions & 0 deletions ...rorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/IgnoredReturnValueSpec.kt
View file
Open in desktop
Expand Up @@ -813,5 +813,28 @@ object IgnoredReturnValueSpec : Spek({
val findings = rule.compileAndLintWithContext(env, code)
assertThat(findings).isEmpty()
}

it("does not report when a function is in skip list") {
val code = """
package foo

fun listOfChecked(value: String) = listOf(value)

fun foo() : Int {
listOfChecked("hello")
return 42
}
"""
val rule = IgnoredReturnValue(
TestConfig(
mapOf(
"skipFunctions" to listOf("foo.listOfChecked"),
"restrictToAnnotatedMethods" to false
)
)
)
val findings = rule.compileAndLintWithContext(env, code)
assertThat(findings).isEmpty()
}
}
})