feat: introduce string_implicit_backslashes as escape_implicit_backslashes replacement

[mvorisek]

fix #7621

if backslashes are disabled to be explicitly escaped, they must be unescaped

[mvorisek]

PR is done. Tested with

'escape_implicit_backslashes' => ['double_quoted' => false, 'heredoc_syntax' => false, 'single_quoted' => true],

configuration on the whole project, ie. the inverse configuration. Once removed and fixer is run again, all code returned to exactly the same version as before.

[mvorisek]

Hi @Wirone, can you please review this PR?

[Wirone]

I hope to do some reviews in the late evening 🤞.

[mvorisek]

@Wirone is now a good late evening?

[julienfalque]

Shouldn't this new behavior be behind an option?

[Wirone]

@Wirone is now a good late evening?

Hah, with 3 kids you can plan and end with totally different reality 😅. Today I am going on business trip for 2 days, many hours in the train, so I again plan to do some reviews 😉.

[mvorisek]

Shouldn't this new behavior be behind an option?

What about unescape: bool option?

It is easy to add, but I am a little worried to whom it will be helpful in reality.

[Wirone]

To be honest, I don't know what to do with this PR 🤷‍♂️. In general, fixers are named after the state desired after fix is applied, in this case: all implicit backslashes should be escaped. It's clear that we can't change the current behaviour just like that, it must be introduced behind configuration option. But even if we do this, the idea behind fixer under this particular name would be broken, so in order to do it properly we should introduce new fixer, e.g. implicit_backslashes and modify escape_implicit_backslashes to be only proxy fixer to the new one, with proper config. I believe the options double_quoted, single_quoted and heredoc_syntax should be a three-state: ?bool or dictionary-like escape|unescape|ignore, in order to achieve both strategies and to be able to leave code as-is.

But it's only my perspective, I believe we need @keradus' opinion.

[mvorisek]

I like the 3-state concept and new fixer name - is there any "proxy migration fixer PR" I can as a reference?

Other options is to keep this PR as is, I do not see any value in not unescaping implicit backslashes when they are not fixed to the oposite.

[Wirone]

You can take a look at #7053, but I would recommend waiting for @keradus' opinion, as I won't merge it anyway without his decision about it 🙂.

[julienfalque]

I also like @Wirone's proposal and agree the current name of the fixer might not be very accurate with this new behavior.

I do think unescaping should be the default behavior but that might be an unexpected change for users, hence the option. We might reconsider this when we'll prepare a new major version.

[Wirone]

I would like to see small improvement in tests, other than that - looks good.

[Wirone]

FYI: diff between new fixer and EscapeImplicitBackslashesFixerTest (not counting supported options, docs etc):

[Wirone]

Thank you @mvorisek 🍻!