Merge remote-tracking branch 'upstream/master' into rustup
This commit is contained in:
commit
87ce1d8069
141 changed files with 3554 additions and 1145 deletions
|
|
@ -26,7 +26,7 @@ pub(super) fn check<'cx>(cx: &LateContext<'cx>, name: Symbol, items: &[NestedMet
|
|||
cx,
|
||||
ALLOW_ATTRIBUTES_WITHOUT_REASON,
|
||||
attr.span,
|
||||
format!("`{}` attribute without specifying a reason", name.as_str()),
|
||||
format!("`{name}` attribute without specifying a reason"),
|
||||
|diag| {
|
||||
diag.help("try adding a reason at the end with `, reason = \"..\"`");
|
||||
},
|
||||
|
|
|
|||
|
|
@ -1,52 +0,0 @@
|
|||
use super::{EMPTY_LINE_AFTER_DOC_COMMENTS, EMPTY_LINE_AFTER_OUTER_ATTR};
|
||||
use clippy_utils::diagnostics::span_lint;
|
||||
use clippy_utils::source::{is_present_in_source, without_block_comments, SpanRangeExt};
|
||||
use rustc_ast::{AttrKind, AttrStyle};
|
||||
use rustc_lint::EarlyContext;
|
||||
use rustc_span::Span;
|
||||
|
||||
/// Check for empty lines after outer attributes.
|
||||
///
|
||||
/// Attributes and documentation comments are both considered outer attributes
|
||||
/// by the AST. However, the average user likely considers them to be different.
|
||||
/// Checking for empty lines after each of these attributes is split into two different
|
||||
/// lints but can share the same logic.
|
||||
pub(super) fn check(cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
|
||||
let mut iter = item.attrs.iter().peekable();
|
||||
while let Some(attr) = iter.next() {
|
||||
if (matches!(attr.kind, AttrKind::Normal(..)) || matches!(attr.kind, AttrKind::DocComment(..)))
|
||||
&& attr.style == AttrStyle::Outer
|
||||
&& is_present_in_source(cx, attr.span)
|
||||
{
|
||||
let begin_of_attr_to_item = Span::new(attr.span.lo(), item.span.lo(), item.span.ctxt(), item.span.parent());
|
||||
let end_of_attr_to_next_attr_or_item = Span::new(
|
||||
attr.span.hi(),
|
||||
iter.peek().map_or(item.span.lo(), |next_attr| next_attr.span.lo()),
|
||||
item.span.ctxt(),
|
||||
item.span.parent(),
|
||||
);
|
||||
|
||||
if let Some(snippet) = end_of_attr_to_next_attr_or_item.get_source_text(cx) {
|
||||
let lines = snippet.split('\n').collect::<Vec<_>>();
|
||||
let lines = without_block_comments(lines);
|
||||
|
||||
if lines.iter().filter(|l| l.trim().is_empty()).count() > 2 {
|
||||
let (lint_msg, lint_type) = match attr.kind {
|
||||
AttrKind::DocComment(..) => (
|
||||
"found an empty line after a doc comment. \
|
||||
Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`?",
|
||||
EMPTY_LINE_AFTER_DOC_COMMENTS,
|
||||
),
|
||||
AttrKind::Normal(..) => (
|
||||
"found an empty line after an outer attribute. \
|
||||
Perhaps you forgot to add a `!` to make it an inner attribute?",
|
||||
EMPTY_LINE_AFTER_OUTER_ATTR,
|
||||
),
|
||||
};
|
||||
|
||||
span_lint(cx, lint_type, begin_of_attr_to_item, lint_msg);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -4,7 +4,6 @@ mod blanket_clippy_restriction_lints;
|
|||
mod deprecated_cfg_attr;
|
||||
mod deprecated_semver;
|
||||
mod duplicated_attributes;
|
||||
mod empty_line_after;
|
||||
mod inline_always;
|
||||
mod mixed_attributes_style;
|
||||
mod non_minimal_cfg;
|
||||
|
|
@ -126,94 +125,6 @@ declare_clippy_lint! {
|
|||
"use of `#[deprecated(since = \"x\")]` where x is not semver"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for empty lines after outer attributes
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// Most likely the attribute was meant to be an inner attribute using a '!'.
|
||||
/// If it was meant to be an outer attribute, then the following item
|
||||
/// should not be separated by empty lines.
|
||||
///
|
||||
/// ### Known problems
|
||||
/// Can cause false positives.
|
||||
///
|
||||
/// From the clippy side it's difficult to detect empty lines between an attributes and the
|
||||
/// following item because empty lines and comments are not part of the AST. The parsing
|
||||
/// currently works for basic cases but is not perfect.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```no_run
|
||||
/// #[allow(dead_code)]
|
||||
///
|
||||
/// fn not_quite_good_code() { }
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```no_run
|
||||
/// // Good (as inner attribute)
|
||||
/// #![allow(dead_code)]
|
||||
///
|
||||
/// fn this_is_fine() { }
|
||||
///
|
||||
/// // or
|
||||
///
|
||||
/// // Good (as outer attribute)
|
||||
/// #[allow(dead_code)]
|
||||
/// fn this_is_fine_too() { }
|
||||
/// ```
|
||||
#[clippy::version = "pre 1.29.0"]
|
||||
pub EMPTY_LINE_AFTER_OUTER_ATTR,
|
||||
nursery,
|
||||
"empty line after outer attribute"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for empty lines after documentation comments.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// The documentation comment was most likely meant to be an inner attribute or regular comment.
|
||||
/// If it was intended to be a documentation comment, then the empty line should be removed to
|
||||
/// be more idiomatic.
|
||||
///
|
||||
/// ### Known problems
|
||||
/// Only detects empty lines immediately following the documentation. If the doc comment is followed
|
||||
/// by an attribute and then an empty line, this lint will not trigger. Use `empty_line_after_outer_attr`
|
||||
/// in combination with this lint to detect both cases.
|
||||
///
|
||||
/// Does not detect empty lines after doc attributes (e.g. `#[doc = ""]`).
|
||||
///
|
||||
/// ### Example
|
||||
/// ```no_run
|
||||
/// /// Some doc comment with a blank line after it.
|
||||
///
|
||||
/// fn not_quite_good_code() { }
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```no_run
|
||||
/// /// Good (no blank line)
|
||||
/// fn this_is_fine() { }
|
||||
/// ```
|
||||
///
|
||||
/// ```no_run
|
||||
/// // Good (convert to a regular comment)
|
||||
///
|
||||
/// fn this_is_fine_too() { }
|
||||
/// ```
|
||||
///
|
||||
/// ```no_run
|
||||
/// //! Good (convert to a comment on an inner attribute)
|
||||
///
|
||||
/// fn this_is_fine_as_well() { }
|
||||
/// ```
|
||||
#[clippy::version = "1.70.0"]
|
||||
pub EMPTY_LINE_AFTER_DOC_COMMENTS,
|
||||
nursery,
|
||||
"empty line after documentation comments"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category.
|
||||
|
|
@ -601,18 +512,12 @@ impl EarlyAttributes {
|
|||
|
||||
impl_lint_pass!(EarlyAttributes => [
|
||||
DEPRECATED_CFG_ATTR,
|
||||
EMPTY_LINE_AFTER_OUTER_ATTR,
|
||||
EMPTY_LINE_AFTER_DOC_COMMENTS,
|
||||
NON_MINIMAL_CFG,
|
||||
DEPRECATED_CLIPPY_CFG_ATTR,
|
||||
UNNECESSARY_CLIPPY_CFG,
|
||||
]);
|
||||
|
||||
impl EarlyLintPass for EarlyAttributes {
|
||||
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
|
||||
empty_line_after::check(cx, item);
|
||||
}
|
||||
|
||||
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
|
||||
deprecated_cfg_attr::check(cx, attr, &self.msrv);
|
||||
deprecated_cfg_attr::check_clippy(cx, attr);
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ declare_clippy_lint! {
|
|||
/// ```ignore
|
||||
/// b"Hello"
|
||||
/// ```
|
||||
#[clippy::version = "1.68.0"]
|
||||
#[clippy::version = "1.81.0"]
|
||||
pub BYTE_CHAR_SLICES,
|
||||
style,
|
||||
"hard to read byte char slice"
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ declare_clippy_lint! {
|
|||
/// # fn important_check() {}
|
||||
/// important_check();
|
||||
/// ```
|
||||
#[clippy::version = "1.73.0"]
|
||||
#[clippy::version = "1.81.0"]
|
||||
pub CFG_NOT_TEST,
|
||||
restriction,
|
||||
"enforce against excluding code from test builds"
|
||||
|
|
|
|||
|
|
@ -49,8 +49,6 @@ pub static LINTS: &[&crate::LintInfo] = &[
|
|||
crate::attrs::DEPRECATED_CLIPPY_CFG_ATTR_INFO,
|
||||
crate::attrs::DEPRECATED_SEMVER_INFO,
|
||||
crate::attrs::DUPLICATED_ATTRIBUTES_INFO,
|
||||
crate::attrs::EMPTY_LINE_AFTER_DOC_COMMENTS_INFO,
|
||||
crate::attrs::EMPTY_LINE_AFTER_OUTER_ATTR_INFO,
|
||||
crate::attrs::INLINE_ALWAYS_INFO,
|
||||
crate::attrs::MIXED_ATTRIBUTES_STYLE_INFO,
|
||||
crate::attrs::NON_MINIMAL_CFG_INFO,
|
||||
|
|
@ -138,6 +136,8 @@ pub static LINTS: &[&crate::LintInfo] = &[
|
|||
crate::doc::DOC_LINK_WITH_QUOTES_INFO,
|
||||
crate::doc::DOC_MARKDOWN_INFO,
|
||||
crate::doc::EMPTY_DOCS_INFO,
|
||||
crate::doc::EMPTY_LINE_AFTER_DOC_COMMENTS_INFO,
|
||||
crate::doc::EMPTY_LINE_AFTER_OUTER_ATTR_INFO,
|
||||
crate::doc::MISSING_ERRORS_DOC_INFO,
|
||||
crate::doc::MISSING_PANICS_DOC_INFO,
|
||||
crate::doc::MISSING_SAFETY_DOC_INFO,
|
||||
|
|
@ -217,6 +217,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
|
|||
crate::implicit_return::IMPLICIT_RETURN_INFO,
|
||||
crate::implicit_saturating_add::IMPLICIT_SATURATING_ADD_INFO,
|
||||
crate::implicit_saturating_sub::IMPLICIT_SATURATING_SUB_INFO,
|
||||
crate::implicit_saturating_sub::INVERTED_SATURATING_SUB_INFO,
|
||||
crate::implied_bounds_in_impls::IMPLIED_BOUNDS_IN_IMPLS_INFO,
|
||||
crate::incompatible_msrv::INCOMPATIBLE_MSRV_INFO,
|
||||
crate::inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR_INFO,
|
||||
|
|
@ -486,6 +487,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
|
|||
crate::misc::SHORT_CIRCUIT_STATEMENT_INFO,
|
||||
crate::misc::TOPLEVEL_REF_ARG_INFO,
|
||||
crate::misc::USED_UNDERSCORE_BINDING_INFO,
|
||||
crate::misc::USED_UNDERSCORE_ITEMS_INFO,
|
||||
crate::misc_early::BUILTIN_TYPE_SHADOW_INFO,
|
||||
crate::misc_early::DOUBLE_NEG_INFO,
|
||||
crate::misc_early::DUPLICATE_UNDERSCORE_ARGUMENT_INFO,
|
||||
|
|
@ -598,6 +600,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
|
|||
crate::pathbuf_init_then_push::PATHBUF_INIT_THEN_PUSH_INFO,
|
||||
crate::pattern_type_mismatch::PATTERN_TYPE_MISMATCH_INFO,
|
||||
crate::permissions_set_readonly_false::PERMISSIONS_SET_READONLY_FALSE_INFO,
|
||||
crate::pointers_in_nomem_asm_block::POINTERS_IN_NOMEM_ASM_BLOCK_INFO,
|
||||
crate::precedence::PRECEDENCE_INFO,
|
||||
crate::ptr::CMP_NULL_INFO,
|
||||
crate::ptr::INVALID_NULL_PTR_USAGE_INFO,
|
||||
|
|
@ -767,4 +770,5 @@ pub static LINTS: &[&crate::LintInfo] = &[
|
|||
crate::zero_div_zero::ZERO_DIVIDED_BY_ZERO_INFO,
|
||||
crate::zero_repeat_side_effects::ZERO_REPEAT_SIDE_EFFECTS_INFO,
|
||||
crate::zero_sized_map_values::ZERO_SIZED_MAP_VALUES_INFO,
|
||||
crate::zombie_processes::ZOMBIE_PROCESSES_INFO,
|
||||
];
|
||||
|
|
|
|||
329
clippy_lints/src/doc/empty_line_after.rs
Normal file
329
clippy_lints/src/doc/empty_line_after.rs
Normal file
|
|
@ -0,0 +1,329 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_then;
|
||||
use clippy_utils::source::{snippet_indent, SpanRangeExt};
|
||||
use clippy_utils::tokenize_with_text;
|
||||
use itertools::Itertools;
|
||||
use rustc_ast::token::CommentKind;
|
||||
use rustc_ast::{AttrKind, AttrStyle, Attribute};
|
||||
use rustc_errors::{Applicability, Diag, SuggestionStyle};
|
||||
use rustc_hir::{ItemKind, Node};
|
||||
use rustc_lexer::TokenKind;
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_span::{ExpnKind, InnerSpan, Span, SpanData};
|
||||
|
||||
use super::{EMPTY_LINE_AFTER_DOC_COMMENTS, EMPTY_LINE_AFTER_OUTER_ATTR};
|
||||
|
||||
#[derive(Debug, PartialEq, Clone, Copy)]
|
||||
enum StopKind {
|
||||
Attr,
|
||||
Doc(CommentKind),
|
||||
}
|
||||
|
||||
impl StopKind {
|
||||
fn is_doc(self) -> bool {
|
||||
matches!(self, StopKind::Doc(_))
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct Stop {
|
||||
span: Span,
|
||||
kind: StopKind,
|
||||
first: usize,
|
||||
last: usize,
|
||||
}
|
||||
|
||||
impl Stop {
|
||||
fn convert_to_inner(&self) -> (Span, String) {
|
||||
let inner = match self.kind {
|
||||
// #|[...]
|
||||
StopKind::Attr => InnerSpan::new(1, 1),
|
||||
// /// or /**
|
||||
// ^ ^
|
||||
StopKind::Doc(_) => InnerSpan::new(2, 3),
|
||||
};
|
||||
(self.span.from_inner(inner), "!".into())
|
||||
}
|
||||
|
||||
fn comment_out(&self, cx: &LateContext<'_>, suggestions: &mut Vec<(Span, String)>) {
|
||||
match self.kind {
|
||||
StopKind::Attr => {
|
||||
if cx.tcx.sess.source_map().is_multiline(self.span) {
|
||||
suggestions.extend([
|
||||
(self.span.shrink_to_lo(), "/* ".into()),
|
||||
(self.span.shrink_to_hi(), " */".into()),
|
||||
]);
|
||||
} else {
|
||||
suggestions.push((self.span.shrink_to_lo(), "// ".into()));
|
||||
}
|
||||
},
|
||||
StopKind::Doc(CommentKind::Line) => suggestions.push((self.span.shrink_to_lo(), "// ".into())),
|
||||
StopKind::Doc(CommentKind::Block) => {
|
||||
// /** outer */ /*! inner */
|
||||
// ^ ^
|
||||
let asterisk = self.span.from_inner(InnerSpan::new(1, 2));
|
||||
suggestions.push((asterisk, String::new()));
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn from_attr(cx: &LateContext<'_>, attr: &Attribute) -> Option<Self> {
|
||||
let SpanData { lo, hi, .. } = attr.span.data();
|
||||
let file = cx.tcx.sess.source_map().lookup_source_file(lo);
|
||||
|
||||
Some(Self {
|
||||
span: attr.span,
|
||||
kind: match attr.kind {
|
||||
AttrKind::Normal(_) => StopKind::Attr,
|
||||
AttrKind::DocComment(comment_kind, _) => StopKind::Doc(comment_kind),
|
||||
},
|
||||
first: file.lookup_line(file.relative_position(lo))?,
|
||||
last: file.lookup_line(file.relative_position(hi))?,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
/// Represents a set of attrs/doc comments separated by 1 or more empty lines
|
||||
///
|
||||
/// ```ignore
|
||||
/// /// chunk 1 docs
|
||||
/// // not an empty line so also part of chunk 1
|
||||
/// #[chunk_1_attrs] // <-- prev_stop
|
||||
///
|
||||
/// /* gap */
|
||||
///
|
||||
/// /// chunk 2 docs // <-- next_stop
|
||||
/// #[chunk_2_attrs]
|
||||
/// ```
|
||||
struct Gap<'a> {
|
||||
/// The span of individual empty lines including the newline at the end of the line
|
||||
empty_lines: Vec<Span>,
|
||||
has_comment: bool,
|
||||
next_stop: &'a Stop,
|
||||
prev_stop: &'a Stop,
|
||||
/// The chunk that includes [`prev_stop`](Self::prev_stop)
|
||||
prev_chunk: &'a [Stop],
|
||||
}
|
||||
|
||||
impl<'a> Gap<'a> {
|
||||
fn new(cx: &LateContext<'_>, prev_chunk: &'a [Stop], next_chunk: &'a [Stop]) -> Option<Self> {
|
||||
let prev_stop = prev_chunk.last()?;
|
||||
let next_stop = next_chunk.first()?;
|
||||
let gap_span = prev_stop.span.between(next_stop.span);
|
||||
let gap_snippet = gap_span.get_source_text(cx)?;
|
||||
|
||||
let mut has_comment = false;
|
||||
let mut empty_lines = Vec::new();
|
||||
|
||||
for (token, source, inner_span) in tokenize_with_text(&gap_snippet) {
|
||||
match token {
|
||||
TokenKind::BlockComment {
|
||||
doc_style: None,
|
||||
terminated: true,
|
||||
}
|
||||
| TokenKind::LineComment { doc_style: None } => has_comment = true,
|
||||
TokenKind::Whitespace => {
|
||||
let newlines = source.bytes().positions(|b| b == b'\n');
|
||||
empty_lines.extend(
|
||||
newlines
|
||||
.tuple_windows()
|
||||
.map(|(a, b)| InnerSpan::new(inner_span.start + a + 1, inner_span.start + b))
|
||||
.map(|inner_span| gap_span.from_inner(inner_span)),
|
||||
);
|
||||
},
|
||||
// Ignore cfg_attr'd out attributes as they may contain empty lines, could also be from macro
|
||||
// shenanigans
|
||||
_ => return None,
|
||||
}
|
||||
}
|
||||
|
||||
(!empty_lines.is_empty()).then_some(Self {
|
||||
empty_lines,
|
||||
has_comment,
|
||||
next_stop,
|
||||
prev_stop,
|
||||
prev_chunk,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
/// If the node the attributes/docs apply to is the first in the module/crate suggest converting
|
||||
/// them to inner attributes/docs
|
||||
fn suggest_inner(cx: &LateContext<'_>, diag: &mut Diag<'_, ()>, kind: StopKind, gaps: &[Gap<'_>]) {
|
||||
let Some(owner) = cx.last_node_with_lint_attrs.as_owner() else {
|
||||
return;
|
||||
};
|
||||
let parent_desc = match cx.tcx.parent_hir_node(owner.into()) {
|
||||
Node::Item(item)
|
||||
if let ItemKind::Mod(parent_mod) = item.kind
|
||||
&& let [first, ..] = parent_mod.item_ids
|
||||
&& first.owner_id == owner =>
|
||||
{
|
||||
"parent module"
|
||||
},
|
||||
Node::Crate(crate_mod)
|
||||
if let Some(first) = crate_mod
|
||||
.item_ids
|
||||
.iter()
|
||||
.map(|&id| cx.tcx.hir().item(id))
|
||||
// skip prelude imports
|
||||
.find(|item| !matches!(item.span.ctxt().outer_expn_data().kind, ExpnKind::AstPass(_)))
|
||||
&& first.owner_id == owner =>
|
||||
{
|
||||
"crate"
|
||||
},
|
||||
_ => return,
|
||||
};
|
||||
|
||||
diag.multipart_suggestion_verbose(
|
||||
match kind {
|
||||
StopKind::Attr => format!("if the attribute should apply to the {parent_desc} use an inner attribute"),
|
||||
StopKind::Doc(_) => format!("if the comment should document the {parent_desc} use an inner doc comment"),
|
||||
},
|
||||
gaps.iter()
|
||||
.flat_map(|gap| gap.prev_chunk)
|
||||
.map(Stop::convert_to_inner)
|
||||
.collect(),
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
}
|
||||
|
||||
fn check_gaps(cx: &LateContext<'_>, gaps: &[Gap<'_>]) -> bool {
|
||||
let Some(first_gap) = gaps.first() else {
|
||||
return false;
|
||||
};
|
||||
let empty_lines = || gaps.iter().flat_map(|gap| gap.empty_lines.iter().copied());
|
||||
let mut has_comment = false;
|
||||
let mut has_attr = false;
|
||||
for gap in gaps {
|
||||
has_comment |= gap.has_comment;
|
||||
if !has_attr {
|
||||
has_attr = gap.prev_chunk.iter().any(|stop| stop.kind == StopKind::Attr);
|
||||
}
|
||||
}
|
||||
let kind = first_gap.prev_stop.kind;
|
||||
let (lint, kind_desc) = match kind {
|
||||
StopKind::Attr => (EMPTY_LINE_AFTER_OUTER_ATTR, "outer attribute"),
|
||||
StopKind::Doc(_) => (EMPTY_LINE_AFTER_DOC_COMMENTS, "doc comment"),
|
||||
};
|
||||
let (lines, are, them) = if empty_lines().nth(1).is_some() {
|
||||
("lines", "are", "them")
|
||||
} else {
|
||||
("line", "is", "it")
|
||||
};
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
lint,
|
||||
first_gap.prev_stop.span.to(empty_lines().last().unwrap()),
|
||||
format!("empty {lines} after {kind_desc}"),
|
||||
|diag| {
|
||||
if let Some(owner) = cx.last_node_with_lint_attrs.as_owner() {
|
||||
let def_id = owner.to_def_id();
|
||||
let def_descr = cx.tcx.def_descr(def_id);
|
||||
diag.span_label(
|
||||
cx.tcx.def_span(def_id),
|
||||
match kind {
|
||||
StopKind::Attr => format!("the attribute applies to this {def_descr}"),
|
||||
StopKind::Doc(_) => format!("the comment documents this {def_descr}"),
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
diag.multipart_suggestion_with_style(
|
||||
format!("if the empty {lines} {are} unintentional remove {them}"),
|
||||
empty_lines().map(|empty_line| (empty_line, String::new())).collect(),
|
||||
Applicability::MaybeIncorrect,
|
||||
SuggestionStyle::HideCodeAlways,
|
||||
);
|
||||
|
||||
if has_comment && kind.is_doc() {
|
||||
// Likely doc comments that applied to some now commented out code
|
||||
//
|
||||
// /// Old docs for Foo
|
||||
// // struct Foo;
|
||||
|
||||
let mut suggestions = Vec::new();
|
||||
for stop in gaps.iter().flat_map(|gap| gap.prev_chunk) {
|
||||
stop.comment_out(cx, &mut suggestions);
|
||||
}
|
||||
let name = match cx.tcx.hir().opt_name(cx.last_node_with_lint_attrs) {
|
||||
Some(name) => format!("`{name}`"),
|
||||
None => "this".into(),
|
||||
};
|
||||
diag.multipart_suggestion_verbose(
|
||||
format!("if the doc comment should not document {name} comment it out"),
|
||||
suggestions,
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
} else {
|
||||
suggest_inner(cx, diag, kind, gaps);
|
||||
}
|
||||
|
||||
if kind == StopKind::Doc(CommentKind::Line)
|
||||
&& gaps
|
||||
.iter()
|
||||
.all(|gap| !gap.has_comment && gap.next_stop.kind == StopKind::Doc(CommentKind::Line))
|
||||
{
|
||||
// Commentless empty gaps between line doc comments, possibly intended to be part of the markdown
|
||||
|
||||
let indent = snippet_indent(cx, first_gap.prev_stop.span).unwrap_or_default();
|
||||
diag.multipart_suggestion_verbose(
|
||||
format!("if the documentation should include the empty {lines} include {them} in the comment"),
|
||||
empty_lines()
|
||||
.map(|empty_line| (empty_line, format!("{indent}///")))
|
||||
.collect(),
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
}
|
||||
},
|
||||
);
|
||||
kind.is_doc()
|
||||
}
|
||||
|
||||
/// Returns `true` if [`EMPTY_LINE_AFTER_DOC_COMMENTS`] triggered, used to skip other doc comment
|
||||
/// lints where they would be confusing
|
||||
///
|
||||
/// [`EMPTY_LINE_AFTER_OUTER_ATTR`] is also here to share an implementation but does not return
|
||||
/// `true` if it triggers
|
||||
pub(super) fn check(cx: &LateContext<'_>, attrs: &[Attribute]) -> bool {
|
||||
let mut outer = attrs
|
||||
.iter()
|
||||
.filter(|attr| attr.style == AttrStyle::Outer && !attr.span.from_expansion())
|
||||
.map(|attr| Stop::from_attr(cx, attr))
|
||||
.collect::<Option<Vec<_>>>()
|
||||
.unwrap_or_default();
|
||||
|
||||
if outer.is_empty() {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Push a fake attribute Stop for the item itself so we check for gaps between the last outer
|
||||
// attr/doc comment and the item they apply to
|
||||
let span = cx.tcx.hir().span(cx.last_node_with_lint_attrs);
|
||||
if !span.from_expansion()
|
||||
&& let Ok(line) = cx.tcx.sess.source_map().lookup_line(span.lo())
|
||||
{
|
||||
outer.push(Stop {
|
||||
span,
|
||||
kind: StopKind::Attr,
|
||||
first: line.line,
|
||||
// last doesn't need to be accurate here, we don't compare it with anything
|
||||
last: line.line,
|
||||
});
|
||||
}
|
||||
|
||||
let mut gaps = Vec::new();
|
||||
let mut last = 0;
|
||||
for pos in outer
|
||||
.array_windows()
|
||||
.positions(|[a, b]| b.first.saturating_sub(a.last) > 1)
|
||||
{
|
||||
// we want to be after the first stop in the window
|
||||
let pos = pos + 1;
|
||||
if let Some(gap) = Gap::new(cx, &outer[last..pos], &outer[pos..]) {
|
||||
last = pos;
|
||||
gaps.push(gap);
|
||||
}
|
||||
}
|
||||
|
||||
check_gaps(cx, &gaps)
|
||||
}
|
||||
|
|
@ -22,7 +22,6 @@ pub(super) fn check(
|
|||
range: Range<usize>,
|
||||
mut span: Span,
|
||||
containers: &[super::Container],
|
||||
line_break_span: Span,
|
||||
) {
|
||||
if doc[range.clone()].contains('\t') {
|
||||
// We don't do tab stops correctly.
|
||||
|
|
@ -52,29 +51,6 @@ pub(super) fn check(
|
|||
"doc list item without indentation"
|
||||
};
|
||||
span_lint_and_then(cx, DOC_LAZY_CONTINUATION, span, msg, |diag| {
|
||||
let snippet = clippy_utils::source::snippet(cx, line_break_span, "");
|
||||
if snippet.chars().filter(|&c| c == '\n').count() > 1
|
||||
&& let Some(doc_comment_start) = snippet.rfind('\n')
|
||||
&& let doc_comment = snippet[doc_comment_start..].trim()
|
||||
&& (doc_comment == "///" || doc_comment == "//!")
|
||||
{
|
||||
// suggest filling in a blank line
|
||||
diag.span_suggestion_verbose(
|
||||
line_break_span.shrink_to_lo(),
|
||||
"if this should be its own paragraph, add a blank doc comment line",
|
||||
format!("\n{doc_comment}"),
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
if ccount > 0 || blockquote_level > 0 {
|
||||
diag.help("if this not intended to be a quote at all, escape it with `\\>`");
|
||||
} else {
|
||||
let indent = list_indentation - lcount;
|
||||
diag.help(format!(
|
||||
"if this is intended to be part of the list, indent {indent} spaces"
|
||||
));
|
||||
}
|
||||
return;
|
||||
}
|
||||
if ccount == 0 && blockquote_level == 0 {
|
||||
// simpler suggestion style for indentation
|
||||
let indent = list_indentation - lcount;
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ use rustc_span::{sym, Span};
|
|||
use std::ops::Range;
|
||||
use url::Url;
|
||||
|
||||
mod empty_line_after;
|
||||
mod link_with_quotes;
|
||||
mod markdown;
|
||||
mod missing_headers;
|
||||
|
|
@ -455,7 +456,82 @@ declare_clippy_lint! {
|
|||
"ensure that the first line of a documentation paragraph isn't too long"
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for empty lines after outer attributes
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// The attribute may have meant to be an inner attribute (`#![attr]`). If
|
||||
/// it was meant to be an outer attribute (`#[attr]`) then the empty line
|
||||
/// should be removed
|
||||
///
|
||||
/// ### Example
|
||||
/// ```no_run
|
||||
/// #[allow(dead_code)]
|
||||
///
|
||||
/// fn not_quite_good_code() {}
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```no_run
|
||||
/// // Good (as inner attribute)
|
||||
/// #![allow(dead_code)]
|
||||
///
|
||||
/// fn this_is_fine() {}
|
||||
///
|
||||
/// // or
|
||||
///
|
||||
/// // Good (as outer attribute)
|
||||
/// #[allow(dead_code)]
|
||||
/// fn this_is_fine_too() {}
|
||||
/// ```
|
||||
#[clippy::version = "pre 1.29.0"]
|
||||
pub EMPTY_LINE_AFTER_OUTER_ATTR,
|
||||
suspicious,
|
||||
"empty line after outer attribute"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for empty lines after doc comments.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// The doc comment may have meant to be an inner doc comment, regular
|
||||
/// comment or applied to some old code that is now commented out. If it was
|
||||
/// intended to be a doc comment, then the empty line should be removed.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```no_run
|
||||
/// /// Some doc comment with a blank line after it.
|
||||
///
|
||||
/// fn f() {}
|
||||
///
|
||||
/// /// Docs for `old_code`
|
||||
/// // fn old_code() {}
|
||||
///
|
||||
/// fn new_code() {}
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```no_run
|
||||
/// //! Convert it to an inner doc comment
|
||||
///
|
||||
/// // Or a regular comment
|
||||
///
|
||||
/// /// Or remove the empty line
|
||||
/// fn f() {}
|
||||
///
|
||||
/// // /// Docs for `old_code`
|
||||
/// // fn old_code() {}
|
||||
///
|
||||
/// fn new_code() {}
|
||||
/// ```
|
||||
#[clippy::version = "1.70.0"]
|
||||
pub EMPTY_LINE_AFTER_DOC_COMMENTS,
|
||||
suspicious,
|
||||
"empty line after doc comments"
|
||||
}
|
||||
|
||||
pub struct Documentation {
|
||||
valid_idents: FxHashSet<String>,
|
||||
check_private_items: bool,
|
||||
|
|
@ -482,6 +558,8 @@ impl_lint_pass!(Documentation => [
|
|||
SUSPICIOUS_DOC_COMMENTS,
|
||||
EMPTY_DOCS,
|
||||
DOC_LAZY_CONTINUATION,
|
||||
EMPTY_LINE_AFTER_OUTER_ATTR,
|
||||
EMPTY_LINE_AFTER_DOC_COMMENTS,
|
||||
TOO_LONG_FIRST_DOC_PARAGRAPH,
|
||||
]);
|
||||
|
||||
|
|
@ -612,12 +690,10 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
|
|||
Some(("fake".into(), "fake".into()))
|
||||
}
|
||||
|
||||
if is_doc_hidden(attrs) {
|
||||
if suspicious_doc_comments::check(cx, attrs) || empty_line_after::check(cx, attrs) || is_doc_hidden(attrs) {
|
||||
return None;
|
||||
}
|
||||
|
||||
suspicious_doc_comments::check(cx, attrs);
|
||||
|
||||
let (fragments, _) = attrs_to_doc_fragments(
|
||||
attrs.iter().filter_map(|attr| {
|
||||
if in_external_macro(cx.sess(), attr.span) {
|
||||
|
|
@ -816,7 +892,6 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
|
|||
range.end..next_range.start,
|
||||
Span::new(span.hi(), next_span.lo(), span.ctxt(), span.parent()),
|
||||
&containers[..],
|
||||
span,
|
||||
);
|
||||
}
|
||||
},
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ use rustc_span::Span;
|
|||
|
||||
use super::SUSPICIOUS_DOC_COMMENTS;
|
||||
|
||||
pub fn check(cx: &LateContext<'_>, attrs: &[Attribute]) {
|
||||
pub fn check(cx: &LateContext<'_>, attrs: &[Attribute]) -> bool {
|
||||
let replacements: Vec<_> = collect_doc_replacements(attrs);
|
||||
|
||||
if let Some((&(lo_span, _), &(hi_span, _))) = replacements.first().zip(replacements.last()) {
|
||||
|
|
@ -24,6 +24,10 @@ pub fn check(cx: &LateContext<'_>, attrs: &[Attribute]) {
|
|||
);
|
||||
},
|
||||
);
|
||||
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -41,7 +41,7 @@ declare_clippy_lint! {
|
|||
/// }
|
||||
/// }
|
||||
/// ```
|
||||
#[clippy::version = "1.78.0"]
|
||||
#[clippy::version = "1.81.0"]
|
||||
pub FIELD_SCOPED_VISIBILITY_MODIFIERS,
|
||||
restriction,
|
||||
"checks for usage of a scoped visibility modifier, like `pub(crate)`, on fields"
|
||||
|
|
|
|||
|
|
@ -181,6 +181,9 @@ fn convert_to_from(
|
|||
let from = self_ty.span.get_source_text(cx)?;
|
||||
let into = target_ty.span.get_source_text(cx)?;
|
||||
|
||||
let return_type = matches!(sig.decl.output, FnRetTy::Return(_))
|
||||
.then_some(String::from("Self"))
|
||||
.unwrap_or_default();
|
||||
let mut suggestions = vec![
|
||||
// impl Into<T> for U -> impl From<T> for U
|
||||
// ~~~~ ~~~~
|
||||
|
|
@ -197,13 +200,10 @@ fn convert_to_from(
|
|||
// fn into([mut] self) -> T -> fn into([mut] v: T) -> T
|
||||
// ~~~~ ~~~~
|
||||
(self_ident.span, format!("val: {from}")),
|
||||
];
|
||||
|
||||
if let FnRetTy::Return(_) = sig.decl.output {
|
||||
// fn into(self) -> T -> fn into(self) -> Self
|
||||
// ~ ~~~~
|
||||
suggestions.push((sig.decl.output.span(), String::from("Self")));
|
||||
}
|
||||
(sig.decl.output.span(), return_type),
|
||||
];
|
||||
|
||||
let mut finder = SelfFinder {
|
||||
cx,
|
||||
|
|
|
|||
|
|
@ -1,11 +1,17 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use clippy_utils::{higher, is_integer_literal, peel_blocks_with_stmt, SpanlessEq};
|
||||
use clippy_config::msrvs::{self, Msrv};
|
||||
use clippy_config::Conf;
|
||||
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
|
||||
use clippy_utils::source::snippet_opt;
|
||||
use clippy_utils::{
|
||||
higher, is_in_const_context, is_integer_literal, path_to_local, peel_blocks, peel_blocks_with_stmt, SpanlessEq,
|
||||
};
|
||||
use rustc_ast::ast::LitKind;
|
||||
use rustc_data_structures::packed::Pu128;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{BinOpKind, Expr, ExprKind, QPath};
|
||||
use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind, HirId, QPath};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::declare_lint_pass;
|
||||
use rustc_session::impl_lint_pass;
|
||||
use rustc_span::Span;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
|
|
@ -39,7 +45,49 @@ declare_clippy_lint! {
|
|||
"Perform saturating subtraction instead of implicitly checking lower bound of data type"
|
||||
}
|
||||
|
||||
declare_lint_pass!(ImplicitSaturatingSub => [IMPLICIT_SATURATING_SUB]);
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for comparisons between integers, followed by subtracting the greater value from the
|
||||
/// lower one.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// This could result in an underflow and is most likely not what the user wants. If this was
|
||||
/// intended to be a saturated subtraction, consider using the `saturating_sub` method directly.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```no_run
|
||||
/// let a = 12u32;
|
||||
/// let b = 13u32;
|
||||
///
|
||||
/// let result = if a > b { b - a } else { 0 };
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```no_run
|
||||
/// let a = 12u32;
|
||||
/// let b = 13u32;
|
||||
///
|
||||
/// let result = a.saturating_sub(b);
|
||||
/// ```
|
||||
#[clippy::version = "1.44.0"]
|
||||
pub INVERTED_SATURATING_SUB,
|
||||
correctness,
|
||||
"Check if a variable is smaller than another one and still subtract from it even if smaller"
|
||||
}
|
||||
|
||||
pub struct ImplicitSaturatingSub {
|
||||
msrv: Msrv,
|
||||
}
|
||||
|
||||
impl_lint_pass!(ImplicitSaturatingSub => [IMPLICIT_SATURATING_SUB, INVERTED_SATURATING_SUB]);
|
||||
|
||||
impl ImplicitSaturatingSub {
|
||||
pub fn new(conf: &'static Conf) -> Self {
|
||||
Self {
|
||||
msrv: conf.msrv.clone(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
|
||||
|
|
@ -50,73 +98,260 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
|
|||
|
||||
// Check if the conditional expression is a binary operation
|
||||
&& let ExprKind::Binary(ref cond_op, cond_left, cond_right) = cond.kind
|
||||
|
||||
// Ensure that the binary operator is >, !=, or <
|
||||
&& (BinOpKind::Ne == cond_op.node || BinOpKind::Gt == cond_op.node || BinOpKind::Lt == cond_op.node)
|
||||
|
||||
// Check if assign operation is done
|
||||
&& let Some(target) = subtracts_one(cx, then)
|
||||
|
||||
// Extracting out the variable name
|
||||
&& let ExprKind::Path(QPath::Resolved(_, ares_path)) = target.kind
|
||||
{
|
||||
// Handle symmetric conditions in the if statement
|
||||
let (cond_var, cond_num_val) = if SpanlessEq::new(cx).eq_expr(cond_left, target) {
|
||||
if BinOpKind::Gt == cond_op.node || BinOpKind::Ne == cond_op.node {
|
||||
(cond_left, cond_right)
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
} else if SpanlessEq::new(cx).eq_expr(cond_right, target) {
|
||||
if BinOpKind::Lt == cond_op.node || BinOpKind::Ne == cond_op.node {
|
||||
(cond_right, cond_left)
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
check_with_condition(cx, expr, cond_op.node, cond_left, cond_right, then);
|
||||
} else if let Some(higher::If {
|
||||
cond,
|
||||
then: if_block,
|
||||
r#else: Some(else_block),
|
||||
}) = higher::If::hir(expr)
|
||||
&& let ExprKind::Binary(ref cond_op, cond_left, cond_right) = cond.kind
|
||||
{
|
||||
check_manual_check(
|
||||
cx, expr, cond_op, cond_left, cond_right, if_block, else_block, &self.msrv,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
extract_msrv_attr!(LateContext);
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn check_manual_check<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
expr: &Expr<'tcx>,
|
||||
condition: &BinOp,
|
||||
left_hand: &Expr<'tcx>,
|
||||
right_hand: &Expr<'tcx>,
|
||||
if_block: &Expr<'tcx>,
|
||||
else_block: &Expr<'tcx>,
|
||||
msrv: &Msrv,
|
||||
) {
|
||||
let ty = cx.typeck_results().expr_ty(left_hand);
|
||||
if ty.is_numeric() && !ty.is_signed() {
|
||||
match condition.node {
|
||||
BinOpKind::Gt | BinOpKind::Ge => check_gt(
|
||||
cx,
|
||||
condition.span,
|
||||
expr.span,
|
||||
left_hand,
|
||||
right_hand,
|
||||
if_block,
|
||||
else_block,
|
||||
msrv,
|
||||
),
|
||||
BinOpKind::Lt | BinOpKind::Le => check_gt(
|
||||
cx,
|
||||
condition.span,
|
||||
expr.span,
|
||||
right_hand,
|
||||
left_hand,
|
||||
if_block,
|
||||
else_block,
|
||||
msrv,
|
||||
),
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn check_gt(
|
||||
cx: &LateContext<'_>,
|
||||
condition_span: Span,
|
||||
expr_span: Span,
|
||||
big_var: &Expr<'_>,
|
||||
little_var: &Expr<'_>,
|
||||
if_block: &Expr<'_>,
|
||||
else_block: &Expr<'_>,
|
||||
msrv: &Msrv,
|
||||
) {
|
||||
if let Some(big_var) = Var::new(big_var)
|
||||
&& let Some(little_var) = Var::new(little_var)
|
||||
{
|
||||
check_subtraction(
|
||||
cx,
|
||||
condition_span,
|
||||
expr_span,
|
||||
big_var,
|
||||
little_var,
|
||||
if_block,
|
||||
else_block,
|
||||
msrv,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
struct Var {
|
||||
span: Span,
|
||||
hir_id: HirId,
|
||||
}
|
||||
|
||||
impl Var {
|
||||
fn new(expr: &Expr<'_>) -> Option<Self> {
|
||||
path_to_local(expr).map(|hir_id| Self {
|
||||
span: expr.span,
|
||||
hir_id,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn check_subtraction(
|
||||
cx: &LateContext<'_>,
|
||||
condition_span: Span,
|
||||
expr_span: Span,
|
||||
big_var: Var,
|
||||
little_var: Var,
|
||||
if_block: &Expr<'_>,
|
||||
else_block: &Expr<'_>,
|
||||
msrv: &Msrv,
|
||||
) {
|
||||
let if_block = peel_blocks(if_block);
|
||||
let else_block = peel_blocks(else_block);
|
||||
if is_integer_literal(if_block, 0) {
|
||||
// We need to check this case as well to prevent infinite recursion.
|
||||
if is_integer_literal(else_block, 0) {
|
||||
// Well, seems weird but who knows?
|
||||
return;
|
||||
}
|
||||
// If the subtraction is done in the `else` block, then we need to also revert the two
|
||||
// variables as it means that the check was reverted too.
|
||||
check_subtraction(
|
||||
cx,
|
||||
condition_span,
|
||||
expr_span,
|
||||
little_var,
|
||||
big_var,
|
||||
else_block,
|
||||
if_block,
|
||||
msrv,
|
||||
);
|
||||
return;
|
||||
}
|
||||
if is_integer_literal(else_block, 0)
|
||||
&& let ExprKind::Binary(op, left, right) = if_block.kind
|
||||
&& let BinOpKind::Sub = op.node
|
||||
{
|
||||
let local_left = path_to_local(left);
|
||||
let local_right = path_to_local(right);
|
||||
if Some(big_var.hir_id) == local_left && Some(little_var.hir_id) == local_right {
|
||||
// This part of the condition is voluntarily split from the one before to ensure that
|
||||
// if `snippet_opt` fails, it won't try the next conditions.
|
||||
if let Some(big_var_snippet) = snippet_opt(cx, big_var.span)
|
||||
&& let Some(little_var_snippet) = snippet_opt(cx, little_var.span)
|
||||
&& (!is_in_const_context(cx) || msrv.meets(msrvs::SATURATING_SUB_CONST))
|
||||
{
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
IMPLICIT_SATURATING_SUB,
|
||||
expr_span,
|
||||
"manual arithmetic check found",
|
||||
"replace it with",
|
||||
format!("{big_var_snippet}.saturating_sub({little_var_snippet})"),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
} else if Some(little_var.hir_id) == local_left
|
||||
&& Some(big_var.hir_id) == local_right
|
||||
&& let Some(big_var_snippet) = snippet_opt(cx, big_var.span)
|
||||
&& let Some(little_var_snippet) = snippet_opt(cx, little_var.span)
|
||||
{
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
INVERTED_SATURATING_SUB,
|
||||
condition_span,
|
||||
"inverted arithmetic check before subtraction",
|
||||
|diag| {
|
||||
diag.span_note(
|
||||
if_block.span,
|
||||
format!("this subtraction underflows when `{little_var_snippet} < {big_var_snippet}`"),
|
||||
);
|
||||
diag.span_suggestion(
|
||||
if_block.span,
|
||||
"try replacing it with",
|
||||
format!("{big_var_snippet} - {little_var_snippet}"),
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_with_condition<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
expr: &Expr<'tcx>,
|
||||
cond_op: BinOpKind,
|
||||
cond_left: &Expr<'tcx>,
|
||||
cond_right: &Expr<'tcx>,
|
||||
then: &Expr<'tcx>,
|
||||
) {
|
||||
// Ensure that the binary operator is >, !=, or <
|
||||
if (BinOpKind::Ne == cond_op || BinOpKind::Gt == cond_op || BinOpKind::Lt == cond_op)
|
||||
|
||||
// Check if assign operation is done
|
||||
&& let Some(target) = subtracts_one(cx, then)
|
||||
|
||||
// Extracting out the variable name
|
||||
&& let ExprKind::Path(QPath::Resolved(_, ares_path)) = target.kind
|
||||
{
|
||||
// Handle symmetric conditions in the if statement
|
||||
let (cond_var, cond_num_val) = if SpanlessEq::new(cx).eq_expr(cond_left, target) {
|
||||
if BinOpKind::Gt == cond_op || BinOpKind::Ne == cond_op {
|
||||
(cond_left, cond_right)
|
||||
} else {
|
||||
return;
|
||||
};
|
||||
|
||||
// Check if the variable in the condition statement is an integer
|
||||
if !cx.typeck_results().expr_ty(cond_var).is_integral() {
|
||||
}
|
||||
} else if SpanlessEq::new(cx).eq_expr(cond_right, target) {
|
||||
if BinOpKind::Lt == cond_op || BinOpKind::Ne == cond_op {
|
||||
(cond_right, cond_left)
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
return;
|
||||
};
|
||||
|
||||
// Get the variable name
|
||||
let var_name = ares_path.segments[0].ident.name.as_str();
|
||||
match cond_num_val.kind {
|
||||
ExprKind::Lit(cond_lit) => {
|
||||
// Check if the constant is zero
|
||||
if let LitKind::Int(Pu128(0), _) = cond_lit.node {
|
||||
if cx.typeck_results().expr_ty(cond_left).is_signed() {
|
||||
} else {
|
||||
print_lint_and_sugg(cx, var_name, expr);
|
||||
};
|
||||
}
|
||||
},
|
||||
ExprKind::Path(QPath::TypeRelative(_, name)) => {
|
||||
if name.ident.as_str() == "MIN"
|
||||
&& let Some(const_id) = cx.typeck_results().type_dependent_def_id(cond_num_val.hir_id)
|
||||
&& let Some(impl_id) = cx.tcx.impl_of_method(const_id)
|
||||
&& let None = cx.tcx.impl_trait_ref(impl_id) // An inherent impl
|
||||
&& cx.tcx.type_of(impl_id).instantiate_identity().is_integral()
|
||||
{
|
||||
// Check if the variable in the condition statement is an integer
|
||||
if !cx.typeck_results().expr_ty(cond_var).is_integral() {
|
||||
return;
|
||||
}
|
||||
|
||||
// Get the variable name
|
||||
let var_name = ares_path.segments[0].ident.name.as_str();
|
||||
match cond_num_val.kind {
|
||||
ExprKind::Lit(cond_lit) => {
|
||||
// Check if the constant is zero
|
||||
if let LitKind::Int(Pu128(0), _) = cond_lit.node {
|
||||
if cx.typeck_results().expr_ty(cond_left).is_signed() {
|
||||
} else {
|
||||
print_lint_and_sugg(cx, var_name, expr);
|
||||
}
|
||||
},
|
||||
ExprKind::Call(func, []) => {
|
||||
if let ExprKind::Path(QPath::TypeRelative(_, name)) = func.kind
|
||||
&& name.ident.as_str() == "min_value"
|
||||
&& let Some(func_id) = cx.typeck_results().type_dependent_def_id(func.hir_id)
|
||||
&& let Some(impl_id) = cx.tcx.impl_of_method(func_id)
|
||||
&& let None = cx.tcx.impl_trait_ref(impl_id) // An inherent impl
|
||||
&& cx.tcx.type_of(impl_id).instantiate_identity().is_integral()
|
||||
{
|
||||
print_lint_and_sugg(cx, var_name, expr);
|
||||
}
|
||||
},
|
||||
_ => (),
|
||||
}
|
||||
};
|
||||
}
|
||||
},
|
||||
ExprKind::Path(QPath::TypeRelative(_, name)) => {
|
||||
if name.ident.as_str() == "MIN"
|
||||
&& let Some(const_id) = cx.typeck_results().type_dependent_def_id(cond_num_val.hir_id)
|
||||
&& let Some(impl_id) = cx.tcx.impl_of_method(const_id)
|
||||
&& let None = cx.tcx.impl_trait_ref(impl_id) // An inherent impl
|
||||
&& cx.tcx.type_of(impl_id).instantiate_identity().is_integral()
|
||||
{
|
||||
print_lint_and_sugg(cx, var_name, expr);
|
||||
}
|
||||
},
|
||||
ExprKind::Call(func, []) => {
|
||||
if let ExprKind::Path(QPath::TypeRelative(_, name)) = func.kind
|
||||
&& name.ident.as_str() == "min_value"
|
||||
&& let Some(func_id) = cx.typeck_results().type_dependent_def_id(func.hir_id)
|
||||
&& let Some(impl_id) = cx.tcx.impl_of_method(func_id)
|
||||
&& let None = cx.tcx.impl_trait_ref(impl_id) // An inherent impl
|
||||
&& cx.tcx.type_of(impl_id).instantiate_identity().is_integral()
|
||||
{
|
||||
print_lint_and_sugg(cx, var_name, expr);
|
||||
}
|
||||
},
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -287,6 +287,7 @@ mod pass_by_ref_or_value;
|
|||
mod pathbuf_init_then_push;
|
||||
mod pattern_type_mismatch;
|
||||
mod permissions_set_readonly_false;
|
||||
mod pointers_in_nomem_asm_block;
|
||||
mod precedence;
|
||||
mod ptr;
|
||||
mod ptr_offset_with_cast;
|
||||
|
|
@ -386,6 +387,7 @@ mod write;
|
|||
mod zero_div_zero;
|
||||
mod zero_repeat_side_effects;
|
||||
mod zero_sized_map_values;
|
||||
mod zombie_processes;
|
||||
// end lints modules, do not remove this comment, it’s used in `update_lints`
|
||||
|
||||
use clippy_config::{get_configuration_metadata, Conf};
|
||||
|
|
@ -623,7 +625,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
|
|||
store.register_late_pass(|_| Box::new(unit_return_expecting_ord::UnitReturnExpectingOrd));
|
||||
store.register_late_pass(|_| Box::new(strings::StringAdd));
|
||||
store.register_late_pass(|_| Box::new(implicit_return::ImplicitReturn));
|
||||
store.register_late_pass(|_| Box::new(implicit_saturating_sub::ImplicitSaturatingSub));
|
||||
store.register_late_pass(move |_| Box::new(implicit_saturating_sub::ImplicitSaturatingSub::new(conf)));
|
||||
store.register_late_pass(|_| Box::new(default_numeric_fallback::DefaultNumericFallback));
|
||||
store.register_late_pass(|_| Box::new(inconsistent_struct_constructor::InconsistentStructConstructor));
|
||||
store.register_late_pass(|_| Box::new(non_octal_unix_permissions::NonOctalUnixPermissions));
|
||||
|
|
@ -933,5 +935,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
|
|||
store.register_late_pass(|_| Box::new(set_contains_or_insert::SetContainsOrInsert));
|
||||
store.register_early_pass(|| Box::new(byte_char_slices::ByteCharSlice));
|
||||
store.register_early_pass(|| Box::new(cfg_not_test::CfgNotTest));
|
||||
store.register_late_pass(|_| Box::new(zombie_processes::ZombieProcesses));
|
||||
store.register_late_pass(|_| Box::new(pointers_in_nomem_asm_block::PointersInNomemAsmBlock));
|
||||
// add lints here, do not remove this comment, it's used in `new_lint`
|
||||
}
|
||||
|
|
|
|||
|
|
@ -77,9 +77,10 @@ impl Num {
|
|||
impl LateLintPass<'_> for ManualRangePatterns {
|
||||
fn check_pat(&mut self, cx: &LateContext<'_>, pat: &'_ rustc_hir::Pat<'_>) {
|
||||
// a pattern like 1 | 2 seems fine, lint if there are at least 3 alternatives
|
||||
// or at least one range
|
||||
// or more then one range (exclude triggering on stylistic using OR with one element
|
||||
// like described https://github.com/rust-lang/rust-clippy/issues/11825)
|
||||
if let PatKind::Or(pats) = pat.kind
|
||||
&& (pats.len() >= 3 || pats.iter().any(|p| matches!(p.kind, PatKind::Range(..))))
|
||||
&& (pats.len() >= 3 || (pats.len() > 1 && pats.iter().any(|p| matches!(p.kind, PatKind::Range(..)))))
|
||||
&& !in_external_macro(cx.sess(), pat.span)
|
||||
{
|
||||
let mut min = Num::dummy(i128::MAX);
|
||||
|
|
|
|||
|
|
@ -14,7 +14,6 @@ use rustc_span::sym;
|
|||
/// - `hashmap.into_iter().map(|(_, v)| v)`
|
||||
///
|
||||
/// on `HashMaps` and `BTreeMaps` in std
|
||||
|
||||
pub(super) fn check<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
map_type: &'tcx str, // iter / into_iter
|
||||
|
|
|
|||
|
|
@ -441,6 +441,17 @@ declare_clippy_lint! {
|
|||
/// }
|
||||
/// }
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```no_run
|
||||
/// # struct X;
|
||||
/// impl X {
|
||||
/// fn as_str(&self) -> &'static str {
|
||||
/// // ..
|
||||
/// # ""
|
||||
/// }
|
||||
/// }
|
||||
/// ```
|
||||
#[clippy::version = "pre 1.29.0"]
|
||||
pub WRONG_SELF_CONVENTION,
|
||||
style,
|
||||
|
|
@ -3964,7 +3975,7 @@ declare_clippy_lint! {
|
|||
/// ```no_run
|
||||
/// let _ = 0;
|
||||
/// ```
|
||||
#[clippy::version = "1.78.0"]
|
||||
#[clippy::version = "1.81.0"]
|
||||
pub UNNECESSARY_MIN_OR_MAX,
|
||||
complexity,
|
||||
"using 'min()/max()' when there is no need for it"
|
||||
|
|
@ -4025,7 +4036,7 @@ declare_clippy_lint! {
|
|||
/// ```
|
||||
#[clippy::version = "1.78.0"]
|
||||
pub MANUAL_C_STR_LITERALS,
|
||||
pedantic,
|
||||
complexity,
|
||||
r#"creating a `CStr` through functions when `c""` literals can be used"#
|
||||
}
|
||||
|
||||
|
|
@ -4099,7 +4110,7 @@ declare_clippy_lint! {
|
|||
/// ```no_run
|
||||
/// "foo".is_ascii();
|
||||
/// ```
|
||||
#[clippy::version = "1.80.0"]
|
||||
#[clippy::version = "1.81.0"]
|
||||
pub NEEDLESS_CHARACTER_ITERATION,
|
||||
suspicious,
|
||||
"is_ascii() called on a char iterator"
|
||||
|
|
|
|||
|
|
@ -1,16 +1,15 @@
|
|||
use std::cmp::Ordering;
|
||||
|
||||
use super::UNNECESSARY_MIN_OR_MAX;
|
||||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
|
||||
use clippy_utils::consts::{ConstEvalCtxt, Constant, ConstantSource, FullInt};
|
||||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use clippy_utils::source::snippet;
|
||||
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::Expr;
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_middle::ty;
|
||||
use rustc_span::Span;
|
||||
use rustc_span::{sym, Span};
|
||||
|
||||
pub(super) fn check<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
|
|
@ -21,26 +20,30 @@ pub(super) fn check<'tcx>(
|
|||
) {
|
||||
let typeck_results = cx.typeck_results();
|
||||
let ecx = ConstEvalCtxt::with_env(cx.tcx, cx.param_env, typeck_results);
|
||||
if let Some((left, ConstantSource::Local | ConstantSource::CoreConstant)) = ecx.eval_with_source(recv)
|
||||
&& let Some((right, ConstantSource::Local | ConstantSource::CoreConstant)) = ecx.eval_with_source(arg)
|
||||
if let Some(id) = typeck_results.type_dependent_def_id(expr.hir_id)
|
||||
&& (cx.tcx.is_diagnostic_item(sym::cmp_ord_min, id) || cx.tcx.is_diagnostic_item(sym::cmp_ord_max, id))
|
||||
{
|
||||
let Some(ord) = Constant::partial_cmp(cx.tcx, typeck_results.expr_ty(recv), &left, &right) else {
|
||||
return;
|
||||
};
|
||||
if let Some((left, ConstantSource::Local | ConstantSource::CoreConstant)) = ecx.eval_with_source(recv)
|
||||
&& let Some((right, ConstantSource::Local | ConstantSource::CoreConstant)) = ecx.eval_with_source(arg)
|
||||
{
|
||||
let Some(ord) = Constant::partial_cmp(cx.tcx, typeck_results.expr_ty(recv), &left, &right) else {
|
||||
return;
|
||||
};
|
||||
|
||||
lint(cx, expr, name, recv.span, arg.span, ord);
|
||||
} else if let Some(extrema) = detect_extrema(cx, recv) {
|
||||
let ord = match extrema {
|
||||
Extrema::Minimum => Ordering::Less,
|
||||
Extrema::Maximum => Ordering::Greater,
|
||||
};
|
||||
lint(cx, expr, name, recv.span, arg.span, ord);
|
||||
} else if let Some(extrema) = detect_extrema(cx, arg) {
|
||||
let ord = match extrema {
|
||||
Extrema::Minimum => Ordering::Greater,
|
||||
Extrema::Maximum => Ordering::Less,
|
||||
};
|
||||
lint(cx, expr, name, recv.span, arg.span, ord);
|
||||
lint(cx, expr, name, recv.span, arg.span, ord);
|
||||
} else if let Some(extrema) = detect_extrema(cx, recv) {
|
||||
let ord = match extrema {
|
||||
Extrema::Minimum => Ordering::Less,
|
||||
Extrema::Maximum => Ordering::Greater,
|
||||
};
|
||||
lint(cx, expr, name, recv.span, arg.span, ord);
|
||||
} else if let Some(extrema) = detect_extrema(cx, arg) {
|
||||
let ord = match extrema {
|
||||
Extrema::Minimum => Ordering::Greater,
|
||||
Extrema::Maximum => Ordering::Less,
|
||||
};
|
||||
lint(cx, expr, name, recv.span, arg.span, ord);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -5,7 +5,8 @@ use clippy_utils::ty::implements_trait;
|
|||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Closure, Expr, ExprKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QPath};
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_middle::ty::{self, GenericArgKind};
|
||||
use rustc_middle::ty;
|
||||
use rustc_middle::ty::GenericArgKind;
|
||||
use rustc_span::sym;
|
||||
use rustc_span::symbol::Ident;
|
||||
use std::iter;
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ use clippy_utils::{
|
|||
};
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::def::Res;
|
||||
use rustc_hir::def_id::LOCAL_CRATE;
|
||||
use rustc_hir::intravisit::FnKind;
|
||||
use rustc_hir::{
|
||||
BinOpKind, BindingMode, Body, ByRef, Expr, ExprKind, FnDecl, Mutability, PatKind, QPath, Stmt, StmtKind,
|
||||
|
|
@ -80,6 +81,45 @@ declare_clippy_lint! {
|
|||
"using a binding which is prefixed with an underscore"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for the use of item with a single leading
|
||||
/// underscore.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// A single leading underscore is usually used to indicate
|
||||
/// that a item will not be used. Using such a item breaks this
|
||||
/// expectation.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```no_run
|
||||
/// fn _foo() {}
|
||||
///
|
||||
/// struct _FooStruct {}
|
||||
///
|
||||
/// fn main() {
|
||||
/// _foo();
|
||||
/// let _ = _FooStruct{};
|
||||
/// }
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```no_run
|
||||
/// fn foo() {}
|
||||
///
|
||||
/// struct FooStruct {}
|
||||
///
|
||||
/// fn main() {
|
||||
/// foo();
|
||||
/// let _ = FooStruct{};
|
||||
/// }
|
||||
/// ```
|
||||
#[clippy::version = "pre 1.29.0"]
|
||||
pub USED_UNDERSCORE_ITEMS,
|
||||
pedantic,
|
||||
"using a item which is prefixed with an underscore"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for the use of short circuit boolean conditions as
|
||||
|
|
@ -104,6 +144,7 @@ declare_clippy_lint! {
|
|||
declare_lint_pass!(LintPass => [
|
||||
TOPLEVEL_REF_ARG,
|
||||
USED_UNDERSCORE_BINDING,
|
||||
USED_UNDERSCORE_ITEMS,
|
||||
SHORT_CIRCUIT_STATEMENT,
|
||||
]);
|
||||
|
||||
|
|
@ -205,51 +246,104 @@ impl<'tcx> LateLintPass<'tcx> for LintPass {
|
|||
{
|
||||
return;
|
||||
}
|
||||
let (definition_hir_id, ident) = match expr.kind {
|
||||
ExprKind::Path(ref qpath) => {
|
||||
if let QPath::Resolved(None, path) = qpath
|
||||
&& let Res::Local(id) = path.res
|
||||
&& is_used(cx, expr)
|
||||
{
|
||||
(id, last_path_segment(qpath).ident)
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
},
|
||||
ExprKind::Field(recv, ident) => {
|
||||
if let Some(adt_def) = cx.typeck_results().expr_ty_adjusted(recv).ty_adt_def()
|
||||
&& let Some(field) = adt_def.all_fields().find(|field| field.name == ident.name)
|
||||
&& let Some(local_did) = field.did.as_local()
|
||||
&& !cx.tcx.type_of(field.did).skip_binder().is_phantom_data()
|
||||
{
|
||||
(cx.tcx.local_def_id_to_hir_id(local_did), ident)
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
},
|
||||
_ => return,
|
||||
};
|
||||
|
||||
let name = ident.name.as_str();
|
||||
if name.starts_with('_')
|
||||
&& !name.starts_with("__")
|
||||
&& let definition_span = cx.tcx.hir().span(definition_hir_id)
|
||||
&& !definition_span.from_expansion()
|
||||
&& !fulfill_or_allowed(cx, USED_UNDERSCORE_BINDING, [expr.hir_id, definition_hir_id])
|
||||
{
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
USED_UNDERSCORE_BINDING,
|
||||
expr.span,
|
||||
format!(
|
||||
"used binding `{name}` which is prefixed with an underscore. A leading \
|
||||
underscore signals that a binding will not be used"
|
||||
),
|
||||
|diag| {
|
||||
diag.span_note(definition_span, format!("`{name}` is defined here"));
|
||||
},
|
||||
);
|
||||
}
|
||||
used_underscore_binding(cx, expr);
|
||||
used_underscore_items(cx, expr);
|
||||
}
|
||||
}
|
||||
|
||||
fn used_underscore_items<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
||||
let (def_id, ident) = match expr.kind {
|
||||
ExprKind::Call(func, ..) => {
|
||||
if let ExprKind::Path(QPath::Resolved(.., path)) = func.kind
|
||||
&& let Some(last_segment) = path.segments.last()
|
||||
&& let Res::Def(_, def_id) = last_segment.res
|
||||
{
|
||||
(def_id, last_segment.ident)
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
},
|
||||
ExprKind::MethodCall(path, ..) => {
|
||||
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) {
|
||||
(def_id, path.ident)
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
},
|
||||
ExprKind::Struct(QPath::Resolved(_, path), ..) => {
|
||||
if let Some(last_segment) = path.segments.last()
|
||||
&& let Res::Def(_, def_id) = last_segment.res
|
||||
{
|
||||
(def_id, last_segment.ident)
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
},
|
||||
_ => return,
|
||||
};
|
||||
|
||||
let name = ident.name.as_str();
|
||||
let definition_span = cx.tcx.def_span(def_id);
|
||||
if name.starts_with('_')
|
||||
&& !name.starts_with("__")
|
||||
&& !definition_span.from_expansion()
|
||||
&& def_id.krate == LOCAL_CRATE
|
||||
{
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
USED_UNDERSCORE_ITEMS,
|
||||
expr.span,
|
||||
"used underscore-prefixed item".to_string(),
|
||||
|diag| {
|
||||
diag.span_note(definition_span, "item is defined here".to_string());
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn used_underscore_binding<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
||||
let (definition_hir_id, ident) = match expr.kind {
|
||||
ExprKind::Path(ref qpath) => {
|
||||
if let QPath::Resolved(None, path) = qpath
|
||||
&& let Res::Local(id) = path.res
|
||||
&& is_used(cx, expr)
|
||||
{
|
||||
(id, last_path_segment(qpath).ident)
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
},
|
||||
ExprKind::Field(recv, ident) => {
|
||||
if let Some(adt_def) = cx.typeck_results().expr_ty_adjusted(recv).ty_adt_def()
|
||||
&& let Some(field) = adt_def.all_fields().find(|field| field.name == ident.name)
|
||||
&& let Some(local_did) = field.did.as_local()
|
||||
&& !cx.tcx.type_of(field.did).skip_binder().is_phantom_data()
|
||||
{
|
||||
(cx.tcx.local_def_id_to_hir_id(local_did), ident)
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
},
|
||||
_ => return,
|
||||
};
|
||||
|
||||
let name = ident.name.as_str();
|
||||
if name.starts_with('_')
|
||||
&& !name.starts_with("__")
|
||||
&& let definition_span = cx.tcx.hir().span(definition_hir_id)
|
||||
&& !definition_span.from_expansion()
|
||||
&& !fulfill_or_allowed(cx, USED_UNDERSCORE_BINDING, [expr.hir_id, definition_hir_id])
|
||||
{
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
USED_UNDERSCORE_BINDING,
|
||||
expr.span,
|
||||
"used underscore-prefixed binding".to_string(),
|
||||
|diag| {
|
||||
diag.span_note(definition_span, "binding is defined here".to_string());
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
|
||||
use clippy_utils::macros::root_macro_call_first_node;
|
||||
use clippy_utils::{get_parent_expr, path_to_local, path_to_local_id};
|
||||
use rustc_hir::intravisit::{walk_expr, Visitor};
|
||||
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, LetStmt, Node, Stmt, StmtKind};
|
||||
|
|
@ -134,6 +135,11 @@ impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
|
|||
}
|
||||
|
||||
fn report_diverging_sub_expr(&mut self, e: &Expr<'_>) {
|
||||
if let Some(macro_call) = root_macro_call_first_node(self.cx, e) {
|
||||
if self.cx.tcx.item_name(macro_call.def_id).as_str() == "todo" {
|
||||
return;
|
||||
}
|
||||
}
|
||||
span_lint(self.cx, DIVERGING_SUB_EXPRESSION, e.span, "sub-expression diverges");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -96,10 +96,6 @@ impl<'a, 'tcx> Visitor<'tcx> for MutArgVisitor<'a, 'tcx> {
|
|||
self.found = true;
|
||||
return;
|
||||
},
|
||||
ExprKind::If(..) => {
|
||||
self.found = true;
|
||||
return;
|
||||
},
|
||||
ExprKind::Path(_) => {
|
||||
if let Some(adj) = self.cx.typeck_results().adjustments().get(expr.hir_id) {
|
||||
if adj
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ declare_clippy_lint! {
|
|||
///
|
||||
/// // or choose alternative bounds for `T` so that it can be unsized
|
||||
/// ```
|
||||
#[clippy::version = "1.79.0"]
|
||||
#[clippy::version = "1.81.0"]
|
||||
pub NEEDLESS_MAYBE_SIZED,
|
||||
suspicious,
|
||||
"a `?Sized` bound that is unusable due to a `Sized` requirement"
|
||||
|
|
|
|||
|
|
@ -129,7 +129,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue {
|
|||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
// Collect moved variables and spans which will need dereferencings from the
|
||||
// Collect moved variables and spans which will need dereferencing from the
|
||||
// function body.
|
||||
let MovedVariablesCtxt { moved_vars } = {
|
||||
let mut ctx = MovedVariablesCtxt::default();
|
||||
|
|
@ -148,12 +148,13 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue {
|
|||
return;
|
||||
}
|
||||
|
||||
// Ignore `self`s.
|
||||
if idx == 0 {
|
||||
if let PatKind::Binding(.., ident, _) = arg.pat.kind {
|
||||
if ident.name == kw::SelfLower {
|
||||
continue;
|
||||
}
|
||||
// Ignore `self`s and params whose variable name starts with an underscore
|
||||
if let PatKind::Binding(.., ident, _) = arg.pat.kind {
|
||||
if idx == 0 && ident.name == kw::SelfLower {
|
||||
continue;
|
||||
}
|
||||
if ident.name.as_str().starts_with('_') {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
use clippy_config::Conf;
|
||||
use clippy_utils::diagnostics::span_lint;
|
||||
use clippy_utils::is_in_test;
|
||||
use clippy_utils::macros::{is_panic, root_macro_call_first_node};
|
||||
use rustc_hir::Expr;
|
||||
use clippy_utils::{is_in_test, match_def_path, paths};
|
||||
use rustc_hir::def::{DefKind, Res};
|
||||
use rustc_hir::{Expr, ExprKind, QPath};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::impl_lint_pass;
|
||||
|
||||
|
|
@ -95,10 +96,49 @@ impl_lint_pass!(PanicUnimplemented => [UNIMPLEMENTED, UNREACHABLE, TODO, PANIC])
|
|||
|
||||
impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
||||
let Some(macro_call) = root_macro_call_first_node(cx, expr) else {
|
||||
return;
|
||||
};
|
||||
if is_panic(cx, macro_call.def_id) {
|
||||
if let Some(macro_call) = root_macro_call_first_node(cx, expr) {
|
||||
if is_panic(cx, macro_call.def_id) {
|
||||
if cx.tcx.hir().is_inside_const_context(expr.hir_id)
|
||||
|| self.allow_panic_in_tests && is_in_test(cx.tcx, expr.hir_id)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
span_lint(
|
||||
cx,
|
||||
PANIC,
|
||||
macro_call.span,
|
||||
"`panic` should not be present in production code",
|
||||
);
|
||||
return;
|
||||
}
|
||||
match cx.tcx.item_name(macro_call.def_id).as_str() {
|
||||
"todo" => {
|
||||
span_lint(
|
||||
cx,
|
||||
TODO,
|
||||
macro_call.span,
|
||||
"`todo` should not be present in production code",
|
||||
);
|
||||
},
|
||||
"unimplemented" => {
|
||||
span_lint(
|
||||
cx,
|
||||
UNIMPLEMENTED,
|
||||
macro_call.span,
|
||||
"`unimplemented` should not be present in production code",
|
||||
);
|
||||
},
|
||||
"unreachable" => {
|
||||
span_lint(cx, UNREACHABLE, macro_call.span, "usage of the `unreachable!` macro");
|
||||
},
|
||||
_ => {},
|
||||
}
|
||||
} else if let ExprKind::Call(func, [_]) = expr.kind
|
||||
&& let ExprKind::Path(QPath::Resolved(None, expr_path)) = func.kind
|
||||
&& let Res::Def(DefKind::Fn, def_id) = expr_path.res
|
||||
&& match_def_path(cx, def_id, &paths::PANIC_ANY)
|
||||
{
|
||||
if cx.tcx.hir().is_inside_const_context(expr.hir_id)
|
||||
|| self.allow_panic_in_tests && is_in_test(cx.tcx, expr.hir_id)
|
||||
{
|
||||
|
|
@ -108,32 +148,10 @@ impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented {
|
|||
span_lint(
|
||||
cx,
|
||||
PANIC,
|
||||
macro_call.span,
|
||||
"`panic` should not be present in production code",
|
||||
expr.span,
|
||||
"`panic_any` should not be present in production code",
|
||||
);
|
||||
return;
|
||||
}
|
||||
match cx.tcx.item_name(macro_call.def_id).as_str() {
|
||||
"todo" => {
|
||||
span_lint(
|
||||
cx,
|
||||
TODO,
|
||||
macro_call.span,
|
||||
"`todo` should not be present in production code",
|
||||
);
|
||||
},
|
||||
"unimplemented" => {
|
||||
span_lint(
|
||||
cx,
|
||||
UNIMPLEMENTED,
|
||||
macro_call.span,
|
||||
"`unimplemented` should not be present in production code",
|
||||
);
|
||||
},
|
||||
"unreachable" => {
|
||||
span_lint(cx, UNREACHABLE, macro_call.span, "usage of the `unreachable!` macro");
|
||||
},
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
88
clippy_lints/src/pointers_in_nomem_asm_block.rs
Normal file
88
clippy_lints/src/pointers_in_nomem_asm_block.rs
Normal file
|
|
@ -0,0 +1,88 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_then;
|
||||
use rustc_ast::InlineAsmOptions;
|
||||
use rustc_hir::{Expr, ExprKind, InlineAsm, InlineAsmOperand};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::declare_lint_pass;
|
||||
use rustc_span::Span;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks if any pointer is being passed to an asm! block with `nomem` option.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// `nomem` forbids any reads or writes to memory and passing a pointer suggests
|
||||
/// that either of those will happen.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```no_run
|
||||
/// fn f(p: *mut u32) {
|
||||
/// unsafe { core::arch::asm!("mov [{p}], 42", p = in(reg) p, options(nomem, nostack)); }
|
||||
/// }
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```no_run
|
||||
/// fn f(p: *mut u32) {
|
||||
/// unsafe { core::arch::asm!("mov [{p}], 42", p = in(reg) p, options(nostack)); }
|
||||
/// }
|
||||
/// ```
|
||||
#[clippy::version = "1.81.0"]
|
||||
pub POINTERS_IN_NOMEM_ASM_BLOCK,
|
||||
suspicious,
|
||||
"pointers in nomem asm block"
|
||||
}
|
||||
|
||||
declare_lint_pass!(PointersInNomemAsmBlock => [POINTERS_IN_NOMEM_ASM_BLOCK]);
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for PointersInNomemAsmBlock {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
|
||||
if let ExprKind::InlineAsm(asm) = &expr.kind {
|
||||
check_asm(cx, asm);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_asm(cx: &LateContext<'_>, asm: &InlineAsm<'_>) {
|
||||
if !asm.options.contains(InlineAsmOptions::NOMEM) {
|
||||
return;
|
||||
}
|
||||
|
||||
let spans = asm
|
||||
.operands
|
||||
.iter()
|
||||
.filter(|(op, _span)| has_in_operand_pointer(cx, op))
|
||||
.map(|(_op, span)| *span)
|
||||
.collect::<Vec<Span>>();
|
||||
|
||||
if spans.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
POINTERS_IN_NOMEM_ASM_BLOCK,
|
||||
spans,
|
||||
"passing pointers to nomem asm block",
|
||||
additional_notes,
|
||||
);
|
||||
}
|
||||
|
||||
fn has_in_operand_pointer(cx: &LateContext<'_>, asm_op: &InlineAsmOperand<'_>) -> bool {
|
||||
let asm_in_expr = match asm_op {
|
||||
InlineAsmOperand::SymStatic { .. }
|
||||
| InlineAsmOperand::Out { .. }
|
||||
| InlineAsmOperand::Const { .. }
|
||||
| InlineAsmOperand::SymFn { .. }
|
||||
| InlineAsmOperand::Label { .. } => return false,
|
||||
InlineAsmOperand::SplitInOut { in_expr, .. } => in_expr,
|
||||
InlineAsmOperand::In { expr, .. } | InlineAsmOperand::InOut { expr, .. } => expr,
|
||||
};
|
||||
|
||||
// This checks for raw ptrs, refs and function pointers - the last one
|
||||
// also technically counts as reading memory.
|
||||
cx.typeck_results().expr_ty(asm_in_expr).is_any_ptr()
|
||||
}
|
||||
|
||||
fn additional_notes(diag: &mut rustc_errors::Diag<'_, ()>) {
|
||||
diag.note("`nomem` means that no memory write or read happens inside the asm! block");
|
||||
diag.note("if this is intentional and no pointers are read or written to, consider allowing the lint");
|
||||
}
|
||||
|
|
@ -42,7 +42,7 @@ declare_clippy_lint! {
|
|||
/// println!("inserted {value:?}");
|
||||
/// }
|
||||
/// ```
|
||||
#[clippy::version = "1.80.0"]
|
||||
#[clippy::version = "1.81.0"]
|
||||
pub SET_CONTAINS_OR_INSERT,
|
||||
nursery,
|
||||
"call to `<set>::contains` followed by `<set>::insert`"
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ declare_clippy_lint! {
|
|||
/// ```no_run
|
||||
/// "Hello World!".trim_end_matches(['.', ',', '!', '?']);
|
||||
/// ```
|
||||
#[clippy::version = "1.80.0"]
|
||||
#[clippy::version = "1.81.0"]
|
||||
pub MANUAL_PATTERN_CHAR_COMPARISON,
|
||||
style,
|
||||
"manual char comparison in string patterns"
|
||||
|
|
|
|||
334
clippy_lints/src/zombie_processes.rs
Normal file
334
clippy_lints/src/zombie_processes.rs
Normal file
|
|
@ -0,0 +1,334 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_then;
|
||||
use clippy_utils::{fn_def_id, get_enclosing_block, match_any_def_paths, match_def_path, path_to_local_id, paths};
|
||||
use rustc_ast::Mutability;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::intravisit::{walk_block, walk_expr, walk_local, Visitor};
|
||||
use rustc_hir::{Expr, ExprKind, HirId, LetStmt, Node, PatKind, Stmt, StmtKind};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::declare_lint_pass;
|
||||
use rustc_span::sym;
|
||||
use std::ops::ControlFlow;
|
||||
use ControlFlow::{Break, Continue};
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Looks for code that spawns a process but never calls `wait()` on the child.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// As explained in the [standard library documentation](https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning),
|
||||
/// calling `wait()` is necessary on Unix platforms to properly release all OS resources associated with the process.
|
||||
/// Not doing so will effectively leak process IDs and/or other limited global resources,
|
||||
/// which can eventually lead to resource exhaustion, so it's recommended to call `wait()` in long-running applications.
|
||||
/// Such processes are called "zombie processes".
|
||||
///
|
||||
/// ### Example
|
||||
/// ```rust
|
||||
/// use std::process::Command;
|
||||
///
|
||||
/// let _child = Command::new("ls").spawn().expect("failed to execute child");
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```rust
|
||||
/// use std::process::Command;
|
||||
///
|
||||
/// let mut child = Command::new("ls").spawn().expect("failed to execute child");
|
||||
/// child.wait().expect("failed to wait on child");
|
||||
/// ```
|
||||
#[clippy::version = "1.74.0"]
|
||||
pub ZOMBIE_PROCESSES,
|
||||
suspicious,
|
||||
"not waiting on a spawned child process"
|
||||
}
|
||||
declare_lint_pass!(ZombieProcesses => [ZOMBIE_PROCESSES]);
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for ZombieProcesses {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
|
||||
if let ExprKind::Call(..) | ExprKind::MethodCall(..) = expr.kind
|
||||
&& let Some(child_adt) = cx.typeck_results().expr_ty(expr).ty_adt_def()
|
||||
&& match_def_path(cx, child_adt.did(), &paths::CHILD)
|
||||
{
|
||||
match cx.tcx.parent_hir_node(expr.hir_id) {
|
||||
Node::LetStmt(local)
|
||||
if let PatKind::Binding(_, local_id, ..) = local.pat.kind
|
||||
&& let Some(enclosing_block) = get_enclosing_block(cx, expr.hir_id) =>
|
||||
{
|
||||
let mut vis = WaitFinder::WalkUpTo(cx, local_id);
|
||||
|
||||
// If it does have a `wait()` call, we're done. Don't lint.
|
||||
if let Break(BreakReason::WaitFound) = walk_block(&mut vis, enclosing_block) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Don't emit a suggestion since the binding is used later
|
||||
check(cx, expr, false);
|
||||
},
|
||||
Node::LetStmt(&LetStmt { pat, .. }) if let PatKind::Wild = pat.kind => {
|
||||
// `let _ = child;`, also dropped immediately without `wait()`ing
|
||||
check(cx, expr, true);
|
||||
},
|
||||
Node::Stmt(&Stmt {
|
||||
kind: StmtKind::Semi(_),
|
||||
..
|
||||
}) => {
|
||||
// Immediately dropped. E.g. `std::process::Command::new("echo").spawn().unwrap();`
|
||||
check(cx, expr, true);
|
||||
},
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
enum BreakReason {
|
||||
WaitFound,
|
||||
EarlyReturn,
|
||||
}
|
||||
|
||||
/// A visitor responsible for finding a `wait()` call on a local variable.
|
||||
///
|
||||
/// Conditional `wait()` calls are assumed to not call wait:
|
||||
/// ```ignore
|
||||
/// let mut c = Command::new("").spawn().unwrap();
|
||||
/// if true {
|
||||
/// c.wait();
|
||||
/// }
|
||||
/// ```
|
||||
///
|
||||
/// Note that this visitor does NOT explicitly look for `wait()` calls directly, but rather does the
|
||||
/// inverse -- checking if all uses of the local are either:
|
||||
/// - a field access (`child.{stderr,stdin,stdout}`)
|
||||
/// - calling `id` or `kill`
|
||||
/// - no use at all (e.g. `let _x = child;`)
|
||||
/// - taking a shared reference (`&`), `wait()` can't go through that
|
||||
///
|
||||
/// None of these are sufficient to prevent zombie processes.
|
||||
/// Doing it like this means more FNs, but FNs are better than FPs.
|
||||
///
|
||||
/// `return` expressions, conditional or not, short-circuit the visitor because
|
||||
/// if a `wait()` call hadn't been found at that point, it might never reach one at a later point:
|
||||
/// ```ignore
|
||||
/// let mut c = Command::new("").spawn().unwrap();
|
||||
/// if true {
|
||||
/// return; // Break(BreakReason::EarlyReturn)
|
||||
/// }
|
||||
/// c.wait(); // this might not be reachable
|
||||
/// ```
|
||||
enum WaitFinder<'a, 'tcx> {
|
||||
WalkUpTo(&'a LateContext<'tcx>, HirId),
|
||||
Found(&'a LateContext<'tcx>, HirId),
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> Visitor<'tcx> for WaitFinder<'a, 'tcx> {
|
||||
type Result = ControlFlow<BreakReason>;
|
||||
|
||||
fn visit_local(&mut self, l: &'tcx LetStmt<'tcx>) -> Self::Result {
|
||||
if let Self::WalkUpTo(cx, local_id) = *self
|
||||
&& let PatKind::Binding(_, pat_id, ..) = l.pat.kind
|
||||
&& local_id == pat_id
|
||||
{
|
||||
*self = Self::Found(cx, local_id);
|
||||
}
|
||||
|
||||
walk_local(self, l)
|
||||
}
|
||||
|
||||
fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) -> Self::Result {
|
||||
let Self::Found(cx, local_id) = *self else {
|
||||
return walk_expr(self, ex);
|
||||
};
|
||||
|
||||
if path_to_local_id(ex, local_id) {
|
||||
match cx.tcx.parent_hir_node(ex.hir_id) {
|
||||
Node::Stmt(Stmt {
|
||||
kind: StmtKind::Semi(_),
|
||||
..
|
||||
}) => {},
|
||||
Node::Expr(expr) if let ExprKind::Field(..) = expr.kind => {},
|
||||
Node::Expr(expr) if let ExprKind::AddrOf(_, Mutability::Not, _) = expr.kind => {},
|
||||
Node::Expr(expr)
|
||||
if let Some(fn_did) = fn_def_id(cx, expr)
|
||||
&& match_any_def_paths(cx, fn_did, &[&paths::CHILD_ID, &paths::CHILD_KILL]).is_some() => {},
|
||||
|
||||
// Conservatively assume that all other kinds of nodes call `.wait()` somehow.
|
||||
_ => return Break(BreakReason::WaitFound),
|
||||
}
|
||||
} else {
|
||||
match ex.kind {
|
||||
ExprKind::Ret(..) => return Break(BreakReason::EarlyReturn),
|
||||
ExprKind::If(cond, then, None) => {
|
||||
walk_expr(self, cond)?;
|
||||
|
||||
// A `wait()` call in an if expression with no else is not enough:
|
||||
//
|
||||
// let c = spawn();
|
||||
// if true {
|
||||
// c.wait();
|
||||
// }
|
||||
//
|
||||
// This might not call wait(). However, early returns are propagated,
|
||||
// because they might lead to a later wait() not being called.
|
||||
if let Break(BreakReason::EarlyReturn) = walk_expr(self, then) {
|
||||
return Break(BreakReason::EarlyReturn);
|
||||
}
|
||||
|
||||
return Continue(());
|
||||
},
|
||||
|
||||
ExprKind::If(cond, then, Some(else_)) => {
|
||||
walk_expr(self, cond)?;
|
||||
|
||||
#[expect(clippy::unnested_or_patterns)]
|
||||
match (walk_expr(self, then), walk_expr(self, else_)) {
|
||||
(Continue(()), Continue(()))
|
||||
|
||||
// `wait()` in one branch but nothing in the other does not count
|
||||
| (Continue(()), Break(BreakReason::WaitFound))
|
||||
| (Break(BreakReason::WaitFound), Continue(())) => {},
|
||||
|
||||
// `wait()` in both branches is ok
|
||||
(Break(BreakReason::WaitFound), Break(BreakReason::WaitFound)) => {
|
||||
return Break(BreakReason::WaitFound);
|
||||
},
|
||||
|
||||
// Propagate early returns in either branch
|
||||
(Break(BreakReason::EarlyReturn), _) | (_, Break(BreakReason::EarlyReturn)) => {
|
||||
return Break(BreakReason::EarlyReturn);
|
||||
},
|
||||
}
|
||||
|
||||
return Continue(());
|
||||
},
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
|
||||
walk_expr(self, ex)
|
||||
}
|
||||
}
|
||||
|
||||
/// This function has shared logic between the different kinds of nodes that can trigger the lint.
|
||||
///
|
||||
/// In particular, `let <binding> = <expr that spawns child>;` requires some custom additional logic
|
||||
/// such as checking that the binding is not used in certain ways, which isn't necessary for
|
||||
/// `let _ = <expr that spawns child>;`.
|
||||
///
|
||||
/// This checks if the program doesn't unconditionally exit after the spawn expression.
|
||||
fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, emit_suggestion: bool) {
|
||||
let Some(block) = get_enclosing_block(cx, spawn_expr.hir_id) else {
|
||||
return;
|
||||
};
|
||||
|
||||
let mut vis = ExitPointFinder {
|
||||
cx,
|
||||
state: ExitPointState::WalkUpTo(spawn_expr.hir_id),
|
||||
};
|
||||
if let Break(ExitCallFound) = vis.visit_block(block) {
|
||||
// Visitor found an unconditional `exit()` call, so don't lint.
|
||||
return;
|
||||
}
|
||||
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
ZOMBIE_PROCESSES,
|
||||
spawn_expr.span,
|
||||
"spawned process is never `wait()`ed on",
|
||||
|diag| {
|
||||
if emit_suggestion {
|
||||
diag.span_suggestion(
|
||||
spawn_expr.span.shrink_to_hi(),
|
||||
"try",
|
||||
".wait()",
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
} else {
|
||||
diag.note("consider calling `.wait()`");
|
||||
}
|
||||
|
||||
diag.note("not doing so might leave behind zombie processes")
|
||||
.note("see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning");
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
/// Checks if the given expression exits the process.
|
||||
fn is_exit_expression(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
|
||||
fn_def_id(cx, expr).is_some_and(|fn_did| {
|
||||
cx.tcx.is_diagnostic_item(sym::process_exit, fn_did) || match_def_path(cx, fn_did, &paths::ABORT)
|
||||
})
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
enum ExitPointState {
|
||||
/// Still walking up to the expression that initiated the visitor.
|
||||
WalkUpTo(HirId),
|
||||
/// We're inside of a control flow construct (e.g. `if`, `match`, `loop`)
|
||||
/// Within this, we shouldn't accept any `exit()` calls in here, but we can leave all of these
|
||||
/// constructs later and still continue looking for an `exit()` call afterwards. Example:
|
||||
/// ```ignore
|
||||
/// Command::new("").spawn().unwrap();
|
||||
///
|
||||
/// if true { // depth=1
|
||||
/// if true { // depth=2
|
||||
/// match () { // depth=3
|
||||
/// () => loop { // depth=4
|
||||
///
|
||||
/// std::process::exit();
|
||||
/// ^^^^^^^^^^^^^^^^^^^^^ conditional exit call, ignored
|
||||
///
|
||||
/// } // depth=3
|
||||
/// } // depth=2
|
||||
/// } // depth=1
|
||||
/// } // depth=0
|
||||
///
|
||||
/// std::process::exit();
|
||||
/// ^^^^^^^^^^^^^^^^^^^^^ this exit call is accepted because we're now unconditionally calling it
|
||||
/// ```
|
||||
/// We can only get into this state from `NoExit`.
|
||||
InControlFlow { depth: u32 },
|
||||
/// No exit call found yet, but looking for one.
|
||||
NoExit,
|
||||
}
|
||||
|
||||
fn expr_enters_control_flow(expr: &Expr<'_>) -> bool {
|
||||
matches!(expr.kind, ExprKind::If(..) | ExprKind::Match(..) | ExprKind::Loop(..))
|
||||
}
|
||||
|
||||
struct ExitPointFinder<'a, 'tcx> {
|
||||
state: ExitPointState,
|
||||
cx: &'a LateContext<'tcx>,
|
||||
}
|
||||
|
||||
struct ExitCallFound;
|
||||
|
||||
impl<'a, 'tcx> Visitor<'tcx> for ExitPointFinder<'a, 'tcx> {
|
||||
type Result = ControlFlow<ExitCallFound>;
|
||||
|
||||
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> Self::Result {
|
||||
match self.state {
|
||||
ExitPointState::WalkUpTo(id) if expr.hir_id == id => {
|
||||
self.state = ExitPointState::NoExit;
|
||||
walk_expr(self, expr)
|
||||
},
|
||||
ExitPointState::NoExit if expr_enters_control_flow(expr) => {
|
||||
self.state = ExitPointState::InControlFlow { depth: 1 };
|
||||
walk_expr(self, expr)?;
|
||||
if let ExitPointState::InControlFlow { .. } = self.state {
|
||||
self.state = ExitPointState::NoExit;
|
||||
}
|
||||
Continue(())
|
||||
},
|
||||
ExitPointState::NoExit if is_exit_expression(self.cx, expr) => Break(ExitCallFound),
|
||||
ExitPointState::InControlFlow { ref mut depth } if expr_enters_control_flow(expr) => {
|
||||
*depth += 1;
|
||||
walk_expr(self, expr)?;
|
||||
match self.state {
|
||||
ExitPointState::InControlFlow { depth: 1 } => self.state = ExitPointState::NoExit,
|
||||
ExitPointState::InControlFlow { ref mut depth } => *depth -= 1,
|
||||
_ => {},
|
||||
}
|
||||
Continue(())
|
||||
},
|
||||
_ => Continue(()),
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue