prefer_adjacent_string_concatenation should not warn for list elements

[zoechi]

If I remove +, no_adjacent_strings_in_list warns instead

Dart VM version: 2.1.0-dev.6.0 (Thu Sep 27 09:51:34 2018 +0200) on "macos_x64"

[MichaelRFairhurst]

Maybe ()s would be better style here?

…

On Wed, Oct 3, 2018 at 11:11 AM Günter Zöchbauer ***@***.***> wrote: [image: image] <https://user-images.githubusercontent.com/405837/46430021-34ab1480-c748-11e8-8dfc-ec8155f0ca47.png> If I remove +, no_adjacent_strings_in_list warns instead — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#57796>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABjWe9broiEfApRPFBZD59wAleqqzfC7ks5uhP23gaJpZM4XGk57> .

[bwilkerson]

This is an interesting case. We have two rules that conflict in one specific scenario. I agree that in this case it makes more sense to change prefer_adjacent_string_concatenation, but it isn't clear to me whether the behavior should only be changed when both rules are enabled or to make the change unconditional. Unconditionally changing the behavior will make the rule's behavior more consistent and predictable, but means that we'll be missing cases where a lint could reasonably be expected by users that only enable that one rule.

@pq Interested to hear your thoughts.

[pq]

In general, I think it'd be interesting to explore special cases where some rules might be mostly but not completely compatible (currently we only have the big hammer of tagging complete incompatibility). For this case we could probably make the change unconditionally --- I can't really imagine anyone wanting adjacent strings in lists [1]. That said, checking for the enablement of no_adjacent_strings_in_list could be done by accession options via the LinterContext.

context.analysisOptions.lintRules
        .map((lint) => lint.name)
        .contains('no_adjacent_strings_in_list');

(Untested.)

I guess I'm on the fence.

Whatever we do, it'd be good to update the rule docs to describe this nuance.

[1] I had to stop typing this to look and see and I admit I'm surprised to not see no_adjacent_strings_in_list in the core or recommended rulesets. @natebosch @munificent @jakemac53 @goderbauer : can any of you recall why we didn't?

[jakemac53]

There is likely a performance cost to concatenating strings instead of using adjacent string literals? If so it would seem wrong to me that we would have a core lint that requires a less performant pattern, even if it does catch some bugs in programs?

The difference is likely small but 🤷‍♂️.

[natebosch]

For this case we could probably make the change unconditionally --- I can't really imagine anyone wanting adjacent strings in lists

I do want adjacent strings in lists. Dartfmt indentation makes these fairly obvious.

[pq]

Given @natebosch's rationale, I'm leaning towards marking this "do not fix". Exceptions can just be ignored.

[srawlins]

Agreed.