Improve API exposed on ExprStringLiteral nodes#16192
Conversation
AlexWaygood
requested review from
MichaReiser,
carljm,
dhruvmanila and
sharkdp
as code owners
AlexWaygood
force-pushed
the
alex/contents-range
branch
from
3d3ce4f to
a25e293
Compare
|
| impl ExprStringLiteral { | ||
| /// Return `Some(literal)` if the string only consists of a single `StringLiteral` part | ||
| /// (indicating that it is not implicitly concatenated). Otherwise, return `None`. | ||
| pub fn as_unconcatenated_literal(&self) -> Option<&StringLiteral> { |
There was a problem hiding this comment.
Nit: I wonder if it should be as_only_literal to align with first_literal but I'm fine with either.
There was a problem hiding this comment.
I think I'd prefer to use as_single_literal / as_only_literal instead as reading unconcatenated and matching against Single confused me at the first glance
There was a problem hiding this comment.
I weakly prefer the current name as I think it makes it clearer that the method is used to differentiate between strings that are implicitly concatenated and ones that aren't
There was a problem hiding this comment.
Oh shoot, I didn't see Dhruv's message until after I merged — sorry. I'll rename it as a followup, since you both dislike the current name!
| impl ExprStringLiteral { | ||
| /// Return `Some(literal)` if the string only consists of a single `StringLiteral` part | ||
| /// (indicating that it is not implicitly concatenated). Otherwise, return `None`. | ||
| pub fn as_unconcatenated_literal(&self) -> Option<&StringLiteral> { |
There was a problem hiding this comment.
I think I'd prefer to use as_single_literal / as_only_literal instead as reading unconcatenated and matching against Single confused me at the first glance
* main: (60 commits) [`refurb`] Manual timezone monkeypatching (`FURB162`) (#16113) [`pyupgrade`] Do not upgrade functional TypedDicts with private field names to the class-based syntax (`UP013`) (#16219) Improve docs for PYI019 (#16229) Refactor `CallOutcome` to `Result` (#16161) Fix minor punctuation errors (#16228) Include document specific debug info (#16215) Update server to return the debug info as string (#16214) [`airflow`] Group `ImportPathMoved` and `ProviderName` to avoid misusing (`AIR303`) (#16157) Fix unstable formatting of trailing end-of-line comments of parenthesized attribute values (#16187) Ignore source code actions for a notebook cell (#16154) Add FAQ entry for `source.*` code actions in Notebook (#16212) red-knot: move symbol lookups in `symbol.rs` (#16152) better error messages while loading configuration `extend`s (#15658) Format `index.css` (#16207) Improve API exposed on `ExprStringLiteral` nodes (#16192) Update Rust crate tempfile to v3.17.0 (#16202) Update cloudflare/wrangler-action action to v3.14.0 (#16203) Update NPM Development dependencies (#16199) Update Rust crate smallvec to v1.14.0 (#16201) Update Rust crate codspeed-criterion-compat to v2.8.0 (#16200) ...
3 participants
Summary
This PR makes the following changes:
ast::StringLiteral::contents_range()method that was introduced in Reduce memory usage ofDocstringstruct #16183. This is less verbose and more type-safe than using theast::str::raw_contents()helper function.ast::ExprStringLiteral::as_unconcatenated_literal()helper method, and adjusts various callsites to use it. This addresses @MichaReiser's review comment at Reduce memory usage ofDocstringstruct #16183 (comment). There is no functional change here, but it helps readability to make it clearer that we're differentiating between implicitly concatenated strings and unconcatenated strings at various points.StringLiteralValue::flags()method toStringLiteralFlags::first_literal_flags(). If you're dealing with an implicitly concatenated stringstring_node,string_node.value.flags().closer_len()could give an incorrect result; this renaming makes it clearer that theStringLiteralFlagsinstance returned by the method is only guaranteed to give accurate information for the firstStringLiteralcontained in theExprStringLiteralnode.BytesLiteralValue::flags()method. This seems prone to misuse in the same way asStringLiteralValue::flags(): if it's an implicitly concatenated bytestring, theBytesLiteralFlagsinstance returned by the method would only give accurate information for the firstBytesLiteralin the bytestring.Test Plan
cargo test