Minor nitpicks for PTH210 (#14814)

This commit is contained in:
Alex Waygood 2024-12-06 12:23:21 +00:00 committed by GitHub
parent 89368a62a8
commit 9ee438b02f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -1,7 +1,7 @@
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::{Expr, ExprAttribute, ExprCall, ExprStringLiteral, StringFlags}; use ruff_python_ast::{self as ast, StringFlags};
use ruff_python_semantic::analyze::typing; use ruff_python_semantic::analyze::typing;
use ruff_python_semantic::SemanticModel; use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
@ -27,9 +27,12 @@ use ruff_text_size::Ranged;
/// ``` /// ```
/// ///
/// ## Known problems /// ## Known problems
/// This rule is prone to false negatives due to type inference limitations, /// This rule is likely to have false negatives, as Ruff can only emit the
/// as it will only detect paths that are either instantiated (`p = Path(...)`) /// lint if it can say for sure that a binding refers to a `Path` object at
/// or annotated (`def f(p: Path)`) as such. /// runtime. Due to type inference limitations, Ruff is currently only
/// confident about this if it can see that the binding originates from a
/// function parameter annotated with `Path` or from a direct assignment to a
/// `Path()` constructor call.
/// ///
/// ## Fix safety /// ## Fix safety
/// The fix for this rule adds a leading period to the string passed /// The fix for this rule adds a leading period to the string passed
@ -55,7 +58,7 @@ impl AlwaysFixableViolation for DotlessPathlibWithSuffix {
} }
/// PTH210 /// PTH210
pub(crate) fn dotless_pathlib_with_suffix(checker: &mut Checker, call: &ExprCall) { pub(crate) fn dotless_pathlib_with_suffix(checker: &mut Checker, call: &ast::ExprCall) {
let (func, arguments) = (&call.func, &call.arguments); let (func, arguments) = (&call.func, &call.arguments);
if !is_path_with_suffix_call(checker.semantic(), func) { if !is_path_with_suffix_call(checker.semantic(), func) {
@ -66,7 +69,7 @@ pub(crate) fn dotless_pathlib_with_suffix(checker: &mut Checker, call: &ExprCall
return; return;
} }
let Some(Expr::StringLiteral(string)) = arguments.find_argument("suffix", 0) else { let Some(ast::Expr::StringLiteral(string)) = arguments.find_argument("suffix", 0) else {
return; return;
}; };
@ -76,16 +79,17 @@ pub(crate) fn dotless_pathlib_with_suffix(checker: &mut Checker, call: &ExprCall
return; return;
} }
let diagnostic = Diagnostic::new(DotlessPathlibWithSuffix, call.range); let [first_part, ..] = string.value.as_slice() else {
let Some(fix) = add_leading_dot_fix(string) else { return;
unreachable!("Expected to always be able to fix this rule");
}; };
let diagnostic = Diagnostic::new(DotlessPathlibWithSuffix, call.range);
let fix = add_leading_dot_fix(first_part);
checker.diagnostics.push(diagnostic.with_fix(fix)); checker.diagnostics.push(diagnostic.with_fix(fix));
} }
fn is_path_with_suffix_call(semantic: &SemanticModel, func: &Expr) -> bool { fn is_path_with_suffix_call(semantic: &SemanticModel, func: &ast::Expr) -> bool {
let Expr::Attribute(ExprAttribute { value, attr, .. }) = func else { let ast::Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func else {
return false; return false;
}; };
@ -93,7 +97,7 @@ fn is_path_with_suffix_call(semantic: &SemanticModel, func: &Expr) -> bool {
return false; return false;
} }
let Expr::Name(name) = value.as_ref() else { let ast::Expr::Name(name) = &**value else {
return false; return false;
}; };
let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else { let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else {
@ -103,13 +107,12 @@ fn is_path_with_suffix_call(semantic: &SemanticModel, func: &Expr) -> bool {
typing::is_pathlib_path(binding, semantic) typing::is_pathlib_path(binding, semantic)
} }
fn add_leading_dot_fix(string: &ExprStringLiteral) -> Option<Fix> { fn add_leading_dot_fix(string: &ast::StringLiteral) -> Fix {
let first_part = string.value.iter().next()?; let opener_length = string.flags.opener_len();
let after_leading_quote = string
let opener_length = first_part.flags.opener_len(); .start()
let after_leading_quote = first_part.start().checked_add(opener_length)?; .checked_add(opener_length)
.expect("Stepping past a string's opener should still be a valid offset");
let edit = Edit::insertion(".".to_string(), after_leading_quote); let edit = Edit::insertion(".".to_string(), after_leading_quote);
Fix::unsafe_edit(edit)
Some(Fix::unsafe_edit(edit))
} }