Extract the analyzers from VSCode extension and add C/C++ support

[Horrih]

Summary

In this rework, lsp-sonarlint downloads and unzips the VSCode extension, which bundles java, the language server, and all available analyzers, including CFamily. This makes things simpler overall.

This is probably more maintainable and adds the benefit of new analyzers like CFamily.

I have used this PR on a c++/linux project at work for a week now, and validated it somewhat on windows as well. I am content with how it works for me, feel free to adapt it to your way of doing things. I remain available to answer any question you might have ! 'll try to implement connected mode in the upcoming week, a basic version does not seem too hard.

Impact on configuration

I felt that the current way of managing analyzers has become cumbersome now that all analyzers are downloaded anyway.

(require 'lsp-sonarlint-php)
(setq lsp-sonarlint-php-enabled t)

(require 'lsp-sonarlint-python)
(setq lsp-sonarlint-python-enabled t)

;; becomes now
(setq lsp-sonarlint-enabled-analyzers '(php python))  ; set to 'all by default

I'll leave it up to you maintainers to judge if this change makes sense to you and your users.

Other impacts

Additionally, many sonarlint LSP protocol extensions have been added/modified. I based my work on the source code of the typescript front end of vscode's extension to remove the obsolete and support the new required requests.

I had to tweak a few of the integration tests for some request/response timing issues on my side. They all passed in the end (tested on linux) but seemed a bit brittle overall so I am not overly confident they'll work as well on your side.

I switched the client from tcp to stdio because I could not seem to make the tcp work on windows. stdio seems to work on both platforms.

[miraz12]

Hi @Horrih! I've tried this PR out in a C++ project on Linux but I'm getting the following error: LSP :: Sending to process failed with the following error: Wrong type argument: utf-8-string-p, "S\366ndag.txt". I've also tried out #20 which you have mentioned in other places which seems to work fine but if you are also working on SonarQube support hopefully this PR will be the way forward.

It looks like some stdio issue but I'm not sure how to debug it, I tried running with debug-on-error but it didn't seem to get triggered. Do you know what the cause could be?

[Horrih]

Hello @miraz12 , thank you for your feeback, do you have this issue as well on the provided example c++ file ? (fixtures/sample.cpp)

If you only have this issue on your project, could you provide a reproducible example for me to investigate ?

If you want to investigate things on your side, here is what I use to enable verbose logs :

(setq lsp-sonarlint-verbose-logs t)
(setq lsp-sonarlint-show-analyzer-logs t)
(setq lsp-log-io t)

You'll get several output buffers, one with the details of the communications with sonarlint's LSP server

[miraz12]

Thanks you! I did not have issues with the example c++ file which surprised me. But after completely removing the folders straight/repo/lsp-sonarlint, straight/repo/lsp-sonarlint and cloned down a clean version it seems to be working in all of my projects. I must have messed something up while trying to switch from the main branch to your PR branch.

So everything works as expected now, nice work! And sorry for the noise.

[miraz12]

Seems I spoke too soon. Today when I started my repo again it fails with the same issue. But I've figured out a bit more about the issue. First "\366" is ISO-8859-1 (ISO Latin 1) for "ö". Secondly if I start sonarlint in a project without such files and then open a project with such files it does not seem to try to parse the none UTF-8 files and only connects to the currently existing instance of sonarlint and works fine.

I was able to create a reproducible example by running touch S\Xf6ndag.txt in the fixtures folder and then opening sample.cpp.

[Horrih]

Thank you, I was able to reproduce your issue by switching my terminal to ISO 8859-1 and creating a file with non ASCII characters in the name.

The issue seems to come from one of sonarlint's recent LSP addition, which requests the list of files in the current project. This server request was not present in previous versions, I had to implement it in this PR.

I'll investigate this week-end hopefully, and figure if this is something that could be fixed, or at least mitigated (i.e ignore the files using non UTF-8 encoding in their filenames)

[Horrih]

Hello @miraz12, Emacs json implementation is quite strict as to the encoding used, and will refuse anything non UTF-8, see https://github.com/emacs-lsp/lsp-mode/issues/1246

As a work aound, I added a filter to remove non UTF-8 encoded filenames from the response to sonarlint. Could you check if this solves your issue ?

Note that even though they should not block the whole project anymore, these individual files will not be analyzed if you open them.

[miraz12]

Hi @Horrih, I just tried it out in my project that had issues and it work flawlessly now. Thank you for your help and efforts!

Luckily the files with non UTF-8 names were not source files so it should be fine.

Hopefully this PR gets accepted soon!

[mjburling]

@Horrih This is great! I really appreciate you putting this together. I'll share that I'm running this for a Java codebase, and unlike the latest version of emacs-lsp/lsp-sonarlint, running your branch is works fine in doom, e.g.

;;..snippet from ~/.config/doom/packages.el
(package! lsp-sonarlint
  :recipe (:host github
           :repo "Horrih/lsp-sonarlint"))

A brief aside... Apple silicon is really nice. With the bifurcation 4 years ago, I wonder how many tens of emacs users are still using intel-based darwin systems... does your lsp-sonarlint-download-url need a default case in the switch statement?

Echoing the above, I hope we see this land soon 🤞

[Horrih]

Thank you @mjburling for your feedback !

As for your question about lsp-sonarlint-download-url, I was wondering how to achieve that.

From my understandingn both arm-based and intel-based macs have 'darwin as system-type.

As I have no mac lying around at home, I was hesitant to implement some logic I could not test, and ended up implementing the supposedly dominant platform (arm), leaving the other (intel) to customize this variable.

If you know of a reliable way to distinguish them, i'll gladly add the corresponding default value. Otherwise, maybe mentioning this issue in the README could be an acceptable compromise ?

[mjburling]

From my understandingn both arm-based and intel-based macs have 'darwin as system-type.

As I have no mac lying around at home, I was hesitant to implement some logic I could not test, and ended up implementing the supposedly dominant platform (arm), leaving the other (intel) to customize this variable.

I don't have a spare intel mac, either. But, I'll own up to ambitiously looking at a solution for this myself only to dismiss it as both trivial to parse the output of the system-configuration variable and... potentially beyond my art? I'm terrible at elisp, so I just settled for mentioning it instead 😉.

However, allow me to stand on the shoulders of giants, or... at least those taller than myself. It looks like the inclusion of (require 'lsp-mode) has you closer than you might think: lsp--system-arch is available, and should be one of x64, x32, or arm64. You could use that directly in your solution and/or borrow from its implementation:

(defconst lsp--system-arch (lambda ()
                             (setq lsp--system-arch
                                   (pcase system-type
                                     ('windows-nt
                                      (pcase system-configuration
                                        ((rx bol "x86_64-") 'x64)
                                        (_ 'x86)))
                                     ('darwin
                                      (pcase system-configuration
                                        ((rx "aarch64-") 'arm64)
                                        (_ 'x64)))
                                     ('gnu/linux
                                       (pcase system-configuration
                                         ((rx bol "x86_64") 'x64)
                                         ((rx bol (| "i386" "i886")) 'x32)))
                                     (_
                                      (pcase system-configuration
                                        ((rx bol "x86_64") 'x64)
                                        ((rx bol (| "i386" "i886")) 'x32))))))
  "Return the system architecture of `Emacs'.
Special values:
  `x64'       64bit
  `x32'       32bit
  `arm64'     ARM 64bit")

[Horrih]

Thanks @mjburling ! Looking for calls of this function in lsp-mode gave me an example which was precisely what we were looking for.

If you can spare a couple of minutes, could you pull again and check that the variable lsp-sonarlint-download-url remains correctly set on your ARM mac with this new change ?

[mjburling]

Very cool, @Horrih! I cleared away the package and the associated sonarlint resources, and re-installed... worked just as it did before! 💯

I don't know the very prolific @jcs090218, but I'll see if I can't nudge them for a review in discord. I hope this aligns with others' expectations. This should resolve #12, #21, and possibly #18! And as you pointed out, your solution overlaps with some of the goals of #20.

[jcs090218]

Can you rebase this? I've fixed the CI. 🤔

This PR looks good, but I'll need the package author to confirm it.

cc @Sasanidas WDYT? 🤔

[Sasanidas]

This looks very nice, I think this rework is well done and removes complexity, It would be nice to fix the CI as @jcs090218 mentioned. I'll merge it :+1: