Skip to content

Commit

Permalink
rustc: Disallow machine applicability in foreign macros
Browse files Browse the repository at this point in the history
Recent changes to lints disallowed lints from being emitted against code located
in foreign macros, except for future-incompatible lints. For a future
incompatible lint, however, the automatic suggestions may not be applicable!

This commit updates this code path to force all applicability suggestions made
to foreign macros to never be `MachineApplicable`. This should avoid rustfix
actually attempting fixing these suggestions, causing non-compiling code to be
produced.

Closes rust-lang/cargo#5799
  • Loading branch information
alexcrichton committed Jul 30, 2018
1 parent 54628c8 commit ca762ba
Show file tree
Hide file tree
Showing 8 changed files with 217 additions and 35 deletions.
30 changes: 16 additions & 14 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,8 @@ pub fn struct_lint_level<'a>(sess: &'a Session,
// Check for future incompatibility lints and issue a stronger warning.
let lints = sess.lint_store.borrow();
let lint_id = LintId::of(lint);
if let Some(future_incompatible) = lints.future_incompatible(lint_id) {
let future_incompatible = lints.future_incompatible(lint_id);
if let Some(future_incompatible) = future_incompatible {
const STANDARD_MESSAGE: &str =
"this was previously accepted by the compiler but is being phased out; \
it will become a hard error";
Expand All @@ -593,20 +594,21 @@ pub fn struct_lint_level<'a>(sess: &'a Session,
future_incompatible.reference);
err.warn(&explanation);
err.note(&citation);
}

// If this lint is *not* a future incompatibility warning then we want to be
// sure to not be too noisy in some situations. If this code originates in a
// foreign macro, aka something that this crate did not itself author, then
// it's likely that there's nothing this crate can do about it. We probably
// want to skip the lint entirely.
//
// For some lints though (like unreachable code) there's clear actionable
// items to take care of (delete the macro invocation). As a result we have
// a few lints we whitelist here for allowing a lint even though it's in a
// foreign macro invocation.
} else if !lint.report_in_external_macro {
if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s)) {
err.cancel();
// If this code originates in a foreign macro, aka something that this crate
// did not itself author, then it's likely that there's nothing this crate
// can do about it. We probably want to skip the lint entirely.
if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s)) {
// Any suggestions made here are likely to be incorrect, so anything we
// emit shouldn't be automatically fixed by rustfix.
err.allow_suggestions(false);

// If this is a future incompatible lint it'll become a hard error, so
// we have to emit *something*. Also allow lints to whitelist themselves
// on a case-by-case basis for emission in a foreign macro.
if future_incompatible.is_none() && !lint.report_in_external_macro {
err.cancel()
}
}

Expand Down
83 changes: 64 additions & 19 deletions src/librustc_errors/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use syntax_pos::{MultiSpan, Span};
pub struct DiagnosticBuilder<'a> {
pub handler: &'a Handler,
diagnostic: Diagnostic,
allow_suggestions: bool,
}

/// In general, the `DiagnosticBuilder` uses deref to allow access to
Expand Down Expand Up @@ -186,27 +187,67 @@ impl<'a> DiagnosticBuilder<'a> {
msg: &str,
suggestions: Vec<String>)
-> &mut Self);
forward!(pub fn span_suggestion_with_applicability(&mut self,
sp: Span,
msg: &str,
suggestion: String,
applicability: Applicability)
-> &mut Self);
forward!(pub fn span_suggestions_with_applicability(&mut self,
sp: Span,
msg: &str,
suggestions: Vec<String>,
applicability: Applicability)
-> &mut Self);
forward!(pub fn span_suggestion_short_with_applicability(&mut self,
sp: Span,
msg: &str,
suggestion: String,
applicability: Applicability)
-> &mut Self);
pub fn span_suggestion_with_applicability(&mut self,
sp: Span,
msg: &str,
suggestion: String,
applicability: Applicability)
-> &mut Self {
if !self.allow_suggestions {
return self
}
self.diagnostic.span_suggestion_with_applicability(
sp,
msg,
suggestion,
applicability,
);
self
}

pub fn span_suggestions_with_applicability(&mut self,
sp: Span,
msg: &str,
suggestions: Vec<String>,
applicability: Applicability)
-> &mut Self {
if !self.allow_suggestions {
return self
}
self.diagnostic.span_suggestions_with_applicability(
sp,
msg,
suggestions,
applicability,
);
self
}

pub fn span_suggestion_short_with_applicability(&mut self,
sp: Span,
msg: &str,
suggestion: String,
applicability: Applicability)
-> &mut Self {
if !self.allow_suggestions {
return self
}
self.diagnostic.span_suggestion_short_with_applicability(
sp,
msg,
suggestion,
applicability,
);
self
}
forward!(pub fn set_span<S: Into<MultiSpan>>(&mut self, sp: S) -> &mut Self);
forward!(pub fn code(&mut self, s: DiagnosticId) -> &mut Self);

pub fn allow_suggestions(&mut self, allow: bool) -> &mut Self {
self.allow_suggestions = allow;
self
}

/// Convenience function for internal use, clients should use one of the
/// struct_* methods on Handler.
pub fn new(handler: &'a Handler, level: Level, message: &str) -> DiagnosticBuilder<'a> {
Expand All @@ -228,7 +269,11 @@ impl<'a> DiagnosticBuilder<'a> {
/// diagnostic.
pub fn new_diagnostic(handler: &'a Handler, diagnostic: Diagnostic)
-> DiagnosticBuilder<'a> {
DiagnosticBuilder { handler, diagnostic }
DiagnosticBuilder {
handler,
diagnostic,
allow_suggestions: true,
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2018 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.

// no-prefer-dynamic

#![crate_type = "proc-macro"]

extern crate proc_macro;

use proc_macro::*;

#[proc_macro_attribute]
pub fn foo(_attr: TokenStream, _f: TokenStream) -> TokenStream {
"pub fn foo() -> ::Foo { ::Foo }".parse().unwrap()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2018 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.

// aux-build:suggestions-not-always-applicable.rs
// compile-flags: --edition 2015
// run-rustfix
// rustfix-only-machine-applicable
// compile-pass

#![feature(rust_2018_preview)]
#![warn(rust_2018_compatibility)]

extern crate suggestions_not_always_applicable as foo;

pub struct Foo;

mod test {
use crate::foo::foo;

#[foo] //~ WARN: absolute paths must start with
//~| WARN: previously accepted
//~| WARN: absolute paths
//~| WARN: previously accepted
fn main() {
}
}

fn main() {
test::foo();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2018 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.

// aux-build:suggestions-not-always-applicable.rs
// compile-flags: --edition 2015
// run-rustfix
// rustfix-only-machine-applicable
// compile-pass

#![feature(rust_2018_preview)]
#![warn(rust_2018_compatibility)]

extern crate suggestions_not_always_applicable as foo;

pub struct Foo;

mod test {
use crate::foo::foo;

#[foo] //~ WARN: absolute paths must start with
//~| WARN: previously accepted
//~| WARN: absolute paths
//~| WARN: previously accepted
fn main() {
}
}

fn main() {
test::foo();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
warning: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
--> $DIR/suggestions-not-always-applicable.rs:27:5
|
LL | #[foo] //~ WARN: absolute paths must start with
| ^^^^^^
|
note: lint level defined here
--> $DIR/suggestions-not-always-applicable.rs:18:9
|
LL | #![warn(rust_2018_compatibility)]
| ^^^^^^^^^^^^^^^^^^^^^^^
= note: #[warn(absolute_paths_not_starting_with_crate)] implied by #[warn(rust_2018_compatibility)]
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition!
= note: for more information, see issue TBD

warning: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
--> $DIR/suggestions-not-always-applicable.rs:27:5
|
LL | #[foo] //~ WARN: absolute paths must start with
| ^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition!
= note: for more information, see issue TBD

11 changes: 11 additions & 0 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ pub struct TestProps {
pub normalize_stderr: Vec<(String, String)>,
pub failure_status: i32,
pub run_rustfix: bool,
pub rustfix_only_machine_applicable: bool,
}

impl TestProps {
Expand Down Expand Up @@ -263,6 +264,7 @@ impl TestProps {
normalize_stderr: vec![],
failure_status: -1,
run_rustfix: false,
rustfix_only_machine_applicable: false,
}
}

Expand Down Expand Up @@ -397,6 +399,11 @@ impl TestProps {
if !self.run_rustfix {
self.run_rustfix = config.parse_run_rustfix(ln);
}

if !self.rustfix_only_machine_applicable {
self.rustfix_only_machine_applicable =
config.parse_rustfix_only_machine_applicable(ln);
}
});

if self.failure_status == -1 {
Expand Down Expand Up @@ -663,6 +670,10 @@ impl Config {
self.parse_name_directive(line, "run-rustfix")
}

fn parse_rustfix_only_machine_applicable(&self, line: &str) -> bool {
self.parse_name_directive(line, "rustfix-only-machine-applicable")
}

fn parse_edition(&self, line: &str) -> Option<String> {
self.parse_name_value_directive(line, "edition")
}
Expand Down
8 changes: 6 additions & 2 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2624,7 +2624,11 @@ impl<'test> TestCx<'test> {
let suggestions = get_suggestions_from_json(
&proc_res.stderr,
&HashSet::new(),
Filter::Everything,
if self.props.rustfix_only_machine_applicable {
Filter::MachineApplicableOnly
} else {
Filter::Everything
},
).unwrap();
let fixed_code = apply_suggestions(&unfixed_code, &suggestions).expect(&format!(
"failed to apply suggestions for {:?} with rustfix",
Expand Down Expand Up @@ -2686,7 +2690,7 @@ impl<'test> TestCx<'test> {
if !res.status.success() {
self.fatal_proc_rec("failed to compile fixed code", &res);
}
if !res.stderr.is_empty() {
if !res.stderr.is_empty() && !self.props.rustfix_only_machine_applicable {
self.fatal_proc_rec("fixed code is still producing diagnostics", &res);
}
}
Expand Down

0 comments on commit ca762ba

Please sign in to comment.