Skip to content

Commit 5330747

Browse files
committed
private no-mangle lints: issue suggestion for restricted visibility
This is probably quite a lot less likely to come up in practice than the "inherited" (no visibility keyword) case, but now that we have visibility spans in the HIR, we can do this, and it presumably doesn't hurt to be exhaustive. (Who can say but that the attention to detail just might knock someone's socks off, someday, somewhere?) This is inspired by #47383.
1 parent 104985b commit 5330747

File tree

3 files changed

+52
-23
lines changed

3 files changed

+52
-23
lines changed

src/librustc_lint/builtin.rs

+24-17
Original file line numberDiff line numberDiff line change
@@ -1177,6 +1177,23 @@ impl LintPass for InvalidNoMangleItems {
11771177

11781178
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
11791179
fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
1180+
let suggest_make_pub = |vis: &hir::Visibility, err: &mut DiagnosticBuilder| {
1181+
let suggestion = match vis.node {
1182+
hir::VisibilityInherited => {
1183+
// inherited visibility is empty span at item start; need an extra space
1184+
Some("pub ".to_owned())
1185+
},
1186+
hir::VisibilityRestricted { .. } |
1187+
hir::VisibilityCrate(_) => {
1188+
Some("pub".to_owned())
1189+
},
1190+
hir::VisibilityPublic => None
1191+
};
1192+
if let Some(replacement) = suggestion {
1193+
err.span_suggestion(vis.span, "try making it public", replacement);
1194+
}
1195+
};
1196+
11801197
match it.node {
11811198
hir::ItemFn(.., ref generics, _) => {
11821199
if let Some(no_mangle_attr) = attr::find_by_name(&it.attrs, "no_mangle") {
@@ -1186,12 +1203,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
11861203
if !cx.access_levels.is_reachable(it.id) {
11871204
let msg = "function is marked #[no_mangle], but not exported";
11881205
let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_FNS, it.span, msg);
1189-
let insertion_span = it.span.shrink_to_lo();
1190-
if it.vis.node == hir::VisibilityInherited {
1191-
err.span_suggestion(insertion_span,
1192-
"try making it public",
1193-
"pub ".to_owned());
1194-
}
1206+
suggest_make_pub(&it.vis, &mut err);
11951207
err.emit();
11961208
}
11971209
for param in &generics.params {
@@ -1214,17 +1226,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
12141226
}
12151227
hir::ItemStatic(..) => {
12161228
if attr::contains_name(&it.attrs, "no_mangle") &&
1217-
!cx.access_levels.is_reachable(it.id) {
1218-
let msg = "static is marked #[no_mangle], but not exported";
1219-
let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, msg);
1220-
let insertion_span = it.span.shrink_to_lo();
1221-
if it.vis.node == hir::VisibilityInherited {
1222-
err.span_suggestion(insertion_span,
1223-
"try making it public",
1224-
"pub ".to_owned());
1225-
}
1226-
err.emit();
1227-
}
1229+
!cx.access_levels.is_reachable(it.id) {
1230+
let msg = "static is marked #[no_mangle], but not exported";
1231+
let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, msg);
1232+
suggest_make_pub(&it.vis, &mut err);
1233+
err.emit();
1234+
}
12281235
}
12291236
hir::ItemConst(..) => {
12301237
if attr::contains_name(&it.attrs, "no_mangle") {

src/test/ui/lint/suggestions.rs

+6
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ mod badlands {
3434
//~^ WARN static is marked
3535
#[no_mangle] pub fn val_jean() {}
3636
//~^ WARN function is marked
37+
38+
// ... but we can suggest just-`pub` instead of restricted
39+
#[no_mangle] pub(crate) static VETAR: bool = true;
40+
//~^ WARN static is marked
41+
#[no_mangle] pub(crate) fn crossfield() {}
42+
//~^ WARN function is marked
3743
}
3844

3945
struct Equinox {

src/test/ui/lint/suggestions.stderr

+22-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
warning: unnecessary parentheses around assigned value
2-
--> $DIR/suggestions.rs:48:21
2+
--> $DIR/suggestions.rs:54:21
33
|
44
LL | let mut a = (1); // should suggest no `mut`, no parens
55
| ^^^ help: remove these parentheses
@@ -11,15 +11,15 @@ LL | #![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issu
1111
| ^^^^^^^^^^^^^
1212

1313
warning: use of deprecated attribute `no_debug`: the `#[no_debug]` attribute was an experimental feature that has been deprecated due to lack of demand. See https://github.com/rust-lang/rust/issues/29721
14-
--> $DIR/suggestions.rs:43:1
14+
--> $DIR/suggestions.rs:49:1
1515
|
1616
LL | #[no_debug] // should suggest removal of deprecated attribute
1717
| ^^^^^^^^^^^ help: remove this attribute
1818
|
1919
= note: #[warn(deprecated)] on by default
2020

2121
warning: variable does not need to be mutable
22-
--> $DIR/suggestions.rs:48:13
22+
--> $DIR/suggestions.rs:54:13
2323
|
2424
LL | let mut a = (1); // should suggest no `mut`, no parens
2525
| ----^
@@ -33,7 +33,7 @@ LL | #![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issu
3333
| ^^^^^^^^^^
3434

3535
warning: variable does not need to be mutable
36-
--> $DIR/suggestions.rs:52:13
36+
--> $DIR/suggestions.rs:58:13
3737
|
3838
LL | let mut
3939
| _____________^
@@ -96,16 +96,32 @@ warning: function is marked #[no_mangle], but not exported
9696
LL | #[no_mangle] pub fn val_jean() {}
9797
| ^^^^^^^^^^^^^^^^^^^^
9898

99+
warning: static is marked #[no_mangle], but not exported
100+
--> $DIR/suggestions.rs:39:18
101+
|
102+
LL | #[no_mangle] pub(crate) static VETAR: bool = true;
103+
| ----------^^^^^^^^^^^^^^^^^^^^^^^^^^^
104+
| |
105+
| help: try making it public: `pub`
106+
107+
warning: function is marked #[no_mangle], but not exported
108+
--> $DIR/suggestions.rs:41:18
109+
|
110+
LL | #[no_mangle] pub(crate) fn crossfield() {}
111+
| ----------^^^^^^^^^^^^^^^^^^^
112+
| |
113+
| help: try making it public: `pub`
114+
99115
warning: denote infinite loops with `loop { ... }`
100-
--> $DIR/suggestions.rs:46:5
116+
--> $DIR/suggestions.rs:52:5
101117
|
102118
LL | while true { // should suggest `loop`
103119
| ^^^^^^^^^^ help: use `loop`
104120
|
105121
= note: #[warn(while_true)] on by default
106122

107123
warning: the `warp_factor:` in this pattern is redundant
108-
--> $DIR/suggestions.rs:57:23
124+
--> $DIR/suggestions.rs:63:23
109125
|
110126
LL | Equinox { warp_factor: warp_factor } => {} // should suggest shorthand
111127
| ------------^^^^^^^^^^^^

0 commit comments

Comments
 (0)