Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SLLS-277 Fix JsonNull conversion error #410

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

jblievremont
Copy link
Member

@jblievremont jblievremont commented Nov 12, 2024

SLLS-277

Internal copy of #402 to let CI checks run

Partially verified

This commit is signed with the committer’s verified signature.
Virv12’s contribution has been verified via GPG key.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
Some LSP clients do not send values outside of of configuration
namespace. For example Neovim does not support sending client
configurations to mimic the VScode behavior. Following in Lua is not
possible.

```lua
require('sonarlint').setup({
   server = {
      cmd = {
         -- …
      },
      settings = {
         sonarlint = {
            -- …
         },
         ["files.exclude"] = { ["**/.git"] = true },
      }
   },
})
```

This commit makes sure that sonarlint language server still functions if
the values won't be sent. Otherwise, the language server errors while
loading the project.

Verified

This commit was signed with the committer’s verified signature.
Virv12 Filippo Casarin
@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot changed the title Fix JsonNull conversion error SLLS-277 Fix JsonNull conversion error Nov 12, 2024
@jblievremont jblievremont force-pushed the contrib/schrieveslaach-sonarlint-language-server branch 3 times, most recently from 5b622c0 to 19912c4 Compare November 12, 2024 17:12

Verified

This commit was signed with the committer’s verified signature.
Virv12 Filippo Casarin
@jblievremont jblievremont force-pushed the contrib/schrieveslaach-sonarlint-language-server branch from 19912c4 to 6eab87d Compare November 14, 2024 16:56
Copy link
Member

@nquinquenel nquinquenel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


@Override
public String[] getVersion() throws Exception {
try(var versionStream = getClass().getResourceAsStream("/slls-version.txt")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
try(var versionStream = getClass().getResourceAsStream("/slls-version.txt")) {
try (var versionStream = getClass().getResourceAsStream("/slls-version.txt")) {

@@ -98,4 +101,14 @@ public static void main(String... args) {
System.exit(exitCode);
}

public static class MavenVersionProvider implements IVersionProvider {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it related to Maven?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My original version in #402 provided that name because it was taken from the Maven attributes in JAR's manifest. Probably, renaming is a good idea.

Suggested change
public static class MavenVersionProvider implements IVersionProvider {
public static class SonarlintVersionProvider implements IVersionProvider {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is related to Maven because slls-version.txt is a Maven-filtered resource that contains the ${project.version} property.

I believe it is slightly more robust than the initial implementation because:

  • This file is provided only by the language server build itself - it is not automagically generated by some Maven plugin or copied from a random JAR when building the über-JAR with dependencies
  • It contains exactly the property that we want to show with the --version option

@jblievremont jblievremont merged commit 9cb7cc1 into master Nov 19, 2024
9 checks passed
@jblievremont jblievremont deleted the contrib/schrieveslaach-sonarlint-language-server branch November 19, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants