[BUG] Index out of range during Singularize

[ArcturusZhang]

When I execute

var result = "S".Singularize();

it throws this exception:

Exception: Index was outside the bounds of the array.
   at System.String.get_Chars(Int32 index)
   at Humanizer.Inflections.Vocabulary.MatchUpperCase(String word, String replacement) in /_/src/Humanizer/Inflections/Vocabulary.cs:line 158
   at Humanizer.Inflections.Vocabulary.Singularize(String word, Boolean inputIsKnownToBePlural, Boolean skipSimpleWords) in /_/src/Humanizer/Inflections/Vocabulary.cs:line 104
   at Humanizer.InflectorExtensions.Singularize(String word, Boolean inputIsKnownToBePlural, Boolean skipSimpleWords) in /_/src/Humanizer/InflectorExtensions.cs:line 54

[twwilliams]

I have been looking at this one, and the problem starts when this rule:

_default.AddSingular("s$", "");

is run in ApplyRules in Inflections/Vocabularies.cs.

This leaves result as the empty string, but not null, so that MatchUpperCase(word, result) is called, and MatchUpperCase() assumes that not only are word and replacement not null, but that they are not the empty string, either. The problem only happens with "S" and not "s" because the lowercase character short-circuits the first part of the conditional check char.IsUpper(word[0]) && char.IsLower(replacement[0]) so there is no attempt to index into replacement which is empty.

I'm not sure if the better approach is to change the conditional in ApplyRules() to be !string.IsNullOrEmpty(result) or to put a guard clause in MatchUpperCase() to protect against the empty string.

I am also not sure what the test case should look like. I triggered the problem by adding yield return new object[] { "S", "S" }; to the PluralTestSource class but I'm not sure if that's the right result for "S" or not since the lowercase "s" returns the empty string.

And if the empty string is the right result, I don't (yet) know how to put that into the test structure so there is a regression test for this issue.

[clairernovotny]

For the specific case of "S".Singularize(), it would seem like "S" is the correct result. As a general rule, I'm suspecting that single letters can't be singularized anyway, so potentially check the length and short-circuit?

A PR with a test and fix would be appreciated!

[ArcturusZhang]

I believe it makes sense to just return myself if myself is only a single character when we are Singularizing. We worked this around by adding uncountable rule to the single character S.

[neilboyd]

What should the plural of a single letter be? Sometimes it makes sense to add 's, for example x's, but this depends on the context, for example a's makes sense for the letter a but not for the word a.
This is relevant because plural is the opposite of singular, as defined in the test cases.

Probably we should keep it as the default case of adding s to pluralize and removing s to singularize, but handle the special case of singularizing s/S.

As far as I can see, Singularize and Pluralize are only supported for English, so we don't need to consider other languages.