Code review changes

crates/ruff_linter/src/checkers/ast/analyze/definitions.rs

@@ -170,7 +170,7 @@ pub(crate) fn definitions(checker: &mut Checker) {
                 expr.start(),
             ));
 
-            if pydocstyle::helpers::should_ignore_docstring(expr) {
+            if expr.implicit_concatenated {
                 #[allow(deprecated)]
                 let location = checker.locator.compute_source_location(expr.start());
                 warn_user!(

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -1238,19 +1238,19 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 flake8_pyi::rules::string_or_bytes_too_long(checker, expr);
             }
         }
-        Expr::StringLiteral(ast::ExprStringLiteral { value, unicode, .. }) => {
+        Expr::StringLiteral(string) => {
             if checker.enabled(Rule::HardcodedBindAllInterfaces) {
                 if let Some(diagnostic) =
-                    flake8_bandit::rules::hardcoded_bind_all_interfaces(value, expr.range())
+                    flake8_bandit::rules::hardcoded_bind_all_interfaces(string)
                 {
                     checker.diagnostics.push(diagnostic);
                 }
             }
             if checker.enabled(Rule::HardcodedTempFile) {
-                flake8_bandit::rules::hardcoded_tmp_directory(checker, expr, value);
+                flake8_bandit::rules::hardcoded_tmp_directory(checker, string);
             }
             if checker.enabled(Rule::UnicodeKindPrefix) {
-                pyupgrade::rules::unicode_kind_prefix(checker, expr, *unicode);
+                pyupgrade::rules::unicode_kind_prefix(checker, string);
             }
             if checker.source_type.is_stub() {
                 if checker.enabled(Rule::StringOrBytesTooLong) {

crates/ruff_linter/src/doc_lines.rs

@@ -69,11 +69,15 @@ struct StringLinesVisitor<'a> {
 
 impl StatementVisitor<'_> for StringLinesVisitor<'_> {
     fn visit_stmt(&mut self, stmt: &Stmt) {
-        if let Stmt::Expr(ast::StmtExpr { value, range: _ }) = stmt {
-            if value.is_string_literal_expr() {
+        if let Stmt::Expr(ast::StmtExpr {
+            value: expr,
+            range: _,
+        }) = stmt
+        {
+            if expr.is_string_literal_expr() {
                 for line in UniversalNewlineIterator::with_offset(
-                    self.locator.slice(value.as_ref()),
-                    value.start(),
+                    self.locator.slice(expr.as_ref()),
+                    expr.start(),
                 ) {
                     self.string_lines.push(line.start());
                 }

crates/ruff_linter/src/docstrings/extraction.rs

@@ -1,24 +1,23 @@
 //! Extract docstrings from an AST.
 
-use ruff_python_ast::{self as ast, Expr, Stmt};
+use ruff_python_ast::{self as ast, Stmt};
 use ruff_python_semantic::{Definition, DefinitionId, Definitions, Member, MemberKind};
 
 /// Extract a docstring from a function or class body.
-pub(crate) fn docstring_from(suite: &[Stmt]) -> Option<&Expr> {
+pub(crate) fn docstring_from(suite: &[Stmt]) -> Option<&ast::ExprStringLiteral> {
     let stmt = suite.first()?;
     // Require the docstring to be a standalone expression.
     let Stmt::Expr(ast::StmtExpr { value, range: _ }) = stmt else {
         return None;
     };
     // Only match strings.
-    if !value.is_string_literal_expr() {
-        return None;
-    }
-    Some(value)
+    value.as_string_literal_expr()
 }
 
 /// Extract a docstring from a `Definition`.
-pub(crate) fn extract_docstring<'a>(definition: &'a Definition<'a>) -> Option<&'a Expr> {
+pub(crate) fn extract_docstring<'a>(
+    definition: &'a Definition<'a>,
+) -> Option<&'a ast::ExprStringLiteral> {
     match definition {
         Definition::Module(module) => docstring_from(module.python_ast),
         Definition::Member(member) => docstring_from(member.body()),

crates/ruff_linter/src/docstrings/mod.rs

@@ -1,7 +1,7 @@
 use std::fmt::{Debug, Formatter};
 use std::ops::Deref;
 
-use ruff_python_ast::Expr;
+use ruff_python_ast::ExprStringLiteral;
 use ruff_python_semantic::Definition;
 use ruff_text_size::{Ranged, TextRange};
 
@@ -14,7 +14,8 @@ pub(crate) mod styles;
 #[derive(Debug)]
 pub(crate) struct Docstring<'a> {
     pub(crate) definition: &'a Definition<'a>,
-    pub(crate) expr: &'a Expr,
+    /// The literal AST node representing the docstring.
+    pub(crate) expr: &'a ExprStringLiteral,
     /// The content of the docstring, including the leading and trailing quotes.
     pub(crate) contents: &'a str,
     /// The range of the docstring body (without the quotes). The range is relative to [`Self::contents`].

crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs

@@ -1,7 +1,6 @@
-use ruff_text_size::TextRange;
-
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::ExprStringLiteral;
 
 /// ## What it does
 /// Checks for hardcoded bindings to all network interfaces (`0.0.0.0`).
@@ -35,9 +34,9 @@ impl Violation for HardcodedBindAllInterfaces {
 }
 
 /// S104
-pub(crate) fn hardcoded_bind_all_interfaces(value: &str, range: TextRange) -> Option<Diagnostic> {
-    if value == "0.0.0.0" {
-        Some(Diagnostic::new(HardcodedBindAllInterfaces, range))
+pub(crate) fn hardcoded_bind_all_interfaces(string: &ExprStringLiteral) -> Option<Diagnostic> {
+    if string.value == "0.0.0.0" {
+        Some(Diagnostic::new(HardcodedBindAllInterfaces, string.range))
     } else {
         None
     }

crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs

@@ -2,7 +2,6 @@ use ruff_python_ast::{self as ast, Expr};
 
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
-use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
 
@@ -52,13 +51,13 @@ impl Violation for HardcodedTempFile {
 }
 
 /// S108
-pub(crate) fn hardcoded_tmp_directory(checker: &mut Checker, expr: &Expr, value: &str) {
+pub(crate) fn hardcoded_tmp_directory(checker: &mut Checker, string: &ast::ExprStringLiteral) {
     if !checker
         .settings
         .flake8_bandit
         .hardcoded_tmp_directory
         .iter()
-        .any(|prefix| value.starts_with(prefix))
+        .any(|prefix| string.value.starts_with(prefix))
     {
         return;
     }
@@ -77,8 +76,8 @@ pub(crate) fn hardcoded_tmp_directory(checker: &mut Checker, expr: &Expr, value:
 
     checker.diagnostics.push(Diagnostic::new(
         HardcodedTempFile {
-            string: value.to_string(),
+            string: string.value.clone(),
         },
-        expr.range(),
+        string.range,
     ));
 }

crates/ruff_linter/src/rules/flake8_pyi/rules/docstring_in_stubs.rs

@@ -1,4 +1,4 @@
-use ruff_python_ast::Expr;
+use ruff_python_ast::ExprStringLiteral;
 
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
@@ -36,8 +36,8 @@ impl Violation for DocstringInStub {
 }
 
 /// PYI021
-pub(crate) fn docstring_in_stubs(checker: &mut Checker, docstring: Option<&Expr>) {
-    if let Some(docstr) = &docstring {
+pub(crate) fn docstring_in_stubs(checker: &mut Checker, docstring: Option<&ExprStringLiteral>) {
+    if let Some(docstr) = docstring {
         checker
             .diagnostics
             .push(Diagnostic::new(DocstringInStub, docstr.range()));

crates/ruff_linter/src/rules/pydocstyle/helpers.rs

@@ -2,7 +2,6 @@ use std::collections::BTreeSet;
 
 use ruff_python_ast::call_path::from_qualified_name;
 use ruff_python_ast::helpers::map_callable;
-use ruff_python_ast::Expr;
 use ruff_python_semantic::{Definition, SemanticModel};
 use ruff_source_file::UniversalNewlines;
 
@@ -61,13 +60,3 @@ pub(crate) fn should_ignore_definition(
             })
     })
 }
-
-/// Check if a docstring should be ignored.
-pub(crate) fn should_ignore_docstring(docstring: &Expr) -> bool {
-    // Avoid analyzing docstrings that contain implicit string concatenations.
-    // Python does consider these docstrings, but they're almost certainly a
-    // user error, and supporting them "properly" is extremely difficult.
-    docstring
-        .as_string_literal_expr()
-        .is_some_and(|string_literal| string_literal.implicit_concatenated)
-}

crates/ruff_linter/src/rules/pydocstyle/rules/capitalized.rs

@@ -86,7 +86,7 @@ pub(crate) fn capitalized(checker: &mut Checker, docstring: &Docstring) {
             first_word: first_word.to_string(),
             capitalized_word: capitalized_word.to_string(),
         },
-        docstring.expr.range(),
+        docstring.range(),
     );
 
     diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(

crates/ruff_linter/src/rules/pydocstyle/rules/newline_after_last_paragraph.rs

@@ -96,8 +96,8 @@ pub(crate) fn newline_after_last_paragraph(checker: &mut Checker, docstring: &Do
                     );
                     diagnostic.set_fix(Fix::safe_edit(Edit::replacement(
                         content,
-                        docstring.expr.end() - num_trailing_quotes - num_trailing_spaces,
-                        docstring.expr.end() - num_trailing_quotes,
+                        docstring.end() - num_trailing_quotes - num_trailing_spaces,
+                        docstring.end() - num_trailing_quotes,
                     )));
                     checker.diagnostics.push(diagnostic);
                 }

crates/ruff_linter/src/rules/pylint/rules/magic_value_comparison.rs

@@ -71,13 +71,15 @@ fn as_literal(expr: &Expr) -> Option<&Expr> {
 }
 
 fn is_magic_value(expr: &Expr, allowed_types: &[ConstantType]) -> bool {
-    if let Ok(constant_type) = ConstantType::try_from(expr) {
+    if let Some(constant_type) = ConstantType::try_from_expr(expr) {
         if allowed_types.contains(&constant_type) {
             return false;
         }
     }
 
     match expr {
+        // Ignore `None`, `Bool`, and `Ellipsis` constants.
+        Expr::NoneLiteral(_) | Expr::BooleanLiteral(_) | Expr::EllipsisLiteral(_) => false,
         // Special-case some common string and integer types.
         Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => {
             !matches!(value.as_str(), "" | "__main__")

crates/ruff_linter/src/rules/pylint/settings.rs

@@ -16,19 +16,17 @@ pub enum ConstantType {
     Str,
 }
 
-impl TryFrom<&Expr> for ConstantType {
-    type Error = ();
-
-    fn try_from(value: &Expr) -> Result<Self, Self::Error> {
-        match value {
-            Expr::StringLiteral(_) => Ok(Self::Str),
-            Expr::BytesLiteral(_) => Ok(Self::Bytes),
+impl ConstantType {
+    pub fn try_from_expr(expr: &Expr) -> Option<Self> {
+        match expr {
+            Expr::StringLiteral(_) => Some(Self::Str),
+            Expr::BytesLiteral(_) => Some(Self::Bytes),
             Expr::NumberLiteral(ExprNumberLiteral { value, .. }) => match value {
-                Number::Int(_) => Ok(Self::Int),
-                Number::Float(_) => Ok(Self::Float),
-                Number::Complex { .. } => Ok(Self::Complex),
+                Number::Int(_) => Some(Self::Int),
+                Number::Float(_) => Some(Self::Float),
+                Number::Complex { .. } => Some(Self::Complex),
             },
-            _ => Err(()),
+            _ => None,
         }
     }
 }

crates/ruff_linter/src/rules/pyupgrade/rules/unicode_kind_prefix.rs

@@ -1,6 +1,6 @@
 use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
-use ruff_python_ast::Expr;
+use ruff_python_ast::ExprStringLiteral;
 use ruff_text_size::{Ranged, TextRange, TextSize};
 
 use crate::checkers::ast::Checker;
@@ -39,11 +39,11 @@ impl AlwaysFixableViolation for UnicodeKindPrefix {
 }
 
 /// UP025
-pub(crate) fn unicode_kind_prefix(checker: &mut Checker, expr: &Expr, is_unicode: bool) {
-    if is_unicode {
-        let mut diagnostic = Diagnostic::new(UnicodeKindPrefix, expr.range());
+pub(crate) fn unicode_kind_prefix(checker: &mut Checker, string: &ExprStringLiteral) {
+    if string.unicode {
+        let mut diagnostic = Diagnostic::new(UnicodeKindPrefix, string.range);
         diagnostic.set_fix(Fix::safe_edit(Edit::range_deletion(TextRange::at(
-            expr.start(),
+            string.start(),
             TextSize::from(1),
         ))));
         checker.diagnostics.push(diagnostic);

crates/ruff_linter/src/rules/refurb/rules/implicit_cwd.rs

@@ -63,10 +63,10 @@ pub(crate) fn no_implicit_cwd(checker: &mut Checker, call: &ExprCall) {
         [] => {}
         // Ex) `Path(".").resolve()`
         [arg] => {
-            let Expr::StringLiteral(ast::ExprStringLiteral { value: str, .. }) = arg else {
+            let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = arg else {
                 return;
             };
-            if !matches!(str.as_str(), "" | ".") {
+            if !matches!(value.as_str(), "" | ".") {
                 return;
             }
         }

crates/ruff_linter/src/rules/ruff/rules/invalid_index_type.rs

@@ -149,7 +149,7 @@ pub(crate) fn invalid_index_type(checker: &mut Checker, expr: &ExprSubscript) {
 /// These are generally "literal" type expressions in that we know their concrete type
 /// without additional analysis; opposed to expressions like a function call where we
 /// cannot determine what type it may return.
-#[derive(Debug, PartialEq)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
 enum CheckableExprType {
     FString,
     StringLiteral,
@@ -220,7 +220,7 @@ impl CheckableExprType {
         }
     }
 
-    fn is_literal(&self) -> bool {
+    fn is_literal(self) -> bool {
         matches!(
             self,
             Self::StringLiteral

crates/ruff_python_codegen/src/generator.rs

@@ -1094,16 +1094,16 @@ impl<'a> Generator<'a> {
                 self.p_bytes_repr(value);
             }
             Expr::NumberLiteral(ast::ExprNumberLiteral { value, .. }) => {
+                static INF_STR: &str = "1e309";
                 assert_eq!(f64::MAX_10_EXP, 308);
-                let inf_str = "1e309";
 
                 match value {
                     ast::Number::Int(i) => {
                         self.p(&format!("{i}"));
                     }
                     ast::Number::Float(fp) => {
                         if fp.is_infinite() {
-                            self.p(inf_str);
+                            self.p(INF_STR);
                         } else {
                             self.p(&ruff_python_literal::float::to_string(*fp));
                         }
@@ -1115,7 +1115,7 @@ impl<'a> Generator<'a> {
                             format!("({real}{imag:+}j)")
                         };
                         if real.is_infinite() || imag.is_infinite() {
-                            self.p(&value.replace("inf", inf_str));
+                            self.p(&value.replace("inf", INF_STR));
                         } else {
                             self.p(&value);
                         }

crates/ruff_python_formatter/src/expression/expr_boolean_literal.rs

@@ -1,7 +1,6 @@
 use ruff_python_ast::AnyNodeRef;
 use ruff_python_ast::ExprBooleanLiteral;
 
-use crate::comments::SourceComment;
 use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
 use crate::prelude::*;
 
@@ -16,14 +15,6 @@ impl FormatNodeRule<ExprBooleanLiteral> for FormatExprBooleanLiteral {
             token("False").fmt(f)
         }
     }
-
-    fn fmt_dangling_comments(
-        &self,
-        _dangling_comments: &[SourceComment],
-        _f: &mut PyFormatter,
-    ) -> FormatResult<()> {
-        Ok(())
-    }
 }
 
 impl NeedsParentheses for ExprBooleanLiteral {