New KDoc resolution #4397
New KDoc resolution #4397
Conversation
...rg/jetbrains/dokka/analysis/kotlin/descriptors/compiler/configuration/AnalysisEnvironment.kt
Show resolved
Hide resolved
AbdullinAM
force-pushed
the
abdullin/kdoc-ambiguity-resolution
branch
from
ce77cc5 to
65dff32
Compare
| compileOnly(libs.kotlinx.coroutines.core) | ||
|
|
||
| // to gain access to com.intellij.diagnostic.LoadingState in the compile time | ||
| compileOnly(libs.intellij.util.base) |
There was a problem hiding this comment.
For some reason LoadingState bundled with the analysis api/compiler does not contains necessary APIs. We have the correct version of the LoadingState in the runtime
...symbols/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/symbols/plugin/KotlinAnalysis.kt
Outdated
Show resolved
Hide resolved
| | * some text | ||
| | * | ||
| | * @see String | ||
| | * @throws Unit |
There was a problem hiding this comment.
This case is quite unexpected without printing the reason. The KEEP only says @exception / @throws must only be resolvable to Throwables. Anyway, Dokka and the IDE should warn or print an error.
Maybe this should be resolved to better understand the misuse. A user should understand where the link refers, especially when it is ambiguous.
General idea:
class MyException : Exception
class Usage {
fun MyException()...
/**
@throws MyException
*/
fun usage() {
}
}
There was a problem hiding this comment.
IDE currently highlights this as a warning, makes sense to add a warning to Dokka too 👍
There was a problem hiding this comment.
It is already logged as a warning:
/**
* @throws String
*/
funfoo(arg: Int) {}
w: [:composeApp:dokkaGeneratePublicationHtml] Couldn't resolve link for String in main.kt/org.example.project.foo
There was a problem hiding this comment.
IMO, this warning should be enough.
Might make sense to add a test for unresolved links in a separate PR?
There was a problem hiding this comment.
I would make this message more straightforward about the reason: In this case, String should be Throwable.
There are two tpes of errors where the link:
- is resolved, but not Throwable
- is unresolved (e.g., because of a typo).
It seems we cannot distinguish between these two cases because the AA returns nothing in both.
Or do we agree Couldn't resolve link for String is enough?
There was a problem hiding this comment.
Yeah, it would be nice to have a better message here, but I think that we will not be able to do it without some additional information from AA.
I think that for now, this warning should be enough, and we should probably create an issue on the AA side to design how this could be supported (in both IDE and Dokka). Perhaps in this case, we need to have some diagnostics inside AA that we can propagate to users? Or still return types from the AA resolver and then additionally check those? (hope not) I don't know.
…ependencies of analysis-kotlin-symbols to gain access to the `LoadingState`
…bResolveStrategy.resolve`
AbdullinAM
force-pushed
the
abdullin/kdoc-ambiguity-resolution
branch
from
65dff32 to
43e6081
Compare
|
The failing tests on CI seem to be caused by external issues, master fails with same errors |
|
Note: TC IT are red because of #4402 (comment) |
whyoleg
left a comment
whyoleg
left a comment
There was a problem hiding this comment.
Nice!
Looks like KT-83152 is the last piece of the puzzle here!
| | * some text | ||
| | * | ||
| | * @see String | ||
| | * @throws Unit |
There was a problem hiding this comment.
IMO, this warning should be enough.
Might make sense to add a test for unresolved links in a separate PR?
|
|
||
| @Test | ||
| @OnlySymbols("KEEP #389: New KDoc resolution") | ||
| fun `short link should lead to package rather than function K2`() = withExperimentalKDocResolution { |
There was a problem hiding this comment.
test should be renamed, as with new resolution, it should lead to the function rather than package (and the test shows it)
|
|
||
| @Test | ||
| @OnlySymbols("KEEP #389: New KDoc resolution") | ||
| fun `fully qualified link should lead to package K2`() = withExperimentalKDocResolution { |
There was a problem hiding this comment.
as in the test below - the name of the test is missleading now
|
|
||
|
|
||
| @Test | ||
| @Ignore("KT-83152 [Analysis API, KDoc] Make class name links on constructors point to the class") |
There was a problem hiding this comment.
Let's create an issue in Dokka to track it and put it here
Enable new KDoc resolution algorithm and add tests