Improved argument injections handling

[vepanimas]

@anstarovoyt

#76

The general idea is to ignore nested styled-components injections which we could assume valid CSS fragments instead of replacing them with EXTERNAL_FRAGMENT.

So this code

const OptionLabel = styled.div`
  padding: 5px;
  ${props => css`
    margin-bottom: 0.3em;
  `};
`;

will be replaced with syntactically correct

const OptionLabel = styled.div`
  padding: 5px;
`;

This kind of injections will be handled as before:

let atStart = styled.div`${getPropName()}: red`
let atEnd = styled.div`color: ${getColor()}`

let atStart = styled.div`EXTERNAL_FRAGMENT: red`
let atEnd = styled.div`color: EXTERNAL_FRAGMENT`

I think this is a safe heuristic that we can use now, and it should cover all the most painful cases.

Also, I provided a temporary registry key to disable this new behavior if something goes wrong for someone: styled.components.experimental.injections.

[anstarovoyt]

does it really work O_o? Wow

[undeadcat]

Hey! Sorry for not paying attention to this project.
Unfortnuately, I don't have much time to maintain it now.

The issue described in #76 seems to appear intermittently (try it with the sample from the original post, paste the sample into a file, usually no error is shown, make some small whitespace edits, error will usually appear), so it seems possible it's a highlighting error, not on the level of building injected PSI content.
It also seems like it started happening at some point, but wasn't present before.

Errors in injected code are filtered by com.intellij.lang.javascript.injections.StringInterpolationErrorFilter on the IDE side that was adapted from this plugin.
It could be that something in the filter was broken (either when adapting from this plugin or due to injection-related changes in platform)...

...
So I would recommend: try investigating if this error could be due to highlighting filter behavior with nested injections.
As to this PR: I'm confident in @anstarovoyt's review, I'd leave it up to him :-)

[vepanimas]

@undeadcat thank you for the details, I'll check it out. It looks like for this case something stopped working. A range containing \n color: blue contains an expected semicolon error in the first character. For example, a range is (25, 36) and an error has (25,25). So it's not filtered out.

div {
  EXTERNAL_FRAGMENT<expected semicolon>
  color: blue; 
}

But I don't see any logic which will ignore this kind of errors. It is caused by an EXTERNAL_FRAGMENT insertion, but it is located in a position where all the errors will be visible.

[undeadcat]

@vepanimas, I don't think we ever handled the case of

css`
div {
  ${foo()}
  color: blue; 
}
`

(NB: without semicolon after argument) without extra highlights.
That's why we had to describe this case in https://github.com/styled-components/webstorm-styled-components/#faq

It would be nice to fix it, but this seems distinct from the issue described in #76
This PR doesn't fix it either, right? Since this checks for presence of nested injection only.

[vepanimas]

@undeadcat I can't agree that they are distinct. At first, all the examples in the related issue contain a nested injection, so we can talk about them as the most common use cases. Second, an example I mentioned in my previous comment is from here, I mistakenly copied the wrong text from the editor to the comment. It also contains an injection and has an error displayed in the middle of the color: blue string, not near the template argument. This PR should fix that case and I don't see any other simple way to make life easier for the user now.

[vepanimas]

After a little investigation, I can confirm that the issue refers to the platform itself, not the plugin. We can configure a standalone LESS injection with prefix="div {" and suffix="}" in WebStorm and will have this error consistently reproduced.

const BackgroundContainer = css`
  ${media.desktop(css`
    padding-left: ${({theme}) => theme.space[16]};
    padding-right: ${({theme}) => theme.space[16]};
  `)}<error here>
`;

Note that if we remove nested injection it will work as expected:

const BackgroundContainer = css`
  ${media.desktop(` // no tag here
    padding-left: ${({theme}) => theme.space[16]};
    padding-right: ${({theme}) => theme.space[16]};
  `)} // no error
`;

Created a separate issue https://youtrack.jetbrains.com/issue/WEB-47350.

[anstarovoyt]

Maybe it is unrelated, but there is a false-positive warning for cases like:

styled.div`${'foo'}: absolute;${'bar'}: none;`

[vepanimas]

@anstarovoyt fixed in 7bb3683 based on the current implementation in IDEA.