Feature #21346: Introduce `String#ensure_suffix`

• Status: Open
• Assignee: -
• Target version: -

[matheusrich (Matheus Richard)]

Problem

Ensuring a string has a specific suffix or prefix is a common operation in many applications.
Bundler itself uses it:

https://github.com/rubygems/rubygems/blob/d409ec8b5fc647fabe30e37e17cd1ea857634f6b/bundler/lib/bundler/uri_normalizer.rb#L17

Here are GitHub search queries that might find this pattern in other places:

1. for Ruby: /end(?:s)?_with\?\(['"].*['"]\) \?/ lang:ruby -is:fork
2. for Crystal (a language very similar to Ruby): /ends_with\?\(['"].*['"]\) \?/ lang:crystal -is:fork

Suggested solution

I believe Ruby would benefit from having a first-class method for this purpose.
I suggest the String#ensure_suffix and String#ensure_prefix methods.

I think these names are intuitive enough (here are 2 examples of people using ensure for this purpose (1, 2)).

I've gone ahead and implemented String#ensure_suffix in a pull request but the suggested behavior is this:

"Hell".ensure_suffix("o!")   # => "Hello!"
"Hello!".ensure_suffix("o!") # => "Hello!"

s = "Hello!"
s.ensure_suffix("!").equal?(s) # => true # returns same object if already suffixed

[matheusrich (Matheus Richard)]

If approved, I can add String#ensure_prefix and the bang versions of those methods.

[duerst (Martin Dürst)]

You say "queries that might find this pattern". That seems to say that you haven't found it yet.

What's the result for

"Hello".ensure_suffix("o!")

Is it "Helloo!", or is it "Hello!"?

[matheusrich (Matheus Richard)]

You say "queries that might find this pattern". That seems to say that you haven't found it yet.

I'm not sure what you mean. I just meant that the regex can find this pattern, but I can't guarantee it will only find that.

What's the result for

"Hello".ensure_suffix("o!")

Is it "Helloo!", or is it "Hello!"?

As currently implemented, it returns "Helloo!"

[nobu (Nobuyoshi Nakada)]

matheusrich (Matheus Richard) wrote in #note-3:

You say "queries that might find this pattern". That seems to say that you haven't found it yet.

I'm not sure what you mean. I just meant that the regex can find this pattern, but I can't guarantee it will only find that.

Maybe like this?

"Hell".sub(/(?<!o!)\z/, "o!")   #=> "Hello!"
"Hello!".sub(/(?<!o!)\z/, "o!") #=> "Hello!"
"o!Hell".sub(/(?<!o!)\z/, "o!") #=> "o!Hello!"

What's the result for

"Hello".ensure_suffix("o!")

Is it "Helloo!", or is it "Hello!"?

As currently implemented, it returns "Helloo!"

You should describe the corner case in the doc, and the test preferably.

[matheusrich (Matheus Richard)]

@nobu (Nobuyoshi Nakada) added! Thanks for the review.

[Eregon (Benoit Daloze)]

I think this would be good to add (both methods).

[matheusrich (Matheus Richard)]

Related: I've proposed both methods to Crystal, and the recently were approved and merged.

[maxfelsher (Max Felsher)]

I like this idea, but I'm concerned about the same method returning self in one case and a new string in another. It seems like it'd be easier to reason about if it either always returned a new string or always affected the original object. Or if there's a bang version, it would be consistent with chomp, sub, etc., for the non-bang version to always return a new string and the bang version to always affect the original object.

Apologies if I'm missing something!

[matz (Yukihiro Matsumoto)]

ensure_suffix accepted. IMO, it should always copy the original string, even when it ends with the suffix. I don't see any reason to add bang-version, right now. Keep it on-demand (because we are lazy).

Matz.

[Eregon (Benoit Daloze)]

What about ensure_prefix?
I think it's good to have parity here, I would be surprised if that doesn't exist but ensure_suffix does.
Also we have start_with?/end_with?, etc.

[matheusrich (Matheus Richard)]

I've updated the PR as per Matz's comment.

[ufuk (Ufuk Kayserilioglu)]

I personally find the "Hello".ensure_suffix("o!") case returning "Helloo!" very unexpected. I would have expected the change in the string to be the minimal operation needed to ensure that the string ends with the given suffix (i.e. for the result of the example to be "Hello!"), but this isn't what's happening.

@matz (Yukihiro Matsumoto) Is this the behaviour that you want?

[matheusrich (Matheus Richard)]

@ufuk (Ufuk Kayserilioglu) see dev meeting discussion

matz: "Hello".ensure_suffix("o!") should return "Helloo!"

[ufuk (Ufuk Kayserilioglu)]

matheusrich (Matheus Richard) wrote in #note-13:

@ufuk (Ufuk Kayserilioglu) see dev meeting discussion

matz: "Hello".ensure_suffix("o!") should return "Helloo!"

Ah, ok, I hadn't read the dev meeting discussion. Thanks for the pointer.

[mame (Yusuke Endoh)]

Let me record the background of the discussion. During the dev meeting, we considered the following use case:

path.ends_with?(".rb") ? path : "#{path}.rb"

When rewriting this as path.ensure_suffix(".rb"), the behavior of appending just "b" when path is "foo.r" to make it "foo.rb" is somewhat unexpected.

This is the rationale behind the idea that "Hello".ensure_suffix("o!") should return "Helloo!".

---

That said, on a personal note, I think the method name ensure_suffix is indeed confusing. As @ufuk (Ufuk Kayserilioglu) pointed out, and as @akr (Akira Tanaka) also mentioned in the meeting, it sounds like a method that makes the minimal necessary addition. I agree with you.

The code snippet path.ends_with?(".rb") ? path : "#{path}.rb" isn't particularly long, and its meaning is quite clear, so maybe we don't need this method at all.

[matheusrich (Matheus Richard)]

I think path.ends_with?(".rb") ? path : "#{path}.rb" is much more common than the version that just appends b to "foo.r". IMO we should optimize for the common operation, which is more predictable and simple as well.

[matheusrich (Matheus Richard)]

Some alternative names:

• with_suffix
• end_with: pairs well with end_with?, but might be too similar and error prone?
• add_suffix: I think it implies that the suffix is always added, which is not true.

[Eregon (Benoit Daloze)]

I think with_suffix would be a bit clearer for the implemented semantics.