Skip to content

Snippet rework #14724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jarcho wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Snippet rework #14724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions clippy_lints/src/cognitive_complexity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::res::MaybeDef as _;
use clippy_utils::source::{IntoSpan as _, SpanExt as _};
use clippy_utils::source::{FileRangeExt as _, SpanExt as _, StrExt as _};
use clippy_utils::visitors::for_each_expr_without_closures;
use clippy_utils::{LimitStack, get_async_fn_body, sym};
use core::ops::ControlFlow;
Expand Down Expand Up @@ -107,12 +107,12 @@ impl CognitiveComplexity {
let fn_span = match kind {
FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span,
FnKind::Closure => {
let header_span = body_span.with_hi(decl.output.span().lo());
if let Some(range) = header_span.map_range(cx, |_, src, range| {
let mut idxs = src.get(range.clone())?.match_indices('|');
Some(range.start + idxs.next()?.0..range.start + idxs.next()?.0 + 1)
if let Some(sp) = body_span.map_range(cx, |scx, range| {
range
.shrink_end_to(scx, decl.output.span().lo_ctxt())?
.map_range_text(scx, |src| src.find_bounded_inclusive('|'))
}) {
range.with_ctxt(header_span.ctxt())
sp
} else {
return;
}
Expand Down
106 changes: 42 additions & 64 deletions clippy_lints/src/collapsible_if.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::msrvs::Msrv;
use clippy_utils::source::{IntoSpan as _, SpanExt as _, snippet, snippet_block_with_applicability};
use clippy_utils::{can_use_if_let_chains, span_contains_cfg, span_contains_non_whitespace, sym, tokenize_with_text};
use clippy_utils::source::{FileRangeExt as _, SpanExt as _, StrExt as _, snippet, snippet_block_with_applicability};
use clippy_utils::{can_use_if_let_chains, span_contains_non_whitespace, sym, text_contains_cfg, tokenize_with_text};
use rustc_ast::{BinOpKind, MetaItemInner};
use rustc_errors::Applicability;
use rustc_hir::{Block, Expr, ExprKind, StmtKind};
Expand Down Expand Up @@ -99,6 +99,7 @@ impl CollapsibleIf {
&& !else_.span.from_expansion()
&& let ExprKind::If(else_if_cond, ..) = else_.kind
&& self.check_significant_tokens_and_expect_attrs(cx, else_block, else_, sym::collapsible_else_if)
&& let Some([_, inner_if_span, _]) = peel_parens(cx, else_.span)
{
span_lint_hir_and_then(
cx,
Expand All @@ -112,15 +113,15 @@ impl CollapsibleIf {
if self.lint_commented_code
&& let Some(else_keyword_span) = span_extract_keyword(cx, up_to_else, "else")
&& let Some(else_if_keyword_span) = span_extract_keyword(cx, else_before_if, "if")
&& let Some(else_keyword_span) =
else_keyword_span.map_range(cx, |scx, range| range.with_leading_whitespace(scx))
&& let Some([else_open_bracket, else_closing_bracket]) =
else_block.span.map_split_range(cx, |scx, range| {
range
.map_split_range_text(scx, |src| src.get_prefix_suffix('{', '}'))?
.try_map(|r| r.with_leading_whitespace(scx))
})
{
let else_keyword_span = else_keyword_span.with_leading_whitespace(cx).into_span();
let else_open_bracket = else_block.span.split_at(1).0.with_leading_whitespace(cx).into_span();
let else_closing_bracket = {
let end = else_block.span.shrink_to_hi();
end.with_lo(end.lo() - BytePos(1))
.with_leading_whitespace(cx)
.into_span()
};
let sugg = vec![
// Remove the outer else block `else`
(else_keyword_span, String::new()),
Expand All @@ -135,9 +136,6 @@ impl CollapsibleIf {
return;
}

// Peel off any parentheses.
let (_, else_block_span, _) = peel_parens(cx, else_.span);

// Prevent "elseif"
// Check that the "else" is followed by whitespace
// Note: We intentionally use char::is_whitespace instead of rustc_lexer::is_whitespace here to
Expand All @@ -152,7 +150,7 @@ impl CollapsibleIf {
if requires_space { " " } else { "" },
snippet_block_with_applicability(
cx,
else_block_span,
inner_if_span,
"..",
Some(else_block.span),
&mut applicability
Expand All @@ -171,11 +169,16 @@ impl CollapsibleIf {
&& self.eligible_condition(cx, check_inner)
&& expr.span.eq_ctxt(inner.span)
&& self.check_significant_tokens_and_expect_attrs(cx, then, inner, sym::collapsible_if)
&& let then_closing_bracket = {
let end = then.span.shrink_to_hi();
end.with_lo(end.lo() - BytePos(1))
}
&& !span_contains_cfg(cx, inner.span.between(then_closing_bracket))
&& let Some([then_open_bracket, then_closing_bracket]) = then.span.map_split_range(cx, |scx, range| {
range
.map_split_range_text(scx, |src| src.get_prefix_suffix('{', '}'))
.filter(|[_, end]| {
scx.get_text(scx.span_to_file_range(inner.span).end..end.start)
.is_some_and(|x| !text_contains_cfg(x))
})?
.try_map(|r| r.with_leading_whitespace(scx))
})
&& let Some([paren_start, inner_if_span, paren_end]) = peel_parens(cx, inner.span)
{
span_lint_hir_and_then(
cx,
Expand All @@ -184,9 +187,6 @@ impl CollapsibleIf {
expr.span,
"this `if` statement can be collapsed",
|diag| {
let then_open_bracket = then.span.split_at(1).0.with_leading_whitespace(cx).into_span();
let then_closing_bracket = then_closing_bracket.with_leading_whitespace(cx).into_span();
let (paren_start, inner_if_span, paren_end) = peel_parens(cx, inner.span);
let inner_if = inner_if_span.split_at(2).0;
let mut sugg = vec![
// Remove the outer then block `{`
Expand Down Expand Up @@ -320,51 +320,29 @@ pub(super) fn parens_around(expr: &Expr<'_>) -> Vec<(Span, String)> {
}

fn span_extract_keyword(cx: &LateContext<'_>, span: Span, keyword: &str) -> Option<Span> {
span.with_source_text(cx, |snippet| {
tokenize_with_text(snippet)
.filter(|(t, s, _)| matches!(t, TokenKind::Ident if *s == keyword))
.map(|(_, _, inner)| {
span.split_at(u32::try_from(inner.start).unwrap())
.1
.split_at(u32::try_from(inner.end - inner.start).unwrap())
.0
})
.next()
span.map_range(cx, |scx, range| {
range.map_range_text(scx, |s| {
tokenize_with_text(s)
.find(|&(t, s, _)| matches!(t, TokenKind::Ident if s == keyword))
.map(|(_, _, inner)| &s[inner.start..inner.end])
})
})
.flatten()
}

/// Peel the parentheses from an `if` expression, e.g. `((if true {} else {}))`.
pub(super) fn peel_parens(cx: &LateContext<'_>, mut span: Span) -> (Span, Span, Span) {
use crate::rustc_span::Pos as _;

let start = span.shrink_to_lo();
let end = span.shrink_to_hi();

span.with_source_text(cx, |snippet| {
if let Some((trim_start, _, trim_end)) = peel_parens_str(snippet) {
let mut data = span.data();
data.lo = data.lo + BytePos::from_usize(trim_start);
data.hi = data.hi - BytePos::from_usize(trim_end);
span = data.span();
}
});

(start.with_hi(span.lo()), span, end.with_lo(span.hi()))
}

fn peel_parens_str(snippet: &str) -> Option<(usize, &str, usize)> {
let trimmed = snippet.trim();
if !(trimmed.starts_with('(') && trimmed.ends_with(')')) {
return None;
}

let trim_start = (snippet.len() - snippet.trim_start().len()) + 1;
let trim_end = (snippet.len() - snippet.trim_end().len()) + 1;

let inner = snippet.get(trim_start..snippet.len() - trim_end)?;
Some(match peel_parens_str(inner) {
None => (trim_start, inner, trim_end),
Some((start, inner, end)) => (trim_start + start, inner, trim_end + end),
pub(super) fn peel_parens(cx: &LateContext<'_>, span: Span) -> Option<[Span; 3]> {
span.map_split_range(cx, |scx, range| {
range.map_split_range_text(scx, |s| {
let mut trimmed = s;
while let Some(s) = trimmed.strip_prefix('(')
&& let Some(s) = s.strip_suffix(')')
{
trimmed = s.trim();
}
let pos = trimmed.as_ptr().addr() - s.as_ptr().addr();
let (pre, s) = s.split_at(pos);
let (mid, post) = s.split_at(trimmed.len());
Some([pre, mid, post])
})
})
}
28 changes: 15 additions & 13 deletions clippy_lints/src/double_parens.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::{SpanExt as _, snippet_with_applicability, snippet_with_context};
use clippy_utils::source::{FileRangeExt as _, SpanExt as _, snippet_with_applicability, snippet_with_context};
use rustc_ast::ast::{Expr, ExprKind, MethodCall};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext as _};
Expand Down Expand Up @@ -102,18 +102,20 @@ impl EarlyLintPass for DoubleParens {

/// Check that the span does indeed look like `( (..) )`
fn check_source(cx: &EarlyContext<'_>, inner: &Expr) -> bool {
if let Some(sfr) = inner.span.get_source_range(cx)
// this is the same as `SourceFileRange::as_str`, but doesn't apply the range right away, because
// we're interested in the source code outside it
&& let Some(src) = sfr.sf.src.as_ref().map(|src| src.as_str())
&& let Some((start, outer_after_inner)) = src.split_at_checked(sfr.range.end)
&& let Some((outer_before_inner, inner)) = start.split_at_checked(sfr.range.start)
&& outer_before_inner.trim_end().ends_with('(')
&& inner.starts_with('(')
&& inner.ends_with(')')
&& outer_after_inner.trim_start().starts_with(')')
// Don't lint macro repetition patterns like `($($result),*)` where parens are necessary
&& !inner.trim_start_matches('(').trim_start().starts_with("$(")
if let Some((scx, range)) = inner.span.mk_edit_cx(cx)
&& let Some(text) = scx.get_text(range.clone())
&& let Some(text) = text.strip_prefix('(')
&& let Some(text) = text.strip_suffix(')')
// Don't lint pattern expansions. e.g. `(($($var)*))` may expand to `((foo))` or
// `((foo, bar))` depending on what `$var` expands to.
&& !text
.trim_start()
.strip_prefix('$')
.is_some_and(|text| text.trim_start().starts_with('('))
&& let Some(range) = range.with_trailing_whitespace(&scx)
&& let Some(range) = range.with_leading_whitespace(&scx)
&& let Some(range) = range.with_trailing_match(&scx, ')')
&& range.with_leading_match(&scx, '(').is_some()
Comment thread
Alexendoo marked this conversation as resolved.
{
true
} else {
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/empty_enums.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ impl LateLintPass<'_> for EmptyEnums {
&& def.variants.is_empty()
// Only suggest the `never_type` if the feature is enabled
&& cx.tcx.features().never_type()
&& !item.span.in_external_macro(cx.tcx.sess.source_map())
&& !span_contains_cfg(cx, item.span)
{
span_lint_and_help(
Expand Down
23 changes: 14 additions & 9 deletions clippy_lints/src/empty_with_brackets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use clippy_utils::attrs::span_contains_cfg;
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::source::SpanExt;
use clippy_utils::span_contains_non_whitespace;
use clippy_utils::source::{FileRangeExt as _, SpanExt as _};
use rustc_data_structures::fx::{FxIndexMap, IndexEntry};
use rustc_errors::Applicability;
use rustc_hir::def::DefKind::Ctor;
Expand All @@ -12,7 +11,7 @@ use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node, Pat, PatKind, Path, QPath,
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, TyCtxt};
use rustc_session::impl_lint_pass;
use rustc_span::{BytePos, Span};
use rustc_span::Span;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -190,13 +189,19 @@ impl LateLintPass<'_> for EmptyWithBrackets {
};

// Span of the parentheses in variant definition
let span = variant.span.with_lo(variant.ident.span.hi());
let span_inner = span
.with_lo(SpanExt::trim_start(span, cx).start + BytePos(1))
.with_hi(span.hi() - BytePos(1));
if span_contains_non_whitespace(cx, span_inner, false) {
let Some(span) = variant.span.map_range(cx, |scx, range| {
let range = range.shrink_start_to(scx, variant.ident.span.hi_ctxt())?;
matches!(
scx.get_text(range.clone())?
.trim_start()
.strip_prefix(['(', '{'])?
.trim_start(),
")" | "}",
)
.then_some(range)
}) else {
continue;
}
};
span_lint_hir_and_then(
cx,
EMPTY_ENUM_VARIANTS_WITH_BRACKETS,
Expand Down
21 changes: 10 additions & 11 deletions clippy_lints/src/ifs/branches_sharing_code.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::res::MaybeResPath as _;
use clippy_utils::source::{IntoSpan as _, SpanExt as _, first_line_of_span, indent_of, reindent_multiline, snippet};
use clippy_utils::source::{
FileRangeExt as _, SpanExt as _, first_line_of_span, indent_of, reindent_multiline, snippet,
};
use clippy_utils::ty::needs_ordered_drop;
use clippy_utils::visitors::for_each_expr_without_closures;
use clippy_utils::{
Expand Down Expand Up @@ -47,21 +49,18 @@ pub(super) fn check<'tcx>(
let suggestion = reindent_multiline(&suggestion, true, cond_indent);
(replace_span, suggestion)
});
let end_suggestion = res.end_span(last_block, sm).map(|span| {
let end_suggestion = res.end_span(last_block, sm).and_then(|span| {
let moved_snipped = reindent_multiline(&snippet(cx, span, "_"), true, None);
let indent = indent_of(cx, expr.span.shrink_to_hi());
let suggestion = "}\n".to_string() + &moved_snipped;
let suggestion = reindent_multiline(&suggestion, true, indent);

let span = span.with_hi(last_block.span.hi());
// Improve formatting if the inner block has indentation (i.e. normal Rust formatting)
let span = span
.map_range(cx, |_, src, range| {
(range.start > 4 && src.get(range.start - 4..range.start)? == " ")
.then_some(range.start - 4..range.end)
})
.map_or(span, |range| range.with_ctxt(span.ctxt()));
(span, suggestion.clone())
span.map_range(cx, |scx, range| {
let range = range.extend_end_to(scx, last_block.span.hi_ctxt())?;
// Improve formatting if the inner block has indentation (i.e. normal Rust formatting)
Some(range.clone().with_leading_match(scx, " ").unwrap_or(range))
})
.map(|sp| (sp, suggestion))
});

let (span, msg, end_span) = match (&start_suggestion, &end_suggestion) {
Expand Down
44 changes: 26 additions & 18 deletions clippy_lints/src/implicit_hasher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_session::declare_lint_pass;
use rustc_span::Span;

use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::{IntoSpan as _, SpanExt as _, snippet, snippet_with_context};
use clippy_utils::source::{FileRangeExt as _, SpanExt as _, snippet, snippet_with_context};
use clippy_utils::sym;

declare_clippy_lint! {
Expand Down Expand Up @@ -118,16 +118,19 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher {
return;
}

let generics_suggestion_span = impl_.generics.span.substitute_dummy({
let range = (item.span.lo()..target.span().lo()).map_range(cx, |_, src, range| {
Some(src.get(range.clone())?.find("impl")? + 4..range.end)
});
if let Some(range) = range {
range.with_ctxt(item.span.ctxt())
let generics_suggestion_span = if impl_.generics.span.is_dummy() {
if let Some(sp) = item.span.map_range(cx, |scx, range| {
range
.shrink_end_to(scx, target.span().lo_ctxt())?
.map_range_text(scx, |src| src.split_once("impl").map(|(_, x)| x))
}) {
sp
} else {
return;
}
});
} else {
impl_.generics.span
};
Comment thread
ada4a marked this conversation as resolved.

let mut ctr_vis = ImplicitHasherConstructorVisitor::new(cx, target);
for item in impl_.items.iter().map(|&item| cx.tcx.hir_impl_item(item)) {
Expand Down Expand Up @@ -164,19 +167,24 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher {
if generics.span.from_expansion() {
continue;
}
let generics_suggestion_span = generics.span.substitute_dummy({
let range =
(item.span.lo()..body.params[0].pat.span.lo()).map_range(cx, |_, src, range| {
let (pre, post) = src.get(range.clone())?.split_once("fn")?;
let pos = post.find('(')? + pre.len() + 2;
Some(pos..pos)
});
if let Some(range) = range {
range.with_ctxt(item.span.ctxt())

let generics_suggestion_span = if generics.span.is_dummy() {
if let Some(sp) = item.span.map_range(cx, |scx, range| {
range
.shrink_end_to(scx, body.params[0].pat.span.lo_ctxt())?
.map_range_text(scx, |src| {
src.split_once("fn")
.and_then(|(_, x)| x.split_once('('))
.map(|(_, x)| x)
})
}) {
sp
} else {
return;
}
});
} else {
generics.span
};
Comment thread
ada4a marked this conversation as resolved.

let mut ctr_vis = ImplicitHasherConstructorVisitor::new(cx, target);
ctr_vis.visit_body(body);
Expand Down
Loading