Auto merge of #50476 - zackmdavis:tame_unreachable_pub_suggestion, r=Manishearth
don't make crazy suggestion for unreachable braced pub-use The Higher Intermediate Representation doesn't have spans for visibility keywords, so we were assuming that the first whitespace-delimited token in the item span was the `pub` to be weakened. This doesn't work for brace-grouped `use`s, which get lowered as if they were several individual `use` statements, but with spans that only cover the braced path-segments. Constructing a correct suggestion here presents some challenges—until someone works those out, we can at least protect the dignity of our compiler by not offering any suggestion at all for `use` items. This resolves #50455 (but again, it would be desirable in the future to make a correct suggestion instead of copping out like this). r? @Manishearth
This commit is contained in:
bors
commit
5f98fe714e
@ -1287,32 +1287,27 @@ impl LintPass for UnreachablePub {
|
||||
|
||||
impl UnreachablePub {
|
||||
fn perform_lint(&self, cx: &LateContext, what: &str, id: ast::NodeId,
|
||||
vis: &hir::Visibility, span: Span, exportable: bool) {
|
||||
vis: &hir::Visibility, span: Span, exportable: bool,
|
||||
mut applicability: Applicability) {
|
||||
if !cx.access_levels.is_reachable(id) && *vis == hir::Visibility::Public {
|
||||
if span.ctxt().outer().expn_info().is_some() {
|
||||
applicability = Applicability::MaybeIncorrect;
|
||||
}
|
||||
let def_span = cx.tcx.sess.codemap().def_span(span);
|
||||
let mut err = cx.struct_span_lint(UNREACHABLE_PUB, def_span,
|
||||
&format!("unreachable `pub` {}", what));
|
||||
// visibility is token at start of declaration (can be macro
|
||||
// variable rather than literal `pub`)
|
||||
// We are presuming that visibility is token at start of
|
||||
// declaration (can be macro variable rather than literal `pub`)
|
||||
let pub_span = cx.tcx.sess.codemap().span_until_char(def_span, ' ');
|
||||
let replacement = if cx.tcx.features().crate_visibility_modifier {
|
||||
"crate"
|
||||
} else {
|
||||
"pub(crate)"
|
||||
}.to_owned();
|
||||
let app = if span.ctxt().outer().expn_info().is_none() {
|
||||
// even if macros aren't involved the suggestion
|
||||
// may be incorrect -- the user may have mistakenly
|
||||
// hidden it behind a private module and this lint is
|
||||
// a helpful way to catch that. However, we're trying
|
||||
// not to change the nature of the code with this lint
|
||||
// so it's marked as machine applicable.
|
||||
Applicability::MachineApplicable
|
||||
} else {
|
||||
Applicability::MaybeIncorrect
|
||||
};
|
||||
err.span_suggestion_with_applicability(pub_span, "consider restricting its visibility",
|
||||
replacement, app);
|
||||
err.span_suggestion_with_applicability(pub_span,
|
||||
"consider restricting its visibility",
|
||||
replacement,
|
||||
applicability);
|
||||
if exportable {
|
||||
err.help("or consider exporting it for use by other crates");
|
||||
}
|
||||
@ -1321,21 +1316,31 @@ impl UnreachablePub {
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnreachablePub {
|
||||
fn check_item(&mut self, cx: &LateContext, item: &hir::Item) {
|
||||
self.perform_lint(cx, "item", item.id, &item.vis, item.span, true);
|
||||
let applicability = match item.node {
|
||||
// suggestion span-manipulation is inadequate for `pub use
|
||||
// module::{item}` (Issue #50455)
|
||||
hir::ItemUse(..) => Applicability::MaybeIncorrect,
|
||||
_ => Applicability::MachineApplicable,
|
||||
};
|
||||
self.perform_lint(cx, "item", item.id, &item.vis, item.span, true, applicability);
|
||||
}
|
||||
|
||||
fn check_foreign_item(&mut self, cx: &LateContext, foreign_item: &hir::ForeignItem) {
|
||||
self.perform_lint(cx, "item", foreign_item.id, &foreign_item.vis, foreign_item.span, true);
|
||||
self.perform_lint(cx, "item", foreign_item.id, &foreign_item.vis,
|
||||
foreign_item.span, true, Applicability::MachineApplicable);
|
||||
}
|
||||
|
||||
fn check_struct_field(&mut self, cx: &LateContext, field: &hir::StructField) {
|
||||
self.perform_lint(cx, "field", field.id, &field.vis, field.span, false);
|
||||
self.perform_lint(cx, "field", field.id, &field.vis, field.span, false,
|
||||
Applicability::MachineApplicable);
|
||||
}
|
||||
|
||||
fn check_impl_item(&mut self, cx: &LateContext, impl_item: &hir::ImplItem) {
|
||||
self.perform_lint(cx, "item", impl_item.id, &impl_item.vis, impl_item.span, false);
|
||||
self.perform_lint(cx, "item", impl_item.id, &impl_item.vis, impl_item.span, false,
|
||||
Applicability::MachineApplicable);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -24,6 +24,7 @@
|
||||
mod private_mod {
|
||||
// non-leaked `pub` items in private module should be linted
|
||||
pub use std::fmt;
|
||||
pub use std::env::{Args}; // braced-use has different item spans than unbraced
|
||||
|
||||
pub struct Hydrogen {
|
||||
// `pub` struct fields, too
|
||||
|
||||
@ -14,7 +14,15 @@ LL | #![warn(unreachable_pub)]
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:28:5
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:27:24
|
||||
|
|
||||
LL | pub use std::env::{Args}; // braced-use has different item spans than unbraced
|
||||
| ^^^^ help: consider restricting its visibility: `pub(crate)`
|
||||
|
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:29:5
|
||||
|
|
||||
LL | pub struct Hydrogen {
|
||||
| ---^^^^^^^^^^^^^^^^
|
||||
@ -24,7 +32,7 @@ LL | pub struct Hydrogen {
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` field
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:30:9
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:31:9
|
||||
|
|
||||
LL | pub neutrons: usize,
|
||||
| ---^^^^^^^^^^^^^^^^
|
||||
@ -32,7 +40,7 @@ LL | pub neutrons: usize,
|
||||
| help: consider restricting its visibility: `pub(crate)`
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:36:9
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:37:9
|
||||
|
|
||||
LL | pub fn count_neutrons(&self) -> usize { self.neutrons }
|
||||
| ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
@ -40,7 +48,7 @@ LL | pub fn count_neutrons(&self) -> usize { self.neutrons }
|
||||
| help: consider restricting its visibility: `pub(crate)`
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:40:5
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:41:5
|
||||
|
|
||||
LL | pub enum Helium {}
|
||||
| ---^^^^^^^^^^^^
|
||||
@ -50,7 +58,7 @@ LL | pub enum Helium {}
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:41:5
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:42:5
|
||||
|
|
||||
LL | pub union Lithium { c1: usize, c2: u8 }
|
||||
| ---^^^^^^^^^^^^^^
|
||||
@ -60,7 +68,7 @@ LL | pub union Lithium { c1: usize, c2: u8 }
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:42:5
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:43:5
|
||||
|
|
||||
LL | pub fn beryllium() {}
|
||||
| ---^^^^^^^^^^^^^^^
|
||||
@ -70,7 +78,7 @@ LL | pub fn beryllium() {}
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:43:5
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:44:5
|
||||
|
|
||||
LL | pub trait Boron {}
|
||||
| ---^^^^^^^^^^^^
|
||||
@ -80,7 +88,7 @@ LL | pub trait Boron {}
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:44:5
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:45:5
|
||||
|
|
||||
LL | pub const CARBON: usize = 1;
|
||||
| ---^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
@ -90,7 +98,7 @@ LL | pub const CARBON: usize = 1;
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:45:5
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:46:5
|
||||
|
|
||||
LL | pub static NITROGEN: usize = 2;
|
||||
| ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
@ -100,7 +108,7 @@ LL | pub static NITROGEN: usize = 2;
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:46:5
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:47:5
|
||||
|
|
||||
LL | pub type Oxygen = bool;
|
||||
| ---^^^^^^^^^^^^^^^^^^^^
|
||||
@ -110,7 +118,7 @@ LL | pub type Oxygen = bool;
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:49:47
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:50:47
|
||||
|
|
||||
LL | ($visibility: vis, $name: ident) => { $visibility struct $name {} }
|
||||
| -----------^^^^^^^^^^^^^
|
||||
@ -123,7 +131,7 @@ LL | define_empty_struct_with_visibility!(pub, Fluorine);
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:54:9
|
||||
--> $DIR/unreachable_pub-pub_crate.rs:55:9
|
||||
|
|
||||
LL | pub fn catalyze() -> bool;
|
||||
| ---^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
@ -19,6 +19,7 @@
|
||||
mod private_mod {
|
||||
// non-leaked `pub` items in private module should be linted
|
||||
pub use std::fmt;
|
||||
pub use std::env::{Args}; // braced-use has different item spans than unbraced
|
||||
|
||||
pub struct Hydrogen {
|
||||
// `pub` struct fields, too
|
||||
|
||||
@ -14,7 +14,15 @@ LL | #![warn(unreachable_pub)]
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:23:5
|
||||
--> $DIR/unreachable_pub.rs:22:24
|
||||
|
|
||||
LL | pub use std::env::{Args}; // braced-use has different item spans than unbraced
|
||||
| ^^^^ help: consider restricting its visibility: `crate`
|
||||
|
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:24:5
|
||||
|
|
||||
LL | pub struct Hydrogen {
|
||||
| ---^^^^^^^^^^^^^^^^
|
||||
@ -24,7 +32,7 @@ LL | pub struct Hydrogen {
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` field
|
||||
--> $DIR/unreachable_pub.rs:25:9
|
||||
--> $DIR/unreachable_pub.rs:26:9
|
||||
|
|
||||
LL | pub neutrons: usize,
|
||||
| ---^^^^^^^^^^^^^^^^
|
||||
@ -32,7 +40,7 @@ LL | pub neutrons: usize,
|
||||
| help: consider restricting its visibility: `crate`
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:31:9
|
||||
--> $DIR/unreachable_pub.rs:32:9
|
||||
|
|
||||
LL | pub fn count_neutrons(&self) -> usize { self.neutrons }
|
||||
| ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
@ -40,7 +48,7 @@ LL | pub fn count_neutrons(&self) -> usize { self.neutrons }
|
||||
| help: consider restricting its visibility: `crate`
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:35:5
|
||||
--> $DIR/unreachable_pub.rs:36:5
|
||||
|
|
||||
LL | pub enum Helium {}
|
||||
| ---^^^^^^^^^^^^
|
||||
@ -50,7 +58,7 @@ LL | pub enum Helium {}
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:36:5
|
||||
--> $DIR/unreachable_pub.rs:37:5
|
||||
|
|
||||
LL | pub union Lithium { c1: usize, c2: u8 }
|
||||
| ---^^^^^^^^^^^^^^
|
||||
@ -60,7 +68,7 @@ LL | pub union Lithium { c1: usize, c2: u8 }
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:37:5
|
||||
--> $DIR/unreachable_pub.rs:38:5
|
||||
|
|
||||
LL | pub fn beryllium() {}
|
||||
| ---^^^^^^^^^^^^^^^
|
||||
@ -70,7 +78,7 @@ LL | pub fn beryllium() {}
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:38:5
|
||||
--> $DIR/unreachable_pub.rs:39:5
|
||||
|
|
||||
LL | pub trait Boron {}
|
||||
| ---^^^^^^^^^^^^
|
||||
@ -80,7 +88,7 @@ LL | pub trait Boron {}
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:39:5
|
||||
--> $DIR/unreachable_pub.rs:40:5
|
||||
|
|
||||
LL | pub const CARBON: usize = 1;
|
||||
| ---^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
@ -90,7 +98,7 @@ LL | pub const CARBON: usize = 1;
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:40:5
|
||||
--> $DIR/unreachable_pub.rs:41:5
|
||||
|
|
||||
LL | pub static NITROGEN: usize = 2;
|
||||
| ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
@ -100,7 +108,7 @@ LL | pub static NITROGEN: usize = 2;
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:41:5
|
||||
--> $DIR/unreachable_pub.rs:42:5
|
||||
|
|
||||
LL | pub type Oxygen = bool;
|
||||
| ---^^^^^^^^^^^^^^^^^^^^
|
||||
@ -110,7 +118,7 @@ LL | pub type Oxygen = bool;
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:44:47
|
||||
--> $DIR/unreachable_pub.rs:45:47
|
||||
|
|
||||
LL | ($visibility: vis, $name: ident) => { $visibility struct $name {} }
|
||||
| -----------^^^^^^^^^^^^^
|
||||
@ -123,7 +131,7 @@ LL | define_empty_struct_with_visibility!(pub, Fluorine);
|
||||
= help: or consider exporting it for use by other crates
|
||||
|
||||
warning: unreachable `pub` item
|
||||
--> $DIR/unreachable_pub.rs:49:9
|
||||
--> $DIR/unreachable_pub.rs:50:9
|
||||
|
|
||||
LL | pub fn catalyze() -> bool;
|
||||
| ---^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user