EmptyLiteral autocorrector respects StringLiterals/EnforcedStyle config

[DNNX]

Before this PR it would replace String.new with '' even if double-quoted string were preferred.

Fixes #2593

[jonas054]

Looks correct, but do we need it? StringLiterals will auto-correct '' to "" in the same run, so from the user's perspective, there's no change. (Unless StringLiterals is disabled, in which case it wouldn't be wrong to respect that decision.)

You forgot to update CHANGELOG, by the way.

[DNNX]

StringLiterals will auto-correct '' to "" in the same run

Hmm, is it because StringLiterals always runs after EmptyLiteral? What determines the order of the cops? Their position in the config file? Or they are always executed in alphabetical order?

[alexdowad]

Hmm, is it because StringLiterals always runs after EmptyLiteral?

Auto-correction goes in a loop until there is nothing left to correct.

[alexdowad]

Looks correct, but do we need it? StringLiterals will auto-correct '' to "" in the same run, so from the user's perspective, there's no change.

From a performance perspective, it's much better to avoid creating new offenses.

[DNNX]

While we're at it. Do I need to check if StringLiterals is enabled here? Is it possible that config.for_cop('Style/StringLiterals') returns nil or raises an error?

[alexdowad]

Is it possible that config.for_cop('Style/StringLiterals') returns nil or raises an error?

Shouldn't happen.

[jonas054]

From a performance perspective, it's much better to avoid creating new offenses.

True, but performance isn't everything. There's also simplicity, clarity, and generality.

[alexdowad]

True, but performance isn't everything. There's also simplicity, clarity, and generality.

Definitely, performance is not everything. But personally, I don't feel that this patch sacrifices anything in clarity. And we have a serious, serious problem with autocorrection performance right now. We have an open issue for this right now -- #2047.

[bbatsov]

I'm ok with the proposed change, provided it gets a proper changelog entry. My only concern are the growing number of inter-cop deps. Maybe some preferences (like string literals, etc) should eventually be moved to a more generic location. At any rate - this is not very important right now.

[DNNX]

CI green, no conflicts