Jakarta EE: Inconsistent behaviour for @Dependent quick fix.

[mrglavas]

While reviewing the Jakarta EE quick fixes, I noticed a difference in behaviour in how the "Replace current scope with @Dependent" quick fix is applied when resolving a diagnostic on a field.

Current LT for IntelliJ behaviour: Applying "Replace current scope with @Dependent" to a field adds the annotation to the field.

LT for VS Code 24.0.3 behaviour: Applying "Replace current scope with @Dependent" to a field replaces the scope annotation on the class.

After applying the quick fix in VS Code it continues to report a diagnostic which cannot be resolved by the quick fix. We should investigate which of these tools (IntelliJ vs. VS Code) has the correct behaviour, assuming either of them are correct.

[scottkurz]

Seems like the IntelliJ behavior is wrong. The LT in VSCode and Eclipse both will update the class-level annotation.

I haven't looked at the whole history or discussion in LSP4Jakarta and am not a CDI expert, but it seems unlikely that the field-level annotation was intended to be updated. Though the field-level annotation can hold a scope annotation, that's more for things like Producer use cases so not sure it applies here.

I assume the logic of linking the field which has the problem/diagnostic to the class which needs the annotation replaced is somewhere in the org.eclipse.lsp4jakarta.jdt.internal.cdi;ManagedBeanQuickFix but I'm not fluent enough to easily make sense of that.

Anyway hopefully that was a bit of help to mention.

[mrglavas]

Seems like the IntelliJ behavior is wrong. The LT in VSCode and Eclipse both will update the class-level annotation.

I haven't looked at the whole history or discussion in LSP4Jakarta and am not a CDI expert, but it seems unlikely that the field-level annotation was intended to be updated. Though the field-level annotation can hold a scope annotation, that's more for things like Producer use cases so not sure it applies here.

I assume the logic of linking the field which has the problem/diagnostic to the class which needs the annotation replaced is somewhere in the org.eclipse.lsp4jakarta.jdt.internal.cdi;ManagedBeanQuickFix but I'm not fluent enough to easily make sense of that.

Anyway hopefully that was a bit of help to mention.

@scottkurz Not sure if you caught my observation but updating the class-level annotation does not resolve the diagnostic, so in VSCode when the user applies the quick fix they still get an error. If the quick fix is correct in VSCode then it would seem that the diagnostic is wrong. If I manually add @Dependent to the field then VSCode stops reporting the error.

[TrevCraw]

For more context, this is an issue and PR set in LSP4Jakarta where the LT IntelliJ changes were based on: https://github.com/eclipse/lsp4jakarta/issues/444 and https://github.com/eclipse/lsp4jakarta/pull/506

[scottkurz]

@scottkurz Not sure if you caught my observation but updating the class-level annotation does not resolve the diagnostic, so in VSCode when the user applies the quick fix they still get an error. If the quick fix is correct in VSCode then it would seem that the diagnostic is wrong. If I manually add @Dependent to the field then VSCode stops reporting the error.

@mrglavas no I didn't notice this before. I just tried in Eclipse and I see the same thing.

So, I think it's clear that what should happen is that the class-level annotation gets replaced with @Dependent. As far as what went wrong, well, you and @TrevCraw might know more than me.

I can imagine the opportunity for confusion since we have a field that accepts a certain annotation, but when we look at this field we want to change the annotation elsewhere in the scope (the class). I'm just recognizing it seems open for possibilities for error but not really claiming to help get us further there.

Seems likely if both VSCode and Eclipse envs show the same error then something could be wrong in LSP4Jakarta (stating the obvious).