Skip to content

Commit daeb6a1

Browse files
y21y21
authored
Misc changes (#14702)
This mainly fixes `with_leading_whitespace` not always adding the whitespace it can. changelog: None
2 parents 9ebfb84 + 77157f5 commit daeb6a1

19 files changed

+90
-55
lines changed

‎clippy_lints/src/cognitive_complexity.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ impl CognitiveComplexity {
104104
FnKind::Closure => {
105105
let header_span = body_span.with_hi(decl.output.span().lo());
106106
#[expect(clippy::range_plus_one)]
107-
if let Some(range) = header_span.map_range(cx, |src, range| {
107+
if let Some(range) = header_span.map_range(cx, |_, src, range| {
108108
let mut idxs = src.get(range.clone())?.match_indices('|');
109109
Some(range.start + idxs.next()?.0..range.start + idxs.next()?.0 + 1)
110110
}) {

‎clippy_lints/src/copies.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ fn lint_branches_sharing_code<'tcx>(
258258
let span = span.with_hi(last_block.span.hi());
259259
// Improve formatting if the inner block has indentation (i.e. normal Rust formatting)
260260
let span = span
261-
.map_range(cx, |src, range| {
261+
.map_range(cx, |_, src, range| {
262262
(range.start > 4 && src.get(range.start - 4..range.start)? == " ")
263263
.then_some(range.start - 4..range.end)
264264
})

‎clippy_lints/src/extra_unused_type_parameters.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ impl<'tcx> LateLintPass<'tcx> for ExtraUnusedTypeParameters {
273273
// Only lint on inherent methods, not trait methods.
274274
if let ImplItemKind::Fn(.., body_id) = item.kind
275275
&& !item.generics.params.is_empty()
276-
&& trait_ref_of_method(cx, item.owner_id.def_id).is_none()
276+
&& trait_ref_of_method(cx, item.owner_id).is_none()
277277
&& !is_empty_body(cx, body_id)
278278
&& (!self.avoid_breaking_exported_api || !cx.effective_visibilities.is_exported(item.owner_id.def_id))
279279
&& !item.span.in_external_macro(cx.sess().source_map())

‎clippy_lints/src/functions/must_use.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Imp
5555
let attr = cx.tcx.get_attr(item.owner_id, sym::must_use);
5656
if let Some(attr) = attr {
5757
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, attrs, sig);
58-
} else if is_public && !is_proc_macro(attrs) && trait_ref_of_method(cx, item.owner_id.def_id).is_none() {
58+
} else if is_public && !is_proc_macro(attrs) && trait_ref_of_method(cx, item.owner_id).is_none() {
5959
check_must_use_candidate(
6060
cx,
6161
sig.decl,

‎clippy_lints/src/functions/result.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pub(super) fn check_impl_item<'tcx>(
5555
// Don't lint if method is a trait's implementation, we can't do anything about those
5656
if let hir::ImplItemKind::Fn(ref sig, _) = item.kind
5757
&& let Some((hir_ty, err_ty)) = result_err_ty(cx, sig.decl, item.owner_id.def_id, item.span)
58-
&& trait_ref_of_method(cx, item.owner_id.def_id).is_none()
58+
&& trait_ref_of_method(cx, item.owner_id).is_none()
5959
{
6060
if cx.effective_visibilities.is_exported(item.owner_id.def_id) {
6161
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());

‎clippy_lints/src/implicit_hasher.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher {
119119
}
120120

121121
let generics_suggestion_span = impl_.generics.span.substitute_dummy({
122-
let range = (item.span.lo()..target.span().lo()).map_range(cx, |src, range| {
122+
let range = (item.span.lo()..target.span().lo()).map_range(cx, |_, src, range| {
123123
Some(src.get(range.clone())?.find("impl")? + 4..range.end)
124124
});
125125
if let Some(range) = range {
@@ -165,11 +165,12 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher {
165165
continue;
166166
}
167167
let generics_suggestion_span = generics.span.substitute_dummy({
168-
let range = (item.span.lo()..body.params[0].pat.span.lo()).map_range(cx, |src, range| {
169-
let (pre, post) = src.get(range.clone())?.split_once("fn")?;
170-
let pos = post.find('(')? + pre.len() + 2;
171-
Some(pos..pos)
172-
});
168+
let range =
169+
(item.span.lo()..body.params[0].pat.span.lo()).map_range(cx, |_, src, range| {
170+
let (pre, post) = src.get(range.clone())?.split_once("fn")?;
171+
let pos = post.find('(')? + pre.len() + 2;
172+
Some(pos..pos)
173+
});
173174
if let Some(range) = range {
174175
range.with_ctxt(item.span.ctxt())
175176
} else {

‎clippy_lints/src/inherent_to_string.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ impl<'tcx> LateLintPass<'tcx> for InherentToString {
106106
// Check if return type is String
107107
&& is_type_lang_item(cx, return_ty(cx, impl_item.owner_id), LangItem::String)
108108
// Filters instances of to_string which are required by a trait
109-
&& trait_ref_of_method(cx, impl_item.owner_id.def_id).is_none()
109+
&& trait_ref_of_method(cx, impl_item.owner_id).is_none()
110110
{
111111
show_lint(cx, impl_item);
112112
}

‎clippy_lints/src/lifetimes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ impl<'tcx> LateLintPass<'tcx> for Lifetimes {
159159

160160
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
161161
if let ImplItemKind::Fn(ref sig, id) = item.kind {
162-
let report_extra_lifetimes = trait_ref_of_method(cx, item.owner_id.def_id).is_none();
162+
let report_extra_lifetimes = trait_ref_of_method(cx, item.owner_id).is_none();
163163
check_fn_inner(
164164
cx,
165165
sig,

‎clippy_lints/src/methods/manual_inspect.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ pub(crate) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &Expr<'_>, name:
101101
UseKind::Return(s) => edits.push((s.with_leading_whitespace(cx).with_ctxt(s.ctxt()), String::new())),
102102
UseKind::Borrowed(s) => {
103103
#[expect(clippy::range_plus_one)]
104-
let range = s.map_range(cx, |src, range| {
104+
let range = s.map_range(cx, |_, src, range| {
105105
let src = src.get(range.clone())?;
106106
let trimmed = src.trim_start_matches([' ', '\t', '\n', '\r', '(']);
107107
trimmed.starts_with('&').then(|| {

‎clippy_lints/src/missing_const_for_fn.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use rustc_abi::ExternAbi;
77
use rustc_errors::Applicability;
88
use rustc_hir::def_id::CRATE_DEF_ID;
99
use rustc_hir::intravisit::FnKind;
10-
use rustc_hir::{self as hir, Body, Constness, FnDecl, GenericParamKind};
10+
use rustc_hir::{self as hir, Body, Constness, FnDecl, GenericParamKind, OwnerId};
1111
use rustc_lint::{LateContext, LateLintPass};
1212
use rustc_middle::ty;
1313
use rustc_session::impl_lint_pass;
@@ -125,7 +125,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn {
125125
}
126126
},
127127
FnKind::Method(_, sig, ..) => {
128-
if already_const(sig.header) || trait_ref_of_method(cx, def_id).is_some() {
128+
if already_const(sig.header) || trait_ref_of_method(cx, OwnerId { def_id }).is_some() {
129129
return;
130130
}
131131
},

‎clippy_lints/src/mut_key.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for MutableKeyType<'tcx> {
8383

8484
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'tcx>) {
8585
if let hir::ImplItemKind::Fn(ref sig, ..) = item.kind
86-
&& trait_ref_of_method(cx, item.owner_id.def_id).is_none()
86+
&& trait_ref_of_method(cx, item.owner_id).is_none()
8787
{
8888
self.check_sig(cx, item.owner_id.def_id, sig.decl);
8989
}

‎clippy_lints/src/operators/assign_op_pattern.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ pub(super) fn check<'tcx>(
2626
let rty = cx.typeck_results().expr_ty(rhs);
2727
if let Some((_, lang_item)) = binop_traits(op.node)
2828
&& let Some(trait_id) = cx.tcx.lang_items().get(lang_item)
29-
&& let parent_fn = cx.tcx.hir_get_parent_item(e.hir_id).def_id
29+
&& let parent_fn = cx.tcx.hir_get_parent_item(e.hir_id)
3030
&& trait_ref_of_method(cx, parent_fn).is_none_or(|t| t.path.res.def_id() != trait_id)
3131
&& implements_trait(cx, ty, trait_id, &[rty.into()])
3232
{

‎clippy_lints/src/suspicious_trait_impl.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ fn check_expr_inner<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, bin
8080
&& let hir::Node::ImplItem(impl_item) = cx.tcx.hir_node_by_def_id(parent_fn)
8181
&& let hir::ImplItemKind::Fn(_, body_id) = impl_item.kind
8282
&& let body = cx.tcx.hir_body(body_id)
83-
&& let parent_fn = cx.tcx.hir_get_parent_item(expr.hir_id).def_id
83+
&& let parent_fn = cx.tcx.hir_get_parent_item(expr.hir_id)
8484
&& let Some(trait_ref) = trait_ref_of_method(cx, parent_fn)
8585
&& let trait_id = trait_ref.path.res.def_id()
8686
&& ![binop_trait_id, op_assign_trait_id].contains(&trait_id)

‎clippy_utils/src/lib.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -523,12 +523,8 @@ pub fn path_def_id<'tcx>(cx: &LateContext<'_>, maybe_path: &impl MaybePath<'tcx>
523523
/// }
524524
/// }
525525
/// ```
526-
pub fn trait_ref_of_method<'tcx>(cx: &LateContext<'tcx>, def_id: LocalDefId) -> Option<&'tcx TraitRef<'tcx>> {
527-
// Get the implemented trait for the current function
528-
let hir_id = cx.tcx.local_def_id_to_hir_id(def_id);
529-
let parent_impl = cx.tcx.hir_get_parent_item(hir_id);
530-
if parent_impl != hir::CRATE_OWNER_ID
531-
&& let Node::Item(item) = cx.tcx.hir_node_by_def_id(parent_impl.def_id)
526+
pub fn trait_ref_of_method<'tcx>(cx: &LateContext<'tcx>, owner: OwnerId) -> Option<&'tcx TraitRef<'tcx>> {
527+
if let Node::Item(item) = cx.tcx.hir_node(cx.tcx.hir_owner_parent(owner))
532528
&& let ItemKind::Impl(impl_) = &item.kind
533529
{
534530
return impl_.of_trait.as_ref();

‎clippy_utils/src/source.rs

Lines changed: 62 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,14 @@ use std::sync::Arc;
77
use rustc_ast::{LitKind, StrStyle};
88
use rustc_errors::Applicability;
99
use rustc_hir::{BlockCheckMode, Expr, ExprKind, UnsafeSource};
10+
use rustc_lexer::{LiteralKind, TokenKind, tokenize};
1011
use rustc_lint::{EarlyContext, LateContext};
1112
use rustc_middle::ty::TyCtxt;
1213
use rustc_session::Session;
1314
use rustc_span::source_map::{SourceMap, original_sp};
1415
use rustc_span::{
15-
BytePos, DUMMY_SP, FileNameDisplayPreference, Pos, SourceFile, SourceFileAndLine, Span, SpanData, SyntaxContext,
16-
hygiene,
16+
BytePos, DUMMY_SP, FileNameDisplayPreference, Pos, RelativeBytePos, SourceFile, SourceFileAndLine, Span, SpanData,
17+
SyntaxContext, hygiene,
1718
};
1819
use std::borrow::Cow;
1920
use std::fmt;
@@ -137,25 +138,25 @@ pub trait SpanRangeExt: SpanRange {
137138
fn map_range(
138139
self,
139140
cx: &impl HasSession,
140-
f: impl for<'a> FnOnce(&'a str, Range<usize>) -> Option<Range<usize>>,
141+
f: impl for<'a> FnOnce(&'a SourceFile, &'a str, Range<usize>) -> Option<Range<usize>>,
141142
) -> Option<Range<BytePos>> {
142143
map_range(cx.sess().source_map(), self.into_range(), f)
143144
}
144145

145146
#[allow(rustdoc::invalid_rust_codeblocks, reason = "The codeblock is intentionally broken")]
146-
/// Extends the range to include all preceding whitespace characters, unless there
147-
/// are non-whitespace characters left on the same line after `self`.
147+
/// Extends the range to include all preceding whitespace characters.
148+
///
149+
/// The range will not be expanded if it would cross a line boundary, the line the range would
150+
/// be extended to ends with a line comment and the text after the range contains a
151+
/// non-whitespace character on the same line. e.g.
148152
///
149-
/// This extra condition prevents a problem when removing the '}' in:
150153
/// ```ignore
151-
/// ( // There was an opening bracket after the parenthesis, which has been removed
152-
/// // This is a comment
153-
/// })
154+
/// ( // Some comment
155+
/// foo)
154156
/// ```
155-
/// Removing the whitespaces, including the linefeed, before the '}', would put the
156-
/// closing parenthesis at the end of the `// This is a comment` line, which would
157-
/// make it part of the comment as well. In this case, it is best to keep the span
158-
/// on the '}' alone.
157+
///
158+
/// When the range points to `foo`, suggesting to remove the range after it's been extended will
159+
/// cause the `)` to be placed inside the line comment as `( // Some comment)`.
159160
fn with_leading_whitespace(self, cx: &impl HasSession) -> Range<BytePos> {
160161
with_leading_whitespace(cx.sess().source_map(), self.into_range())
161162
}
@@ -254,11 +255,11 @@ fn with_source_text_and_range<T>(
254255
fn map_range(
255256
sm: &SourceMap,
256257
sp: Range<BytePos>,
257-
f: impl for<'a> FnOnce(&'a str, Range<usize>) -> Option<Range<usize>>,
258+
f: impl for<'a> FnOnce(&'a SourceFile, &'a str, Range<usize>) -> Option<Range<usize>>,
258259
) -> Option<Range<BytePos>> {
259260
if let Some(src) = get_source_range(sm, sp.clone())
260261
&& let Some(text) = &src.sf.src
261-
&& let Some(range) = f(text, src.range.clone())
262+
&& let Some(range) = f(&src.sf, text, src.range.clone())
262263
{
263264
debug_assert!(
264265
range.start <= text.len() && range.end <= text.len(),
@@ -275,20 +276,57 @@ fn map_range(
275276
}
276277
}
277278

279+
fn ends_with_line_comment_or_broken(text: &str) -> bool {
280+
let Some(last) = tokenize(text).last() else {
281+
return false;
282+
};
283+
match last.kind {
284+
// Will give the wrong result on text like `" // "` where the first quote ends a string
285+
// started earlier. The only workaround is to lex the whole file which we don't really want
286+
// to do.
287+
TokenKind::LineComment { .. } | TokenKind::BlockComment { terminated: false, .. } => true,
288+
TokenKind::Literal { kind, .. } => matches!(
289+
kind,
290+
LiteralKind::Byte { terminated: false }
291+
| LiteralKind::ByteStr { terminated: false }
292+
| LiteralKind::CStr { terminated: false }
293+
| LiteralKind::Char { terminated: false }
294+
| LiteralKind::RawByteStr { n_hashes: None }
295+
| LiteralKind::RawCStr { n_hashes: None }
296+
| LiteralKind::RawStr { n_hashes: None }
297+
),
298+
_ => false,
299+
}
300+
}
301+
302+
fn with_leading_whitespace_inner(lines: &[RelativeBytePos], src: &str, range: Range<usize>) -> Option<usize> {
303+
debug_assert!(lines.is_empty() || lines[0].to_u32() == 0);
304+
305+
let start = src.get(..range.start)?.trim_end();
306+
let next_line = lines.partition_point(|&pos| pos.to_usize() <= start.len());
307+
if let Some(line_end) = lines.get(next_line)
308+
&& line_end.to_usize() <= range.start
309+
&& let prev_start = lines.get(next_line - 1).map_or(0, |&x| x.to_usize())
310+
&& ends_with_line_comment_or_broken(&start[prev_start..])
311+
&& let next_line = lines.partition_point(|&pos| pos.to_usize() < range.end)
312+
&& let next_start = lines.get(next_line).map_or(src.len(), |&x| x.to_usize())
313+
&& tokenize(src.get(range.end..next_start)?).any(|t| !matches!(t.kind, TokenKind::Whitespace))
314+
{
315+
Some(range.start)
316+
} else {
317+
Some(start.len())
318+
}
319+
}
320+
278321
fn with_leading_whitespace(sm: &SourceMap, sp: Range<BytePos>) -> Range<BytePos> {
279-
map_range(sm, sp, |src, range| {
280-
let non_blank_after = src.len() - src.get(range.end..)?.trim_start().len();
281-
if src.get(range.end..non_blank_after)?.contains(['\r', '\n']) {
282-
Some(src.get(..range.start)?.trim_end().len()..range.end)
283-
} else {
284-
Some(range)
285-
}
322+
map_range(sm, sp.clone(), |sf, src, range| {
323+
Some(with_leading_whitespace_inner(sf.lines(), src, range.clone())?..range.end)
286324
})
287-
.unwrap()
325+
.unwrap_or(sp)
288326
}
289327

290328
fn trim_start(sm: &SourceMap, sp: Range<BytePos>) -> Range<BytePos> {
291-
map_range(sm, sp.clone(), |src, range| {
329+
map_range(sm, sp.clone(), |_, src, range| {
292330
let src = src.get(range.clone())?;
293331
Some(range.start + (src.len() - src.trim_start().len())..range.end)
294332
})

‎tests/ui-toml/collapsible_if/collapsible_if.fixed

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ fn main() {
1313
//~^^^^^^ collapsible_if
1414

1515
// The following tests check for the fix of https://github.com/rust-lang/rust-clippy/issues/798
16-
if x == "hello" // Inner comment
16+
if x == "hello" // Inner comment
1717
&& y == "world" {
1818
println!("Hello world!");
1919
}
@@ -26,7 +26,7 @@ fn main() {
2626
}
2727
//~^^^^^^ collapsible_if
2828

29-
if x == "hello" /* Inner comment */
29+
if x == "hello" /* Inner comment */
3030
&& y == "world" {
3131
println!("Hello world!");
3232
}

‎tests/ui-toml/collapsible_if/collapsible_if.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ LL | | }
3232
|
3333
help: collapse nested if block
3434
|
35-
LL ~ if x == "hello" // Inner comment
35+
LL ~ if x == "hello" // Inner comment
3636
LL ~ && y == "world" {
3737
LL | println!("Hello world!");
3838
LL ~ }
@@ -70,7 +70,7 @@ LL | | }
7070
|
7171
help: collapse nested if block
7272
|
73-
LL ~ if x == "hello" /* Inner comment */
73+
LL ~ if x == "hello" /* Inner comment */
7474
LL ~ && y == "world" {
7575
LL | println!("Hello world!");
7676
LL ~ }

‎tests/ui/manual_inspect.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ fn main() {
107107
let _ = || {
108108
let _x = x;
109109
};
110-
return ;
110+
return;
111111
}
112112
println!("test");
113113
});

‎tests/ui/manual_inspect.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ LL | if x.is_empty() {
9898
LL | let _ = || {
9999
LL ~ let _x = x;
100100
LL | };
101-
LL ~ return ;
101+
LL ~ return;
102102
LL | }
103103
LL ~ println!("test");
104104
|

0 commit comments

Comments
 (0)