Rollup merge of #42874 - zackmdavis:overzealous_by_outer_forbid, r=nikomatsakis

only set "overruled by outer forbid" once for lint groups, by group name

Previously, conflicting forbid/allow attributes for a lint group would
result in a separate "allow(L) overruled by outer forbid(L)" error for
every lint L in the group. This was needlessly and annoyingly verbose;
we prefer to just have one error pointing out the conflicting
attributes.

(Also, while we're touching context.rs, clean up some unused arguments.)

Resolves #42873.

3 files changed, 73 insertions, 17 deletions

diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs
index a9e0ef51102..466d163854f 100644
--- a/src/librustc/lint/context.rs
+++ b/src/librustc/lint/context.rs
@@ -291,16 +291,13 @@ impl LintStore {
         self.by_name.insert(name.into(), Removed(reason.into()));
     }
 
-    #[allow(unused_variables)]
-    fn find_lint(&self, lint_name: &str, sess: &Session, span: Option<Span>)
-                 -> Result<LintId, FindLintError>
-    {
+    fn find_lint(&self, lint_name: &str) -> Result<LintId, FindLintError> {
         match self.by_name.get(lint_name) {
             Some(&Id(lint_id)) => Ok(lint_id),
             Some(&Renamed(_, lint_id)) => {
                 Ok(lint_id)
             },
-            Some(&Removed(ref reason)) => {
+            Some(&Removed(_)) => {
                 Err(FindLintError::Removed)
             },
             None => Err(FindLintError::NotFound)
@@ -313,7 +310,7 @@ impl LintStore {
                                     &lint_name[..], level);
 
             let lint_flag_val = Symbol::intern(&lint_name);
-            match self.find_lint(&lint_name[..], sess, None) {
+            match self.find_lint(&lint_name[..]) {
                 Ok(lint_id) => self.levels.set(lint_id, (level, CommandLine(lint_flag_val))),
                 Err(FindLintError::Removed) => { }
                 Err(_) => {
@@ -724,21 +721,22 @@ pub trait LintContext<'tcx>: Sized {
         let mut pushed = 0;
 
         for result in gather_attrs(attrs) {
-            let v = match result {
+            let (is_group, lint_level_spans) = match result {
                 Err(span) => {
                     span_err!(self.sess(), span, E0452,
                               "malformed lint attribute");
                     continue;
                 }
                 Ok((lint_name, level, span)) => {
-                    match self.lints().find_lint(&lint_name.as_str(), &self.sess(), Some(span)) {
-                        Ok(lint_id) => vec![(lint_id, level, span)],
+                    match self.lints().find_lint(&lint_name.as_str()) {
+                        Ok(lint_id) => (false, vec![(lint_id, level, span)]),
                         Err(FindLintError::NotFound) => {
                             match self.lints().lint_groups.get(&*lint_name.as_str()) {
-                                Some(&(ref v, _)) => v.iter()
+                                Some(&(ref v, _)) => (true,
+                                                      v.iter()
                                                       .map(|lint_id: &LintId|
                                                            (*lint_id, level, span))
-                                                      .collect(),
+                                                      .collect()),
                                 None => {
                                     // The lint or lint group doesn't exist.
                                     // This is an error, but it was handled
@@ -754,14 +752,18 @@ pub trait LintContext<'tcx>: Sized {
 
             let lint_attr_name = result.expect("lint attribute should be well-formed").0;
 
-            for (lint_id, level, span) in v {
+            for (lint_id, level, span) in lint_level_spans {
                 let (now, now_source) = self.lint_sess().get_source(lint_id);
                 if now == Forbid && level != Forbid {
-                    let lint_name = lint_id.to_string();
+                    let forbidden_lint_name = match now_source {
+                        LintSource::Default => lint_id.to_string(),
+                        LintSource::Node(name, _) => name.to_string(),
+                        LintSource::CommandLine(name) => name.to_string(),
+                    };
                     let mut diag_builder = struct_span_err!(self.sess(), span, E0453,
                                                             "{}({}) overruled by outer forbid({})",
-                                                            level.as_str(), lint_name,
-                                                            lint_name);
+                                                            level.as_str(), lint_attr_name,
+                                                            forbidden_lint_name);
                     diag_builder.span_label(span, "overruled by previous forbid");
                     match now_source {
                         LintSource::Default => &mut diag_builder,
@@ -772,7 +774,10 @@ pub trait LintContext<'tcx>: Sized {
                         LintSource::CommandLine(_) => {
                             diag_builder.note("`forbid` lint level was set on command line")
                         }
-                    }.emit()
+                    }.emit();
+                    if is_group { // don't set a separate error for every lint in the group
+                        break;
+                    }
                 } else if now != level {
                     let cx = self.lint_sess_mut();
                     cx.stack.push((lint_id, (now, now_source)));
@@ -1420,7 +1425,7 @@ impl Decodable for LintId {
     fn decode<D: Decoder>(d: &mut D) -> Result<LintId, D::Error> {
         let s = d.read_str()?;
         ty::tls::with(|tcx| {
-            match tcx.sess.lint_store.borrow().find_lint(&s, tcx.sess, None) {
+            match tcx.sess.lint_store.borrow().find_lint(&s) {
                 Ok(id) => Ok(id),
                 Err(_) => panic!("invalid lint-id `{}`", s),
             }
diff --git a/src/test/ui/lint/outer-forbid.rs b/src/test/ui/lint/outer-forbid.rs
new file mode 100644
index 00000000000..d71da58829a
--- /dev/null
+++ b/src/test/ui/lint/outer-forbid.rs
@@ -0,0 +1,22 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Forbidding a group (here, `unused`) overrules subsequent allowance of both
+// the group, and an individual lint in the group (here, `unused_variables`);
+// and, forbidding an individual lint (here, `non_snake_case`) overrules
+// subsequent allowance of a lint group containing it (here, `bad_style`). See
+// Issue #42873.
+
+#![forbid(unused, non_snake_case)]
+
+#[allow(unused, unused_variables, bad_style)]
+fn main() {
+    println!("hello forbidden world")
+}
diff --git a/src/test/ui/lint/outer-forbid.stderr b/src/test/ui/lint/outer-forbid.stderr
new file mode 100644
index 00000000000..831b3f65634
--- /dev/null
+++ b/src/test/ui/lint/outer-forbid.stderr
@@ -0,0 +1,29 @@
+error[E0453]: allow(unused) overruled by outer forbid(unused)
+  --> $DIR/outer-forbid.rs:19:9
+   |
+17 | #![forbid(unused, non_snake_case)]
+   |           ------ `forbid` level set here
+18 | 
+19 | #[allow(unused, unused_variables, bad_style)]
+   |         ^^^^^^ overruled by previous forbid
+
+error[E0453]: allow(unused_variables) overruled by outer forbid(unused)
+  --> $DIR/outer-forbid.rs:19:17
+   |
+17 | #![forbid(unused, non_snake_case)]
+   |           ------ `forbid` level set here
+18 | 
+19 | #[allow(unused, unused_variables, bad_style)]
+   |                 ^^^^^^^^^^^^^^^^ overruled by previous forbid
+
+error[E0453]: allow(bad_style) overruled by outer forbid(non_snake_case)
+  --> $DIR/outer-forbid.rs:19:35
+   |
+17 | #![forbid(unused, non_snake_case)]
+   |                   -------------- `forbid` level set here
+18 | 
+19 | #[allow(unused, unused_variables, bad_style)]
+   |                                   ^^^^^^^^^ overruled by previous forbid
+
+error: aborting due to previous error(s)
+