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

Add rules to suggest usage of check(), require() and error(). #1570

Merged
merged 8 commits into from Apr 8, 2019
Expand Up @@ -44,12 +44,15 @@ class UseCheckOrError(config: Config = Config.empty) : Rule(config) {
)

override fun visitThrowExpression(expression: KtThrowExpression) {
if (expression.isIllegalStateException()) {
if (expression.isIllegalStateExceptionWithoutCause()) {
report(CodeSmell(issue, Entity.from(expression), issue.description))
}
}

private fun KtThrowExpression.isIllegalStateException(): Boolean {
return findDescendantOfType<KtCallExpression>()?.firstChild?.text == "IllegalStateException"
private fun KtThrowExpression.isIllegalStateExceptionWithoutCause(): Boolean {
val callExpression = findDescendantOfType<KtCallExpression>()
Copy link
Member

Choose a reason for hiding this comment

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

Code duplication here
Can we group this together and pass the type of the exception?

val argumentCount = callExpression?.valueArgumentList?.children?.size ?: 0

return callExpression?.firstChild?.text == "IllegalStateException" && argumentCount < 2
}
}
Expand Up @@ -42,6 +42,9 @@ class UseRequire(config: Config = Config.empty) : Rule(config) {
}

private fun KtThrowExpression.isIllegalArgumentException(): Boolean {
return findDescendantOfType<KtCallExpression>()?.firstChild?.text == "IllegalArgumentException"
val callExpression = findDescendantOfType<KtCallExpression>()
val argumentCount = callExpression?.valueArgumentList?.children?.size ?: 0
Copy link
Member

Choose a reason for hiding this comment

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

Code duplication here
Can we group this together and pass the type of the exception?


return callExpression?.firstChild?.text == "IllegalArgumentException" && argumentCount < 2
}
}
Expand Up @@ -66,5 +66,15 @@ class UseCheckOrErrorSpec : Spek({
}"""
assertThat(subject.lint(code)).isEmpty()
}

it("does not report an issue if the exception thrown has a message and a cause") {
val code = """
private fun missing(): Nothing {
if (cause != null) {
throw IllegalStateException("message", cause)
}
}"""
assertThat(subject.lint(code)).isEmpty()
}
}
})
Expand Up @@ -56,5 +56,13 @@ class UseRequireSpec : Spek({
}"""
assertThat(subject.lint(code)).isEmpty()
}

it("does not report an issue if the exception thrown has a message and a cause") {
val code = """
private fun x(a: Int): Nothing {
throw IllegalArgumentException("message", cause)
}"""
assertThat(subject.lint(code)).isEmpty()
}
}
})