fix: false positive in quoted-expressions (#209)

[jpradelle]

Proposal to almost fix #209

This is not a perfect solution as there are still some false positive, but much less. When ${...} is preceded by = there will be issue, for exemple there is still issue with:

html`
  <x-foo attr="foo=${v}"></x-foo>
`

[43081j]

i've had a play around with this and im still unsure what the best way we can do this is

basically, we should be able to solve it entirely but just not sure how yet

these cases probably fail right now:

• attr="${v} foo"
• attr="foo ${v}"
• attr="foo ${v} bar"

since we will be looking for /"$/ in the prefix and /^"/ in the suffix

without doing a lookahead, we can't really account for things like attr="${v} foo", as the following quasi doesn't start with ". we'd have to iterate through the all following quasis until we find the closing quote, which is a waste.

so i wonder if the only real fix to this is loosening the check, such that we only check if we're inside a quoted value but don't check for the closing quote anymore

[jpradelle]

I scratched my head trying to fix it, I think you need to find a way to go back to HTML tag and then parse its attributes from the beginning if you want to handle all cases

Cases like that can be very complicated to handle otherwise <x-foo attr="fake-like-attr='${...}'"></x-foo>

these cases probably fail right now:

• attr="${v} foo"
• attr="foo ${v}"
• attr="foo ${v} bar"

In fact only the 1st one is currently reported as error. 2nd and 3rd are valide because they are not checked, considered as attribute by this check: https://github.com/43081j/eslint-plugin-lit/blob/406b902996c8903cc54d96acfe37db45a21352a1/src/rules/quoted-expressions.ts#L62 But to be part of attribute value, it must be in quotes due to space, so no need to check it. That's why I ended up by removing closing quote check to handle the 1st case.

Side effect of removing closing quote check makes that case not reported anymore as issue <x-foo attr="${v}></x-foo>

[jpradelle]

I checked this test case too

    {
      code: 'html`<x-foo attr=foo${v}></x-foo>`',
      options: ['always']
    }

It's not reported as error (with and without this PR), where it should be :/

[43081j]

ok i think i have a solution

quotes.patch

basically the following behaviour:

• always quote means that standalone expressions should always be quoted (e.g. attr="${foo}")
• never quote means that standalone expressions should never be quoted (e.g. attr=${foo})
• in both cases, quoted values are accepted for mixed expressions (e.g. attr="foo ${v} bar")

[jpradelle]

Your patch is great :) Fixes my issues.

[43081j]

if you're happy with it, feel free to apply it to this PR (or open another) and ill review again

i think it makes sense since we don't need to care about mixed values then (quoted values with multiple expressions/strings)

[jpradelle]

PR is updated with your patch

[jpradelle]

Could you please run a release with last merged PRs ?

[43081j]

Released in 1.15.0

let me know if all goes well 🙏

[jpradelle]

Awesome, everything works as expected :)