Skip to content

Commit f26d123

Browse files
rramarrama
authored
fix: use pointers for optional LSP settings [IDE-899] (#811)
Previously we had no way to distinguish between the settings object being omitted and being set to "empty" value for the struct. As such we were ignoring settings objects that were set to the same as the "empty" struct value, as it would have broken / overridden the default LSP settings for clients that did not support the setting. With this change to using pointers, we can distinguish between a client not sending the settings object (because they don't support it yet) and a client sending the settings object but deliberately setting the values to be the same as the empty struct.
1 parent bf16f10 commit f26d123

13 files changed

+112
-48
lines changed

‎README.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -492,13 +492,13 @@ within `initializationOptions?: LSPAny;` we support the following settings:
492492
"integrationVersion": "1.0.0", // The version of the IDE or editor the LS is running in
493493
"automaticAuthentication": "true", // Whether LS will automatically authenticate on scan start (default: true)
494494
"deviceId": "a UUID", // A unique ID from the running the LS, used for telemetry
495-
"filterSeverity": { // Filters to be applied for the determined issues
495+
"filterSeverity": { // Optional filter to be applied for the determined issues (if omitted: no filtering)
496496
"critical": true,
497497
"high": true,
498498
"medium": true,
499499
"low": true,
500500
},
501-
"issueViewOptions": { // Another filter to be applied for the determined issues
501+
"issueViewOptions": { // Optional filter to be applied for the determined issues (if omitted: no filtering)
502502
"openIssues": true,
503503
"ignoredIssues": false,
504504
},

‎application/config/config.go

+8-10
Original file line numberDiff line numberDiff line change
@@ -585,29 +585,27 @@ func (c *Config) SetSnykAdvisorEnabled(enabled bool) {
585585
c.isSnykAdvisorEnabled = enabled
586586
}
587587

588-
func (c *Config) SetSeverityFilter(severityFilter types.SeverityFilter) bool {
588+
func (c *Config) SetSeverityFilter(severityFilter *types.SeverityFilter) bool {
589589
c.m.Lock()
590590
defer c.m.Unlock()
591-
emptySeverityFilter := types.SeverityFilter{}
592-
if severityFilter == emptySeverityFilter {
591+
if severityFilter == nil {
593592
return false
594593
}
595-
filterModified := c.filterSeverity != severityFilter
594+
filterModified := c.filterSeverity != *severityFilter
596595
c.logger.Debug().Str("method", "SetSeverityFilter").Interface("severityFilter", severityFilter).Msg("Setting severity filter:")
597-
c.filterSeverity = severityFilter
596+
c.filterSeverity = *severityFilter
598597
return filterModified
599598
}
600599

601-
func (c *Config) SetIssueViewOptions(issueViewOptions types.IssueViewOptions) bool {
600+
func (c *Config) SetIssueViewOptions(issueViewOptions *types.IssueViewOptions) bool {
602601
c.m.Lock()
603602
defer c.m.Unlock()
604-
emptyIssueViewOptions := types.IssueViewOptions{}
605-
if issueViewOptions == emptyIssueViewOptions {
603+
if issueViewOptions == nil {
606604
return false
607605
}
608-
issueViewOptionsModified := c.issueViewOptions != issueViewOptions
606+
issueViewOptionsModified := c.issueViewOptions != *issueViewOptions
609607
c.logger.Debug().Str("method", "SetIssueViewOptions").Interface("issueViewOptions", issueViewOptions).Msg("Setting issue view options:")
610-
c.issueViewOptions = issueViewOptions
608+
c.issueViewOptions = *issueViewOptions
611609
return issueViewOptionsModified
612610
}
613611

‎application/config/config_test.go

+8-6
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import (
2121
"testing"
2222
"time"
2323

24+
"github.com/snyk/snyk-ls/internal/util"
25+
2426
"github.com/google/uuid"
2527
"github.com/stretchr/testify/assert"
2628
"github.com/stretchr/testify/require"
@@ -168,37 +170,37 @@ func TestSnykCodeApi(t *testing.T) {
168170
func Test_SetSeverityFilter(t *testing.T) {
169171
t.Run("Saves filter", func(t *testing.T) {
170172
c := New()
171-
c.SetSeverityFilter(types.NewSeverityFilter(true, true, false, false))
173+
c.SetSeverityFilter(util.Ptr(types.NewSeverityFilter(true, true, false, false)))
172174
assert.Equal(t, types.NewSeverityFilter(true, true, false, false), c.FilterSeverity())
173175
})
174176

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

179-
modified := c.SetSeverityFilter(lowExcludedFilter)
181+
modified := c.SetSeverityFilter(&lowExcludedFilter)
180182
assert.True(t, modified)
181183

182-
modified = c.SetSeverityFilter(lowExcludedFilter)
184+
modified = c.SetSeverityFilter(&lowExcludedFilter)
183185
assert.False(t, modified)
184186
})
185187
}
186188

187189
func Test_SetIssueViewOptions(t *testing.T) {
188190
t.Run("Saves filter", func(t *testing.T) {
189191
c := New()
190-
c.SetIssueViewOptions(types.NewIssueViewOptions(false, true))
192+
c.SetIssueViewOptions(util.Ptr(types.NewIssueViewOptions(false, true)))
191193
assert.Equal(t, types.NewIssueViewOptions(false, true), c.IssueViewOptions())
192194
})
193195

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

198-
modified := c.SetIssueViewOptions(ignoredOnlyFilter)
200+
modified := c.SetIssueViewOptions(&ignoredOnlyFilter)
199201
assert.True(t, modified)
200202

201-
modified = c.SetIssueViewOptions(ignoredOnlyFilter)
203+
modified = c.SetIssueViewOptions(&ignoredOnlyFilter)
202204
assert.False(t, modified)
203205
})
204206
}

‎application/server/authentication_smoke_test.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import (
2323
"testing"
2424
"time"
2525

26+
"github.com/snyk/snyk-ls/internal/util"
27+
2628
"github.com/stretchr/testify/assert"
2729
"github.com/stretchr/testify/require"
2830
"golang.org/x/oauth2"
@@ -78,8 +80,8 @@ func checkInvalidCredentialsMessageRequest(t *testing.T, expected string, tokenS
7880
InitializationOptions: types.Settings{
7981
Token: tokenString,
8082
EnableTrustedFoldersFeature: "false",
81-
FilterSeverity: types.DefaultSeverityFilter(),
82-
IssueViewOptions: types.DefaultIssueViewOptions(),
83+
FilterSeverity: util.Ptr(types.DefaultSeverityFilter()),
84+
IssueViewOptions: util.Ptr(types.DefaultIssueViewOptions()),
8385
AuthenticationMethod: types.OAuthAuthentication,
8486
AutomaticAuthentication: "false",
8587
},

‎application/server/configuration.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ func updateProductEnablement(c *config.Config, settings types.Settings) {
395395
}
396396
}
397397

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

@@ -404,7 +404,7 @@ func updateIssueViewOptions(c *config.Config, s types.IssueViewOptions) {
404404
}
405405
}
406406

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

‎application/server/configuration_test.go

+36-6
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,8 @@ func Test_UpdateSettings(t *testing.T) {
171171

172172
tempDir1 := filepath.Join(t.TempDir(), "tempDir1")
173173
tempDir2 := filepath.Join(t.TempDir(), "tempDir2")
174+
nonDefaultSeverityFilter := types.NewSeverityFilter(false, true, false, true)
175+
nonDefaultIssueViewOptions := types.NewIssueViewOptions(false, true)
174176
hoverVerbosity := 1
175177
outputFormat := "html"
176178
settings := types.Settings{
@@ -187,8 +189,8 @@ func Test_UpdateSettings(t *testing.T) {
187189
ManageBinariesAutomatically: "false",
188190
CliPath: filepath.Join(t.TempDir(), "cli"),
189191
Token: "a fancy token",
190-
FilterSeverity: types.DefaultSeverityFilter(),
191-
IssueViewOptions: types.DefaultIssueViewOptions(),
192+
FilterSeverity: &nonDefaultSeverityFilter,
193+
IssueViewOptions: &nonDefaultIssueViewOptions,
192194
TrustedFolders: []string{"trustedPath1", "trustedPath2"},
193195
OsPlatform: "windows",
194196
OsArch: "amd64",
@@ -234,8 +236,8 @@ func Test_UpdateSettings(t *testing.T) {
234236
assert.Equal(t, expectedOrgId, c.Organization())
235237
assert.False(t, c.ManageBinariesAutomatically())
236238
assert.Equal(t, settings.CliPath, c.CliSettings().Path())
237-
assert.Equal(t, types.DefaultSeverityFilter(), c.FilterSeverity())
238-
assert.Equal(t, types.DefaultIssueViewOptions(), c.IssueViewOptions())
239+
assert.Equal(t, nonDefaultSeverityFilter, c.FilterSeverity())
240+
assert.Equal(t, nonDefaultIssueViewOptions, c.IssueViewOptions())
239241
assert.Subset(t, []types.FilePath{"trustedPath1", "trustedPath2"}, c.TrustedFolders())
240242
assert.Equal(t, settings.OsPlatform, c.OsPlatform())
241243
assert.Equal(t, settings.OsArch, c.OsArch())
@@ -405,18 +407,46 @@ func Test_UpdateSettings(t *testing.T) {
405407
c := testutil.UnitTest(t)
406408
t.Run("filtering gets passed", func(t *testing.T) {
407409
mixedSeverityFilter := types.NewSeverityFilter(true, false, true, false)
408-
UpdateSettings(c, types.Settings{FilterSeverity: mixedSeverityFilter})
410+
UpdateSettings(c, types.Settings{FilterSeverity: &mixedSeverityFilter})
409411

410412
assert.Equal(t, mixedSeverityFilter, c.FilterSeverity())
411413
})
414+
t.Run("equivalent of the \"empty\" struct as a filter gets passed", func(t *testing.T) {
415+
emptyLikeSeverityFilter := types.NewSeverityFilter(false, false, false, false)
416+
UpdateSettings(c, types.Settings{FilterSeverity: &emptyLikeSeverityFilter})
417+
418+
assert.Equal(t, emptyLikeSeverityFilter, c.FilterSeverity())
419+
})
420+
t.Run("omitting filter does not cause an update", func(t *testing.T) {
421+
mixedSeverityFilter := types.NewSeverityFilter(false, false, true, false)
422+
UpdateSettings(c, types.Settings{FilterSeverity: &mixedSeverityFilter})
423+
assert.Equal(t, mixedSeverityFilter, c.FilterSeverity())
424+
425+
UpdateSettings(c, types.Settings{})
426+
assert.Equal(t, mixedSeverityFilter, c.FilterSeverity())
427+
})
412428
})
413429

414430
t.Run("issue view options", func(t *testing.T) {
415431
c := testutil.UnitTest(t)
416432
t.Run("filtering gets passed", func(t *testing.T) {
417433
mixedIssueViewOptions := types.NewIssueViewOptions(false, true)
418-
UpdateSettings(c, types.Settings{IssueViewOptions: mixedIssueViewOptions})
434+
UpdateSettings(c, types.Settings{IssueViewOptions: &mixedIssueViewOptions})
435+
436+
assert.Equal(t, mixedIssueViewOptions, c.IssueViewOptions())
437+
})
438+
t.Run("equivalent of the \"empty\" struct as a filter gets passed", func(t *testing.T) {
439+
emptyLikeIssueViewOptions := types.NewIssueViewOptions(false, false)
440+
UpdateSettings(c, types.Settings{IssueViewOptions: &emptyLikeIssueViewOptions})
441+
442+
assert.Equal(t, emptyLikeIssueViewOptions, c.IssueViewOptions())
443+
})
444+
t.Run("omitting filter does not cause an update", func(t *testing.T) {
445+
mixedIssueViewOptions := types.NewIssueViewOptions(false, true)
446+
UpdateSettings(c, types.Settings{IssueViewOptions: &mixedIssueViewOptions})
447+
assert.Equal(t, mixedIssueViewOptions, c.IssueViewOptions())
419448

449+
UpdateSettings(c, types.Settings{})
420450
assert.Equal(t, mixedIssueViewOptions, c.IssueViewOptions())
421451
})
422452
})

‎application/server/parallelization_test.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import (
2424
"testing"
2525
"time"
2626

27+
"github.com/snyk/snyk-ls/internal/util"
28+
2729
"github.com/stretchr/testify/assert"
2830
"github.com/stretchr/testify/require"
2931

@@ -80,8 +82,8 @@ func Test_Concurrent_CLI_Runs(t *testing.T) {
8082
Endpoint: os.Getenv("SNYK_API"),
8183
Token: os.Getenv("SNYK_TOKEN"),
8284
EnableTrustedFoldersFeature: "false",
83-
FilterSeverity: types.DefaultSeverityFilter(),
84-
IssueViewOptions: types.DefaultIssueViewOptions(),
85+
FilterSeverity: util.Ptr(types.DefaultSeverityFilter()),
86+
IssueViewOptions: util.Ptr(types.DefaultIssueViewOptions()),
8587
AuthenticationMethod: types.TokenAuthentication,
8688
AutomaticAuthentication: "false",
8789
ManageBinariesAutomatically: "true",

‎application/server/server_smoke_test.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import (
2727
"testing"
2828
"time"
2929

30+
"github.com/snyk/snyk-ls/internal/util"
31+
3032
"github.com/creachadair/jrpc2"
3133
"github.com/creachadair/jrpc2/server"
3234
"github.com/go-git/go-git/v5"
@@ -793,8 +795,8 @@ func prepareInitParams(t *testing.T, cloneTargetDir types.FilePath, c *config.Co
793795
Endpoint: os.Getenv("SNYK_API"),
794796
Token: os.Getenv("SNYK_TOKEN"),
795797
EnableTrustedFoldersFeature: "false",
796-
FilterSeverity: types.DefaultSeverityFilter(),
797-
IssueViewOptions: types.DefaultIssueViewOptions(),
798+
FilterSeverity: util.Ptr(types.DefaultSeverityFilter()),
799+
IssueViewOptions: util.Ptr(types.DefaultIssueViewOptions()),
798800
AuthenticationMethod: types.TokenAuthentication,
799801
EnableDeltaFindings: strconv.FormatBool(c.IsDeltaFindingsEnabled()),
800802
ActivateSnykCode: strconv.FormatBool(c.IsSnykCodeEnabled()),
@@ -867,8 +869,8 @@ func Test_SmokeSnykCodeFileScan(t *testing.T) {
867869
Endpoint: os.Getenv("SNYK_API"),
868870
Token: os.Getenv("SNYK_TOKEN"),
869871
EnableTrustedFoldersFeature: "false",
870-
FilterSeverity: types.DefaultSeverityFilter(),
871-
IssueViewOptions: types.DefaultIssueViewOptions(),
872+
FilterSeverity: util.Ptr(types.DefaultSeverityFilter()),
873+
IssueViewOptions: util.Ptr(types.DefaultIssueViewOptions()),
872874
},
873875
}
874876

‎application/server/server_test.go

+8-6
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import (
2525
"testing"
2626
"time"
2727

28+
"github.com/snyk/snyk-ls/internal/util"
29+
2830
"github.com/snyk/snyk-ls/domain/snyk"
2931
"github.com/snyk/snyk-ls/domain/snyk/scanner"
3032
"github.com/snyk/snyk-ls/internal/storedconfig"
@@ -367,8 +369,8 @@ func Test_TextDocumentCodeLenses_shouldReturnCodeLenses(t *testing.T) {
367369
Token: "xxx",
368370
ManageBinariesAutomatically: "true",
369371
CliPath: filepath.Join(t.TempDir(), "cli"),
370-
FilterSeverity: types.DefaultSeverityFilter(),
371-
IssueViewOptions: types.DefaultIssueViewOptions(),
372+
FilterSeverity: util.Ptr(types.DefaultSeverityFilter()),
373+
IssueViewOptions: util.Ptr(types.DefaultIssueViewOptions()),
372374
EnableTrustedFoldersFeature: "false",
373375
},
374376
}
@@ -430,8 +432,8 @@ func Test_TextDocumentCodeLenses_dirtyFileShouldFilterCodeLenses(t *testing.T) {
430432
Token: "xxx",
431433
ManageBinariesAutomatically: "true",
432434
CliPath: filepath.Join(t.TempDir(), "cli"),
433-
FilterSeverity: types.DefaultSeverityFilter(),
434-
IssueViewOptions: types.DefaultIssueViewOptions(),
435+
FilterSeverity: util.Ptr(types.DefaultSeverityFilter()),
436+
IssueViewOptions: util.Ptr(types.DefaultIssueViewOptions()),
435437
EnableTrustedFoldersFeature: "false",
436438
},
437439
}
@@ -486,8 +488,8 @@ func Test_initialize_updatesSettings(t *testing.T) {
486488
InitializationOptions: types.Settings{
487489
Organization: expectedOrgId,
488490
Token: "xxx",
489-
FilterSeverity: types.DefaultSeverityFilter(),
490-
IssueViewOptions: types.DefaultIssueViewOptions(),
491+
FilterSeverity: util.Ptr(types.DefaultSeverityFilter()),
492+
IssueViewOptions: util.Ptr(types.DefaultIssueViewOptions()),
491493
},
492494
}
493495

‎domain/ide/workspace/folder_test.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func TestProcessResults_whenFilteringSeverity_ProcessesOnlyFilteredIssues(t *tes
201201
c := testutil.UnitTest(t)
202202

203203
severityFilter := types.NewSeverityFilter(true, false, true, false)
204-
config.CurrentConfig().SetSeverityFilter(severityFilter)
204+
config.CurrentConfig().SetSeverityFilter(&severityFilter)
205205

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

@@ -251,7 +251,7 @@ func TestProcessResults_whenFilteringIssueViewOptions_ProcessesOnlyFilteredIssue
251251
c := testutil.UnitTest(t)
252252

253253
issueViewOptions := types.NewIssueViewOptions(false, true)
254-
config.CurrentConfig().SetIssueViewOptions(issueViewOptions)
254+
config.CurrentConfig().SetIssueViewOptions(&issueViewOptions)
255255

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

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

438-
c.SetSeverityFilter(types.NewSeverityFilter(true, true, false, false))
438+
c.SetSeverityFilter(util.Ptr(types.NewSeverityFilter(true, true, false, false)))
439439

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

492-
c.SetSeverityFilter(types.NewSeverityFilter(true, true, false, false))
493-
c.SetIssueViewOptions(types.NewIssueViewOptions(true, false))
492+
c.SetSeverityFilter(util.Ptr(types.NewSeverityFilter(true, true, false, false)))
493+
c.SetIssueViewOptions(util.Ptr(types.NewIssueViewOptions(true, false)))
494494

495495
// act
496496
f.ScanFile(ctx, filePath)

‎internal/testutil/test_setup.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import (
2222
"path/filepath"
2323
"testing"
2424

25+
"github.com/snyk/snyk-ls/internal/util"
26+
2527
"github.com/snyk/go-application-framework/pkg/configuration"
2628
"github.com/snyk/go-application-framework/pkg/mocks"
2729
"github.com/stretchr/testify/require"
@@ -124,7 +126,7 @@ func prepareTestHelper(t *testing.T, envVar string, useConsistentIgnores bool) *
124126
c.SetAuthenticationMethod(types.TokenAuthentication)
125127
c.SetErrorReportingEnabled(false)
126128
c.SetTrustedFolderFeatureEnabled(false)
127-
c.SetIssueViewOptions(types.IssueViewOptions{OpenIssues: true, IgnoredIssues: true})
129+
c.SetIssueViewOptions(util.Ptr(types.NewIssueViewOptions(true, true)))
128130
setMCPServerURL(t, c)
129131
redirectConfigAndDataHome(t, c)
130132

‎internal/types/lsp.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -581,8 +581,8 @@ type Settings struct {
581581
IntegrationVersion string `json:"integrationVersion,omitempty"`
582582
AutomaticAuthentication string `json:"automaticAuthentication,omitempty"`
583583
DeviceId string `json:"deviceId,omitempty"`
584-
FilterSeverity SeverityFilter `json:"filterSeverity,omitempty"`
585-
IssueViewOptions IssueViewOptions `json:"issueViewOptions,omitempty"`
584+
FilterSeverity *SeverityFilter `json:"filterSeverity,omitempty"`
585+
IssueViewOptions *IssueViewOptions `json:"issueViewOptions,omitempty"`
586586
SendErrorReports string `json:"sendErrorReports,omitempty"`
587587
ManageBinariesAutomatically string `json:"manageBinariesAutomatically,omitempty"`
588588
EnableTrustedFoldersFeature string `json:"enableTrustedFoldersFeature,omitempty"`

0 commit comments

Comments
 (0)