From 6d1aaac6c373e4781f779bddf992242abb53c7c3 Mon Sep 17 00:00:00 2001 From: Jens Hausdorf Date: Sun, 10 Mar 2019 11:06:19 +0100 Subject: [PATCH 1/7] Avoid reporting string_lit_as_bytes for long strings Port of @jens1o code ([b76f939][jens1o_commit]) Fixes #1208 [jens1o_commit]: https://github.com/jens1o/rust-clippy/commit/b76f939ac2efcfe24900c286b3b7713d972d9088 Co-authored-by: Thiago Arrais --- clippy_lints/src/strings.rs | 1 + tests/ui/string_lit_as_bytes.fixed | 5 +++-- tests/ui/string_lit_as_bytes.rs | 5 +++-- tests/ui/string_lit_as_bytes.stderr | 6 +++--- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index eae464ee249..9be2d40bae1 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -173,6 +173,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringLitAsBytes { ); } else if callsite == expanded && lit_content.as_str().chars().all(|c| c.is_ascii()) + && lit_content.as_str().len() <= 32 && !in_macro_or_desugar(args[0].span) { span_lint_and_sugg( diff --git a/tests/ui/string_lit_as_bytes.fixed b/tests/ui/string_lit_as_bytes.fixed index a70504656d9..1922478165f 100644 --- a/tests/ui/string_lit_as_bytes.fixed +++ b/tests/ui/string_lit_as_bytes.fixed @@ -6,10 +6,11 @@ fn str_lit_as_bytes() { let bs = b"hello there"; - let bs = br###"raw string with three ### in it and some " ""###; + let bs = br###"raw string with 3# plus " ""###; - // no warning, because this cannot be written as a byte string literal: + // no warning, because these cannot be written as byte string literals: let ubs = "☃".as_bytes(); + let ubs = "hello there! this is a very long string".as_bytes(); let strify = stringify!(foobar).as_bytes(); diff --git a/tests/ui/string_lit_as_bytes.rs b/tests/ui/string_lit_as_bytes.rs index ea8c712854b..560cbcb657b 100644 --- a/tests/ui/string_lit_as_bytes.rs +++ b/tests/ui/string_lit_as_bytes.rs @@ -6,10 +6,11 @@ fn str_lit_as_bytes() { let bs = "hello there".as_bytes(); - let bs = r###"raw string with three ### in it and some " ""###.as_bytes(); + let bs = r###"raw string with 3# plus " ""###.as_bytes(); - // no warning, because this cannot be written as a byte string literal: + // no warning, because these cannot be written as byte string literals: let ubs = "☃".as_bytes(); + let ubs = "hello there! this is a very long string".as_bytes(); let strify = stringify!(foobar).as_bytes(); diff --git a/tests/ui/string_lit_as_bytes.stderr b/tests/ui/string_lit_as_bytes.stderr index f51cd71a6dc..59aaec75bd2 100644 --- a/tests/ui/string_lit_as_bytes.stderr +++ b/tests/ui/string_lit_as_bytes.stderr @@ -9,11 +9,11 @@ LL | let bs = "hello there".as_bytes(); error: calling `as_bytes()` on a string literal --> $DIR/string_lit_as_bytes.rs:9:14 | -LL | let bs = r###"raw string with three ### in it and some " ""###.as_bytes(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `br###"raw string with three ### in it and some " ""###` +LL | let bs = r###"raw string with 3# plus " ""###.as_bytes(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `br###"raw string with 3# plus " ""###` error: calling `as_bytes()` on `include_str!(..)` - --> $DIR/string_lit_as_bytes.rs:16:22 + --> $DIR/string_lit_as_bytes.rs:17:22 | LL | let includestr = include_str!("entry.rs").as_bytes(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `include_bytes!(..)` instead: `include_bytes!("entry.rs")` From 2fb73fe03782f1485e6e2305afa66edec7f26d83 Mon Sep 17 00:00:00 2001 From: Bara Date: Mon, 8 Jul 2019 20:45:51 +0200 Subject: [PATCH 2/7] Use empty block instead of unit type for needless return --- clippy_lints/src/returns.rs | 10 +++++----- tests/ui/needless_return.stderr | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 2245b719c23..f08945b53d5 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -86,7 +86,7 @@ declare_clippy_lint! { #[derive(PartialEq, Eq, Copy, Clone)] enum RetReplacement { Empty, - Unit, + Block, } declare_lint_pass!(Return => [NEEDLESS_RETURN, LET_AND_RETURN, UNUSED_UNIT]); @@ -139,7 +139,7 @@ impl Return { // a match expr, check all arms ast::ExprKind::Match(_, ref arms) => { for arm in arms { - self.check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Unit); + self.check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Block); } }, _ => (), @@ -176,12 +176,12 @@ impl Return { ); }); }, - RetReplacement::Unit => { + RetReplacement::Block => { span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| { db.span_suggestion( ret_span, - "replace `return` with the unit type", - "()".to_string(), + "replace `return` with an empty block", + "{}".to_string(), Applicability::MachineApplicable, ); }); diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index 7858ecfba97..ee700ab8408 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -70,7 +70,7 @@ error: unneeded return statement --> $DIR/needless_return.rs:64:14 | LL | _ => return, - | ^^^^^^ help: replace `return` with the unit type: `()` + | ^^^^^^ help: replace `return` with an empty block: `{}` error: aborting due to 12 previous errors From 7d1a9447ea15e7b7ce845876c8850f0feed54185 Mon Sep 17 00:00:00 2001 From: Thiago Arrais Date: Mon, 8 Jul 2019 13:12:51 -0300 Subject: [PATCH 3/7] Extract semantic constant --- clippy_lints/src/strings.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 9be2d40bae1..57f63a600a7 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -138,6 +138,9 @@ fn is_add(cx: &LateContext<'_, '_>, src: &Expr, target: &Expr) -> bool { } } +// Max length a b"foo" string can take +const MAX_LENGTH_BYTE_STRING_LIT: usize = 32; + declare_lint_pass!(StringLitAsBytes => [STRING_LIT_AS_BYTES]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringLitAsBytes { @@ -173,7 +176,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringLitAsBytes { ); } else if callsite == expanded && lit_content.as_str().chars().all(|c| c.is_ascii()) - && lit_content.as_str().len() <= 32 + && lit_content.as_str().len() <= MAX_LENGTH_BYTE_STRING_LIT && !in_macro_or_desugar(args[0].span) { span_lint_and_sugg( From 70cffef3b24756b46ae5bb6f590b45f0e5c02e20 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Tue, 9 Jul 2019 13:34:22 +0200 Subject: [PATCH 4/7] Disable AppVeyor builds on the master branch AppVeyor is already checked on every merge of a PR, rechecking it immediately after on the master branch is not necessary. --- appveyor.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 2d8704a703b..675c16bcc4b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -8,11 +8,10 @@ environment: - TARGET: x86_64-pc-windows-msvc branches: - # Only build AppVeyor on r+, try and the master branch + # Only build AppVeyor on r+ and try branch only: - auto - try - - master install: - curl -sSf -o rustup-init.exe https://win.rustup.rs/ From f831b0979e6a694c499c37570252fe787e04123c Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Tue, 9 Jul 2019 14:48:48 +0200 Subject: [PATCH 5/7] cast_ptr_alignment: Mention legal use under known problems Refs #2881. --- clippy_lints/src/types.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index b674b3888ab..7b1d832f249 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -789,7 +789,8 @@ declare_clippy_lint! { /// **Why is this bad?** Dereferencing the resulting pointer may be undefined /// behavior. /// - /// **Known problems:** None. + /// **Known problems:** Using `std::ptr::read_unaligned` and `std::ptr::write_unaligned` or similar + /// on the resulting pointer is fine. /// /// **Example:** /// ```rust From aa72cac87a81d4a0a87d90c0e0c9f5b94d1a93f6 Mon Sep 17 00:00:00 2001 From: Florian Gilcher Date: Sat, 6 Jul 2019 11:40:14 +0200 Subject: [PATCH 6/7] Improve cast_ptr_alignment lint * print alignment in bytes in the lint message * ignore ZST left-hand types --- clippy_lints/src/types.rs | 16 ++++++++++++---- tests/ui/cast_alignment.rs | 2 ++ tests/ui/cast_alignment.stderr | 4 ++-- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 36a4b699fb6..b5023e26873 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -1210,17 +1210,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Casts { if_chain! { if let ty::RawPtr(from_ptr_ty) = &cast_from.sty; if let ty::RawPtr(to_ptr_ty) = &cast_to.sty; - if let Some(from_align) = cx.layout_of(from_ptr_ty.ty).ok().map(|a| a.align.abi); - if let Some(to_align) = cx.layout_of(to_ptr_ty.ty).ok().map(|a| a.align.abi); - if from_align < to_align; + if let Ok(from_layout) = cx.layout_of(from_ptr_ty.ty); + if let Ok(to_layout) = cx.layout_of(to_ptr_ty.ty); + if from_layout.align.abi < to_layout.align.abi; // with c_void, we inherently need to trust the user if !is_c_void(cx, from_ptr_ty.ty); + // when casting from a ZST, we don't know enough to properly lint + if !from_layout.is_zst(); then { span_lint( cx, CAST_PTR_ALIGNMENT, expr.span, - &format!("casting from `{}` to a more-strictly-aligned pointer (`{}`)", cast_from, cast_to) + &format!( + "casting from `{}` to a more-strictly-aligned pointer (`{}`) ({} < {} bytes)", + cast_from, + cast_to, + from_layout.align.abi.bytes(), + to_layout.align.abi.bytes(), + ), ); } } diff --git a/tests/ui/cast_alignment.rs b/tests/ui/cast_alignment.rs index 08450ba1176..4c08935639f 100644 --- a/tests/ui/cast_alignment.rs +++ b/tests/ui/cast_alignment.rs @@ -22,4 +22,6 @@ fn main() { // For c_void, we should trust the user. See #2677 (&1u32 as *const u32 as *const std::os::raw::c_void) as *const u32; (&1u32 as *const u32 as *const libc::c_void) as *const u32; + // For ZST, we should trust the user. See #4256 + (&1u32 as *const u32 as *const ()) as *const u32; } diff --git a/tests/ui/cast_alignment.stderr b/tests/ui/cast_alignment.stderr index 0077be1b570..79219f86155 100644 --- a/tests/ui/cast_alignment.stderr +++ b/tests/ui/cast_alignment.stderr @@ -1,4 +1,4 @@ -error: casting from `*const u8` to a more-strictly-aligned pointer (`*const u16`) +error: casting from `*const u8` to a more-strictly-aligned pointer (`*const u16`) (1 < 2 bytes) --> $DIR/cast_alignment.rs:12:5 | LL | (&1u8 as *const u8) as *const u16; @@ -6,7 +6,7 @@ LL | (&1u8 as *const u8) as *const u16; | = note: `-D clippy::cast-ptr-alignment` implied by `-D warnings` -error: casting from `*mut u8` to a more-strictly-aligned pointer (`*mut u16`) +error: casting from `*mut u8` to a more-strictly-aligned pointer (`*mut u16`) (1 < 2 bytes) --> $DIR/cast_alignment.rs:13:5 | LL | (&mut 1u8 as *mut u8) as *mut u16; From 27c53487937ffbd7a67aad4511a5d72f637b9ebb Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Thu, 11 Jul 2019 21:45:34 +0700 Subject: [PATCH 7/7] Rustup `macro expansion and resolution` --- clippy_lints/src/misc.rs | 14 +++++++++----- clippy_lints/src/returns.rs | 2 +- clippy_lints/src/types.rs | 4 ++-- clippy_lints/src/utils/mod.rs | 8 ++++---- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index d77f71c2ab2..acca50e3df8 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -7,7 +7,7 @@ use rustc::ty; use rustc::{declare_lint_pass, declare_tool_lint}; use rustc_errors::Applicability; use syntax::ast::LitKind; -use syntax::source_map::{ExpnFormat, Span}; +use syntax::source_map::{ExpnKind, Span}; use crate::consts::{constant, Constant}; use crate::utils::sugg::Sugg; @@ -596,10 +596,14 @@ fn is_used(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { /// Tests whether an expression is in a macro expansion (e.g., something /// generated by `#[derive(...)]` or the like). fn in_attributes_expansion(expr: &Expr) -> bool { - expr.span - .ctxt() - .outer_expn_info() - .map_or(false, |info| matches!(info.format, ExpnFormat::MacroAttribute(_))) + use syntax::ext::hygiene::MacroKind; + expr.span.ctxt().outer_expn_info().map_or(false, |info| { + if let ExpnKind::Macro(MacroKind::Attr, _) = info.kind { + true + } else { + false + } + }) } /// Tests whether `res` is a variable defined outside a macro. diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index f08945b53d5..0f2084e819e 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -317,7 +317,7 @@ fn attr_is_cfg(attr: &ast::Attribute) -> bool { // get the def site fn get_def(span: Span) -> Option { - span.ctxt().outer_expn_info().and_then(|info| info.def_site) + span.ctxt().outer_expn_info().and_then(|info| Some(info.def_site)) } // is this expr a `()` unit? diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index dd578176f23..4f35337292e 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -621,9 +621,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnitArg { } fn is_questionmark_desugar_marked_call(expr: &Expr) -> bool { - use syntax_pos::hygiene::CompilerDesugaringKind; + use syntax_pos::hygiene::DesugaringKind; if let ExprKind::Call(ref callee, _) = expr.node { - callee.span.is_compiler_desugaring(CompilerDesugaringKind::QuestionMark) + callee.span.is_desugaring(DesugaringKind::QuestionMark) } else { false } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 53665506c39..9e45c453ae9 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -43,7 +43,7 @@ use rustc_errors::Applicability; use smallvec::SmallVec; use syntax::ast::{self, LitKind}; use syntax::attr; -use syntax::ext::hygiene::ExpnFormat; +use syntax::ext::hygiene::ExpnKind; use syntax::source_map::{Span, DUMMY_SP}; use syntax::symbol::{kw, Symbol}; @@ -100,7 +100,7 @@ pub fn in_macro_or_desugar(span: Span) -> bool { /// Returns `true` if this `expn_info` was expanded by any macro. pub fn in_macro(span: Span) -> bool { if let Some(info) = span.ctxt().outer_expn_info() { - if let ExpnFormat::CompilerDesugaring(..) = info.format { + if let ExpnKind::Desugaring(..) = info.kind { false } else { true @@ -686,7 +686,7 @@ pub fn is_adjusted(cx: &LateContext<'_, '_>, e: &Expr) -> bool { /// See also `is_direct_expn_of`. pub fn is_expn_of(mut span: Span, name: &str) -> Option { loop { - let span_name_span = span.ctxt().outer_expn_info().map(|ei| (ei.format.name(), ei.call_site)); + let span_name_span = span.ctxt().outer_expn_info().map(|ei| (ei.kind.descr(), ei.call_site)); match span_name_span { Some((mac_name, new_span)) if mac_name.as_str() == name => return Some(new_span), @@ -706,7 +706,7 @@ pub fn is_expn_of(mut span: Span, name: &str) -> Option { /// `bar!` by /// `is_direct_expn_of`. pub fn is_direct_expn_of(span: Span, name: &str) -> Option { - let span_name_span = span.ctxt().outer_expn_info().map(|ei| (ei.format.name(), ei.call_site)); + let span_name_span = span.ctxt().outer_expn_info().map(|ei| (ei.kind.descr(), ei.call_site)); match span_name_span { Some((mac_name, new_span)) if mac_name.as_str() == name => Some(new_span),