Skip to content

Commit 0213442

Browse files
committed
New lint bare_dos_device_names
1 parent 9f0cbfd commit 0213442

File tree

7 files changed

+269
-0
lines changed

7 files changed

+269
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4702,6 +4702,7 @@ Released 2018-09-13
47024702
[`await_holding_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock
47034703
[`await_holding_refcell_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_refcell_ref
47044704
[`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
4705+
[`bare_dos_device_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#bare_dos_device_names
47054706
[`big_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#big_endian_bytes
47064707
[`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map
47074708
[`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
use clippy_utils::{
2+
diagnostics::span_lint_and_then, get_parent_expr, is_from_proc_macro, match_def_path, path_res, paths::PATH_NEW,
3+
ty::is_type_diagnostic_item,
4+
};
5+
use rustc_ast::LitKind;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::def_id::DefId;
8+
use rustc_hir::*;
9+
use rustc_lint::{LateContext, LateLintPass, LintContext};
10+
use rustc_middle::{lint::in_external_macro, ty};
11+
use rustc_session::{declare_lint_pass, declare_tool_lint};
12+
use rustc_span::{sym, Symbol};
13+
14+
declare_clippy_lint! {
15+
/// ### What it does
16+
/// TODO please do soon
17+
///
18+
/// ### Why is this bad?
19+
/// TODO please
20+
///
21+
/// ### Example
22+
/// ```rust
23+
/// // TODO
24+
/// ```
25+
/// Use instead:
26+
/// ```rust
27+
/// // TODO
28+
/// ```
29+
#[clippy::version = "1.72.0"]
30+
pub BARE_DOS_DEVICE_NAMES,
31+
suspicious,
32+
"usage of paths that, on Windows, will implicitly refer to a DOS device"
33+
}
34+
declare_lint_pass!(BareDosDeviceNames => [BARE_DOS_DEVICE_NAMES]);
35+
36+
impl<'tcx> LateLintPass<'tcx> for BareDosDeviceNames {
37+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
38+
if !in_external_macro(cx.sess(), expr.span)
39+
&& let ExprKind::Lit(arg) = expr.kind
40+
&& let LitKind::Str(str_sym, _) = arg.node
41+
&& matches!(
42+
&*str_sym.as_str().to_ascii_lowercase(),
43+
"aux"
44+
| "con"
45+
| "conin$"
46+
// ^^^^^^
47+
| "conout$"
48+
// ^^^^^^^
49+
// TODO: Maybe these two can be an exception.
50+
//
51+
// Using `CONIN$` and `CONOUT$` is common enough in other languages that it may
52+
// trip up a couple newbies coming to rust. Besides, it's unlikely someone will
53+
// ever use `CONIN$` as a filename.
54+
| "com1"
55+
| "com2"
56+
| "com3"
57+
| "com4"
58+
| "com5"
59+
| "com6"
60+
| "com7"
61+
| "com8"
62+
| "com9"
63+
| "lpt1"
64+
| "lpt2"
65+
| "lpt3"
66+
| "lpt4"
67+
| "lpt5"
68+
| "lpt6"
69+
| "lpt7"
70+
| "lpt8"
71+
| "lpt9"
72+
| "nul"
73+
| "prn"
74+
)
75+
&& let Some(parent) = get_parent_expr(cx, expr)
76+
&& (is_path_buf_from_or_path_new(cx, parent) || is_path_ty(cx, expr, parent))
77+
&& !is_from_proc_macro(cx, expr)
78+
{
79+
span_lint_and_then(
80+
cx,
81+
BARE_DOS_DEVICE_NAMES,
82+
expr.span,
83+
"this path refers to a DOS device",
84+
|diag| {
85+
// Suggest making current behavior explicit
86+
diag.span_suggestion_verbose(
87+
expr.span,
88+
"if this is intended, try",
89+
format!(r#""\\.\{str_sym}""#),
90+
Applicability::MaybeIncorrect,
91+
);
92+
93+
// Suggest making the code refer to a file or folder in the current directory
94+
diag.span_suggestion_verbose(
95+
expr.span,
96+
"if this was intended to point to a file or folder, try",
97+
format!("\"./{str_sym}\""),
98+
Applicability::MaybeIncorrect,
99+
);
100+
}
101+
);
102+
}
103+
}
104+
}
105+
106+
/// Gets whether the `Expr` is an argument to `Path::new` or `PathBuf::from`. The caller must
107+
/// provide the parent `Expr`, for performance's sake.
108+
///
109+
/// TODO: We can likely refactor this like we did with `LINTED_TRAITS`.
110+
fn is_path_buf_from_or_path_new(cx: &LateContext<'_>, parent: &Expr<'_>) -> bool {
111+
if let ExprKind::Call(path, _) = parent.kind
112+
&& let ExprKind::Path(qpath) = path.kind
113+
&& let QPath::TypeRelative(ty, last_segment) = qpath
114+
&& let Some(call_def_id) = path_res(cx, path).opt_def_id()
115+
&& let Some(ty_def_id) = path_res(cx, ty).opt_def_id()
116+
&& (match_def_path(cx, call_def_id, &PATH_NEW)
117+
// `PathBuf::from` is unfortunately tricky, as all we end up having for `match_def_path`
118+
// is `core::convert::From::from`, not `std::path::PathBuf::from`. Basically useless.
119+
|| cx.tcx.is_diagnostic_item(sym::PathBuf, ty_def_id) && last_segment.ident.as_str() == "from")
120+
{
121+
return true;
122+
}
123+
124+
false
125+
}
126+
127+
/// Gets the `DefId` and arguments of `expr`, if it's a `Call` or `MethodCall`
128+
fn get_def_id_and_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<(DefId, &'tcx [Expr<'tcx>])> {
129+
match expr.kind {
130+
ExprKind::Call(path, args) => Some((path_res(cx, path).opt_def_id()?, args)),
131+
ExprKind::MethodCall(_, _, args, _) => Some((cx.typeck_results().type_dependent_def_id(expr.hir_id)?, args)),
132+
_ => None,
133+
}
134+
}
135+
136+
/// Given a `Ty`, returns whether it is likely a path type, like `Path` or `PathBuf`. Also returns
137+
/// true if it's `impl AsRef<Path>`, `T: AsRef<Path>`, etc. You get the idea.
138+
fn is_path_ty<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, parent: &'tcx Expr<'tcx>) -> bool {
139+
const LINTED_TRAITS: &[(Symbol, Symbol)] = &[
140+
(sym::AsRef, sym::Path),
141+
(sym::Into, sym::PathBuf),
142+
(sym::Into, sym::Path),
143+
// TODO: Let's add more traits here.
144+
];
145+
146+
let Some((callee, callee_args)) = get_def_id_and_args(cx, parent) else {
147+
return false;
148+
};
149+
let Some(arg_index) = callee_args.iter().position(|arg| arg.hir_id == expr.hir_id) else {
150+
return false;
151+
};
152+
let arg_ty = cx.tcx.fn_sig(callee).subst_identity().inputs().skip_binder()[arg_index].peel_refs();
153+
154+
// If we find `PathBuf` or `Path`, no need to check `impl <trait>` or `T`.
155+
if let Some(def) = arg_ty.ty_adt_def()
156+
&& let def_id = def.did()
157+
&& (cx.tcx.is_diagnostic_item(sym::PathBuf, def_id) || cx.tcx.is_diagnostic_item(sym::Path, def_id))
158+
{
159+
return true;
160+
}
161+
162+
for predicate in cx
163+
.tcx
164+
.param_env(callee)
165+
.caller_bounds()
166+
.iter()
167+
.filter_map(|predicate| predicate.kind().no_bound_vars())
168+
{
169+
if let ty::ClauseKind::Trait(trit) = predicate
170+
&& trit.trait_ref.self_ty() == arg_ty
171+
// I believe `0` is always `Self`, so `T` or `impl <trait>`
172+
&& let [_, subst] = trit.trait_ref.substs.as_slice()
173+
&& let Some(as_ref_ty) = subst.as_type()
174+
{
175+
for (trait_sym, ty_sym) in LINTED_TRAITS {
176+
if cx.tcx.is_diagnostic_item(*trait_sym, trit.trait_ref.def_id)
177+
&& is_type_diagnostic_item(cx, as_ref_ty, *ty_sym)
178+
{
179+
return true;
180+
}
181+
}
182+
}
183+
}
184+
185+
false
186+
}

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
6161
crate::await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE_INFO,
6262
crate::await_holding_invalid::AWAIT_HOLDING_LOCK_INFO,
6363
crate::await_holding_invalid::AWAIT_HOLDING_REFCELL_REF_INFO,
64+
crate::bare_dos_device_names::BARE_DOS_DEVICE_NAMES_INFO,
6465
crate::blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS_INFO,
6566
crate::bool_assert_comparison::BOOL_ASSERT_COMPARISON_INFO,
6667
crate::bool_to_int_with_if::BOOL_TO_INT_WITH_IF_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ mod assertions_on_result_states;
7676
mod async_yields_async;
7777
mod attrs;
7878
mod await_holding_invalid;
79+
mod bare_dos_device_names;
7980
mod blocks_in_if_conditions;
8081
mod bool_assert_comparison;
8182
mod bool_to_int_with_if;
@@ -1080,6 +1081,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10801081
store.register_late_pass(move |_| Box::new(tuple_array_conversions::TupleArrayConversions { msrv: msrv() }));
10811082
store.register_late_pass(|_| Box::new(manual_float_methods::ManualFloatMethods));
10821083
store.register_late_pass(|_| Box::new(four_forward_slashes::FourForwardSlashes));
1084+
store.register_late_pass(move |_| Box::new(bare_dos_device_names::BareDosDeviceNames));
10831085
// add lints here, do not remove this comment, it's used in `new_lint`
10841086
}
10851087

clippy_utils/src/paths.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 3] = ["lock_api", "rwlock", "RwL
6868
pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 3] = ["lock_api", "rwlock", "RwLockWriteGuard"];
6969
pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"];
7070
pub const PATH_MAIN_SEPARATOR: [&str; 3] = ["std", "path", "MAIN_SEPARATOR"];
71+
pub const PATH_NEW: [&str; 4] = ["std", "path", "Path", "new"];
7172
pub const PATH_TO_PATH_BUF: [&str; 4] = ["std", "path", "Path", "to_path_buf"];
7273
pub const PEEKABLE: [&str; 5] = ["core", "iter", "adapters", "peekable", "Peekable"];
7374
pub const PERMISSIONS: [&str; 3] = ["std", "fs", "Permissions"];

tests/ui/bare_dos_device_names.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
//@aux-build:proc_macros.rs:proc-macro
2+
#![allow(clippy::no_effect, unused)]
3+
#![warn(clippy::bare_dos_device_names)]
4+
5+
#[macro_use]
6+
extern crate proc_macros;
7+
8+
use std::fs::File;
9+
use std::path::Path;
10+
use std::path::PathBuf;
11+
12+
fn a<T: AsRef<Path>>(t: T) {}
13+
14+
fn b(t: impl AsRef<Path>) {}
15+
16+
fn main() {
17+
a("con");
18+
b("conin$");
19+
File::open("conin$");
20+
std::path::PathBuf::from("a");
21+
PathBuf::from("a");
22+
Path::new("a");
23+
external! {
24+
a("con");
25+
}
26+
with_span! {
27+
span
28+
a("con");
29+
}
30+
}

tests/ui/bare_dos_device_names.stderr

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
error: this path refers to a DOS device
2+
--> $DIR/bare_dos_device_names.rs:17:7
3+
|
4+
LL | a("con");
5+
| ^^^^^
6+
|
7+
= note: `-D clippy::bare-dos-device-names` implied by `-D warnings`
8+
help: if this is intended, try
9+
|
10+
LL | a("//./con");
11+
| ~~~~~~~~~
12+
help: if this was intended to point to a file or folder, try
13+
|
14+
LL | a("./con");
15+
| ~~~~~~~
16+
17+
error: this path refers to a DOS device
18+
--> $DIR/bare_dos_device_names.rs:18:7
19+
|
20+
LL | b("conin$");
21+
| ^^^^^^^^
22+
|
23+
help: if this is intended, try
24+
|
25+
LL | b("//./conin$");
26+
| ~~~~~~~~~~~~
27+
help: if this was intended to point to a file or folder, try
28+
|
29+
LL | b("./conin$");
30+
| ~~~~~~~~~~
31+
32+
error: this path refers to a DOS device
33+
--> $DIR/bare_dos_device_names.rs:19:16
34+
|
35+
LL | File::open("conin$");
36+
| ^^^^^^^^
37+
|
38+
help: if this is intended, try
39+
|
40+
LL | File::open("//./conin$");
41+
| ~~~~~~~~~~~~
42+
help: if this was intended to point to a file or folder, try
43+
|
44+
LL | File::open("./conin$");
45+
| ~~~~~~~~~~
46+
47+
error: aborting due to 3 previous errors
48+

0 commit comments

Comments
 (0)