Skip to content

Commit b901ba7

Browse files
ada4aada4a
committed
fix(ref_option): don't lint in external and proc-macros
1 parent 25cf36a commit b901ba7

File tree

9 files changed

+230
-36
lines changed

9 files changed

+230
-36
lines changed

‎clippy_lints/src/functions/ref_option.rs‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
11
use crate::functions::REF_OPTION;
22
use clippy_utils::diagnostics::span_lint_and_then;
3-
use clippy_utils::is_trait_impl_item;
43
use clippy_utils::source::snippet;
54
use clippy_utils::ty::option_arg_ty;
5+
use clippy_utils::{is_from_proc_macro, is_trait_impl_item};
66
use rustc_errors::Applicability;
77
use rustc_hir::intravisit::FnKind;
88
use rustc_hir::{self as hir, FnDecl, HirId};
9-
use rustc_lint::LateContext;
9+
use rustc_lint::{LateContext, LintContext};
1010
use rustc_middle::ty::{self, Mutability, Ty};
1111
use rustc_span::Span;
1212
use rustc_span::def_id::LocalDefId;
1313

1414
fn check_ty<'a>(cx: &LateContext<'a>, param: &hir::Ty<'a>, param_ty: Ty<'a>, fixes: &mut Vec<(Span, String)>) {
15-
if let ty::Ref(_, opt_ty, Mutability::Not) = param_ty.kind()
15+
if !param.span.in_external_macro(cx.sess().source_map())
16+
&& let ty::Ref(_, opt_ty, Mutability::Not) = param_ty.kind()
1617
&& let Some(gen_ty) = option_arg_ty(cx, *opt_ty)
1718
&& !gen_ty.is_ref()
1819
// Need to gen the original spans, so first parsing mid, and hir parsing afterward
@@ -23,6 +24,7 @@ fn check_ty<'a>(cx: &LateContext<'a>, param: &hir::Ty<'a>, param_ty: Ty<'a>, fix
2324
args: [hir::GenericArg::Type(opt_ty)],
2425
..
2526
}) = last.args
27+
&& !is_from_proc_macro(cx, param)
2628
{
2729
let lifetime = snippet(cx, lifetime.ident.span, "..");
2830
fixes.push((
@@ -63,21 +65,24 @@ fn check_fn_sig<'a>(cx: &LateContext<'a>, decl: &FnDecl<'a>, span: Span, sig: ty
6365
#[allow(clippy::too_many_arguments)]
6466
pub(crate) fn check_fn<'a>(
6567
cx: &LateContext<'a>,
66-
kind: FnKind<'_>,
68+
kind: FnKind<'a>,
6769
decl: &FnDecl<'a>,
6870
span: Span,
6971
hir_id: HirId,
7072
def_id: LocalDefId,
71-
body: &hir::Body<'_>,
73+
body: &hir::Body<'a>,
7274
avoid_breaking_exported_api: bool,
7375
) {
7476
if avoid_breaking_exported_api && cx.effective_visibilities.is_exported(def_id) {
7577
return;
7678
}
79+
if span.in_external_macro(cx.sess().source_map()) {
80+
return;
81+
}
7782

7883
if let FnKind::Closure = kind {
7984
// Compute the span of the closure parameters + return type if set
80-
let span = if let hir::FnRetTy::Return(out_ty) = &decl.output {
85+
let inputs_output_span = if let hir::FnRetTy::Return(out_ty) = &decl.output {
8186
if decl.inputs.is_empty() {
8287
out_ty.span
8388
} else {
@@ -96,9 +101,18 @@ pub(crate) fn check_fn<'a>(
96101
};
97102
let sig = args.as_closure().sig().skip_binder();
98103

99-
check_fn_sig(cx, decl, span, sig);
104+
if is_from_proc_macro(cx, &(&kind, body, hir_id, span)) {
105+
return;
106+
}
107+
108+
check_fn_sig(cx, decl, inputs_output_span, sig);
100109
} else if !is_trait_impl_item(cx, hir_id) {
101110
let sig = cx.tcx.fn_sig(def_id).instantiate_identity().skip_binder();
111+
112+
if is_from_proc_macro(cx, &(&kind, body, hir_id, span)) {
113+
return;
114+
}
115+
102116
check_fn_sig(cx, decl, span, sig);
103117
}
104118
}
@@ -108,8 +122,10 @@ pub(super) fn check_trait_item<'a>(
108122
trait_item: &hir::TraitItem<'a>,
109123
avoid_breaking_exported_api: bool,
110124
) {
111-
if let hir::TraitItemKind::Fn(ref sig, _) = trait_item.kind
125+
if !trait_item.span.in_external_macro(cx.sess().source_map())
126+
&& let hir::TraitItemKind::Fn(ref sig, _) = trait_item.kind
112127
&& !(avoid_breaking_exported_api && cx.effective_visibilities.is_exported(trait_item.owner_id.def_id))
128+
&& !is_from_proc_macro(cx, trait_item)
113129
{
114130
let def_id = trait_item.owner_id.def_id;
115131
let ty_sig = cx.tcx.fn_sig(def_id).instantiate_identity().skip_binder();

‎tests/ui-toml/ref_option/ref_option.all.fixed‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
//@aux-build:../../ui/auxiliary/proc_macros.rs
12
//@revisions: private all
23
//@[private] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/ref_option/private
34
//@[all] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/ref_option/all
@@ -62,4 +63,52 @@ fn lambdas() {
6263
let x = |a: &Option<String>| -> &Option<String> { panic!() };
6364
}
6465

66+
pub mod external {
67+
proc_macros::external!(
68+
fn opt_u8(a: &Option<u8>) {}
69+
fn ret_u8<'a>(p: &'a str) -> &'a Option<u8> {
70+
panic!()
71+
}
72+
pub fn pub_opt_u8(a: &Option<u8>) {}
73+
74+
pub struct PubStruct;
75+
impl PubStruct {
76+
pub fn pub_opt_params(&self, a: &Option<()>) {}
77+
pub fn pub_opt_ret(&self) -> &Option<String> {
78+
panic!()
79+
}
80+
81+
fn private_opt_params(&self, a: &Option<()>) {}
82+
fn private_opt_ret(&self) -> &Option<String> {
83+
panic!()
84+
}
85+
}
86+
);
87+
}
88+
89+
pub mod proc_macros {
90+
proc_macros::with_span!(
91+
span
92+
93+
fn opt_u8(a: &Option<u8>) {}
94+
fn ret_u8<'a>(p: &'a str) -> &'a Option<u8> {
95+
panic!()
96+
}
97+
pub fn pub_opt_u8(a: &Option<u8>) {}
98+
99+
pub struct PubStruct;
100+
impl PubStruct {
101+
pub fn pub_opt_params(&self, a: &Option<()>) {}
102+
pub fn pub_opt_ret(&self) -> &Option<String> {
103+
panic!()
104+
}
105+
106+
fn private_opt_params(&self, a: &Option<()>) {}
107+
fn private_opt_ret(&self) -> &Option<String> {
108+
panic!()
109+
}
110+
}
111+
);
112+
}
113+
65114
fn main() {}

‎tests/ui-toml/ref_option/ref_option.all.stderr‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
2-
--> tests/ui-toml/ref_option/ref_option.rs:8:1
2+
--> tests/ui-toml/ref_option/ref_option.rs:9:1
33
|
44
LL | fn opt_u8(a: &Option<u8>) {}
55
| ^^^^^^^^^^^^^-----------^^^^
@@ -10,23 +10,23 @@ LL | fn opt_u8(a: &Option<u8>) {}
1010
= help: to override `-D warnings` add `#[allow(clippy::ref_option)]`
1111

1212
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
13-
--> tests/ui-toml/ref_option/ref_option.rs:10:1
13+
--> tests/ui-toml/ref_option/ref_option.rs:11:1
1414
|
1515
LL | fn opt_gen<T>(a: &Option<T>) {}
1616
| ^^^^^^^^^^^^^^^^^----------^^^^
1717
| |
1818
| help: change this to: `Option<&T>`
1919

2020
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
21-
--> tests/ui-toml/ref_option/ref_option.rs:12:1
21+
--> tests/ui-toml/ref_option/ref_option.rs:13:1
2222
|
2323
LL | fn opt_string(a: &std::option::Option<String>) {}
2424
| ^^^^^^^^^^^^^^^^^----------------------------^^^^
2525
| |
2626
| help: change this to: `std::option::Option<&String>`
2727

2828
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
29-
--> tests/ui-toml/ref_option/ref_option.rs:14:1
29+
--> tests/ui-toml/ref_option/ref_option.rs:15:1
3030
|
3131
LL | fn ret_u8<'a>(p: &'a str) -> &'a Option<u8> {
3232
| ^ -------------- help: change this to: `Option<&'a u8>`
@@ -38,7 +38,7 @@ LL | | }
3838
| |_^
3939

4040
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
41-
--> tests/ui-toml/ref_option/ref_option.rs:18:1
41+
--> tests/ui-toml/ref_option/ref_option.rs:19:1
4242
|
4343
LL | fn ret_u8_static() -> &'static Option<u8> {
4444
| ^ ------------------- help: change this to: `Option<&'static u8>`
@@ -50,7 +50,7 @@ LL | | }
5050
| |_^
5151

5252
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
53-
--> tests/ui-toml/ref_option/ref_option.rs:22:1
53+
--> tests/ui-toml/ref_option/ref_option.rs:23:1
5454
|
5555
LL | fn mult_string(a: &Option<String>, b: &Option<Vec<u8>>) {}
5656
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -62,7 +62,7 @@ LL + fn mult_string(a: Option<&String>, b: Option<&Vec<u8>>) {}
6262
|
6363

6464
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
65-
--> tests/ui-toml/ref_option/ref_option.rs:24:1
65+
--> tests/ui-toml/ref_option/ref_option.rs:25:1
6666
|
6767
LL | fn ret_box<'a>() -> &'a Option<Box<u8>> {
6868
| ^ ------------------- help: change this to: `Option<&'a Box<u8>>`
@@ -74,15 +74,15 @@ LL | | }
7474
| |_^
7575

7676
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
77-
--> tests/ui-toml/ref_option/ref_option.rs:29:1
77+
--> tests/ui-toml/ref_option/ref_option.rs:30:1
7878
|
7979
LL | pub fn pub_opt_string(a: &Option<String>) {}
8080
| ^^^^^^^^^^^^^^^^^^^^^^^^^---------------^^^^
8181
| |
8282
| help: change this to: `Option<&String>`
8383

8484
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
85-
--> tests/ui-toml/ref_option/ref_option.rs:31:1
85+
--> tests/ui-toml/ref_option/ref_option.rs:32:1
8686
|
8787
LL | pub fn pub_mult_string(a: &Option<String>, b: &Option<Vec<u8>>) {}
8888
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -94,15 +94,15 @@ LL + pub fn pub_mult_string(a: Option<&String>, b: Option<&Vec<u8>>) {}
9494
|
9595

9696
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
97-
--> tests/ui-toml/ref_option/ref_option.rs:37:5
97+
--> tests/ui-toml/ref_option/ref_option.rs:38:5
9898
|
9999
LL | pub fn pub_opt_params(&self, a: &Option<()>) {}
100100
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------^^^^
101101
| |
102102
| help: change this to: `Option<&()>`
103103

104104
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
105-
--> tests/ui-toml/ref_option/ref_option.rs:39:5
105+
--> tests/ui-toml/ref_option/ref_option.rs:40:5
106106
|
107107
LL | pub fn pub_opt_ret(&self) -> &Option<String> {
108108
| ^ --------------- help: change this to: `Option<&String>`
@@ -114,15 +114,15 @@ LL | | }
114114
| |_____^
115115

116116
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
117-
--> tests/ui-toml/ref_option/ref_option.rs:44:5
117+
--> tests/ui-toml/ref_option/ref_option.rs:45:5
118118
|
119119
LL | fn private_opt_params(&self, a: &Option<()>) {}
120120
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------^^^^
121121
| |
122122
| help: change this to: `Option<&()>`
123123

124124
error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>`
125-
--> tests/ui-toml/ref_option/ref_option.rs:46:5
125+
--> tests/ui-toml/ref_option/ref_option.rs:47:5
126126
|
127127
LL | fn private_opt_ret(&self) -> &Option<String> {
128128
| ^ --------------- help: change this to: `Option<&String>`

‎tests/ui-toml/ref_option/ref_option.private.fixed‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
//@aux-build:../../ui/auxiliary/proc_macros.rs
12
//@revisions: private all
23
//@[private] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/ref_option/private
34
//@[all] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/ref_option/all
@@ -62,4 +63,52 @@ fn lambdas() {
6263
let x = |a: &Option<String>| -> &Option<String> { panic!() };
6364
}
6465

66+
pub mod external {
67+
proc_macros::external!(
68+
fn opt_u8(a: &Option<u8>) {}
69+
fn ret_u8<'a>(p: &'a str) -> &'a Option<u8> {
70+
panic!()
71+
}
72+
pub fn pub_opt_u8(a: &Option<u8>) {}
73+
74+
pub struct PubStruct;
75+
impl PubStruct {
76+
pub fn pub_opt_params(&self, a: &Option<()>) {}
77+
pub fn pub_opt_ret(&self) -> &Option<String> {
78+
panic!()
79+
}
80+
81+
fn private_opt_params(&self, a: &Option<()>) {}
82+
fn private_opt_ret(&self) -> &Option<String> {
83+
panic!()
84+
}
85+
}
86+
);
87+
}
88+
89+
pub mod proc_macros {
90+
proc_macros::with_span!(
91+
span
92+
93+
fn opt_u8(a: &Option<u8>) {}
94+
fn ret_u8<'a>(p: &'a str) -> &'a Option<u8> {
95+
panic!()
96+
}
97+
pub fn pub_opt_u8(a: &Option<u8>) {}
98+
99+
pub struct PubStruct;
100+
impl PubStruct {
101+
pub fn pub_opt_params(&self, a: &Option<()>) {}
102+
pub fn pub_opt_ret(&self) -> &Option<String> {
103+
panic!()
104+
}
105+
106+
fn private_opt_params(&self, a: &Option<()>) {}
107+
fn private_opt_ret(&self) -> &Option<String> {
108+
panic!()
109+
}
110+
}
111+
);
112+
}
113+
65114
fn main() {}

0 commit comments

Comments
 (0)