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

New Rule: UnnecessaryRequireNotNull #5204

Closed
pbreault opened this issue Aug 8, 2022 · 4 comments
Closed

New Rule: UnnecessaryRequireNotNull #5204

pbreault opened this issue Aug 8, 2022 · 4 comments
Labels

Comments

@pbreault
Copy link

pbreault commented Aug 8, 2022

Expected Behavior of the rule

Would there be any appetite for a new UnnecessaryRequireNotNull rule?

This would be similar to UnnecessaryNotNullOperator but it would flag calls to requireNotNull with a non-nullable parameter.

// non compliant code 
var string = "foo"
println(requireNotNull(string))

// compliant code
var string : String? = "foo"
println(requireNotNull(string))

Context

We have seen optional field become required after refactorings, leaving behind unnecessary calls to requireNotNull.
We have built a custom rule to detect these cases and we would like to contribute it back.

@pbreault pbreault added the rules label Aug 8, 2022
@3flex
Copy link
Member

3flex commented Aug 9, 2022

I think this would be a great addition, and should also check for the same case with checkNotNull. We'd need a different rule name if it checks both cases though (or a separate rule for check, but I don't think that makes sense).

@schalkms
Copy link
Member

schalkms commented Aug 9, 2022

I second this. This rule would be a great addition to detekt.
Shall the rule be called UnnecessaryNotNullCheck?

@pbreault may I ask you to take a stab at this rule?

@pbreault
Copy link
Author

pbreault commented Aug 9, 2022

I think a single UnnecessaryNotNullCheck rule that encompasses checkNotNull and requireNotNull makes sense. Definitely more future-proof. I'll take a stab at it.

@pbreault
Copy link
Author

My colleague @VirtualParticle just opened #5218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants