From 05f0dee04a39912222f7237502e2230f0da1b551 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 7 Feb 2019 16:38:23 +0100 Subject: [PATCH 1/3] Improve the error messages for missing stability attributes This makes the capitalisation consistent and provides more context (especially for missing top-level attributes). --- src/librustc/middle/stability.rs | 25 +++++++++++-------- src/test/ui/missing/missing-stability.rs | 4 +-- src/test/ui/missing/missing-stability.stderr | 6 ++--- .../missing-stability-attr-at-top-level.rs | 4 +++ ...missing-stability-attr-at-top-level.stderr | 11 ++++++++ .../stability-attribute-issue-43027.rs | 2 +- .../stability-attribute-issue-43027.stderr | 4 +-- .../stability-attribute-sanity-3.rs | 2 +- .../stability-attribute-sanity-3.stderr | 4 +-- 9 files changed, 41 insertions(+), 21 deletions(-) create mode 100644 src/test/ui/stability-attribute/missing-stability-attr-at-top-level.rs create mode 100644 src/test/ui/stability-attribute/missing-stability-attr-at-top-level.stderr diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 34c77d08f5a7e..1f7345dde8e8e 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -322,14 +322,17 @@ struct MissingStabilityAnnotations<'a, 'tcx: 'a> { } impl<'a, 'tcx: 'a> MissingStabilityAnnotations<'a, 'tcx> { - fn check_missing_stability(&self, id: NodeId, span: Span) { + fn check_missing_stability(&self, id: NodeId, span: Span, name: &str) { let hir_id = self.tcx.hir().node_to_hir_id(id); let stab = self.tcx.stability().local_stability(hir_id); let is_error = !self.tcx.sess.opts.test && stab.is_none() && self.access_levels.is_reachable(id); if is_error { - self.tcx.sess.span_err(span, "This node does not have a stability attribute"); + self.tcx.sess.span_err( + span, + &format!("{} has missing stability attribute", name), + ); } } } @@ -347,42 +350,44 @@ impl<'a, 'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'a, 'tcx> { // optional. They inherit stability from their parents when unannotated. hir::ItemKind::Impl(.., None, _, _) | hir::ItemKind::ForeignMod(..) => {} - _ => self.check_missing_stability(i.id, i.span) + hir::ItemKind::Mod(..) => self.check_missing_stability(i.id, i.span, "module"), + + _ => self.check_missing_stability(i.id, i.span, "node") } intravisit::walk_item(self, i) } fn visit_trait_item(&mut self, ti: &'tcx hir::TraitItem) { - self.check_missing_stability(ti.id, ti.span); + self.check_missing_stability(ti.id, ti.span, "node"); intravisit::walk_trait_item(self, ti); } fn visit_impl_item(&mut self, ii: &'tcx hir::ImplItem) { let impl_def_id = self.tcx.hir().local_def_id(self.tcx.hir().get_parent(ii.id)); if self.tcx.impl_trait_ref(impl_def_id).is_none() { - self.check_missing_stability(ii.id, ii.span); + self.check_missing_stability(ii.id, ii.span, "node"); } intravisit::walk_impl_item(self, ii); } fn visit_variant(&mut self, var: &'tcx Variant, g: &'tcx Generics, item_id: NodeId) { - self.check_missing_stability(var.node.data.id(), var.span); + self.check_missing_stability(var.node.data.id(), var.span, "variant"); intravisit::walk_variant(self, var, g, item_id); } fn visit_struct_field(&mut self, s: &'tcx StructField) { - self.check_missing_stability(s.id, s.span); + self.check_missing_stability(s.id, s.span, "field"); intravisit::walk_struct_field(self, s); } fn visit_foreign_item(&mut self, i: &'tcx hir::ForeignItem) { - self.check_missing_stability(i.id, i.span); + self.check_missing_stability(i.id, i.span, "node"); intravisit::walk_foreign_item(self, i); } fn visit_macro_def(&mut self, md: &'tcx hir::MacroDef) { - self.check_missing_stability(md.id, md.span); + self.check_missing_stability(md.id, md.span, "macro"); } } @@ -867,7 +872,7 @@ pub fn check_unused_or_stable_features<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { tcx, access_levels, }; - missing.check_missing_stability(ast::CRATE_NODE_ID, krate.span); + missing.check_missing_stability(ast::CRATE_NODE_ID, krate.span, "crate"); intravisit::walk_crate(&mut missing, krate); krate.visit_all_item_likes(&mut missing.as_deep_visitor()); } diff --git a/src/test/ui/missing/missing-stability.rs b/src/test/ui/missing/missing-stability.rs index 86841706325f7..c7d721e836d1f 100644 --- a/src/test/ui/missing/missing-stability.rs +++ b/src/test/ui/missing/missing-stability.rs @@ -6,7 +6,7 @@ #![stable(feature = "stable_test_feature", since = "1.0.0")] pub fn unmarked() { - //~^ ERROR This node does not have a stability attribute + //~^ ERROR node has missing stability attribute () } @@ -20,5 +20,5 @@ pub mod foo { pub mod bar { // #[stable] is not inherited pub fn unmarked() {} - //~^ ERROR This node does not have a stability attribute + //~^ ERROR node has missing stability attribute } diff --git a/src/test/ui/missing/missing-stability.stderr b/src/test/ui/missing/missing-stability.stderr index e55bd00e2c673..bced3efd83da3 100644 --- a/src/test/ui/missing/missing-stability.stderr +++ b/src/test/ui/missing/missing-stability.stderr @@ -1,13 +1,13 @@ -error: This node does not have a stability attribute +error: node has missing stability attribute --> $DIR/missing-stability.rs:8:1 | LL | / pub fn unmarked() { -LL | | //~^ ERROR This node does not have a stability attribute +LL | | //~^ ERROR node has missing stability attribute LL | | () LL | | } | |_^ -error: This node does not have a stability attribute +error: node has missing stability attribute --> $DIR/missing-stability.rs:22:5 | LL | pub fn unmarked() {} diff --git a/src/test/ui/stability-attribute/missing-stability-attr-at-top-level.rs b/src/test/ui/stability-attribute/missing-stability-attr-at-top-level.rs new file mode 100644 index 0000000000000..8f750ae62f5e4 --- /dev/null +++ b/src/test/ui/stability-attribute/missing-stability-attr-at-top-level.rs @@ -0,0 +1,4 @@ +#![feature(staged_api)] +//~^ ERROR crate has missing stability attribute + +fn main() {} diff --git a/src/test/ui/stability-attribute/missing-stability-attr-at-top-level.stderr b/src/test/ui/stability-attribute/missing-stability-attr-at-top-level.stderr new file mode 100644 index 0000000000000..f674797694557 --- /dev/null +++ b/src/test/ui/stability-attribute/missing-stability-attr-at-top-level.stderr @@ -0,0 +1,11 @@ +error: crate has missing stability attribute + --> $DIR/missing-stability-attr-at-top-level.rs:1:1 + | +LL | / #![feature(staged_api)] +LL | | //~^ ERROR crate has missing stability attribute +LL | | +LL | | fn main() {} + | |____________^ + +error: aborting due to previous error + diff --git a/src/test/ui/stability-attribute/stability-attribute-issue-43027.rs b/src/test/ui/stability-attribute/stability-attribute-issue-43027.rs index 596a6eb6ed366..0b243bb52119b 100644 --- a/src/test/ui/stability-attribute/stability-attribute-issue-43027.rs +++ b/src/test/ui/stability-attribute/stability-attribute-issue-43027.rs @@ -2,7 +2,7 @@ #![stable(feature = "test", since = "0")] #[stable(feature = "test", since = "0")] -pub struct Reverse(pub T); //~ ERROR This node does not have a stability attribute +pub struct Reverse(pub T); //~ ERROR field has missing stability attribute fn main() { // Make sure the field is used to fill the stability cache diff --git a/src/test/ui/stability-attribute/stability-attribute-issue-43027.stderr b/src/test/ui/stability-attribute/stability-attribute-issue-43027.stderr index e123f42023324..7ffb4bb487a7b 100644 --- a/src/test/ui/stability-attribute/stability-attribute-issue-43027.stderr +++ b/src/test/ui/stability-attribute/stability-attribute-issue-43027.stderr @@ -1,7 +1,7 @@ -error: This node does not have a stability attribute +error: field has missing stability attribute --> $DIR/stability-attribute-issue-43027.rs:5:23 | -LL | pub struct Reverse(pub T); //~ ERROR This node does not have a stability attribute +LL | pub struct Reverse(pub T); //~ ERROR field has missing stability attribute | ^^^^^ error: aborting due to previous error diff --git a/src/test/ui/stability-attribute/stability-attribute-sanity-3.rs b/src/test/ui/stability-attribute/stability-attribute-sanity-3.rs index 0c132e8857550..13ef3d3f53d2b 100644 --- a/src/test/ui/stability-attribute/stability-attribute-sanity-3.rs +++ b/src/test/ui/stability-attribute/stability-attribute-sanity-3.rs @@ -5,7 +5,7 @@ #![stable(feature = "stable_test_feature", since = "1.0.0")] #[macro_export] -macro_rules! mac { //~ ERROR This node does not have a stability attribute +macro_rules! mac { //~ ERROR macro has missing stability attribute () => () } diff --git a/src/test/ui/stability-attribute/stability-attribute-sanity-3.stderr b/src/test/ui/stability-attribute/stability-attribute-sanity-3.stderr index d42dcc0c778a7..1c759d49b9947 100644 --- a/src/test/ui/stability-attribute/stability-attribute-sanity-3.stderr +++ b/src/test/ui/stability-attribute/stability-attribute-sanity-3.stderr @@ -1,7 +1,7 @@ -error: This node does not have a stability attribute +error: macro has missing stability attribute --> $DIR/stability-attribute-sanity-3.rs:8:1 | -LL | / macro_rules! mac { //~ ERROR This node does not have a stability attribute +LL | / macro_rules! mac { //~ ERROR macro has missing stability attribute LL | | () => () LL | | } | |_^ From 03d4fd973aaa6dca0d1e756ecab5d4f17947337b Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 8 Feb 2019 14:30:13 +0100 Subject: [PATCH 2/3] Use descriptive variant name --- src/librustc/middle/stability.rs | 4 ++-- src/test/ui/missing/missing-stability.rs | 4 ++-- src/test/ui/missing/missing-stability.stderr | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 1f7345dde8e8e..45f53dee98582 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -352,7 +352,7 @@ impl<'a, 'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'a, 'tcx> { hir::ItemKind::Mod(..) => self.check_missing_stability(i.id, i.span, "module"), - _ => self.check_missing_stability(i.id, i.span, "node") + _ => self.check_missing_stability(i.id, i.span, i.node.descriptive_variant()) } intravisit::walk_item(self, i) @@ -382,7 +382,7 @@ impl<'a, 'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'a, 'tcx> { } fn visit_foreign_item(&mut self, i: &'tcx hir::ForeignItem) { - self.check_missing_stability(i.id, i.span, "node"); + self.check_missing_stability(i.id, i.span, i.node.descriptive_variant()); intravisit::walk_foreign_item(self, i); } diff --git a/src/test/ui/missing/missing-stability.rs b/src/test/ui/missing/missing-stability.rs index c7d721e836d1f..469c22fdb17d7 100644 --- a/src/test/ui/missing/missing-stability.rs +++ b/src/test/ui/missing/missing-stability.rs @@ -6,7 +6,7 @@ #![stable(feature = "stable_test_feature", since = "1.0.0")] pub fn unmarked() { - //~^ ERROR node has missing stability attribute + //~^ ERROR function has missing stability attribute () } @@ -20,5 +20,5 @@ pub mod foo { pub mod bar { // #[stable] is not inherited pub fn unmarked() {} - //~^ ERROR node has missing stability attribute + //~^ ERROR function has missing stability attribute } diff --git a/src/test/ui/missing/missing-stability.stderr b/src/test/ui/missing/missing-stability.stderr index bced3efd83da3..6c81f2bac5788 100644 --- a/src/test/ui/missing/missing-stability.stderr +++ b/src/test/ui/missing/missing-stability.stderr @@ -1,13 +1,13 @@ -error: node has missing stability attribute +error: function has missing stability attribute --> $DIR/missing-stability.rs:8:1 | LL | / pub fn unmarked() { -LL | | //~^ ERROR node has missing stability attribute +LL | | //~^ ERROR function has missing stability attribute LL | | () LL | | } | |_^ -error: node has missing stability attribute +error: function has missing stability attribute --> $DIR/missing-stability.rs:22:5 | LL | pub fn unmarked() {} From bb1eed0ec8537a09666978ad92d71ee1b5aba13b Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 8 Feb 2019 15:07:26 +0100 Subject: [PATCH 3/3] Correct descriptive item name for impl --- src/librustc/hir/mod.rs | 2 +- src/librustc/middle/stability.rs | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 3e7dd1432e1e3..d520acf950a23 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2288,7 +2288,7 @@ impl ItemKind { ItemKind::Union(..) => "union", ItemKind::Trait(..) => "trait", ItemKind::TraitAlias(..) => "trait alias", - ItemKind::Impl(..) => "item", + ItemKind::Impl(..) => "impl", } } diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 45f53dee98582..04046fa3b00ab 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -350,8 +350,6 @@ impl<'a, 'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'a, 'tcx> { // optional. They inherit stability from their parents when unannotated. hir::ItemKind::Impl(.., None, _, _) | hir::ItemKind::ForeignMod(..) => {} - hir::ItemKind::Mod(..) => self.check_missing_stability(i.id, i.span, "module"), - _ => self.check_missing_stability(i.id, i.span, i.node.descriptive_variant()) } @@ -359,14 +357,14 @@ impl<'a, 'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'a, 'tcx> { } fn visit_trait_item(&mut self, ti: &'tcx hir::TraitItem) { - self.check_missing_stability(ti.id, ti.span, "node"); + self.check_missing_stability(ti.id, ti.span, "item"); intravisit::walk_trait_item(self, ti); } fn visit_impl_item(&mut self, ii: &'tcx hir::ImplItem) { let impl_def_id = self.tcx.hir().local_def_id(self.tcx.hir().get_parent(ii.id)); if self.tcx.impl_trait_ref(impl_def_id).is_none() { - self.check_missing_stability(ii.id, ii.span, "node"); + self.check_missing_stability(ii.id, ii.span, "item"); } intravisit::walk_impl_item(self, ii); }