[BUG] Unexpected and inconsistent handling of escaped interpolation arguments

[Bilka2]

What I tried to do

Escaped interpolation arguments in translations with %, e.g. This is %%{escaped} and not interpolated..

Also, tried to escape that escaping, e.g. This is %%%{not_escaped} and interpolated..

What I expected to happen

The first example This is %%{escaped} and not interpolated. should result in This is %{escaped} and not interpolated..

The second example This is %%%{not_escaped} and interpolated. given the substitution :not_escaped => 'foo' should result in This is %foo and interpolated.. If no substitution is given, it should result in a MissingInterpolationArgument error.

The above is based on the behaviour of sprintf:

irb(main):004> sprintf("%%{test}")
=> "%{test}"
irb(main):005> sprintf("%%%{test}")
(irb):5:in `sprintf': one hash required (ArgumentError)

sprintf("%%%{test}")
        ^^^^^^^^^^^
        from (irb):5:in `<main>'
        from <internal:kernel>:187:in `loop'
        from /usr/local/bundle/gems/irb-1.12.0/exe/irb:9:in `<top (required)>'
        from /usr/local/bundle/bin/irb:25:in `load'
        from /usr/local/bundle/bin/irb:25:in `<main>'
irb(main):006> sprintf("%%%{test}", test: "foo")
=> "%foo"

What actually happened

Various methods handle this differently, depending on whether any substitutions were provided and whether the InterpolationCompiler is used. Mostly the escape characters do not get stripped when no substitutions are provided. See tests linked below.

Versions of i18n, rails, and anything else you think is necessary

i18n 1.14.4, no Rails

Tests for the issue

master...Bilka2:i18n:escaped-interpolations-test

I commented out the lines that were failing. As you can see, the actual I18n.interpolate implementation handles everything correctly, but the various paths that call it do not. The cause for this is simple: When no substitutions are provided the string is never interpolated. See #688 (comment).

The InterpolationCompiler is only incorrect for the case with %%%. Note that one of the previous asserts in the test was incorrect.

[Bilka2]

I started looking into the MissingInterpolationArgument error and found https://github.com/ruby-i18n/i18n/blob/master/lib/i18n/tests/interpolation.rb#L6-L18, apologies for not seeing this sooner.

According to that code comment, i18n should not alter the string if no substitutions are provided, including not raising the MissingInterpolationArgument error. This invalidates all the test cases I linked above, with the exception of the incorrect handling of %%%{a} (with substitution provided) by the InterpolationCompiler backend. This means the behaviour I reported is in fact expected.

However, based on the code comment, the ReservedInterpolationKey error that led me to discover this issue should not be raised either - it was added specifically for the "no substitutions provided" case, which should not be raising interpolation errors.

I see a few ways forward that allow the usecase of i18n-tasks, which is to have a translation t("bar") that results in output such as Foo %{scope}:

• A) #678 is reverted to keep in line with the code comment and the behaviour of the MissingInterpolationArgument error.
• B) The changes from #678 are adjusted to only potentially raise ReservedInterpolationKey when only reserved keywords are provided, so when options is a subset of RESERVED_KEYS.
  This allows t("bar") but raises in cases like t("bar", scope: "baz"). This still solves the original issue raised with Rails but allows the i18n-tasks usecase.
• C) The error is kept as is, #688 is merged, nothing else is changed.
  This allows to work around this issue by escaping the interpolation and adding a dummy substitution to the translation to force the escape character to be removed. Escaped interpolations will still not result in the desired string if no substitution is provided.
• D) Strings with no substitutions are changed to: Escape interpolations and raise MissingInterpolationArgument.
  This allows to use escaped interpolations of reserved keywords without worrying about providing other substitutions. Raising more errors may break existing applications in the same way that the extra ReservedInterpolationKey broke i18n-tasks. This completely disagrees with the linked code comment.

[radar]

I think C is the ticket here. It won't break upstream Rails, and it will fix the underlying issue.

[radar]

Fixed by #688.