Fix conflicts
- New ineffective_open_options had to be fixed. - Now not raising an issue on missing `truncate` when `append(true)` makes the intent clear. - Try implementing more advanced tests for non-chained operations. Fail
This commit is contained in:
parent
84588a8815
commit
515fe65ba8
@ -1,8 +1,8 @@
|
|||||||
use rustc_data_structures::fx::FxHashMap;
|
use rustc_data_structures::fx::FxHashMap;
|
||||||
|
|
||||||
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
|
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
|
||||||
use clippy_utils::paths;
|
|
||||||
use clippy_utils::ty::{is_type_diagnostic_item, match_type};
|
use clippy_utils::ty::{is_type_diagnostic_item, match_type};
|
||||||
|
use clippy_utils::{match_any_def_paths, paths};
|
||||||
use rustc_ast::ast::LitKind;
|
use rustc_ast::ast::LitKind;
|
||||||
use rustc_hir::{Expr, ExprKind};
|
use rustc_hir::{Expr, ExprKind};
|
||||||
use rustc_lint::LateContext;
|
use rustc_lint::LateContext;
|
||||||
@ -19,7 +19,7 @@ fn is_open_options(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
|
|||||||
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) {
|
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) {
|
||||||
if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
|
if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
|
||||||
&& let Some(impl_id) = cx.tcx.impl_of_method(method_id)
|
&& let Some(impl_id) = cx.tcx.impl_of_method(method_id)
|
||||||
&& is_open_options(cx, cx.tcx.type_of(impl_id).instantiate_identity())
|
//&& is_open_options(cx, cx.tcx.type_of(impl_id).instantiate_identity())
|
||||||
{
|
{
|
||||||
let mut options = Vec::new();
|
let mut options = Vec::new();
|
||||||
get_open_options(cx, recv, &mut options);
|
get_open_options(cx, recv, &mut options);
|
||||||
@ -27,7 +27,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Eq, PartialEq, Clone)]
|
#[derive(Eq, PartialEq, Clone, Debug)]
|
||||||
enum Argument {
|
enum Argument {
|
||||||
Set(bool),
|
Set(bool),
|
||||||
Unknown,
|
Unknown,
|
||||||
@ -55,7 +55,11 @@ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec<(OpenOption, Argument, Span)>) {
|
fn get_open_options(
|
||||||
|
cx: &LateContext<'_>,
|
||||||
|
argument: &Expr<'_>,
|
||||||
|
options: &mut Vec<(OpenOption, Argument, Span)>,
|
||||||
|
) -> bool {
|
||||||
if let ExprKind::MethodCall(path, receiver, arguments, span) = argument.kind {
|
if let ExprKind::MethodCall(path, receiver, arguments, span) = argument.kind {
|
||||||
let obj_ty = cx.typeck_results().expr_ty(receiver).peel_refs();
|
let obj_ty = cx.typeck_results().expr_ty(receiver).peel_refs();
|
||||||
|
|
||||||
@ -72,7 +76,8 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec
|
|||||||
} else {
|
} else {
|
||||||
// The function is called with a literal which is not a boolean literal.
|
// The function is called with a literal which is not a boolean literal.
|
||||||
// This is theoretically possible, but not very likely.
|
// This is theoretically possible, but not very likely.
|
||||||
return;
|
// We'll ignore it for now
|
||||||
|
return get_open_options(cx, receiver, options);
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
_ => Argument::Unknown,
|
_ => Argument::Unknown,
|
||||||
@ -100,8 +105,19 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec
|
|||||||
_ => (),
|
_ => (),
|
||||||
}
|
}
|
||||||
|
|
||||||
get_open_options(cx, receiver, options);
|
get_open_options(cx, receiver, options)
|
||||||
|
} else {
|
||||||
|
false
|
||||||
}
|
}
|
||||||
|
} else if let ExprKind::Call(callee, _) = argument.kind
|
||||||
|
&& let ExprKind::Path(path) = callee.kind
|
||||||
|
&& let Some(did) = cx.qpath_res(&path, callee.hir_id).opt_def_id()
|
||||||
|
{
|
||||||
|
match_any_def_paths(cx, did, &[&paths::TOKIO_IO_OPEN_OPTIONS]).is_some()
|
||||||
|
//is_type_diagnostic_item(cx, ty, sym::FsOpenOptions) || match_type(cx, ty,
|
||||||
|
// &paths::TOKIO_IO_OPEN_OPTIONS)
|
||||||
|
} else {
|
||||||
|
false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -144,6 +160,7 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, S
|
|||||||
|
|
||||||
if let Some((Argument::Set(true), create_span)) = options.get(&OpenOption::Create)
|
if let Some((Argument::Set(true), create_span)) = options.get(&OpenOption::Create)
|
||||||
&& let None = options.get(&OpenOption::Truncate)
|
&& let None = options.get(&OpenOption::Truncate)
|
||||||
|
&& let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Append)
|
||||||
{
|
{
|
||||||
span_lint_and_then(
|
span_lint_and_then(
|
||||||
cx,
|
cx,
|
||||||
@ -153,7 +170,7 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, S
|
|||||||
|diag| {
|
|diag| {
|
||||||
diag
|
diag
|
||||||
.span_suggestion(create_span.shrink_to_hi(), "add", ".truncate(true)".to_string(), rustc_errors::Applicability::MaybeIncorrect)
|
.span_suggestion(create_span.shrink_to_hi(), "add", ".truncate(true)".to_string(), rustc_errors::Applicability::MaybeIncorrect)
|
||||||
.help("if you intend to overwrite an existing file entirely, call `.truncate(true)`. if you instead know that you may want to keep some parts of the old file, call `.truncate(false)`");
|
.help("if you intend to overwrite an existing file entirely, call `.truncate(true)`. if you instead know that you may want to keep some parts of the old file, call `.truncate(false)`. Alternatively, use `.append` to append to the file instead of overwriting it.");
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
@ -26,16 +26,23 @@ fn main() {
|
|||||||
.unwrap();
|
.unwrap();
|
||||||
let file = OpenOptions::new()
|
let file = OpenOptions::new()
|
||||||
.create(true)
|
.create(true)
|
||||||
|
.truncate(true)
|
||||||
.write(true)
|
.write(true)
|
||||||
.append(false)
|
.append(false)
|
||||||
.open("dump.json")
|
.open("dump.json")
|
||||||
.unwrap();
|
.unwrap();
|
||||||
let file = OpenOptions::new()
|
let file = OpenOptions::new()
|
||||||
.create(true)
|
.create(true)
|
||||||
|
.truncate(true)
|
||||||
.write(false)
|
.write(false)
|
||||||
.append(false)
|
.append(false)
|
||||||
.open("dump.json")
|
.open("dump.json")
|
||||||
.unwrap();
|
.unwrap();
|
||||||
let file = OpenOptions::new().create(true).append(true).open("dump.json").unwrap();
|
let file = OpenOptions::new().create(true).append(true).open("dump.json").unwrap();
|
||||||
let file = OpenOptions::new().create(true).write(true).open("dump.json").unwrap();
|
let file = OpenOptions::new()
|
||||||
|
.create(true)
|
||||||
|
.truncate(true)
|
||||||
|
.write(true)
|
||||||
|
.open("dump.json")
|
||||||
|
.unwrap();
|
||||||
}
|
}
|
||||||
|
@ -26,16 +26,23 @@ fn main() {
|
|||||||
.unwrap();
|
.unwrap();
|
||||||
let file = OpenOptions::new()
|
let file = OpenOptions::new()
|
||||||
.create(true)
|
.create(true)
|
||||||
|
.truncate(true)
|
||||||
.write(true)
|
.write(true)
|
||||||
.append(false)
|
.append(false)
|
||||||
.open("dump.json")
|
.open("dump.json")
|
||||||
.unwrap();
|
.unwrap();
|
||||||
let file = OpenOptions::new()
|
let file = OpenOptions::new()
|
||||||
.create(true)
|
.create(true)
|
||||||
|
.truncate(true)
|
||||||
.write(false)
|
.write(false)
|
||||||
.append(false)
|
.append(false)
|
||||||
.open("dump.json")
|
.open("dump.json")
|
||||||
.unwrap();
|
.unwrap();
|
||||||
let file = OpenOptions::new().create(true).append(true).open("dump.json").unwrap();
|
let file = OpenOptions::new().create(true).append(true).open("dump.json").unwrap();
|
||||||
let file = OpenOptions::new().create(true).write(true).open("dump.json").unwrap();
|
let file = OpenOptions::new()
|
||||||
|
.create(true)
|
||||||
|
.truncate(true)
|
||||||
|
.write(true)
|
||||||
|
.open("dump.json")
|
||||||
|
.unwrap();
|
||||||
}
|
}
|
||||||
|
@ -29,5 +29,6 @@ fn main() {
|
|||||||
let mut options = std::fs::OpenOptions::new();
|
let mut options = std::fs::OpenOptions::new();
|
||||||
options.read(true);
|
options.read(true);
|
||||||
options.read(false);
|
options.read(false);
|
||||||
//~^ ERROR: the method `read` is called more than once
|
//#~^ ERROR: the method `read` is called more than once
|
||||||
|
options.open("foo.txt");
|
||||||
}
|
}
|
||||||
|
@ -4,9 +4,9 @@ error: file opened with `create`, but `truncate` behavior not defined
|
|||||||
LL | OpenOptions::new().create(true).open("foo.txt");
|
LL | OpenOptions::new().create(true).open("foo.txt");
|
||||||
| ^^^^^^^^^^^^- help: add: `.truncate(true)`
|
| ^^^^^^^^^^^^- help: add: `.truncate(true)`
|
||||||
|
|
|
|
||||||
= help: if you intend to overwrite an existing file entirely, call `.truncate(true)`. if you instead know that you may want to keep some parts of the old file, call `.truncate(false)`
|
= help: if you intend to overwrite an existing file entirely, call `.truncate(true)`. if you instead know that you may want to keep some parts of the old file, call `.truncate(false)`. Alternatively, use `.append` to append to the file instead of overwriting it.
|
||||||
= note: `-D clippy::suspicious-open-options` implied by `-D warnings`
|
= note: `-D clippy::suspicious-open-options` implied by `-D warnings`
|
||||||
= help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]`
|
= help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]`
|
||||||
|
|
||||||
error: aborting due to previous error
|
error: aborting due to 1 previous error
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user