Update SonarLint Core to use the same version

thahnen commented 1 month ago

Prerequisites

[X] This improvement has not already been suggested.
[X] This improvement would be generally useful, not specific to my code or setup.
[X] This improvement is not related to analysis quality (e.g. new rules, improvements to existing rules), which should be raised on the SonarDelphi repository.

Engine area

Other

Improvement description

Hey there,

This is Tobias from the team at SonarSource working on SonarLint (in my case, Eclipse as well as Core the most) :smiley:

I've followed both the SonarDelphi plug-in for SQ and this very nice community "SonarLint-like" linter for some time now, and I have to say: Kudos to all of you, excellent work!

I've noticed that you use different versions of the SonarLint Core (9.8.0.76914 and 9.3.1.74774) for the server part where the patching of SLCORE is done and the Delphi server is implemented. Why don't you use SonarLint Core 9.8.0.76914 modules only? I recommend doing so, as the versions are quite apart.

Additionally, the 9.x versions are long gone, and we moved to another approach where SonarLint Core would run out of process (starting with 10.x) and communicate with the client part via JSON RPC. The implementation would be more extensive (featuring an RPC server that is running out of process), but this will be the long-run solution instead of patching SLCORE to work with newer versions of SonarQube and SonarCloud (as well as your analyzer) at some point. But this is a secondary step as working with the 9.x versions should be good for now :smile:

Best, Tobias

Rationale

Why not?

fourls commented 1 month ago

Hi Tobias! Great to hear from you and thank you for the kudos 😄 We do put a lot of effort into both projects, and it's certainly paid off for us in terms of benefits to our code quality and processes - it seems to resonate with the community as well.

Great catch with the version mismatches - that was just an oversight. I'll update them all to 9.8.0.76914 soon.

The implementation would be more extensive (featuring an RPC server that is running out of process), but this will be the long-run solution instead of patching SLCORE to work with newer versions of SonarQube and SonarCloud (as well as your analyzer) at some point.

Sounds interesting! As you say, upgrading to 10.x is the future if we want to keep supporting new SonarQube versions indefinitely (which of course we do).

From a quick look, it looks like in 10.x the supported languages are still hardcoded (org.sonarsource.sonarlint.core.rpc.protocol.common.Language), so we would still need to patch for Delphi support - further complicated by the need to have the patch on both the RPC client and server. It would be nice to have sonarlint-core be decoupled from the specific languages SonarLint offers 😉 although I appreciate that it is developed primarily to support the SonarLint product, rather than to be an extensible project for the community.

I've put upgrading to 10.x down as something to look into in the medium- to long-term - as you say, it would require a more extensive implementation and the cost-benefit ratio isn't quite there yet in my opinion.

I'll get those modules upgraded - thanks for dropping by, it's great to have some interest from SonarSource!

Cheers, Elliot

thahnen commented 1 month ago

Heyho,

updating SonarLint dependencies to 9.8.0.76914 will be a good first step and sufficient for now indeed! All of the 10.x line is extra and more work, indeed. Just future-proof :smile:

That you have to patch SLCORE is still a thing, I forgot about that, sorry. Moving the languages definition out of it won't be done on the long-term as this will impact also our other products (due to the Sonar Plug-in API which is the glue for everything here).

Best, Tobias

fourls commented 1 month ago

Hi @thahnen,

I've found out why I didn't upgrade sonarlint-analysis-engine or sonarlint-issue-tracking with sonarlint-core in 146923e5e8f47b91381e5190c99390adcf79aff0 - they aren't available on Maven Central for version 9.8.0.76914 (see mvnrepository). Would you be able to chase that up internally?

That you have to patch SLCORE is still a thing, I forgot about that, sorry. Moving the languages definition out of it won't be done on the long-term as this will impact also our other products (due to the Sonar Plug-in API which is the glue for everything here).

All good, no worries :)

Thanks, Elliot

thahnen commented 1 month ago

Hi @fourls,

I didn't know that :confused: I will raise the point with my colleagues mostly working on SonarLint Core, but I have to say that they probably won't do anything about it as the version is not supported by us anymore officially. But I will try and keep you posted!

Best, Tobias

thahnen commented 1 week ago

Heu @fourls,

Long time no see. I've reached out to my colleagues, but sadly, as this version is very old and not supported anymore, we won't do anything about the artifacts that are not synchronized to Maven Central and will only focus on the most recent version to fix something like this.

So I would say keep everything as it is. And continue with your great work!

Best, Tobias

fourls commented 1 week ago

Hi @thahnen,

No problem, we will stay on the older version for now. Thanks for looking into this internally, it's much appreciated!

Thanks, Elliot

integrated-application-development / delphilint