sts4 icon indicating copy to clipboard operation
sts4 copied to clipboard

@Resource annotation are not processed by the language server

Open gayanper opened this issue 5 years ago • 16 comments

fields and methods which are annotation @Resource are not processed and listed in the symbols list. But STS3 was able to list the @Resource.

gayanper avatar Jan 07 '19 10:01 gayanper

The current implementation takes Spring annotations into account, but nothing else (at the moment), therefore @Resource is not being processed for the symbols.

But it certainly makes sense to change that, so that other annotations are being processed, too. But I think @Resource is not the only one that we should take into account. I guess everything from the javax.inject package, too. Anything else?

martinlippert avatar Jan 07 '19 10:01 martinlippert

@martinlippert agree, we should consider all javax.inject package, may we can start from there and add anything more on the way. Would you like a PR for this ?

gayanper avatar Jan 07 '19 10:01 gayanper

@martinlippert i belive the @Resource annotation must belong to the BootJavaHoverProvider right ? There is no purpose over indexing the @Resource through SpringIndexer is it ? Let me know what you think.

gayanper avatar Jan 07 '19 15:01 gayanper

image How does this looks ?

gayanper avatar Jan 07 '19 16:01 gayanper

PRs are always welcome, of course... :-) Give us a few days to review and comment on them, but if it takes a while doesn't mean we don't like it or won't look at it. We are more than happy to review, comment, and merge PRs !!!

With regards to the screenshot, it would be good to see the source code to which the symbols belong, too. I wonder why there is a @Bean showing up in the highlighted line. We always try to show the exact annotation on the symbol that is in the source, so if the source contains @Resource, the symbol should also contain @Resource (just as an example).

martinlippert avatar Jan 07 '19 17:01 martinlippert

If you want to provide a special symbol, you need to implement a SymbolProvider and register that:

https://github.com/spring-projects/sts4/blob/master/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/BootJavaLanguageServerComponents.java#L376

The hover providers are responsible for showing data from live running applications (which is a whole different game for non-boot spring apps). But yes, if you want to support the @Resource (or whatever) annotation there as well, you would need to implement a HoverProvider for that annotation type as well.

martinlippert avatar Jan 07 '19 17:01 martinlippert

With regards to the screenshot, it would be good to see the source code to which the symbols belong, too. I wonder why there is a @Bean showing up in the highlighted line. We always try to show the exact

Basically the code was some thing like this

@Bean
@Singleton
@Named(value="jason-parser")
@Parser // this is a Qualifier i made for testing.
public JsonParser getJsonParser() {
 ...
}

I know this dosn't make much sense but i use the RestrictedDefaultSymbolProvider since it was used for conditions and these annotation where something like conditions.

gayanper avatar Jan 08 '19 10:01 gayanper

If you want to provide a special symbol, you need to implement a SymbolProvider and register that:

@martinlippert for @Resource @Inject @Autowire annotation do you think it will be good a index them as a new symbol since these are injection points or some what like assignments. So would you think its a good idea to represent then with a "=@" ?

gayanper avatar Jan 08 '19 11:01 gayanper

Any recommendations on the suggestions ?

gayanper avatar Jan 10 '19 01:01 gayanper

How about these for above mentioned Injection annotation types and other annotations like Named, Singleton etc.

image

gayanper avatar Jan 15 '19 16:01 gayanper

@martinlippert any feedback on this ? If you all like the way it is i can update tests and create PR.

gayanper avatar Jan 16 '19 12:01 gayanper

great to see you working on this, I just need a bit of time to process this... Sorry for the delay. Will be back as soon as possible on this.

martinlippert avatar Jan 16 '19 12:01 martinlippert

OK, I looked at this in more detail now, here is what I think:

The code example that you provided:

@Bean
@Singleton
@Named(value="jason-parser")
@Parser // this is a Qualifier i made for testing.
public JsonParser getJsonParser() {
 ...
}

produces the symbol in the way the screenshot shows this - and that looks perfectly fine to me.

Now moving on to the injection points like @Autowired, @Inject and @Resource: I think they should indeed be showing up in the same way in the list of symbols, so they should produce similar symbol informations.

I also like the idea of having a specialized shortcut like @=, but that need to be chosen carefully, since some of those characters have a special meaning in Visual Studio Code and cause the symbol navigation to do special things. And I think the = belongs to that group. Maybe we can try something like @*, which would somehow relate to the @+ nicely.

An important feature for this would indeed be to show all the other annotations on that field or method (like @Qualifier) and so on, much in the same way than we do for the @Bean symbol.

I think we should start to collect a few samples here (code snippet + corresponding symbol) as a first step. We can use that data also in the unit tests.

As a side note: At the moment, the symbol indexer creates a default symbol for every spring-related annotation. We need to avoid that in case this annotation showed up in a context of a @Bean or @Autowired or @Inject or @Resource annotation as soon as those symbols integrate the other annotations of the field or method. Otherwise we would end up having two symbols that contain the @Qualifier, for example - the default symbol and the symbol for the @Autowired annotation where the @Qualifier annotation belongs to. This is a bit tricky to realize due to the way we made the symbol indexer configurable for different annotations. The SymbolProvider implementations usually don't know anything about each other and don't share any state. That means if we create the default symbol, we would need to check whether the annotation is in the context of another one and then not create the default symbol.

Attention: I committed a few changes to the Spring symbol indexer recently, so a few things got renamed and have changed, don't be surprised. The underlying mechanics haven't changed, so you can still apply your existing knowledge... :-)

martinlippert avatar Jan 17 '19 10:01 martinlippert

Thanks for the feedback @martinlippert.

I think we should start to collect a few samples here (code snippet + corresponding symbol) as a first step. We can use that data also in the unit tests.

will try to come up with some code samples. May be the community members here can help me out as well.

As a side note: At the moment, the symbol indexer creates a default symbol for every spring-related annotation. We need

Yes this might be a problem. What if we don't have explicit SymbolProviders for such supporting annotations like Qualifier, because they will never be annotated alone, But may be process them in the parent annotations like Autowire and Bean for example. That way we can void duplicate Symbols and just creating a Qualifier symbol without its context might not valuable as well.

gayanper avatar Jan 18 '19 04:01 gayanper

As a side note: At the moment, the symbol indexer creates a default symbol for every spring-related annotation. We need

@martinlippert i got your point on this, and implementation is wrong. Actually the screenshot i posted at first on Bean annotation is provided by the BeanSymboleProvider, not from my crazy changes :). But i now have some understanding what i need to do, Will get back to you after the modifications. Thanks for the support.

I also like the idea of having a specialized shortcut like @=, but that need to be chosen carefully, since some

I didn't checked on VSCode, but what about having "@>" for Injection annotations.

gayanper avatar Jan 18 '19 13:01 gayanper

At the moment @> is used for function definitions. Lets choose something random and change that later if needed.

martinlippert avatar Jan 23 '19 11:01 martinlippert