Skip to content

Commit ee1e330

Browse files
authored
fix: delta improve matching on dir name and similar path names [IDE-1051] (#810)
1 parent 1f7125f commit ee1e330

File tree

16 files changed

+202
-46
lines changed

16 files changed

+202
-46
lines changed

application/config/config.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,10 +365,11 @@ func (c *Config) CliSettings() *CliSettings {
365365
}
366366

367367
func (c *Config) Format() string {
368-
c.m.Lock()
369-
defer c.m.Unlock()
368+
c.m.RLock()
369+
defer c.m.RUnlock()
370370
return c.format
371371
}
372+
372373
func (c *Config) CLIDownloadLockFileName() (string, error) {
373374
c.cliSettings.cliPathAccessMutex.Lock()
374375
defer c.cliSettings.cliPathAccessMutex.Unlock()

application/server/server_smoke_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,33 @@ func Test_SmokeSnykCodeDelta_NoNewIssuesFound(t *testing.T) {
977977
assert.Equal(t, 0, len(issueList), "no issues expected, as delta and no new change")
978978
}
979979

980+
func Test_SmokeSnykCodeDelta_NoNewIssuesFound_JavaGoof(t *testing.T) {
981+
c := testutil.SmokeTest(t, false)
982+
loc, jsonRPCRecorder := setupServer(t, c)
983+
c.SetSnykCodeEnabled(true)
984+
c.SetDeltaFindingsEnabled(true)
985+
cleanupChannels()
986+
di.Init()
987+
scanAggregator := di.ScanStateAggregator()
988+
989+
var cloneTargetDir, err = storedconfig.SetupCustomTestRepo(t, types.FilePath(t.TempDir()), "https://github.com/snyk-labs/java-goof", "f5719ae", c.Logger())
990+
assert.NoError(t, err)
991+
992+
cloneTargetDirString := string(cloneTargetDir)
993+
994+
initParams := prepareInitParams(t, cloneTargetDir, c)
995+
996+
ensureInitialized(t, c, loc, initParams)
997+
998+
waitForScan(t, cloneTargetDirString, c)
999+
1000+
waitForDeltaScan(t, scanAggregator)
1001+
checkForScanParams(t, jsonRPCRecorder, cloneTargetDirString, product.ProductCode)
1002+
issueList := getIssueListFromPublishDiagnosticsNotification(t, jsonRPCRecorder, product.ProductCode, cloneTargetDir)
1003+
1004+
assert.Equal(t, 0, len(issueList), "no issues expected, as delta and no new change")
1005+
}
1006+
9801007
func ensureInitialized(t *testing.T, c *config.Config, loc server.Local, initParams types.InitializeParams) {
9811008
t.Helper()
9821009
t.Setenv("SNYK_LOG_LEVEL", "info")

domain/snyk/issues.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"github.com/snyk/snyk-ls/internal/delta"
2727
"github.com/snyk/snyk-ls/internal/product"
2828
"github.com/snyk/snyk-ls/internal/types"
29-
"github.com/snyk/snyk-ls/internal/util"
3029
)
3130

3231
var (
@@ -53,6 +52,8 @@ type Issue struct {
5352
Message string
5453
// todo [jc] this contains a formatted longest message for hovers, this needs to be pushed up and rendered in presentation. [bd] shouldn't the content and formatting be decided by the product?
5554
FormattedMessage string
55+
// ContentRoot is the root directory where is the issue was found
56+
ContentRoot types.FilePath
5657
// AffectedFilePath is the file path to the file where the issue was found
5758
AffectedFilePath types.FilePath
5859
// Product is the Snyk product, e.g. Snyk Open Source
@@ -94,6 +95,7 @@ func (i *Issue) Clone() *Issue {
9495
Message: i.Message,
9596
FormattedMessage: i.FormattedMessage,
9697
AffectedFilePath: i.AffectedFilePath,
98+
ContentRoot: i.ContentRoot,
9799
Product: i.Product,
98100
References: i.References,
99101
IssueDescriptionURL: i.IssueDescriptionURL,
@@ -190,6 +192,12 @@ func (i *Issue) GetID() string {
190192
return i.ID
191193
}
192194

195+
func (i *Issue) GetContentRoot() types.FilePath {
196+
i.m.RLock()
197+
defer i.m.RUnlock()
198+
return i.ContentRoot
199+
}
200+
193201
func (i *Issue) GetDescription() string {
194202
i.m.RLock()
195203
defer i.m.RUnlock()
@@ -328,7 +336,6 @@ func (i *Issue) SetFingerPrint(fingerprint string) {
328336
defer i.m.Unlock()
329337

330338
i.Fingerprint = fingerprint
331-
i.GlobalIdentity = util.HashWithoutConversion([]byte(fingerprint))
332339
}
333340

334341
func (i *Issue) GetRuleID() string {

domain/snyk/persistence/git_persistence_provider.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import (
3636

3737
const (
3838
CacheFolder = "snyk"
39-
SchemaVersion = "v1"
39+
SchemaVersion = "v1_1"
4040
)
4141

4242
var (

infrastructure/code/convert.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,7 @@ func (s *SarifConverter) toIssues(baseDir types.FilePath) (issues []types.Issue,
409409
Message: message,
410410
FormattedMessage: formattedMessage,
411411
IssueType: issueType,
412+
ContentRoot: baseDir,
412413
AffectedFilePath: types.FilePath(absPath),
413414
Product: product.ProductCode,
414415
IssueDescriptionURL: ruleLink,

infrastructure/iac/iac.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ func (iac *Scanner) retrieveAnalysis(scanResult iacScanResult, workspacePath typ
302302
issue.LineNumber = 0
303303
}
304304

305-
i, err := iac.toIssue(types.FilePath(targetFile), issue, fileContentString)
305+
i, err := iac.toIssue(workspacePath, types.FilePath(targetFile), issue, fileContentString)
306306
if err != nil {
307307
return nil, pkgerrors.Wrap(err, "unable to convert IaC issue to Snyk issue")
308308
}
@@ -332,7 +332,7 @@ func (iac *Scanner) getExtendedMessage(issue iacIssue) string {
332332
)
333333
}
334334

335-
func (iac *Scanner) toIssue(affectedFilePath types.FilePath, issue iacIssue, fileContent string) (*snyk.Issue, error) {
335+
func (iac *Scanner) toIssue(workspacePath types.FilePath, affectedFilePath types.FilePath, issue iacIssue, fileContent string) (*snyk.Issue, error) {
336336
const defaultRangeStart = 0
337337
const defaultRangeEnd = 80
338338
title := issue.IacDescription.Issue
@@ -377,6 +377,7 @@ func (iac *Scanner) toIssue(affectedFilePath types.FilePath, issue iacIssue, fil
377377
Message: title,
378378
FormattedMessage: iac.getExtendedMessage(issue),
379379
Severity: iac.toIssueSeverity(issue.Severity),
380+
ContentRoot: workspacePath,
380381
AffectedFilePath: affectedFilePath,
381382
Product: product.ProductInfrastructureAsCode,
382383
IssueDescriptionURL: issueURL,

infrastructure/iac/iac_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func Test_createIssueDataForCustomUI_SuccessfullyParses(t *testing.T) {
126126
c := testutil.UnitTest(t)
127127
sampleIssue := sampleIssue()
128128
scanner := New(c, performance.NewInstrumentor(), error_reporting.NewTestErrorReporter(), cli.NewTestExecutor())
129-
issue, err := scanner.toIssue("test.yml", sampleIssue, "")
129+
issue, err := scanner.toIssue("/path/to/issue", "test.yml", sampleIssue, "")
130130

131131
expectedAdditionalData := snyk.IaCIssueData{
132132
Key: "6a4df51fc4d53f1cfbdb4b46c165859b",
@@ -170,7 +170,7 @@ func Test_toIssue_issueHasHtmlTemplate(t *testing.T) {
170170
c := testutil.UnitTest(t)
171171
sampleIssue := sampleIssue()
172172
scanner := New(c, performance.NewInstrumentor(), error_reporting.NewTestErrorReporter(), cli.NewTestExecutor())
173-
issue, err := scanner.toIssue("test.yml", sampleIssue, "")
173+
issue, err := scanner.toIssue("/path/to/issue", "test.yml", sampleIssue, "")
174174

175175
assert.NoError(t, err)
176176

infrastructure/oss/cli_scanner.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ func (cliScanner *CLIScanner) unmarshallAndRetrieveAnalysis(ctx context.Context,
344344
logger.Error().Err(reportedErr).Send()
345345
continue
346346
}
347-
issues = append(issues, cliScanner.retrieveIssues(&scanResult, targetFilePath, fileContent)...)
347+
issues = append(issues, cliScanner.retrieveIssues(&scanResult, workDir, targetFilePath, fileContent)...)
348348
}
349349

350350
return issues
@@ -493,15 +493,11 @@ func determineTargetFile(displayTargetFile string) string {
493493
return strings.Replace(displayTargetFile, fileName, manifestFileName, 1)
494494
}
495495

496-
func (cliScanner *CLIScanner) retrieveIssues(
497-
res *scanResult,
498-
targetFilePath types.FilePath,
499-
fileContent []byte,
500-
) []types.Issue {
496+
func (cliScanner *CLIScanner) retrieveIssues(res *scanResult, workDir types.FilePath, targetFilePath types.FilePath, fileContent []byte) []types.Issue {
501497
// we are updating the cli scanner maps/attributes in parallel, so we need to lock
502498
cliScanner.mutex.Lock()
503499
defer cliScanner.mutex.Unlock()
504-
issues := convertScanResultToIssues(cliScanner.config, res, targetFilePath, fileContent, cliScanner.learnService, cliScanner.errorReporter, cliScanner.packageIssueCache)
500+
issues := convertScanResultToIssues(cliScanner.config, res, workDir, targetFilePath, fileContent, cliScanner.learnService, cliScanner.errorReporter, cliScanner.packageIssueCache)
505501

506502
// repopulate
507503
cliScanner.addVulnerabilityCountsToCache(issues)

infrastructure/oss/issue.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ var issuesSeverity = map[string]types.Severity{
4141
"medium": types.Medium,
4242
}
4343

44-
func toIssue(affectedFilePath types.FilePath, issue ossIssue, scanResult *scanResult, issueDepNode *ast.Node, learnService learn.Service, ep error_reporting.ErrorReporter) *snyk.Issue {
44+
func toIssue(c *config.Config, workDir types.FilePath, affectedFilePath types.FilePath, issue ossIssue, scanResult *scanResult, issueDepNode *ast.Node, learnService learn.Service, ep error_reporting.ErrorReporter) *snyk.Issue {
4545
// this needs to be first so that the lesson from Snyk Learn is added
4646
codeActions := issue.AddCodeActions(learnService, ep, affectedFilePath, issueDepNode)
4747

@@ -74,7 +74,7 @@ func toIssue(affectedFilePath types.FilePath, issue ossIssue, scanResult *scanRe
7474
additionalData := issue.toAdditionalData(scanResult, matchingIssues, affectedFilePath)
7575

7676
title := issue.Title
77-
if config.CurrentConfig().Format() == config.FormatHtml {
77+
if c.Format() == config.FormatHtml {
7878
title = string(markdown.ToHTML([]byte(title), nil, nil))
7979
}
8080

@@ -96,6 +96,7 @@ func toIssue(affectedFilePath types.FilePath, issue ossIssue, scanResult *scanRe
9696
FormattedMessage: issue.GetExtendedMessage(issue),
9797
Range: getRangeFromNode(issueDepNode),
9898
Severity: issue.ToIssueSeverity(),
99+
ContentRoot: workDir,
99100
AffectedFilePath: affectedFilePath,
100101
Product: product.ProductOpenSource,
101102
IssueDescriptionURL: issue.CreateIssueURL(),
@@ -125,7 +126,7 @@ func getRangeFromNode(issueDepNode *ast.Node) types.Range {
125126
return r
126127
}
127128

128-
func convertScanResultToIssues(c *config.Config, res *scanResult, targetFilePath types.FilePath, fileContent []byte, ls learn.Service, ep error_reporting.ErrorReporter, packageIssueCache map[string][]types.Issue) []types.Issue {
129+
func convertScanResultToIssues(c *config.Config, res *scanResult, workDir types.FilePath, targetFilePath types.FilePath, fileContent []byte, ls learn.Service, ep error_reporting.ErrorReporter, packageIssueCache map[string][]types.Issue) []types.Issue {
129130
var issues []types.Issue
130131

131132
duplicateCheckMap := map[string]bool{}
@@ -137,7 +138,7 @@ func convertScanResultToIssues(c *config.Config, res *scanResult, targetFilePath
137138
continue
138139
}
139140
node := getDependencyNode(c, targetFilePath, issue, fileContent)
140-
snykIssue := toIssue(targetFilePath, issue, res, node, ls, ep)
141+
snykIssue := toIssue(c, workDir, targetFilePath, issue, res, node, ls, ep)
141142
packageIssueCache[packageKey] = append(packageIssueCache[packageKey], snykIssue)
142143
issues = append(issues, snykIssue)
143144
duplicateCheckMap[duplicateKey] = true

infrastructure/oss/oss_test.go

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -102,17 +102,19 @@ func Test_introducingPackageAndVersion(t *testing.T) {
102102
}
103103

104104
func Test_toIssue_LearnParameterConversion(t *testing.T) {
105+
c := testutil.UnitTest(t)
105106
sampleOssIssue := sampleIssue()
106107
scanner := CLIScanner{
107108
learnService: getLearnMock(t),
108109
}
109-
110-
issue := toIssue("testPath", sampleOssIssue, &scanResult{}, nonEmptyNode(), scanner.learnService, scanner.errorReporter)
110+
contentRoot := types.FilePath("/path/to/issue")
111+
issue := toIssue(c, contentRoot, "testPath", sampleOssIssue, &scanResult{}, nonEmptyNode(), scanner.learnService, scanner.errorReporter)
111112

112113
assert.Equal(t, sampleOssIssue.Id, issue.ID)
113114
assert.Equal(t, sampleOssIssue.Identifiers.CWE, issue.CWEs)
114115
assert.Equal(t, sampleOssIssue.Identifiers.CVE, issue.CVEs)
115116
assert.Equal(t, sampleOssIssue.PackageManager, issue.Ecosystem)
117+
assert.Equal(t, contentRoot, issue.ContentRoot)
116118
assert.Equal(t, "url", (issue.AdditionalData).(snyk.OssIssueData).Lesson)
117119
}
118120

@@ -121,6 +123,7 @@ func nonEmptyNode() *ast.Node {
121123
}
122124

123125
func Test_toIssue_CodeActions(t *testing.T) {
126+
c := testutil.UnitTest(t)
124127
const flashy = "⚡️ "
125128
tests := []struct {
126129
name string
@@ -138,21 +141,23 @@ func Test_toIssue_CodeActions(t *testing.T) {
138141
}
139142
for _, test := range tests {
140143
t.Run(test.name, func(t *testing.T) {
141-
config.CurrentConfig().SetSnykOSSQuickFixCodeActionsEnabled(true)
142-
config.CurrentConfig().SetSnykOpenBrowserActionsEnabled(test.openBrowserEnabled)
144+
c.SetSnykOSSQuickFixCodeActionsEnabled(true)
145+
c.SetSnykOpenBrowserActionsEnabled(test.openBrowserEnabled)
143146

144147
sampleOssIssue := sampleIssue()
145148
scanner := CLIScanner{
146149
learnService: getLearnMock(t),
147150
}
148151
sampleOssIssue.PackageManager = test.packageManager
149152
sampleOssIssue.UpgradePath = []any{"false", test.packageName}
153+
contentRoot := types.FilePath("/path/to/issue")
150154

151-
issue := toIssue("testPath", sampleOssIssue, &scanResult{}, nonEmptyNode(), scanner.learnService, scanner.errorReporter)
155+
issue := toIssue(c, contentRoot, "testPath", sampleOssIssue, &scanResult{}, nonEmptyNode(), scanner.learnService, scanner.errorReporter)
152156

153157
assert.Equal(t, sampleOssIssue.Id, issue.ID)
154158
assert.Equal(t, flashy+test.expectedUpgrade, issue.CodeActions[0].GetTitle())
155159
assert.Equal(t, 1, len(issue.CodelensCommands))
160+
assert.Equal(t, contentRoot, issue.ContentRoot)
156161
assert.Equal(t, flashy+test.expectedUpgrade, issue.CodelensCommands[0].Title)
157162

158163
if test.openBrowserEnabled {
@@ -168,16 +173,21 @@ func Test_toIssue_CodeActions(t *testing.T) {
168173
}
169174

170175
func Test_toIssue_CodeActions_WithoutFix(t *testing.T) {
176+
c := testutil.UnitTest(t)
177+
c.SetSnykOpenBrowserActionsEnabled(true)
178+
171179
sampleOssIssue := sampleIssue()
172180
scanner := CLIScanner{
173181
learnService: getLearnMock(t),
174182
}
175183
sampleOssIssue.UpgradePath = []any{"*"}
184+
contentRoot := types.FilePath("/path/to/issue")
176185

177-
issue := toIssue("testPath", sampleOssIssue, &scanResult{}, nonEmptyNode(), scanner.learnService, scanner.errorReporter)
186+
issue := toIssue(c, contentRoot, "testPath", sampleOssIssue, &scanResult{}, nonEmptyNode(), scanner.learnService, scanner.errorReporter)
178187

179188
assert.Equal(t, sampleOssIssue.Id, issue.ID)
180189
assert.Equal(t, 2, len(issue.CodeActions))
190+
assert.Equal(t, contentRoot, issue.ContentRoot)
181191
assert.Equal(t, "Open description of 'THOU SHALL NOT PASS affecting package pkg' in browser (Snyk)",
182192
issue.CodeActions[0].GetTitle())
183193
assert.Equal(t, "Learn more about THOU SHALL NOT PASS (Snyk)", issue.CodeActions[1].GetTitle())
@@ -434,7 +444,7 @@ func Test_scheduleNewScanWithProductDisabled_NoScanRun(t *testing.T) {
434444
c := testutil.UnitTest(t)
435445

436446
// Arrange
437-
config.CurrentConfig().SetSnykOssEnabled(false)
447+
c.SetSnykOssEnabled(false)
438448
fakeCli := cli.NewTestExecutor()
439449
fakeCli.ExecuteDuration = time.Millisecond
440450
scanner := NewCLIScanner(c, performance.NewInstrumentor(), error_reporting.NewTestErrorReporter(), fakeCli, getLearnMock(t), notification.NewMockNotifier()).(*CLIScanner)

0 commit comments

Comments
 (0)