Skip to content

Commit faebef5

Browse files
CBSpeirCBSpeir
committed
Switch [extern_without_abi] to LateLintPass
Also: - Add test if inside external macro and/or proc macro - Update lint documentation - Emit `MachineApplicable` suggestion with lint
1 parent 3d62203 commit faebef5

File tree

5 files changed

+219
-33
lines changed

5 files changed

+219
-33
lines changed

‎clippy_lints/src/extern_without_abi.rs‎

Lines changed: 71 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,35 @@
1-
use clippy_utils::diagnostics::span_lint;
2-
use rustc_ast::visit::FnKind;
3-
use rustc_ast::{Extern, FnHeader, FnSig, ForeignMod, Item, ItemKind, NodeId};
4-
use rustc_lint::{EarlyContext, EarlyLintPass};
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::is_from_proc_macro;
3+
use clippy_utils::source::snippet;
4+
use rustc_errors::Applicability;
5+
use rustc_hir::def_id::LocalDefId;
6+
use rustc_hir::intravisit::FnKind;
7+
use rustc_hir::{Body, FnDecl, FnHeader, Item, ItemKind};
8+
use rustc_lint::{LateContext, LateLintPass, LintContext};
9+
use rustc_middle::lint::in_external_macro;
510
use rustc_session::declare_lint_pass;
611
use rustc_span::Span;
12+
use rustc_target::spec::abi::Abi;
713

814
const LINT_MSG: &str = "`extern` missing explicit ABI";
15+
const LINT_HELP_MSG: &str = "consider using";
16+
17+
const EXTERN: &str = "extern";
18+
const FN: &str = "fn";
19+
const OPEN_BRACE: &str = "{";
20+
const ABI: &str = "\"C\"";
921

1022
declare_clippy_lint! {
1123
/// ### What it does
1224
/// Checks for usage of `extern` without an explicit ABI.
1325
///
1426
/// ### Why is this bad?
15-
/// Explicitly declaring the ABI improves readability.
16-
//
27+
/// Explicitly declaring the ABI is the recommended convention. See:
28+
/// [Rust Style Guide - `extern` items](https://doc.rust-lang.org/nightly/style-guide/items.html#extern-items)
29+
///
30+
/// It's also enforced by `rustfmt` when the `force_explicit_abi` option is enabled. See:
31+
/// [Configuring Rustfmt](https://rust-lang.github.io/rustfmt/?version=master&search=#force_explicit_abi)
32+
///
1733
/// ### Example
1834
/// ```no_run
1935
/// extern fn foo() {}
@@ -38,24 +54,60 @@ declare_clippy_lint! {
3854

3955
declare_lint_pass!(ExternWithoutAbi => [EXTERN_WITHOUT_ABI]);
4056

41-
impl EarlyLintPass for ExternWithoutAbi {
42-
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
43-
if let ItemKind::ForeignMod(ref foreign_mod) = item.kind
44-
&& let ForeignMod { abi: None, .. } = foreign_mod
57+
impl<'tcx> LateLintPass<'tcx> for ExternWithoutAbi {
58+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
59+
if let ItemKind::ForeignMod { abi: Abi::C { .. }, .. } = item.kind
60+
&& !in_external_macro(cx.sess(), item.span)
61+
&& !is_from_proc_macro(cx, item)
62+
&& let snippet = snippet(cx.sess(), item.span, "").as_ref()
63+
&& is_extern_followed_by(OPEN_BRACE, snippet)
4564
{
46-
span_lint(cx, EXTERN_WITHOUT_ABI, item.span, LINT_MSG);
65+
emit_lint(cx, item.span, snippet);
4766
}
4867
}
4968

50-
fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, _: Span, _: NodeId) {
51-
if let FnKind::Fn(_, _, sig, ..) = kind
52-
&& let FnSig { header, .. } = sig
53-
&& let FnHeader {
54-
ext: Extern::Implicit(span),
55-
..
56-
} = header
69+
fn check_fn(
70+
&mut self,
71+
cx: &LateContext<'tcx>,
72+
kind: FnKind<'tcx>,
73+
_: &'tcx FnDecl<'tcx>,
74+
body: &'tcx Body<'tcx>,
75+
span: Span,
76+
_: LocalDefId,
77+
) {
78+
if let FnKind::ItemFn(_, _, header) = kind
79+
&& let FnHeader { abi: Abi::C { .. }, .. } = header
80+
&& !in_external_macro(cx.sess(), span)
81+
&& let hir_id = cx.tcx.hir().body_owner(body.id())
82+
&& !is_from_proc_macro(cx, &(&kind, body, hir_id, span))
83+
&& let snippet = snippet(cx.sess(), span, "").as_ref()
84+
&& is_extern_followed_by(FN, snippet)
5785
{
58-
span_lint(cx, EXTERN_WITHOUT_ABI, *span, LINT_MSG);
86+
emit_lint(cx, span, snippet);
5987
}
6088
}
6189
}
90+
91+
fn is_extern_followed_by(item: &str, snippet: &str) -> bool {
92+
let mut tokens = snippet.split_whitespace();
93+
94+
if let (_, Some(i)) = (tokens.next(), tokens.next())
95+
&& i.starts_with(item)
96+
{
97+
return true;
98+
}
99+
false
100+
}
101+
102+
fn emit_lint(cx: &LateContext<'_>, span: Span, snippet: &str) {
103+
let sugg = snippet.replacen(EXTERN, format!("{EXTERN} {ABI}").as_str(), 1);
104+
span_lint_and_sugg(
105+
cx,
106+
EXTERN_WITHOUT_ABI,
107+
span,
108+
LINT_MSG,
109+
LINT_HELP_MSG,
110+
sugg,
111+
Applicability::MachineApplicable,
112+
);
113+
}

‎clippy_lints/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
943943
store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf)));
944944
store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
945945
store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions));
946-
store.register_early_pass(|| Box::new(extern_without_abi::ExternWithoutAbi));
946+
store.register_late_pass(|_| Box::new(extern_without_abi::ExternWithoutAbi));
947947
// add lints here, do not remove this comment, it's used in `new_lint`
948948
}

‎tests/ui/extern_without_abi.fixed‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
//@aux-build:proc_macros.rs
2+
3+
#![warn(clippy::extern_without_abi)]
4+
5+
extern crate proc_macros;
6+
use proc_macros::{external, with_span};
7+
8+
#[rustfmt::skip]
9+
extern "C" fn foo() {}
10+
//~^ ERROR: `extern` missing explicit ABI
11+
12+
#[rustfmt::skip]
13+
extern "C"
14+
fn foo_two() {}
15+
//~^^ ERROR: `extern` missing explicit ABI
16+
17+
extern "C" fn bar() {}
18+
19+
#[rustfmt::skip]
20+
extern
21+
"C"
22+
fn bar_two() {}
23+
24+
extern "system" fn baz() {}
25+
26+
#[rustfmt::skip]
27+
extern "C" {
28+
//~^ ERROR: `extern` missing explicit ABI
29+
fn qux();
30+
}
31+
32+
#[rustfmt::skip]
33+
extern "C"
34+
{
35+
//~^^ ERROR: `extern` missing explicit ABI
36+
fn qux_two();
37+
}
38+
39+
#[rustfmt::skip]
40+
extern "C" {fn qux_three();}
41+
//~^ ERROR: `extern` missing explicit ABI
42+
43+
extern "C" {
44+
fn grault();
45+
}
46+
47+
extern "system" {
48+
fn grault_two();
49+
}
50+
51+
external! {
52+
extern fn waldo() {}
53+
}
54+
55+
with_span! {
56+
span
57+
extern fn waldo_two() {}
58+
}
59+
60+
fn main() {}

‎tests/ui/extern_without_abi.rs‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,60 @@
1+
//@aux-build:proc_macros.rs
2+
13
#![warn(clippy::extern_without_abi)]
24

3-
fn main() {}
5+
extern crate proc_macros;
6+
use proc_macros::{external, with_span};
47

58
#[rustfmt::skip]
69
extern fn foo() {}
710
//~^ ERROR: `extern` missing explicit ABI
811

12+
#[rustfmt::skip]
13+
extern
14+
fn foo_two() {}
15+
//~^^ ERROR: `extern` missing explicit ABI
16+
917
extern "C" fn bar() {}
1018

19+
#[rustfmt::skip]
20+
extern
21+
"C"
22+
fn bar_two() {}
23+
1124
extern "system" fn baz() {}
1225

1326
#[rustfmt::skip]
1427
extern {
1528
//~^ ERROR: `extern` missing explicit ABI
16-
static QUX: std::ffi::c_int;
29+
fn qux();
30+
}
1731

18-
fn quux();
32+
#[rustfmt::skip]
33+
extern
34+
{
35+
//~^^ ERROR: `extern` missing explicit ABI
36+
fn qux_two();
1937
}
2038

21-
extern "C" {
22-
static CORGE: std::ffi::c_int;
39+
#[rustfmt::skip]
40+
extern {fn qux_three();}
41+
//~^ ERROR: `extern` missing explicit ABI
2342

43+
extern "C" {
2444
fn grault();
2545
}
2646

2747
extern "system" {
28-
static GARPLY: std::ffi::c_int;
48+
fn grault_two();
49+
}
2950

30-
fn waldo();
51+
external! {
52+
extern fn waldo() {}
3153
}
54+
55+
with_span! {
56+
span
57+
extern fn waldo_two() {}
58+
}
59+
60+
fn main() {}

‎tests/ui/extern_without_abi.stderr‎

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,66 @@
11
error: `extern` missing explicit ABI
2-
--> tests/ui/extern_without_abi.rs:6:1
2+
--> tests/ui/extern_without_abi.rs:9:1
33
|
44
LL | extern fn foo() {}
5-
| ^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^ help: consider using: `extern "C" fn foo() {}`
66
|
77
= note: `-D clippy::extern-without-abi` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::extern_without_abi)]`
99

1010
error: `extern` missing explicit ABI
11-
--> tests/ui/extern_without_abi.rs:14:1
11+
--> tests/ui/extern_without_abi.rs:13:1
12+
|
13+
LL | / extern
14+
LL | | fn foo_two() {}
15+
| |_______________^
16+
|
17+
help: consider using
18+
|
19+
LL ~ extern "C"
20+
LL + fn foo_two() {}
21+
|
22+
23+
error: `extern` missing explicit ABI
24+
--> tests/ui/extern_without_abi.rs:27:1
1225
|
1326
LL | / extern {
1427
LL | |
15-
LL | | static QUX: std::ffi::c_int;
16-
... |
28+
LL | | fn qux();
1729
LL | | }
1830
| |_^
31+
|
32+
help: consider using
33+
|
34+
LL + extern "C" {
35+
LL +
36+
LL + fn qux();
37+
LL + }
38+
|
39+
40+
error: `extern` missing explicit ABI
41+
--> tests/ui/extern_without_abi.rs:33:1
42+
|
43+
LL | / extern
44+
LL | | {
45+
LL | |
46+
LL | | fn qux_two();
47+
LL | | }
48+
| |_^
49+
|
50+
help: consider using
51+
|
52+
LL ~ extern "C"
53+
LL + {
54+
LL +
55+
LL + fn qux_two();
56+
LL + }
57+
|
58+
59+
error: `extern` missing explicit ABI
60+
--> tests/ui/extern_without_abi.rs:40:1
61+
|
62+
LL | extern {fn qux_three();}
63+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `extern "C" {fn qux_three();}`
1964

20-
error: aborting due to 2 previous errors
65+
error: aborting due to 5 previous errors
2166

0 commit comments

Comments
 (0)