From 2892a2b0f578edd290b3be6f5e5c3280bc160f24 Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Thu, 24 Sep 2020 23:22:54 +0900 Subject: [PATCH 1/5] Fix FP in `print_stdout` This lint shouldn't be emitted in `build.rs` as `println!` and `print!` are used for the build script. --- clippy_lints/src/write.rs | 23 ++++++++++++++++++++--- tests/ui/build.rs | 10 ++++++++++ tests/ui/build.stderr | 0 3 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 tests/ui/build.rs create mode 100644 tests/ui/build.stderr diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index fac63bcb993..780d474ee96 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; use std::ops::Range; use crate::utils::{snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then}; +use if_chain::if_chain; use rustc_ast::ast::{Expr, ExprKind, Item, ItemKind, MacCall, StrLit, StrStyle}; use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; @@ -11,7 +12,7 @@ use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_parse::parser; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::symbol::Symbol; -use rustc_span::{BytePos, Span}; +use rustc_span::{BytePos, FileName, Span}; declare_clippy_lint! { /// **What it does:** This lint warns when you use `println!("")` to @@ -236,7 +237,15 @@ impl EarlyLintPass for Write { fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &MacCall) { if mac.path == sym!(println) { - span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`"); + let filename = cx.sess.source_map().span_to_filename(mac.span()); + if_chain! { + if let FileName::Real(filename) = filename; + if let Some(filename) = filename.local_path().file_name(); + if filename != "build.rs"; + then { + span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`"); + } + } if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { if fmt_str.symbol == Symbol::intern("") { span_lint_and_sugg( @@ -251,7 +260,15 @@ impl EarlyLintPass for Write { } } } else if mac.path == sym!(print) { - span_lint(cx, PRINT_STDOUT, mac.span(), "use of `print!`"); + if_chain! { + let filename = cx.sess.source_map().span_to_filename(mac.span()); + if let FileName::Real(filename) = filename; + if let Some(filename) = filename.local_path().file_name(); + if filename != "build.rs"; + then { + span_lint(cx, PRINT_STDOUT, mac.span(), "use of `print!`"); + } + } if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { if check_newlines(&fmt_str) { span_lint_and_then( diff --git a/tests/ui/build.rs b/tests/ui/build.rs new file mode 100644 index 00000000000..2d43d452a4f --- /dev/null +++ b/tests/ui/build.rs @@ -0,0 +1,10 @@ +#![warn(clippy::print_stdout)] + +fn main() { + // Fix #6041 + // + // The `print_stdout` shouldn't be linted in `build.rs` + // as these methods are used for the build script. + println!("Hello"); + print!("Hello"); +} diff --git a/tests/ui/build.stderr b/tests/ui/build.stderr new file mode 100644 index 00000000000..e69de29bb2d From 5b484b405748fc8d7476f9a8d68d2e7227767271 Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Fri, 25 Sep 2020 23:32:18 +0900 Subject: [PATCH 2/5] Fix the detection of build scripts --- clippy_lints/src/write.rs | 33 +++++++++---------- ...{build.rs => print_stdout_build_script.rs} | 2 ++ ...tderr => print_stdout_build_script.stderr} | 0 3 files changed, 17 insertions(+), 18 deletions(-) rename tests/ui/{build.rs => print_stdout_build_script.rs} (81%) rename tests/ui/{build.stderr => print_stdout_build_script.stderr} (100%) diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index 780d474ee96..0e9c7098af8 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -2,7 +2,6 @@ use std::borrow::Cow; use std::ops::Range; use crate::utils::{snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then}; -use if_chain::if_chain; use rustc_ast::ast::{Expr, ExprKind, Item, ItemKind, MacCall, StrLit, StrStyle}; use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; @@ -12,7 +11,7 @@ use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_parse::parser; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::symbol::Symbol; -use rustc_span::{BytePos, FileName, Span}; +use rustc_span::{BytePos, Span}; declare_clippy_lint! { /// **What it does:** This lint warns when you use `println!("")` to @@ -236,15 +235,19 @@ impl EarlyLintPass for Write { } fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &MacCall) { + fn is_build_scripts(cx: &EarlyContext<'_>) -> bool { + // We could leverage the fact that Cargo sets the crate name + // for build scripts to `build_script_build`. + cx.sess + .opts + .crate_name + .as_ref() + .map_or(false, |crate_name| crate_name == "build_script_build") + } + if mac.path == sym!(println) { - let filename = cx.sess.source_map().span_to_filename(mac.span()); - if_chain! { - if let FileName::Real(filename) = filename; - if let Some(filename) = filename.local_path().file_name(); - if filename != "build.rs"; - then { - span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`"); - } + if !is_build_scripts(cx) { + span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`"); } if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { if fmt_str.symbol == Symbol::intern("") { @@ -260,14 +263,8 @@ impl EarlyLintPass for Write { } } } else if mac.path == sym!(print) { - if_chain! { - let filename = cx.sess.source_map().span_to_filename(mac.span()); - if let FileName::Real(filename) = filename; - if let Some(filename) = filename.local_path().file_name(); - if filename != "build.rs"; - then { - span_lint(cx, PRINT_STDOUT, mac.span(), "use of `print!`"); - } + if !is_build_scripts(cx) { + span_lint(cx, PRINT_STDOUT, mac.span(), "use of `print!`"); } if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { if check_newlines(&fmt_str) { diff --git a/tests/ui/build.rs b/tests/ui/print_stdout_build_script.rs similarity index 81% rename from tests/ui/build.rs rename to tests/ui/print_stdout_build_script.rs index 2d43d452a4f..b84bf9124fc 100644 --- a/tests/ui/build.rs +++ b/tests/ui/print_stdout_build_script.rs @@ -1,3 +1,5 @@ +// compile-flags: --crate-name=build_script_build + #![warn(clippy::print_stdout)] fn main() { diff --git a/tests/ui/build.stderr b/tests/ui/print_stdout_build_script.stderr similarity index 100% rename from tests/ui/build.stderr rename to tests/ui/print_stdout_build_script.stderr From 83294f894d477def457d54f2391a287bcb949f06 Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Sat, 26 Sep 2020 23:10:25 +0900 Subject: [PATCH 3/5] Some small fixes --- clippy_lints/src/write.rs | 9 ++++----- tests/ui/print_stdout_build_script.rs | 2 +- tests/ui/print_stdout_build_script.stderr | 0 3 files changed, 5 insertions(+), 6 deletions(-) delete mode 100644 tests/ui/print_stdout_build_script.stderr diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index 0e9c7098af8..d9d60fffcd7 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -235,9 +235,8 @@ impl EarlyLintPass for Write { } fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &MacCall) { - fn is_build_scripts(cx: &EarlyContext<'_>) -> bool { - // We could leverage the fact that Cargo sets the crate name - // for build scripts to `build_script_build`. + fn is_build_script(cx: &EarlyContext<'_>) -> bool { + // Cargo sets the crate name for build scripts to `build_script_build` cx.sess .opts .crate_name @@ -246,7 +245,7 @@ impl EarlyLintPass for Write { } if mac.path == sym!(println) { - if !is_build_scripts(cx) { + if !is_build_script(cx) { span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`"); } if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { @@ -263,7 +262,7 @@ impl EarlyLintPass for Write { } } } else if mac.path == sym!(print) { - if !is_build_scripts(cx) { + if !is_build_script(cx) { span_lint(cx, PRINT_STDOUT, mac.span(), "use of `print!`"); } if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { diff --git a/tests/ui/print_stdout_build_script.rs b/tests/ui/print_stdout_build_script.rs index b84bf9124fc..997ebef8a69 100644 --- a/tests/ui/print_stdout_build_script.rs +++ b/tests/ui/print_stdout_build_script.rs @@ -5,7 +5,7 @@ fn main() { // Fix #6041 // - // The `print_stdout` shouldn't be linted in `build.rs` + // The `print_stdout` lint shouldn't emit in `build.rs` // as these methods are used for the build script. println!("Hello"); print!("Hello"); diff --git a/tests/ui/print_stdout_build_script.stderr b/tests/ui/print_stdout_build_script.stderr deleted file mode 100644 index e69de29bb2d..00000000000 From 71b6d54cd911bdbbc8564dfb17e991cfa1e9a9a8 Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Sat, 26 Sep 2020 23:54:18 +0900 Subject: [PATCH 4/5] Add build script but does not work in the dogfood test --- clippy_workspace_tests/build.rs | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 clippy_workspace_tests/build.rs diff --git a/clippy_workspace_tests/build.rs b/clippy_workspace_tests/build.rs new file mode 100644 index 00000000000..3cc95765210 --- /dev/null +++ b/clippy_workspace_tests/build.rs @@ -0,0 +1,5 @@ +fn main() { + // Test for #6041 + println!("Hello"); + print!("Hello"); +} From 1214a858e0804aa1707e5b87a8eeefa4d207e8c8 Mon Sep 17 00:00:00 2001 From: Eduardo Broto Date: Sat, 26 Sep 2020 22:22:47 +0200 Subject: [PATCH 5/5] Add missing attr to clippy_workspace_tests/build.rs --- clippy_workspace_tests/build.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clippy_workspace_tests/build.rs b/clippy_workspace_tests/build.rs index 3cc95765210..3507168a3a9 100644 --- a/clippy_workspace_tests/build.rs +++ b/clippy_workspace_tests/build.rs @@ -1,3 +1,5 @@ +#![deny(clippy::print_stdout)] + fn main() { // Test for #6041 println!("Hello");