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

Make UnsafeCast less aggressive #1601

Closed
3flex opened this issue Apr 20, 2019 · 2 comments · Fixed by #1879
Closed

Make UnsafeCast less aggressive #1601

3flex opened this issue Apr 20, 2019 · 2 comments · Fixed by #1879

Comments

@3flex
Copy link
Member

3flex commented Apr 20, 2019

Expected Behavior of the rule

Make UnsafeCast rule less aggressive so it only flags usage of the "unsafe" cast operator when the cast is known to fail:

    val something: String = 3 as String
    val somethingElse: String? = 3 as? String

but do not flag usage of the operator when there's not enough information for the compiler to be sure at compile time that the case will fail.

Context

With type and symbol resolution we can flag usage of the "unsafe" cast operator when we know at compile time that the cast will fail, but ignore usage of the operator when it only might fail to avoid false positives.

There are some redundant usages of the "safe" cast operator within detekt that can't currently be changed to the "unsafe" operator without flagging UnsafeCast even when it would be OK, like:

// safe cast to KtValueArgument not necessary since type was already checked
parent is KtValueArgument && (parent as? KtValueArgument)?.isNamed() == true
// PomTransaction is a subclass of PomTransactionBase so "unsafe" cast is safe
        override fun runTransaction(transaction: PomTransaction) {
            (transaction as? PomTransactionBase)?.run()
        }
// docs say "project.version" returns a string and is never null, so safe cast not needed
override = (project.version as? String)?.endsWith("-SNAPSHOT") == true

Changing it to be less aggressive and match behavior of the compiler would help with this.

@3flex 3flex changed the title UnsafeCast - loose & strict mode Make UnsafeCast less aggressive Apr 20, 2019
@3flex
Copy link
Member Author

3flex commented Aug 27, 2019

@arturbosch I'm happy to work on this one but want to check first that you're ok with this approach since it will also reduce the positive detection rate. I think it's better to reduce false positives as much as possible even if the rule starts to miss some real issues. Thoughts?

@arturbosch
Copy link
Member

@3flex yes I totally agree. This would improve a lot.

@3flex 3flex self-assigned this Aug 28, 2019
@3flex 3flex removed their assignment Sep 3, 2019
@arturbosch arturbosch modified the milestones: 1.1.0, 1.0.2 Sep 14, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants