From 4803135ba4c7e1314bc0e8ccdce81e3a7fa5ca0c Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 27 Jan 2025 15:39:22 -0500 Subject: [PATCH 01/10] use quote from each bytes_literal --- crates/ruff_python_codegen/src/generator.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index 410a6ef68c46e..a4f507f657d6d 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -153,8 +153,8 @@ impl<'a> Generator<'a> { self.p(s.as_str()); } - fn p_bytes_repr(&mut self, s: &[u8]) { - let escape = AsciiEscape::with_preferred_quote(s, self.quote); + fn p_bytes_repr(&mut self, s: &[u8], quote: Quote) { + let escape = AsciiEscape::with_preferred_quote(s, quote); if let Some(len) = escape.layout().len { self.buffer.reserve(len); } @@ -1100,7 +1100,7 @@ impl<'a> Generator<'a> { let mut first = true; for bytes_literal in value { self.p_delim(&mut first, " "); - self.p_bytes_repr(&bytes_literal.value); + self.p_bytes_repr(&bytes_literal.value, bytes_literal.flags.quote_style()); } } Expr::NumberLiteral(ast::ExprNumberLiteral { value, .. }) => { From 7eec20e0df42513b516fd17fd2d7859859f7226a Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 27 Jan 2025 16:04:12 -0500 Subject: [PATCH 02/10] add BytesLiteralValue::flags --- crates/ruff_python_ast/src/nodes.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index 1d4d9c78b6c6a..9999bd03360d2 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -1714,6 +1714,16 @@ impl BytesLiteralValue { pub fn bytes(&self) -> impl Iterator + '_ { self.iter().flat_map(|part| part.as_slice().iter().copied()) } + + /// Returns the [`BytesLiteralFlags`] associated with this literal. + /// + /// For an implicitly concatenated literal, it returns the flags for the first literal. + pub fn flags(&self) -> BytesLiteralFlags { + self.iter() + .next() + .expect("There should always be at least one literal in an `ExprBytesLiteral` node") + .flags + } } impl<'a> IntoIterator for &'a BytesLiteralValue { From 431d3329fdb6f6af900dbe8d231201ee13b5b7f8 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 27 Jan 2025 16:04:24 -0500 Subject: [PATCH 03/10] add Checker::default_bytes_flags --- crates/ruff_linter/src/checkers/ast/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index b9f1e2bdf77d2..7e5ae5863da31 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -313,6 +313,12 @@ impl<'a> Checker<'a> { ast::StringLiteralFlags::empty().with_quote_style(self.preferred_quote()) } + /// Return the default bytestring flags a generated `ByteStringLiteral` node should use, given + /// where we are in the AST. + pub(crate) fn default_bytes_flags(&self) -> ast::BytesLiteralFlags { + ast::BytesLiteralFlags::default().with_quote_style(self.preferred_quote()) + } + /// Returns the appropriate quoting for f-string by reversing the one used outside of /// the f-string. /// From 459a09a27b423ca4abf56e0257b85c5d96b62bf2 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 27 Jan 2025 16:09:36 -0500 Subject: [PATCH 04/10] pass Checker to get byte flags too --- .../src/rules/pyupgrade/rules/native_literals.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs index 32b1a8152731c..42ffe9cc9331a 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs @@ -3,7 +3,7 @@ use std::str::FromStr; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; -use ruff_python_ast::{self as ast, Expr, Int, LiteralExpressionRef, StringLiteralFlags, UnaryOp}; +use ruff_python_ast::{self as ast, Expr, Int, LiteralExpressionRef, UnaryOp}; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; @@ -33,15 +33,20 @@ impl FromStr for LiteralType { } impl LiteralType { - fn as_zero_value_expr(self, flags: StringLiteralFlags) -> Expr { + fn as_zero_value_expr(self, checker: &Checker) -> Expr { match self { LiteralType::Str => ast::StringLiteral { value: Box::default(), range: TextRange::default(), - flags, + flags: checker.default_string_flags(), + } + .into(), + LiteralType::Bytes => ast::BytesLiteral { + value: Box::default(), + range: TextRange::default(), + flags: checker.default_bytes_flags(), } .into(), - LiteralType::Bytes => ast::ExprBytesLiteral::default().into(), LiteralType::Int => ast::ExprNumberLiteral { value: ast::Number::Int(Int::from(0u8)), range: TextRange::default(), @@ -191,7 +196,7 @@ pub(crate) fn native_literals( return; } - let expr = literal_type.as_zero_value_expr(checker.default_string_flags()); + let expr = literal_type.as_zero_value_expr(checker); let content = checker.generator().expr(&expr); diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( content, From c2ad78a5faeb18dc57f4d7e785e9ea3894f62505 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 27 Jan 2025 16:13:31 -0500 Subject: [PATCH 05/10] accept new bytes round trip --- crates/ruff_python_codegen/src/generator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index a4f507f657d6d..6910f4fa36106 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1743,7 +1743,7 @@ class Foo: assert_round_trip!(r"'hello'"); assert_round_trip!(r"u'hello'"); assert_round_trip!(r"r'hello'"); - assert_eq!(round_trip(r"b'hello'"), r#"b"hello""#); + assert_round_trip!(r"b'hello'"); assert_eq!(round_trip(r#"("abc" "def" "ghi")"#), r#""abc" "def" "ghi""#); assert_eq!(round_trip(r#""he\"llo""#), r#"'he"llo'"#); assert_eq!(round_trip(r#"f"abc{'def'}{1}""#), r#"f"abc{'def'}{1}""#); From a13ab8e4cdef68b535e4859d5c090ee8837b336d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 27 Jan 2025 16:14:15 -0500 Subject: [PATCH 06/10] use assert_round_trip on existing test --- crates/ruff_python_codegen/src/generator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index 6910f4fa36106..875db97ae238b 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1739,7 +1739,7 @@ class Foo: #[test] fn quote() { - assert_eq!(round_trip(r#""hello""#), r#""hello""#); + assert_round_trip!(r#""hello""#); assert_round_trip!(r"'hello'"); assert_round_trip!(r"u'hello'"); assert_round_trip!(r"r'hello'"); From 074b325301c8ca2bd4d3f3440492b5b18277b2e3 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 27 Jan 2025 16:19:00 -0500 Subject: [PATCH 07/10] update set_quote test for bytestrings; add two-arg macro variant --- crates/ruff_python_codegen/src/generator.rs | 27 ++++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index 875db97ae238b..fb10809b03789 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1806,25 +1806,28 @@ if True: $end, ); }; + ($quote:expr, $start:expr) => { + round_trip_with!($quote, $start, $start); + }; } - // setting Generator::quote works for bytestrings - round_trip_with!(Quote::Double, r#"b"hello""#, r#"b"hello""#); - round_trip_with!(Quote::Single, r#"b"hello""#, r"b'hello'"); - round_trip_with!(Quote::Double, r"b'hello'", r#"b"hello""#); - round_trip_with!(Quote::Single, r"b'hello'", r"b'hello'"); - - // and for f-strings + // setting Generator::quote works for f-strings round_trip_with!(Quote::Double, r#"f"hello""#, r#"f"hello""#); round_trip_with!(Quote::Single, r#"f"hello""#, r"f'hello'"); round_trip_with!(Quote::Double, r"f'hello'", r#"f"hello""#); round_trip_with!(Quote::Single, r"f'hello'", r"f'hello'"); - // but not for string literals, where the `Quote` is taken directly from their flags - round_trip_with!(Quote::Double, r#""hello""#, r#""hello""#); - round_trip_with!(Quote::Single, r#""hello""#, r#""hello""#); // no effect - round_trip_with!(Quote::Double, r"'hello'", r#"'hello'"#); // no effect - round_trip_with!(Quote::Single, r"'hello'", r"'hello'"); + // but not for bytestrings + round_trip_with!(Quote::Double, r#"b"hello""#); + round_trip_with!(Quote::Single, r#"b"hello""#); + round_trip_with!(Quote::Double, r"b'hello'"); + round_trip_with!(Quote::Single, r"b'hello'"); + + // or for string literals, where the `Quote` is taken directly from their flags + round_trip_with!(Quote::Double, r#""hello""#); + round_trip_with!(Quote::Single, r#""hello""#); // no effect + round_trip_with!(Quote::Double, r"'hello'"); // no effect + round_trip_with!(Quote::Single, r"'hello'"); } #[test] From aa35e2c29347768ed9f49a7f039599d8c731ece1 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 27 Jan 2025 16:34:33 -0500 Subject: [PATCH 08/10] copy docs from StringLiteral and its flags; remove default impls --- crates/ruff_linter/src/checkers/ast/mod.rs | 2 +- crates/ruff_python_ast/src/nodes.rs | 41 +++++++++++++------ .../ruff_python_formatter/tests/normalizer.rs | 2 +- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 7e5ae5863da31..641f9575eb462 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -316,7 +316,7 @@ impl<'a> Checker<'a> { /// Return the default bytestring flags a generated `ByteStringLiteral` node should use, given /// where we are in the AST. pub(crate) fn default_bytes_flags(&self) -> ast::BytesLiteralFlags { - ast::BytesLiteralFlags::default().with_quote_style(self.preferred_quote()) + ast::BytesLiteralFlags::empty().with_quote_style(self.preferred_quote()) } /// Returns the appropriate quoting for f-string by reversing the one used outside of diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index 9999bd03360d2..c9d71ea0e4787 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -1634,14 +1634,14 @@ impl Debug for ConcatenatedStringLiteral { /// An AST node that represents either a single bytes literal or an implicitly /// concatenated bytes literals. -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub struct ExprBytesLiteral { pub range: TextRange, pub value: BytesLiteralValue, } /// The value representing a [`ExprBytesLiteral`]. -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub struct BytesLiteralValue { inner: BytesLiteralValueInner, } @@ -1781,12 +1781,6 @@ enum BytesLiteralValueInner { Concatenated(Vec), } -impl Default for BytesLiteralValueInner { - fn default() -> Self { - Self::Single(BytesLiteral::default()) - } -} - bitflags! { #[derive(Default, Copy, Clone, PartialEq, Eq, Hash)] struct BytesLiteralFlagsInner: u8 { @@ -1815,10 +1809,33 @@ bitflags! { /// Flags that can be queried to obtain information /// regarding the prefixes and quotes used for a bytes literal. -#[derive(Default, Copy, Clone, Eq, PartialEq, Hash)] +/// +/// ## Notes on usage +/// +/// If you're using a `Generator` from the `ruff_python_codegen` crate to generate a lint-rule fix +/// from an existing string literal, consider passing along the [`BytesLiteral::flags`] field or the +/// result of the [`BytesLiteralValue::flags`] method. If you don't have an existing string but have +/// a `Checker` from the `ruff_linter` crate available, consider using +/// `Checker::default_bytes_flags` to create instances of this struct; this method will properly +/// handle surrounding f-strings. For usage that doesn't fit into one of these categories, the +/// public constructor [`BytesLiteralFlags::empty`] can be used. +#[derive(Copy, Clone, Eq, PartialEq, Hash)] pub struct BytesLiteralFlags(BytesLiteralFlagsInner); impl BytesLiteralFlags { + /// Construct a new [`BytesLiteralFlags`] with **no flags set**. + /// + /// See [`BytesLiteralFlags::with_quote_style`], [`BytesLiteralFlags::with_triple_quotes`], and + /// [`BytesLiteralFlags::with_prefix`] for ways of setting the quote style (single or double), + /// enabling triple quotes, and adding prefixes (such as `r`), respectively. + /// + /// See the documentation for [`BytesLiteralFlags`] for additional caveats on this constructor, + /// and situations in which alternative ways to construct this struct should be used, especially + /// when writing lint rules. + pub fn empty() -> Self { + Self(BytesLiteralFlagsInner::empty()) + } + #[must_use] pub fn with_quote_style(mut self, quote_style: Quote) -> Self { self.0 @@ -1904,7 +1921,7 @@ impl fmt::Debug for BytesLiteralFlags { /// An AST node that represents a single bytes literal which is part of an /// [`ExprBytesLiteral`]. -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub struct BytesLiteral { pub range: TextRange, pub value: Box<[u8]>, @@ -1930,7 +1947,7 @@ impl BytesLiteral { Self { range, value: Box::new([]), - flags: BytesLiteralFlags::default().with_invalid(), + flags: BytesLiteralFlags::empty().with_invalid(), } } } @@ -2183,7 +2200,7 @@ impl From for BytesLiteralFlags { value.prefix() ) }; - let new = BytesLiteralFlags::default() + let new = BytesLiteralFlags::empty() .with_quote_style(value.quote_style()) .with_prefix(bytestring_prefix); if value.is_triple_quoted() { diff --git a/crates/ruff_python_formatter/tests/normalizer.rs b/crates/ruff_python_formatter/tests/normalizer.rs index c2b3fb76da812..c10ed93720b73 100644 --- a/crates/ruff_python_formatter/tests/normalizer.rs +++ b/crates/ruff_python_formatter/tests/normalizer.rs @@ -97,7 +97,7 @@ impl Transformer for Normalizer { bytes.value = ast::BytesLiteralValue::single(ast::BytesLiteral { value: bytes.value.bytes().collect(), range: bytes.range, - flags: BytesLiteralFlags::default(), + flags: BytesLiteralFlags::empty(), }); } } From 664eac4906e1bc006e2d95f44ba75dbb187f3ca1 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 27 Jan 2025 16:52:27 -0500 Subject: [PATCH 09/10] missed strings --- crates/ruff_python_ast/src/nodes.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index c9d71ea0e4787..894ad649e8bf9 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -1813,9 +1813,9 @@ bitflags! { /// ## Notes on usage /// /// If you're using a `Generator` from the `ruff_python_codegen` crate to generate a lint-rule fix -/// from an existing string literal, consider passing along the [`BytesLiteral::flags`] field or the -/// result of the [`BytesLiteralValue::flags`] method. If you don't have an existing string but have -/// a `Checker` from the `ruff_linter` crate available, consider using +/// from an existing bytes literal, consider passing along the [`BytesLiteral::flags`] field or the +/// result of the [`BytesLiteralValue::flags`] method. If you don't have an existing literal but +/// have a `Checker` from the `ruff_linter` crate available, consider using /// `Checker::default_bytes_flags` to create instances of this struct; this method will properly /// handle surrounding f-strings. For usage that doesn't fit into one of these categories, the /// public constructor [`BytesLiteralFlags::empty`] can be used. From 7c18b1d0eb7ecc6cebef984ab8f3a4e9ca6786a7 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 27 Jan 2025 16:55:06 -0500 Subject: [PATCH 10/10] mirror string comments --- crates/ruff_python_codegen/src/generator.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index fb10809b03789..308fc9c22d68d 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1819,8 +1819,8 @@ if True: // but not for bytestrings round_trip_with!(Quote::Double, r#"b"hello""#); - round_trip_with!(Quote::Single, r#"b"hello""#); - round_trip_with!(Quote::Double, r"b'hello'"); + round_trip_with!(Quote::Single, r#"b"hello""#); // no effect + round_trip_with!(Quote::Double, r"b'hello'"); // no effect round_trip_with!(Quote::Single, r"b'hello'"); // or for string literals, where the `Quote` is taken directly from their flags