From 9ee438b02f10d40a11076cfbcf2bc02396f8bf4f Mon Sep 17 00:00:00 2001
From: Alex Waygood <Alex.Waygood@Gmail.com>
Date: Fri, 6 Dec 2024 12:23:21 +0000
Subject: [PATCH] Minor nitpicks for PTH210 (#14814)

---
 .../rules/dotless_pathlib_with_suffix.rs      | 43 ++++++++++---------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/dotless_pathlib_with_suffix.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/dotless_pathlib_with_suffix.rs
index 3fd1baa80d..fc5ff0a2d8 100644
--- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/dotless_pathlib_with_suffix.rs
+++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/dotless_pathlib_with_suffix.rs
@@ -1,7 +1,7 @@
 use crate::checkers::ast::Checker;
 use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
 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::SemanticModel;
 use ruff_text_size::Ranged;
@@ -27,9 +27,12 @@ use ruff_text_size::Ranged;
 /// ```
 ///
 /// ## Known problems
-/// This rule is prone to false negatives due to type inference limitations,
-/// as it will only detect paths that are either instantiated (`p = Path(...)`)
-/// or annotated (`def f(p: Path)`) as such.
+/// This rule is likely to have false negatives, as Ruff can only emit the
+/// lint if it can say for sure that a binding refers to a `Path` object at
+/// 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
 /// The fix for this rule adds a leading period to the string passed
@@ -55,7 +58,7 @@ impl AlwaysFixableViolation for DotlessPathlibWithSuffix {
 }
 
 /// 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);
 
     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;
     }
 
-    let Some(Expr::StringLiteral(string)) = arguments.find_argument("suffix", 0) else {
+    let Some(ast::Expr::StringLiteral(string)) = arguments.find_argument("suffix", 0) else {
         return;
     };
 
@@ -76,16 +79,17 @@ pub(crate) fn dotless_pathlib_with_suffix(checker: &mut Checker, call: &ExprCall
         return;
     }
 
-    let diagnostic = Diagnostic::new(DotlessPathlibWithSuffix, call.range);
-    let Some(fix) = add_leading_dot_fix(string) else {
-        unreachable!("Expected to always be able to fix this rule");
+    let [first_part, ..] = string.value.as_slice() else {
+        return;
     };
 
+    let diagnostic = Diagnostic::new(DotlessPathlibWithSuffix, call.range);
+    let fix = add_leading_dot_fix(first_part);
     checker.diagnostics.push(diagnostic.with_fix(fix));
 }
 
-fn is_path_with_suffix_call(semantic: &SemanticModel, func: &Expr) -> bool {
-    let Expr::Attribute(ExprAttribute { value, attr, .. }) = func else {
+fn is_path_with_suffix_call(semantic: &SemanticModel, func: &ast::Expr) -> bool {
+    let ast::Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func else {
         return false;
     };
 
@@ -93,7 +97,7 @@ fn is_path_with_suffix_call(semantic: &SemanticModel, func: &Expr) -> bool {
         return false;
     }
 
-    let Expr::Name(name) = value.as_ref() else {
+    let ast::Expr::Name(name) = &**value else {
         return false;
     };
     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)
 }
 
-fn add_leading_dot_fix(string: &ExprStringLiteral) -> Option<Fix> {
-    let first_part = string.value.iter().next()?;
-
-    let opener_length = first_part.flags.opener_len();
-    let after_leading_quote = first_part.start().checked_add(opener_length)?;
-
+fn add_leading_dot_fix(string: &ast::StringLiteral) -> Fix {
+    let opener_length = string.flags.opener_len();
+    let after_leading_quote = string
+        .start()
+        .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);
-
-    Some(Fix::unsafe_edit(edit))
+    Fix::unsafe_edit(edit)
 }