Repository metrics
- Stars
- (6,942 stars)
- PR merge metrics
- (平均マージ 4d 9h) (30d で 65 merged PRs)
説明
- Version of detekt used: commit da8b164c5da3a2c8017452355c1754df7344c8a7
I have observed that Retention of annotations used by Detekt sometimes matters, and sometimes doesn't but this is never mentioned in the documentation. This leads to confusion and observed behavior not matching expectations, though I'm unsure if this is primarily a bug or a documentation issue.
As an example (though the issue is likely much more broad and applies to many issus) take IgnoredReturnValue and its configuration property ignoreReturnValueAnnotations. The example in the test "does not report when a function has a custom annotation" (https://github.com/detekt/detekt/blob/cb298b62ab452d89dd8400185b922e4afe11f07b/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/IgnoredReturnValueSpec.kt#L987C13-L987C70) works as expected no matter what the Retention of annotation class CustomIgnoreReturn is. This is what I have observed on Kotlin/JVM in Kotlin 2.1.0 when everything is in the same file. However, if CustomIgnoreReturn has Retention(SOURCE) and is defined in a common module of a dependency, the annotation will not be seen by detekt, causing detekt to behave as if the annotation was not there (in this case, it emits a finding even though that annotation should have prevented it from emitting).
All of these observations make sense if we assume that the an annotation with retention SOURCE is essentially erased before Detekt sees it if the annotation is in a dependency. However, what is problematic here is that the annotation with rentention SOURCE will still work if it is defined in the same file (or probably the same module) as the file being currently analyzed. This is inconsistent, and might lead people on to thinking that an annotation with retention SOURCE is working when really it will stop working (silently) when its placed on something in a dependency.
If all of my understanding above is correct, I suggest some action is taken. The best solution I can think of would be to do these two together:
- Proactively check if an annotation used by Detekt in any way has
Retention(SOURCE). I'm guessing it will still show up in the Psi if it is in the same module, but this should be reported to the user with a warning and/or error or something so that they are aware that this annotation will stop working if placed in a dependency. - Update the documentation to discuss this. Searching the current docs we can see that this topic is never discussed anywhere.
An alternative solution to the above could be that instead of adding new warnings/errors/etc, just make detekt consistently ignore annotations that have Retention(SOURCE) unformly and consistently no matter where they are defined. This would be a potentially breaking change, but would be more "fail-fast" in the sense that a detekt annotation that is wrongly annotated Retention(SOURCE) will always not work instead of confusingly sometimes work, sometimes not work.