tomblachut / svelte-intellij

Svelte components in WebStorm and friends
MIT License
485 stars 38 forks source link

Handle imports used only in expressions properly #145

Closed tomblachut closed 4 years ago

tomblachut commented 4 years ago

Import statement must not be marked as unused in following example

image

It happens because ES6UnusedImportsHelper#isUnusedSpecifier -> ES6UnusedImportsHelper#getLocalReferencesQuery creates LocalSearchScope containing only script tag, it should be containingFile in case of Svelte.

My first dumb idea is to copy whole ES6UnusedImportsInspection and change necessary lines, but this will be hard to maintain.

@anstarovoyt I'd appreciate your input here, thanks.

anstarovoyt commented 4 years ago

@tomblachut I believe it must be fixed after implementing https://github.com/tomblachut/svelte-intellij/pull/146 with proper scopes.

tomblachut commented 4 years ago

This is actually unrelated to stores, I could clarify it more.

Any declaration used only in template will be marked as unused, e.g. imported functions. But if you e.g. console.log them inside script tag, they will be marked as used.

anstarovoyt commented 4 years ago

@tomblachut I will take a look. Actually we definitely had the same problem in vue files but it works now without re-writing ES6UnusedImportsInspection

anstarovoyt commented 4 years ago

Ok. So now I can explain. There are two cases:

Unfortunately the solution with re-writing ES6UnusedImportsInspection won't work well because we use ES6UnusedImportsHelper directly in import optimizer so even if the error is suppressed the optimizer still will remove the import.

Another solution would be re-write ES6UnusedImportsHelper but the changes will be available only in WS 2020.3 :(

tomblachut commented 4 years ago

fix parser

seems complicated. But OTOH we could split $ and name tokens so they highlight separately, right? That would be huge.

I've went through JS parsing to some extent before and didn't find way to change it in an elegant way, but today I noticed JavaScriptParser#buildTokenElement.

For each call to this.myJavaScriptParser.buildTokenElement(JSElementTypes.REFERENCE_EXPRESSION) we could replace (marker.collapse) this token with some lazy element with two tokens inside, right? Preferably JSCallLikeExpression because $ is actually a call, i.e. it takes a type and returns an inner one

tomblachut commented 4 years ago

Will need to also change (highlighting) lexer, for GlobalSearchScope to work, right?

anstarovoyt commented 4 years ago

@tomblachut actually you can use a custom reference expression similar to VueJSFilterReferenceExpressionImpl but with changed getReferenceName() method.

tomblachut commented 4 years ago

oh nice

tomblachut commented 4 years ago

@anstarovoyt Your suggestion with custom SvelteJSReferenceExpression works great, I just need to refine types evaluation. I'm gonna break my code into 2 PRs and redo #146 because as you've said weird reference search is no longer needed.

Bonus points for fixed rename refactoring and '$' not being treated as part of reference

anstarovoyt commented 4 years ago

@tomblachut sounds good!

unlocomqx commented 4 years ago

There is a similar issue where imports used in use:action only are marked as unused

tomblachut commented 4 years ago

@unlocomqx this is related to directives support, they're not references yet, so import mechanism can't find them