Skip to content

fix: use pointers for optional LSP settings [IDE-899] #811

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

Merged
merged 6 commits into from
Apr 17, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions application/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,29 +581,27 @@ func (c *Config) SetSnykAdvisorEnabled(enabled bool) {
c.isSnykAdvisorEnabled = enabled
}

func (c *Config) SetSeverityFilter(severityFilter types.SeverityFilter) bool {
func (c *Config) SetSeverityFilter(severityFilter *types.SeverityFilter) bool {
c.m.Lock()
defer c.m.Unlock()
emptySeverityFilter := types.SeverityFilter{}
if severityFilter == emptySeverityFilter {
if severityFilter == nil {
return false
}
filterModified := c.filterSeverity != severityFilter
filterModified := c.filterSeverity != *severityFilter
c.logger.Debug().Str("method", "SetSeverityFilter").Interface("severityFilter", severityFilter).Msg("Setting severity filter:")
c.filterSeverity = severityFilter
c.filterSeverity = *severityFilter
return filterModified
}

func (c *Config) SetIssueViewOptions(issueViewOptions types.IssueViewOptions) bool {
func (c *Config) SetIssueViewOptions(issueViewOptions *types.IssueViewOptions) bool {
c.m.Lock()
defer c.m.Unlock()
emptyIssueViewOptions := types.IssueViewOptions{}
if issueViewOptions == emptyIssueViewOptions {
if issueViewOptions == nil {
return false
}
issueViewOptionsModified := c.issueViewOptions != issueViewOptions
issueViewOptionsModified := c.issueViewOptions != *issueViewOptions
c.logger.Debug().Str("method", "SetIssueViewOptions").Interface("issueViewOptions", issueViewOptions).Msg("Setting issue view options:")
c.issueViewOptions = issueViewOptions
c.issueViewOptions = *issueViewOptions
return issueViewOptionsModified
}

Expand Down
14 changes: 8 additions & 6 deletions application/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"testing"
"time"

"github.com/snyk/snyk-ls/internal/util"

"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -168,37 +170,37 @@ func TestSnykCodeApi(t *testing.T) {
func Test_SetSeverityFilter(t *testing.T) {
t.Run("Saves filter", func(t *testing.T) {
c := New()
c.SetSeverityFilter(types.NewSeverityFilter(true, true, false, false))
c.SetSeverityFilter(util.PtrOf(types.NewSeverityFilter(true, true, false, false)))
assert.Equal(t, types.NewSeverityFilter(true, true, false, false), c.FilterSeverity())
})

t.Run("Returns correctly", func(t *testing.T) {
c := New()
lowExcludedFilter := types.NewSeverityFilter(true, true, false, false)

modified := c.SetSeverityFilter(lowExcludedFilter)
modified := c.SetSeverityFilter(&lowExcludedFilter)
assert.True(t, modified)

modified = c.SetSeverityFilter(lowExcludedFilter)
modified = c.SetSeverityFilter(&lowExcludedFilter)
assert.False(t, modified)
})
}

func Test_SetIssueViewOptions(t *testing.T) {
t.Run("Saves filter", func(t *testing.T) {
c := New()
c.SetIssueViewOptions(types.NewIssueViewOptions(false, true))
c.SetIssueViewOptions(util.PtrOf(types.NewIssueViewOptions(false, true)))
assert.Equal(t, types.NewIssueViewOptions(false, true), c.IssueViewOptions())
})

t.Run("Returns correctly", func(t *testing.T) {
c := New()
ignoredOnlyFilter := types.NewIssueViewOptions(false, true)

modified := c.SetIssueViewOptions(ignoredOnlyFilter)
modified := c.SetIssueViewOptions(&ignoredOnlyFilter)
assert.True(t, modified)

modified = c.SetIssueViewOptions(ignoredOnlyFilter)
modified = c.SetIssueViewOptions(&ignoredOnlyFilter)
assert.False(t, modified)
})
}
Expand Down
6 changes: 4 additions & 2 deletions application/server/authentication_smoke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
"testing"
"time"

"github.com/snyk/snyk-ls/internal/util"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/oauth2"
Expand Down Expand Up @@ -78,8 +80,8 @@ func checkInvalidCredentialsMessageRequest(t *testing.T, expected string, tokenS
InitializationOptions: types.Settings{
Token: tokenString,
EnableTrustedFoldersFeature: "false",
FilterSeverity: types.DefaultSeverityFilter(),
IssueViewOptions: types.DefaultIssueViewOptions(),
FilterSeverity: util.PtrOf(types.DefaultSeverityFilter()),
IssueViewOptions: util.PtrOf(types.DefaultIssueViewOptions()),
AuthenticationMethod: types.OAuthAuthentication,
AutomaticAuthentication: "false",
},
Expand Down
4 changes: 2 additions & 2 deletions application/server/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ func updateProductEnablement(c *config.Config, settings types.Settings) {
}
}

func updateIssueViewOptions(c *config.Config, s types.IssueViewOptions) {
func updateIssueViewOptions(c *config.Config, s *types.IssueViewOptions) {
c.Logger().Debug().Str("method", "updateIssueViewOptions").Interface("issueViewOptions", s).Msg("Updating issue view options:")
modified := c.SetIssueViewOptions(s)

Expand All @@ -402,7 +402,7 @@ func updateIssueViewOptions(c *config.Config, s types.IssueViewOptions) {
}
}

func updateSeverityFilter(c *config.Config, s types.SeverityFilter) {
func updateSeverityFilter(c *config.Config, s *types.SeverityFilter) {
c.Logger().Debug().Str("method", "updateSeverityFilter").Interface("severityFilter", s).Msg("Updating severity filter:")
modified := c.SetSeverityFilter(s)

Expand Down
42 changes: 36 additions & 6 deletions application/server/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ func Test_UpdateSettings(t *testing.T) {

tempDir1 := filepath.Join(t.TempDir(), "tempDir1")
tempDir2 := filepath.Join(t.TempDir(), "tempDir2")
nonDefaultSeverityFilter := types.NewSeverityFilter(false, true, false, true)
nonDefaultIssueViewOptions := types.NewIssueViewOptions(false, true)
hoverVerbosity := 1
outputFormat := "html"
settings := types.Settings{
Expand All @@ -187,8 +189,8 @@ func Test_UpdateSettings(t *testing.T) {
ManageBinariesAutomatically: "false",
CliPath: filepath.Join(t.TempDir(), "cli"),
Token: "a fancy token",
FilterSeverity: types.DefaultSeverityFilter(),
IssueViewOptions: types.DefaultIssueViewOptions(),
FilterSeverity: &nonDefaultSeverityFilter,
IssueViewOptions: &nonDefaultIssueViewOptions,
TrustedFolders: []string{"trustedPath1", "trustedPath2"},
OsPlatform: "windows",
OsArch: "amd64",
Expand Down Expand Up @@ -234,8 +236,8 @@ func Test_UpdateSettings(t *testing.T) {
assert.Equal(t, expectedOrgId, c.Organization())
assert.False(t, c.ManageBinariesAutomatically())
assert.Equal(t, settings.CliPath, c.CliSettings().Path())
assert.Equal(t, types.DefaultSeverityFilter(), c.FilterSeverity())
assert.Equal(t, types.DefaultIssueViewOptions(), c.IssueViewOptions())
assert.Equal(t, nonDefaultSeverityFilter, c.FilterSeverity())
assert.Equal(t, nonDefaultIssueViewOptions, c.IssueViewOptions())
assert.Subset(t, []types.FilePath{"trustedPath1", "trustedPath2"}, c.TrustedFolders())
assert.Equal(t, settings.OsPlatform, c.OsPlatform())
assert.Equal(t, settings.OsArch, c.OsArch())
Expand Down Expand Up @@ -405,18 +407,46 @@ func Test_UpdateSettings(t *testing.T) {
c := testutil.UnitTest(t)
t.Run("filtering gets passed", func(t *testing.T) {
mixedSeverityFilter := types.NewSeverityFilter(true, false, true, false)
UpdateSettings(c, types.Settings{FilterSeverity: mixedSeverityFilter})
UpdateSettings(c, types.Settings{FilterSeverity: &mixedSeverityFilter})

assert.Equal(t, mixedSeverityFilter, c.FilterSeverity())
})
t.Run("equivalent of the \"empty\" struct as a filter gets passed", func(t *testing.T) {
emptyLikeSeverityFilter := types.NewSeverityFilter(false, false, false, false)
UpdateSettings(c, types.Settings{FilterSeverity: &emptyLikeSeverityFilter})

assert.Equal(t, emptyLikeSeverityFilter, c.FilterSeverity())
})
t.Run("omitting filter does not cause an update", func(t *testing.T) {
mixedSeverityFilter := types.NewSeverityFilter(false, false, true, false)
UpdateSettings(c, types.Settings{FilterSeverity: &mixedSeverityFilter})
assert.Equal(t, mixedSeverityFilter, c.FilterSeverity())

UpdateSettings(c, types.Settings{})
assert.Equal(t, mixedSeverityFilter, c.FilterSeverity())
})
})

t.Run("issue view options", func(t *testing.T) {
c := testutil.UnitTest(t)
t.Run("filtering gets passed", func(t *testing.T) {
mixedIssueViewOptions := types.NewIssueViewOptions(false, true)
UpdateSettings(c, types.Settings{IssueViewOptions: mixedIssueViewOptions})
UpdateSettings(c, types.Settings{IssueViewOptions: &mixedIssueViewOptions})

assert.Equal(t, mixedIssueViewOptions, c.IssueViewOptions())
})
t.Run("equivalent of the \"empty\" struct as a filter gets passed", func(t *testing.T) {
emptyLikeIssueViewOptions := types.NewIssueViewOptions(false, false)
UpdateSettings(c, types.Settings{IssueViewOptions: &emptyLikeIssueViewOptions})

assert.Equal(t, emptyLikeIssueViewOptions, c.IssueViewOptions())
})
t.Run("omitting filter does not cause an update", func(t *testing.T) {
mixedIssueViewOptions := types.NewIssueViewOptions(false, false)
UpdateSettings(c, types.Settings{IssueViewOptions: &mixedIssueViewOptions})
assert.Equal(t, mixedIssueViewOptions, c.IssueViewOptions())

UpdateSettings(c, types.Settings{})
assert.Equal(t, mixedIssueViewOptions, c.IssueViewOptions())
})
})
Expand Down
6 changes: 4 additions & 2 deletions application/server/parallelization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"testing"
"time"

"github.com/snyk/snyk-ls/internal/util"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -80,8 +82,8 @@ func Test_Concurrent_CLI_Runs(t *testing.T) {
Endpoint: os.Getenv("SNYK_API"),
Token: os.Getenv("SNYK_TOKEN"),
EnableTrustedFoldersFeature: "false",
FilterSeverity: types.DefaultSeverityFilter(),
IssueViewOptions: types.DefaultIssueViewOptions(),
FilterSeverity: util.PtrOf(types.DefaultSeverityFilter()),
IssueViewOptions: util.PtrOf(types.DefaultIssueViewOptions()),
AuthenticationMethod: types.TokenAuthentication,
AutomaticAuthentication: "false",
ManageBinariesAutomatically: "true",
Expand Down
10 changes: 6 additions & 4 deletions application/server/server_smoke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"testing"
"time"

"github.com/snyk/snyk-ls/internal/util"

"github.com/creachadair/jrpc2"
"github.com/creachadair/jrpc2/server"
"github.com/go-git/go-git/v5"
Expand Down Expand Up @@ -776,8 +778,8 @@ func prepareInitParams(t *testing.T, cloneTargetDir types.FilePath, c *config.Co
Endpoint: os.Getenv("SNYK_API"),
Token: os.Getenv("SNYK_TOKEN"),
EnableTrustedFoldersFeature: "false",
FilterSeverity: types.DefaultSeverityFilter(),
IssueViewOptions: types.DefaultIssueViewOptions(),
FilterSeverity: util.PtrOf(types.DefaultSeverityFilter()),
IssueViewOptions: util.PtrOf(types.DefaultIssueViewOptions()),
AuthenticationMethod: types.TokenAuthentication,
EnableDeltaFindings: strconv.FormatBool(c.IsDeltaFindingsEnabled()),
ActivateSnykCode: strconv.FormatBool(c.IsSnykCodeEnabled()),
Expand Down Expand Up @@ -850,8 +852,8 @@ func Test_SmokeSnykCodeFileScan(t *testing.T) {
Endpoint: os.Getenv("SNYK_API"),
Token: os.Getenv("SNYK_TOKEN"),
EnableTrustedFoldersFeature: "false",
FilterSeverity: types.DefaultSeverityFilter(),
IssueViewOptions: types.DefaultIssueViewOptions(),
FilterSeverity: util.PtrOf(types.DefaultSeverityFilter()),
IssueViewOptions: util.PtrOf(types.DefaultIssueViewOptions()),
},
}

Expand Down
14 changes: 8 additions & 6 deletions application/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"testing"
"time"

"github.com/snyk/snyk-ls/internal/util"

"github.com/snyk/snyk-ls/domain/snyk"
"github.com/snyk/snyk-ls/domain/snyk/scanner"
"github.com/snyk/snyk-ls/internal/storedconfig"
Expand Down Expand Up @@ -406,8 +408,8 @@ func Test_TextDocumentCodeLenses_shouldReturnCodeLenses(t *testing.T) {
Token: "xxx",
ManageBinariesAutomatically: "true",
CliPath: filepath.Join(t.TempDir(), "cli"),
FilterSeverity: types.DefaultSeverityFilter(),
IssueViewOptions: types.DefaultIssueViewOptions(),
FilterSeverity: util.PtrOf(types.DefaultSeverityFilter()),
IssueViewOptions: util.PtrOf(types.DefaultIssueViewOptions()),
EnableTrustedFoldersFeature: "false",
},
}
Expand Down Expand Up @@ -469,8 +471,8 @@ func Test_TextDocumentCodeLenses_dirtyFileShouldFilterCodeLenses(t *testing.T) {
Token: "xxx",
ManageBinariesAutomatically: "true",
CliPath: filepath.Join(t.TempDir(), "cli"),
FilterSeverity: types.DefaultSeverityFilter(),
IssueViewOptions: types.DefaultIssueViewOptions(),
FilterSeverity: util.PtrOf(types.DefaultSeverityFilter()),
IssueViewOptions: util.PtrOf(types.DefaultIssueViewOptions()),
EnableTrustedFoldersFeature: "false",
},
}
Expand Down Expand Up @@ -525,8 +527,8 @@ func Test_initialize_updatesSettings(t *testing.T) {
InitializationOptions: types.Settings{
Organization: expectedOrgId,
Token: "xxx",
FilterSeverity: types.DefaultSeverityFilter(),
IssueViewOptions: types.DefaultIssueViewOptions(),
FilterSeverity: util.PtrOf(types.DefaultSeverityFilter()),
IssueViewOptions: util.PtrOf(types.DefaultIssueViewOptions()),
},
}

Expand Down
10 changes: 5 additions & 5 deletions domain/ide/workspace/folder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func TestProcessResults_whenFilteringSeverity_ProcessesOnlyFilteredIssues(t *tes
c := testutil.UnitTest(t)

severityFilter := types.NewSeverityFilter(true, false, true, false)
config.CurrentConfig().SetSeverityFilter(severityFilter)
config.CurrentConfig().SetSeverityFilter(&severityFilter)

f := NewMockFolder(c, notification.NewNotifier())

Expand Down Expand Up @@ -251,7 +251,7 @@ func TestProcessResults_whenFilteringIssueViewOptions_ProcessesOnlyFilteredIssue
c := testutil.UnitTest(t)

issueViewOptions := types.NewIssueViewOptions(false, true)
config.CurrentConfig().SetIssueViewOptions(issueViewOptions)
config.CurrentConfig().SetIssueViewOptions(&issueViewOptions)

f := NewMockFolder(c, notification.NewNotifier())

Expand Down Expand Up @@ -435,7 +435,7 @@ func Test_FilterCachedDiagnostics_filtersDisabledSeverity(t *testing.T) {
f := NewFolder(c, folderPath, "Test", scannerRecorder, hover.NewFakeHoverService(), scanner.NewMockScanNotifier(), notification.NewMockNotifier(), persistence.NewNopScanPersister(), scanstates.NewNoopStateAggregator())
ctx := context.Background()

c.SetSeverityFilter(types.NewSeverityFilter(true, true, false, false))
c.SetSeverityFilter(util.PtrOf(types.NewSeverityFilter(true, true, false, false)))

// act
f.ScanFile(ctx, filePath)
Expand Down Expand Up @@ -489,8 +489,8 @@ func Test_FilterCachedDiagnostics_filtersIgnoredIssues(t *testing.T) {
f := NewFolder(c, folderPath, "Test", scannerRecorder, hover.NewFakeHoverService(), scanner.NewMockScanNotifier(), notification.NewMockNotifier(), persistence.NewNopScanPersister(), scanstates.NewNoopStateAggregator())
ctx := context.Background()

c.SetSeverityFilter(types.NewSeverityFilter(true, true, false, false))
c.SetIssueViewOptions(types.NewIssueViewOptions(true, false))
c.SetSeverityFilter(util.PtrOf(types.NewSeverityFilter(true, true, false, false)))
c.SetIssueViewOptions(util.PtrOf(types.NewIssueViewOptions(true, false)))

// act
f.ScanFile(ctx, filePath)
Expand Down
4 changes: 3 additions & 1 deletion internal/testutil/test_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"path/filepath"
"testing"

"github.com/snyk/snyk-ls/internal/util"

"github.com/snyk/go-application-framework/pkg/configuration"
"github.com/snyk/go-application-framework/pkg/mocks"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -124,7 +126,7 @@ func prepareTestHelper(t *testing.T, envVar string, useConsistentIgnores bool) *
c.SetAuthenticationMethod(types.TokenAuthentication)
c.SetErrorReportingEnabled(false)
c.SetTrustedFolderFeatureEnabled(false)
c.SetIssueViewOptions(types.IssueViewOptions{OpenIssues: true, IgnoredIssues: true})
c.SetIssueViewOptions(util.PtrOf(types.NewIssueViewOptions(true, true)))
setMCPServerURL(t, c)
redirectConfigAndDataHome(t, c)

Expand Down
4 changes: 2 additions & 2 deletions internal/types/lsp.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,8 @@ type Settings struct {
IntegrationVersion string `json:"integrationVersion,omitempty"`
AutomaticAuthentication string `json:"automaticAuthentication,omitempty"`
DeviceId string `json:"deviceId,omitempty"`
FilterSeverity SeverityFilter `json:"filterSeverity,omitempty"`
IssueViewOptions IssueViewOptions `json:"issueViewOptions,omitempty"`
FilterSeverity *SeverityFilter `json:"filterSeverity,omitempty"`
IssueViewOptions *IssueViewOptions `json:"issueViewOptions,omitempty"`
SendErrorReports string `json:"sendErrorReports,omitempty"`
ManageBinariesAutomatically string `json:"manageBinariesAutomatically,omitempty"`
EnableTrustedFoldersFeature string `json:"enableTrustedFoldersFeature,omitempty"`
Expand Down
26 changes: 26 additions & 0 deletions internal/util/ptr_of.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* © 2025 Snyk Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package util

// PtrOf creates a pointer on the heap for the value provided.
// Because in Go you can't do something like `takesPtr(&(returnsStruct()))`.
// So instead do `takesPtr(PtrOf(returnsStruct()))`.
func PtrOf[T any](value T) *T {
pointerToValue := new(T) // Heap may be safer than `&value`
*pointerToValue = value
return pointerToValue
}