Skip to content

Commit 2e9406c

Browse files
afetisovyopox
authored andcommitted
QDOC: print actual visibility for items with restricted visibility
Previously the plugin either printed no visibility qualifiers in the quick documentation (for private items with no visibility specified), or always printed "pub", even for items with restricted visibility. That's confusing: you invoke completion and pick a function from the list looking at the documentation, but the function turns out private to some module, even though it was listed as public. It's also confusing to get a "cannot use private item" error from the compiler while the item is "pub" in the docs. I have changed it to print the actual qualifiers specified in the source code. Benefits: * It doesn't provide false information (e.g. `pub(crate)` items printed as if they are public); * It faithfully represents the visibility specified in the code, which may be relevant to the user; * It is simple in implementation; * It automatically supports any other possible future visibility qualifiers (e.g. when the `crate` visibility is used, the items are automatically printed as `crate fn foo`, which will be relevant if the feature becomes stable and idiomatic in a future edition). A possible downside is that the visibility may be hard to interpret. E.g. a `pub(super) fn foo` can be defined in `crate_foo::mod_foo`, but used in `crate_bar::mod_bar`. Does the `super` refer to `crate_foo` or `crate_bar`? It may be a good idea to provide an explicit answer in the quick documentation, resolving `super` visibility path to specifically `crate_foo`. However, the implementation of that feature would be more complicated, and it also raises new questions, e.g. should it take the visibility of containing modules into account? Perhaps the item is reexported from `crate_zux` which has `crate_foo` as private dependency. E.g. we have `pub fn bar()` in `crate_foo`, but `crate_zux` reexports it as `pub(crate) use crate_foo::bar;`. It may be confusing for the user if the documentation stated `pub fn foo` instead of `pub(in crate_zux) fn bar();`. But what if `crate_foo::bar` is visible from several locations? What if the user is interested in the original definition, rather than reexported paths? Calculating effective visibility would mean that the quick documentation would change depending on the location of the referencing element. Would it be confusing, and would it even be feasible to implement given a multitude of various PSI references? Given that those questions don't have any obvious answer and that the current behaviour is very predictable and easily understandable, I postpone any further improvements.
1 parent 3b3a41c commit 2e9406c

File tree

2 files changed

+48
-4
lines changed

2 files changed

+48
-4
lines changed

src/main/kotlin/org/rust/ide/docs/RsDocumentationProvider.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,9 +349,7 @@ private val RsTraitItem.declarationText: List<String>
349349
private val RsItemElement.declarationModifiers: List<String>
350350
get() {
351351
val modifiers = mutableListOf<String>()
352-
if (isPublic) {
353-
modifiers += "pub"
354-
}
352+
vis?.text?.let { modifiers += it }
355353
when (this) {
356354
is RsFunction -> {
357355
if (isAsync) {

src/test/kotlin/org/rust/ide/docs/RsQuickDocumentationTest.kt

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,52 @@ class RsQuickDocumentationTest : RsDocumentationProviderTest() {
4646
pub fn <b>foo</b>()</pre></div>
4747
""")
4848

49+
fun `test pub(crate) fn`() = doTest("""
50+
pub(crate) fn foo() {}
51+
//^
52+
""", """
53+
<div class='definition'><pre>test_package
54+
pub(crate) fn <b>foo</b>()</pre></div>
55+
""")
56+
57+
fun `test pub(self) fn`() = doTest("""
58+
pub(self) fn foo() {}
59+
//^
60+
""", """
61+
<div class='definition'><pre>test_package
62+
pub(self) fn <b>foo</b>()</pre></div>
63+
""")
64+
65+
fun `test pub(super) fn`() = doTest("""
66+
mod bar {
67+
pub(super) fn foo() {}
68+
//^
69+
}
70+
""", """
71+
<div class='definition'><pre>test_package::bar
72+
pub(super) fn <b>foo</b>()</pre></div>
73+
""")
74+
75+
fun `test crate fn`() = doTest("""
76+
mod bar {
77+
crate fn foo() {}
78+
//^
79+
}
80+
""", """
81+
<div class='definition'><pre>test_package::bar
82+
crate fn <b>foo</b>()</pre></div>
83+
""")
84+
85+
fun `test pub(in crate-bar) fn`() = doTest("""
86+
mod bar {
87+
pub(in crate::bar) fn foo() {}
88+
//^
89+
}
90+
""", """
91+
<div class='definition'><pre>test_package::bar
92+
pub(in crate::bar) fn <b>foo</b>()</pre></div>
93+
""")
94+
4995
fun `test const fn`() = doTest("""
5096
const fn foo() {}
5197
//^
@@ -56,7 +102,7 @@ class RsQuickDocumentationTest : RsDocumentationProviderTest() {
56102

57103
fun `test unsafe fn`() = doTest("""
58104
unsafe fn foo() {}
59-
//^
105+
//^
60106
""", """
61107
<div class='definition'><pre>test_package
62108
unsafe fn <b>foo</b>()</pre></div>

0 commit comments

Comments
 (0)