The :allow_empty option is ignored when given a frozen string

[ridiculous]

This snippet should return nil, since its given a blank string and allow_empty: false

StripAttributes.strip(String(nil), allow_empty: false) # => ""

The reason is this guard against frozen strings:
https://github.com/rmm5t/strip_attributes/blob/master/lib/strip_attributes.rb#L55

[rmm5t]

Given #53, please try to convince me why you think this is a bug?

String(nil) is equivalent to "".frozen, and strip_attributes skips frozen strings.

[ridiculous]

Thanks for the response, and the context with regard to the previous issue. That's helpful.

I think it's a regression, primarily because it doesn't respect the allow_empty: false option. Which in the previous version would return nil instead of an empty string.

Secondly, it leads to unexpected behavior. Which in turn can be challenging to debug. Let me explain:

An often preferred coding style is using coercion over type checking. Which is why it'd be somewhat common to coerce the given value to the expected type, which is how we end up passing nil.to_s as the string argument.

Showing an example, I would expect these to behave similar, since they are functionally the same:

StripAttributes.strip("") # => nil
StripAttributes.strip(nil.to_s) # => ""

But as you can see, the results are different. And confusing. Since many devs may not know that nil.to_s returns a frozen string or why that would matter.

It does make sense to return the given value instead of nil. However, that breaks down when used in conjunction with the allow_empty flag, which explicitly alters this behavior. That's it's entire purpose.

Leading to this confusing behavior:

StripAttributes.strip("", allow_empty: false) # => nil <-- Correct. The flag specifies no empty values
StripAttributes.strip(nil.to_s, allow_empty: false) # => "" <-- Wrong. I specified "no empty values". this should be nil

Whether the empty string is frozen or not, doesn't matter here. It's essentially a noop. But the return value is not correct.

We need to apply the same logic as we do at the bottom of the method.

I can open a PR to update it, if you're on the same page.

[rmm5t]

I believe this was fixed by #78