Skip to content

Conversation

@BraisGabin
Copy link
Member

Match generic names in signature functions.

Now ForbiddenMethod call allows to forbid kotlin.runCatching {} but allowing runCatching {}.

closes #4448

@BraisGabin BraisGabin added this to the 1.20.0 milestone Jan 6, 2022
@codecov
Copy link

codecov bot commented Jan 6, 2022

Codecov Report

Merging #4460 (568c2a4) into main (d7ac501) will increase coverage by 0.00%.
The diff coverage is 62.50%.

@@            Coverage Diff            @@
##               main    #4460   +/-   ##
=========================================
  Coverage     84.55%   84.56%           
  Complexity     3397     3397           
=========================================
  Files           484      484           
  Lines         11272    11276    +4     
  Branches       2059     2060    +1     
=========================================
+ Hits           9531     9535    +4     
  Misses          698      698           
  Partials       1043     1043           
Impacted Files Coverage Δ
...in/io/github/detekt/tooling/api/FunctionMatcher.kt 87.50% <62.50%> (+0.73%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7ac501...568c2a4. Read the comment docs.

@BraisGabin BraisGabin marked this pull request as draft January 8, 2022 11:48
@chao2zhang chao2zhang self-requested a review January 23, 2022 20:19
@BraisGabin BraisGabin force-pushed the function-extension-matcher branch from f73771a to 5d8588f Compare January 25, 2022 08:44
@BraisGabin BraisGabin force-pushed the function-generics-matcher branch from eefa7bc to 746dcaa Compare January 25, 2022 08:45
@BraisGabin BraisGabin force-pushed the function-extension-matcher branch from 5d8588f to dc617af Compare January 30, 2022 11:47
@BraisGabin BraisGabin force-pushed the function-generics-matcher branch from 746dcaa to aa6655a Compare January 30, 2022 11:52
) : FunctionMatcher() {
override fun match(callableDescriptor: CallableDescriptor): Boolean {
if (callableDescriptor.fqNameOrNull()?.asString() != fullyQualifiedName) return false
val descriptor = callableDescriptor.original
Copy link
Member

Choose a reason for hiding this comment

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

Q: What is .original doing here? Why is it needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

That get's the function definition instead of the function call. If I request the type of the parameter of list("hello") it will say String but if I ask for the type of the original I get T.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha 👍 Thanks for clarifying

@BraisGabin BraisGabin force-pushed the function-extension-matcher branch from dc617af to 6a9f0ca Compare January 31, 2022 08:40
@BraisGabin BraisGabin force-pushed the function-generics-matcher branch from aa6655a to 5c370c3 Compare January 31, 2022 08:40
@cortinico cortinico removed this from the 1.20.0 milestone Mar 23, 2022
Base automatically changed from function-extension-matcher to main March 24, 2022 07:54
@BraisGabin BraisGabin marked this pull request as ready for review March 24, 2022 07:55
@BraisGabin
Copy link
Member Author

I would like to merge this one in 1.20 too if possible. This PR was waiting for #4459 to be merged because it depends on that code.

@BraisGabin BraisGabin force-pushed the function-generics-matcher branch from 5c370c3 to 568c2a4 Compare March 24, 2022 08:04
@BraisGabin BraisGabin requested a review from cortinico March 24, 2022 08:05
@cortinico cortinico added this to the 1.20.0 milestone Mar 24, 2022
@cortinico cortinico added the notable changes Marker for notable changes in the changelog label Mar 24, 2022
@cortinico
Copy link
Member

I'm fine adding this, but we need some follow-up docs for this to the website

@BraisGabin
Copy link
Member Author

🤔 I don't think we need to add new documentation here. The current documentation matchs what this matcher does. This one is more a "bug-fix" than a new feature. I mean, for sure it is a new feature but for a user it could seems like the same. The PR that introduced real changes was #4459 and I added the documentation directly in it. https://github.com/detekt/detekt/pull/4459/files#diff-76703cf8efe31f0c7e06926568e0d1e248eb82cb63bd40b0b37698dc35bed70fR52

@BraisGabin BraisGabin merged commit e052dbe into main Mar 31, 2022
@BraisGabin BraisGabin deleted the function-generics-matcher branch March 31, 2022 06:48
chao2zhang pushed a commit that referenced this pull request Apr 8, 2022
* Match functions with generics

* Add test to ensure that #4448 is fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notable changes Marker for notable changes in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add rule to use runCatching instead of kotlin.runCatching

3 participants