ShortScalarCastFixer - Change binary cast to string cast as well

[ntzm]

Not sure if this fixer is the best place for this

[keradus]

imo a feature, thus 2.12

[julienfalque]

The default behavior of the fixer is changing, shouldn't we add an option?

[ntzm]

Agreed, will update shortly

[keradus]

👎 for option, but I agree it's always fuzzy
it's not that we now change possible behaviour, we simply fix more cases, imo it;s good without option
we simply fix more cases, "rule" remains the same, "fixer" that implements it implement it more completely.

@julienfalque , do you remember #3445 ? it was not just refactoring, it was refactoring to cover (detect & fix) more edge cases. but we didn't introduce any option shall newly detected cases be fixed or not, we just took it as good coin - we fix more.

[julienfalque]

#3445 is not a good example IMO as it was a bugfix (it didn't aim to cover more cases, it aimed to fix cases that didn't work as they should have). I think a better example would be #3478. But yes, it's really fuzzy. 👍 for tweaking the default behavior directly.

[keradus]

in #3445 we had to adjust existing unittests to follow new impelemntation, thus it was "behaviuor change", but ultimately not "rule change"

#3478 is good example as well ;)

[ntzm]

So keep this PR as-is?

[julienfalque]

Yes, sorry if you started working on something :/

Travis is failing on 7.2 but I'm not sure this is related to changes from this PR.

[ntzm]

Nope I didn't no worries! Thanks

[SpacePossum]

The test failed because of a time out.
Ignore my the rest of my old comment, failed to read the test script...

[keradus]

Thank you @ntzm.