# Should there be a fallible version of \`String::replace\_range\`? **URL:** \ **Category:** libs\ **Created:** [February 13, 2023, 8:35pm UTC](https://internals.rust-lang.org/t/should-there-be-a-fallible-version-of-string-replace-range/18343 "2023-02-13T20:35:12Z")\ **Posts on this page:** 9\ **Page:** 1
**Author:** ![zackw](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/zackw/32/2071_2.png) [@zackw](https://internals.rust-lang.org/u/zackw)\ **Post date:** [February 13, 2023, 8:35pm UTC](https://internals.rust-lang.org/t/should-there-be-a-fallible-version-of-string-replace-range/18343/1 "2023-02-13T20:35:12Z")
Prompted by [this rust-analyzer bug](https://github.com/rust-lang/rust-analyzer/issues/13709): apparently it is difficult for rust-analyzer to report certain classes of protocol errors in any way other by panic-ing, because the error is detected by `String::replace_range`, which panics on invalid offsets, and has no fallible version. Abstractly one _could_ hand-validate offsets coming from outside the program up front, but, as pointed out near the bottom of the bug, this involves doing duplicate work, and I imagine it would be easy to get wrong. What do you think of adding a fallible version, presumably with the signature ```rust pub fn try_replace_range(&mut self, range: R, replace_with: &str) -> Result<(), InvalidRangeError> ``` Exactly the same semantics, but returns InvalidRangeError where the existing function panics. ---
**Author:** ![matklad](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/matklad/32/12266_2.png) [@matklad](https://internals.rust-lang.org/u/matklad)\ **Post date:** [February 13, 2023, 8:46pm UTC](https://internals.rust-lang.org/t/should-there-be-a-fallible-version-of-string-replace-range/18343/2 "2023-02-13T20:46:12Z")
It’s actually would not be difficult to report for rust-analyzer: one can use get ([String in std::string - Rust](https://doc.rust-lang.org/stable/std/string/struct.String.html#method.get)) to check if the range is valid, and then bubble the error up. In general, indexing APIs in Rust idiomatically panic on invalid indexes, so I don’t think any change is necessary here. For some context, historically rust-analyzer used to deliberately and loudly crash for any kind of invalid input. I stand behind that design decision, as it allowed us to discover and fix numerous bugs across rust-analyzer, official VS Code LSP library and other clients. Roughly when we started the process of making rust-analyzer official, we downgraded this strategy to more classical “gracefully handle invalid inputs” for more or less every API except `textDocument/didChange`. This one doesn’t have a reasonable recovery strategy, and would completely break any _other_ feature. Really, here the client and server just have to agree on the protocol. So in the current implementation we continue to deliberately crash. The best solution here is to send a PR to specific client libraries which send invalid offsets. EDIT: this was worded in a rather unhelpful way, sorry about that, rephrased a bit! ---
**Author:** ![zackw](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/zackw/32/2071_2.png) [@zackw](https://internals.rust-lang.org/u/zackw)\ **Post date:** [February 13, 2023, 9:25pm UTC](https://internals.rust-lang.org/t/should-there-be-a-fallible-version-of-string-replace-range/18343/3 "2023-02-13T21:25:20Z")
Let's keep the question of whether rust-analyzer _should_ panic, in this instance, over on the rust-analyzer issue tracker 🙂 (I admit I'm being pretty dogmatic about it over there.) Here, I'd like to focus discussion on (1) whether anyone _besides_ rust-analyzer could potentially make use of a `try_replace_range` API, and (2) whether there are good alternatives in the stdlib today, and how error-prone they are in practice. On that second note, it seems to me that recommending `.get` to validate offsets might lead people into a trap where they forget that _both_ ends of the range need to be validated. ---
**Author:** ![matklad](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/matklad/32/12266_2.png) [@matklad](https://internals.rust-lang.org/u/matklad)\ **Post date:** [February 13, 2023, 9:28pm UTC](https://internals.rust-lang.org/t/should-there-be-a-fallible-version-of-string-replace-range/18343/4 "2023-02-13T21:28:49Z")
`get` works with ranges as well: > **[Rust Playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=58698e461471f1bac20edc8ac8719e71)** > > A browser interface to the Rust compiler to experiment with the language ---
**Author:** ![programmerjake](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/programmerjake/32/5893_2.png) [@programmerjake](https://internals.rust-lang.org/u/programmerjake)\ **Post date:** [February 14, 2023, 9:34am UTC](https://internals.rust-lang.org/t/should-there-be-a-fallible-version-of-string-replace-range/18343/5 "2023-02-14T09:34:33Z")
another function I noticed was missing is `replace_range_from_within`, ... essentially `s.replace_range(target_range, &s[source_range].to_string())` but without the temporary string ---
**Author:** ![dlight](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/dlight/32/8462_2.png) [@dlight](https://internals.rust-lang.org/u/dlight)\ **Post date:** [February 14, 2023, 1:40pm UTC](https://internals.rust-lang.org/t/should-there-be-a-fallible-version-of-string-replace-range/18343/6 "2023-02-14T13:40:07Z")
A way to frame this question is to think if this API didn't exist, what's the better addition, the panicking version or the fallible version? I think that the only way to justify the panicking version is if both inputs are known at compile time or somehow all instances of invalid offsets are known at compile time (probably because the offset was taken from processing the string itself). In that case, panics are definitely a programmer error and the panicking version is warranted. Otherwise, even if you want to bail on error, using the fallible version (and maybe unwrapping it) makes way more sense. So if both existed I think that the fallible version would be way more useful ---
**Author:** ![eggyal](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/eggyal/32/7947_2.png) [@eggyal](https://internals.rust-lang.org/u/eggyal)\ **Post date:** [February 14, 2023, 4:04pm UTC](https://internals.rust-lang.org/t/should-there-be-a-fallible-version-of-string-replace-range/18343/7 "2023-02-14T16:04:34Z")
It's also possible that you have already established at runtime that the offsets are valid, but I agree such occasions are likely to be in the minority. ---
**Author:** ![scottmcm](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/scottmcm/32/2355_2.png) [@scottmcm](https://internals.rust-lang.org/u/scottmcm)\ **Post date:** [February 14, 2023, 7:50pm UTC](https://internals.rust-lang.org/t/should-there-be-a-fallible-version-of-string-replace-range/18343/8 "2023-02-14T19:50:41Z")
> [@dlight](#): > > what's the better addition, the panicking version or the fallible version? Well, my usual question is where would someone usually get the indexes from? My instinct is that you get the indexes from doing some kind of search on the string, in which case it's already a previous logic bug if the index isn't valid, at which point the panicking version is better than making almost everyone just `.unwrap()` it because _of course_ the index they got from `regex` or whatever is correct. In general, indexes aren't just summoned from the æther -- aka they have an implied expectation that they're valid -- so I lean towards the panicking versions. Indexes are very different that way from fundamentally-not-precheckable things like paths. ---
**Author:** ![system](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/system/32/14092_2.png) [@system](https://internals.rust-lang.org/u/system)\ **Post date:** [May 15, 2023, 7:50pm UTC](https://internals.rust-lang.org/t/should-there-be-a-fallible-version-of-string-replace-range/18343/9 "2023-05-15T19:50:45Z")
Closed on May 15, 2023, 7:50pm UTC This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.