fix: dont scan if workspace didnt change (#606)

application/server/server_smoke_test.go

@@ -478,11 +478,11 @@ func runSmokeTest(t *testing.T, repo string, commit string, file1 string, file2
 	}
 }
 
-func newTestFileWithVulns(t *testing.T, cloneTargetDir string, fileName string) {
+func newFileInCurrentDir(t *testing.T, cloneTargetDir string, fileName string, content string) {
 	t.Helper()
 
 	testFile := filepath.Join(cloneTargetDir, fileName)
-	err := os.WriteFile(testFile, []byte("var token = 'SECRET_TOKEN_f8ed84e8f41e4146403dd4a6bbcea5e418d23a9';"), 0600)
+	err := os.WriteFile(testFile, []byte(content), 0600)
 	assert.NoError(t, err)
 }
 
@@ -754,7 +754,7 @@ func Test_SmokeSnykCodeDelta_OneNewVuln(t *testing.T) {
 	var cloneTargetDir, err = testutil.SetupCustomTestRepo(t, t.TempDir(), "https://github.com/snyk-labs/nodejs-goof", "0336589", c.Logger())
 	assert.NoError(t, err)
 
-	newTestFileWithVulns(t, cloneTargetDir, fileWithNewVulns)
+	newFileInCurrentDir(t, cloneTargetDir, fileWithNewVulns, "var token = 'SECRET_TOKEN_f8ed84e8f41e4146403dd4a6bbcea5e418d23a9';")
 
 	initParams := prepareInitParams(t, cloneTargetDir, c)
 
@@ -768,6 +768,53 @@ func Test_SmokeSnykCodeDelta_OneNewVuln(t *testing.T) {
 	assert.Contains(t, snykCodeScanParams.Issues[0].FilePath, fileWithNewVulns)
 }
 
+func Test_SmokeSnykCodeDelta_NoScanNecessary(t *testing.T) {
+	loc, jsonRPCRecorder := setupServer(t)
+	c := testutil.SmokeTest(t, false)
+	c.SetSnykCodeEnabled(true)
+	c.SetDeltaFindingsEnabled(true)
+	cleanupChannels()
+	di.Init()
+
+	var cloneTargetDir, err = testutil.SetupCustomTestRepo(t, t.TempDir(), "https://github.com/snyk-labs/nodejs-goof", "0336589", c.Logger())
+	assert.NoError(t, err)
+
+	initParams := prepareInitParams(t, cloneTargetDir, c)
+
+	ensureInitialized(t, loc, initParams)
+
+	waitForScan(t, cloneTargetDir)
+
+	snykCodeScanParams := checkForScanParams(t, jsonRPCRecorder, cloneTargetDir, product.ProductCode)
+
+	assert.Equal(t, len(snykCodeScanParams.Issues), 0)
+}
+
+func Test_SmokeSnykCodeDelta_NoNewIssuesFound(t *testing.T) {
+	loc, jsonRPCRecorder := setupServer(t)
+	c := testutil.SmokeTest(t, false)
+	c.SetSnykCodeEnabled(true)
+	c.SetDeltaFindingsEnabled(true)
+	cleanupChannels()
+	di.Init()
+
+	fileWithNewVulns := "vulns.js"
+	var cloneTargetDir, err = testutil.SetupCustomTestRepo(t, t.TempDir(), "https://github.com/snyk-labs/nodejs-goof", "0336589", c.Logger())
+	assert.NoError(t, err)
+
+	newFileInCurrentDir(t, cloneTargetDir, fileWithNewVulns, "// no problems")
+
+	initParams := prepareInitParams(t, cloneTargetDir, c)
+
+	ensureInitialized(t, loc, initParams)
+
+	waitForScan(t, cloneTargetDir)
+
+	snykCodeScanParams := checkForScanParams(t, jsonRPCRecorder, cloneTargetDir, product.ProductCode)
+
+	assert.Equal(t, len(snykCodeScanParams.Issues), 0)
+}
+
 func ensureInitialized(t *testing.T, loc server.Local, initParams types.InitializeParams) {
 	t.Helper()
 

domain/ide/workspace/folder.go

@@ -19,7 +19,6 @@ package workspace
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"strings"
 	"sync"
@@ -305,11 +304,7 @@ func (f *Folder) scan(ctx context.Context, path string) {
 
 func (f *Folder) processResults(scanData snyk.ScanData) {
 	if scanData.Err != nil {
-		f.scanNotifier.SendError(scanData.Product, f.path, scanData.Err.Error())
-		f.c.Logger().Err(scanData.Err).
-			Str("method", "processResults").
-			Str("product", string(scanData.Product)).
-			Msg("Product returned an error")
+		f.sendScanError(scanData.Product, scanData.Err)
 		return
 	}
 	// this also updates the severity counts in scan data, therefore we pass a pointer
@@ -321,6 +316,15 @@ func (f *Folder) processResults(scanData snyk.ScanData) {
 	f.FilterAndPublishDiagnostics(&scanData.Product)
 }
 
+func (f *Folder) sendScanError(product product.Product, err error) {
+	f.scanNotifier.SendError(product, f.path, err.Error())
+	f.c.Logger().Err(err).
+		Str("method", "processResults").
+		Str("product", string(product)).
+		Msg("Product returned an error")
+	f.notifier.SendErrorDiagnostic(f.path, err)
+}
+
 func (f *Folder) updateGlobalCacheAndSeverityCounts(scanData *snyk.ScanData) {
 	var newCache = snyk.IssuesByFile{}
 	var dedupMap = map[string]bool{}
@@ -471,12 +475,10 @@ func appendTestResults(sic snyk.SeverityIssueCounts, results []json_schemas.Test
 func (f *Folder) FilterAndPublishDiagnostics(p *product.Product) {
 	productIssuesByFile, err := f.getDelta(f.IssuesByProduct(), p)
 	if err != nil {
-		f.c.Logger().Error().Err(err).Msg("Error getting delta for product issues")
-		if errors.Is(err, delta.ErrNoDeltaCalculated) {
-			deltaErr := fmt.Errorf("Couldn't determine the difference between current and base branch for %s scan. %s"+
-				"Falling back to showing full scan results. Please check the error log for more information.", p.ToProductNamesString(), err)
-			f.notifier.SendErrorDiagnostic(f.path, deltaErr)
-		}
+		// Error can only be returned from delta analysis. Other non delta scans are skipped with no errors.
+		deltaErr := fmt.Errorf("couldn't determine the difference between current and base branch for %s scan. %w", p.ToProductNamesString(), err)
+		f.sendScanError(*p, deltaErr)
+		return
 	}
 	if p != nil {
 		filteredIssues := f.filterDiagnostics(productIssuesByFile[*p])
@@ -489,21 +491,25 @@ func (f *Folder) FilterAndPublishDiagnostics(p *product.Product) {
 	}
 }
 
+// Error can only be returned from delta analysis. Other non delta scans are skipped with no errors.
 func (f *Folder) getDelta(productIssueByFile snyk.ProductIssuesByFile, p *product.Product) (snyk.ProductIssuesByFile, error) {
 	logger := f.c.Logger().With().Str("method", "getDelta").Logger()
 	currentProduct := *p
+
 	// Delete product check when base scanning is implemented for other products
 	if !f.c.IsDeltaFindingsEnabled() || currentProduct != product.ProductCode {
 		return productIssueByFile, nil
 	}
 
+	if len(productIssueByFile[currentProduct]) == 0 {
+		// If no issues found in current branch scan. We can't have deltas.
+		return productIssueByFile, nil
+	}
+
 	baseIssueList, err := f.scanPersister.GetPersistedIssueList(f.path, currentProduct)
 	if err != nil {
 		logger.Err(err).Msg("Error getting persisted issue list")
-		return productIssueByFile, delta.ErrNoDeltaCalculated
-	}
-	if len(baseIssueList) == 0 {
-		return productIssueByFile, delta.ErrNoDeltaCalculated
+		return nil, delta.ErrNoDeltaCalculated
 	}
 
 	currentFlatIssueList := getFlatIssueList(productIssueByFile, currentProduct)

infrastructure/code/code.go

@@ -32,8 +32,6 @@ import (
 	"github.com/pkg/errors"
 	"github.com/puzpuzpuz/xsync"
 
-	gitconfig "github.com/snyk/snyk-ls/internal/git_config"
-
 	codeClient "github.com/snyk/code-client-go"
 	codeClientObservability "github.com/snyk/code-client-go/observability"
 	"github.com/snyk/code-client-go/scan"
@@ -185,12 +183,30 @@ func (sc *Scanner) Scan(ctx context.Context, path string, folderPath string) (is
 	filesToBeScanned := sc.getFilesToBeScanned(folderPath)
 	sc.changedFilesMutex.Unlock()
 
+	if c.IsDeltaFindingsEnabled() {
+		hasChanges, gitErr := vcs.LocalRepoHasChanges(c.Logger(), folderPath)
+		if gitErr != nil {
+			logger.Error().Err(gitErr).Msg("couldn't check if working dir is clean")
+			return nil, gitErr
+		}
+		if !hasChanges {
+			// If delta is enabled but there are no changes. There can be no delta.
+			// else it should start scanning.
+			logger.Debug().Msg("skipping scanning. working dir is clean")
+			return []snyk.Issue{}, nil // Returning an empty slice implies that no issues were found
+		}
+	}
+
 	results, err := internalScan(ctx, sc, folderPath, logger, filesToBeScanned)
+	if err != nil {
+		return nil, err
+	}
 
-	if err == nil && c.IsDeltaFindingsEnabled() {
-		baseScanErr := scanAndPersistBaseBranch(ctx, sc, folderPath)
-		if baseScanErr != nil {
-			logger.Error().Err(baseScanErr).Msg("couldn't scan base branch for folder " + folderPath)
+	if c.IsDeltaFindingsEnabled() && len(results) > 0 {
+		err = scanAndPersistBaseBranch(ctx, sc, folderPath)
+		if err != nil {
+			logger.Error().Err(err).Msg("couldn't scan base branch for folder " + folderPath)
+			return nil, err
 		}
 	}
 
@@ -224,16 +240,7 @@ func internalScan(ctx context.Context, sc *Scanner, folderPath string, logger ze
 func scanAndPersistBaseBranch(ctx context.Context, sc *Scanner, folderPath string) error {
 	logger := sc.c.Logger().With().Str("method", "scanAndPersistBaseBranch").Logger()
 
-	baseBranchName := getBaseBranchName(folderPath)
-	shouldClone, err := vcs.ShouldClone(&logger, folderPath, baseBranchName)
-	if err != nil {
-		return err
-	}
-
-	if !shouldClone {
-		return nil
-	}
-
+	baseBranchName := vcs.GetBaseBranchName(folderPath)
 	headRef, err := vcs.HeadRefHashForBranch(&logger, folderPath, baseBranchName)
 
 	if err != nil {
@@ -295,14 +302,6 @@ func scanAndPersistBaseBranch(ctx context.Context, sc *Scanner, folderPath strin
 	return nil
 }
 
-func getBaseBranchName(folderPath string) string {
-	folderConfig, err := gitconfig.GetOrCreateFolderConfig(folderPath)
-	if err != nil {
-		return "master"
-	}
-	return folderConfig.BaseBranch
-}
-
 // Populate HTML template
 func (sc *Scanner) enhanceIssuesDetails(issues []snyk.Issue) {
 	logger := sc.c.Logger().With().Str("method", "issue_enhancer.enhanceIssuesDetails").Logger()

internal/vcs/git_cloner.go

@@ -47,6 +47,7 @@ func Clone(logger *zerolog.Logger, srcRepoPath string, destinationPath string, t
 		}
 		return targetRepo, nil
 	}
+
 	return clonedRepo, nil
 }
 
@@ -77,13 +78,15 @@ func cloneRepoWithFsCopy(logger *zerolog.Logger, srcRepoPath string, destination
 	return targetRepo
 }
 
-func ShouldClone(logger *zerolog.Logger, repoPath string, branchName string) (bool, error) {
+func LocalRepoHasChanges(logger *zerolog.Logger, repoPath string) (bool, error) {
 	currentRepo, err := git.PlainOpen(repoPath)
 	if err != nil {
 		logger.Error().Err(err).Msg("Failed to open current repo " + repoPath)
 		return false, err
 	}
 
+	branchName := GetBaseBranchName(repoPath)
+
 	currentRepoBranch, err := currentRepo.Head()
 	if err != nil {
 		logger.Error().Err(err).Msg("Failed to get HEAD for " + repoPath)

internal/vcs/git_cloner_test.go

@@ -137,49 +137,53 @@ func TestClone_InvalidGitRepo(t *testing.T) {
 	assert.Error(t, err)
 }
 
-func TestShouldClone_SameBranchNames_NoModification_SkipClone(t *testing.T) {
+func TestLocalRepoHasChanges_SameBranchNames_NoModification_SkipClone(t *testing.T) {
 	c := testutil.UnitTest(t)
 	repoPath := t.TempDir()
 	initGitRepo(t, repoPath, false)
-	cloneTargetBranchName := "master"
-	shouldclone, err := ShouldClone(c.Logger(), repoPath, cloneTargetBranchName)
+	shouldclone, err := LocalRepoHasChanges(c.Logger(), repoPath)
 
 	assert.NoError(t, err)
 	assert.False(t, shouldclone)
 }
 
-func TestShouldClone_SameBranchNames_WithModification_Clone(t *testing.T) {
+func TestLocalRepoHasChanges_SameBranchNames_WithModification_Clone(t *testing.T) {
 	c := testutil.UnitTest(t)
 	repoPath := t.TempDir()
 	initGitRepo(t, repoPath, true)
-	cloneTargetBranchName := "master"
-	shouldclone, err := ShouldClone(c.Logger(), repoPath, cloneTargetBranchName)
+	shouldclone, err := LocalRepoHasChanges(c.Logger(), repoPath)
 
 	assert.NoError(t, err)
 	assert.True(t, shouldclone)
 }
 
-func TestShouldClone_DifferentBranchNames_Clone(t *testing.T) {
+func TestLocalRepoHasChanges_DifferentBranchNames_Clone(t *testing.T) {
 	c := testutil.UnitTest(t)
 	repoPath := t.TempDir()
-	initGitRepo(t, repoPath, true)
-	cloneTargetBranchName := "feat/new"
+	repo, _ := initGitRepo(t, repoPath, true)
+	wt, err := repo.Worktree()
+	assert.NoError(t, err)
+	err = wt.Checkout(&git.CheckoutOptions{
+		Branch: "feat/new",
+		Create: true,
+	})
+	assert.NoError(t, err)
 
-	shouldclone, err := ShouldClone(c.Logger(), repoPath, cloneTargetBranchName)
+	shouldclone, err := LocalRepoHasChanges(c.Logger(), repoPath)
 
 	assert.True(t, shouldclone)
 	assert.NoError(t, err)
 }
 
-func TestShouldClone_HasUncommittedChanges(t *testing.T) {
+func TestLocalRepoHasChanges_HasUncommittedChanges(t *testing.T) {
 	repo, _ := initGitRepo(t, t.TempDir(), true)
 
 	hasChanges := hasUncommitedChanges(repo)
 
 	assert.True(t, hasChanges)
 }
 
-func TestShouldClone_HasCommittedChanges(t *testing.T) {
+func TestLocalRepoHasChanges_HasCommittedChanges(t *testing.T) {
 	repo, _ := initGitRepo(t, t.TempDir(), false)
 
 	hasChanges := hasUncommitedChanges(repo)

internal/vcs/git_utils.go

@@ -22,6 +22,7 @@ import (
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/storage/filesystem"
 	"github.com/rs/zerolog"
+	gitconfig "github.com/snyk/snyk-ls/internal/git_config"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -102,6 +103,14 @@ func hasUncommitedChanges(repo *git.Repository) bool {
 	return false
 }
 
+func GetBaseBranchName(folderPath string) string {
+	folderConfig, err := gitconfig.GetOrCreateFolderConfig(folderPath)
+	if err != nil {
+		return "master"
+	}
+	return folderConfig.BaseBranch
+}
+
 func NormalizeBranchName(branchName string) string {
 	normalized := strings.TrimSpace(branchName)
 	normalized = strings.ToLower(normalized)