Remove duplicated function definitions (#1666)

Co-authored-by: Azeem Shaikh <[email protected]>
ossf · Feb 22, 2022 · c03085a · c03085a
1 parent e5b62b5
commit c03085a
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 174 deletions.
diff --git a/checks/fileparser/listing.go b/checks/fileparser/listing.go
@@ -196,42 +196,6 @@ func CheckIfFileExistsV6(repoClient clients.RepoClient,
 	return nil
 }
 
-// FileCb represents a callback fn.
-type FileCb func(path string,
-	dl checker.DetailLogger, data FileCbData) (bool, error)
-
-// CheckIfFileExists downloads the tar of the repository and calls the onFile() to check
-// for the occurrence.
-func CheckIfFileExists(c *checker.CheckRequest, onFile FileCb, data FileCbData) error {
-	matchedFiles, err := c.RepoClient.ListFiles(func(string) (bool, error) { return true, nil })
-	if err != nil {
-		// nolint: wrapcheck
-		return err
-	}
-	for _, filename := range matchedFiles {
-		continueIter, err := onFile(filename, c.Dlogger, data)
-		if err != nil {
-			return err
-		}
-
-		if !continueIter {
-			break
-		}
-	}
-
-	return nil
-}
-
-// FileGetCbDataAsBoolPointer returns callback data as bool.
-func FileGetCbDataAsBoolPointer(data FileCbData) *bool {
-	pdata, ok := data.(*bool)
-	if !ok {
-		// This never happens.
-		panic("invalid type")
-	}
-	return pdata
-}
-
 // CheckFileContainsCommands checks if the file content contains commands or not.
 // `comment` is the string or character that indicates a comment:
 // for example for Dockerfiles, it would be `#`.

diff --git a/checks/fileparser/listing_test.go b/checks/fileparser/listing_test.go
@@ -379,54 +379,6 @@ func Test_isTestdataFile(t *testing.T) {
 	}
 }
 
-// TestFileGetCbDataAsBoolPointer tests the FileGetCbDataAsBoolPointer function.
-func TestFileGetCbDataAsBoolPointer(t *testing.T) {
-	t.Parallel()
-	type args struct {
-		data FileCbData
-	}
-	b := true
-	//nolint
-	tests := []struct {
-		name      string
-		args      args
-		want      *bool
-		wantPanic bool
-	}{
-		{
-			name: "true",
-			args: args{
-				data: &b,
-			},
-			want: &b,
-		},
-		{
-			name:      "nil",
-			args:      args{},
-			want:      &b,
-			wantPanic: true,
-		},
-	}
-	for _, tt := range tests {
-		tt := tt // Re-initializing variable so it is not changed while executing the closure below
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			if tt.wantPanic {
-				defer func() {
-					if r := recover(); r == nil {
-						t.Errorf("FileGetCbDataAsBoolPointer() did not panic")
-					}
-				}()
-				FileGetCbDataAsBoolPointer(tt.args.data)
-				return
-			}
-			if got := FileGetCbDataAsBoolPointer(tt.args.data); got != tt.want {
-				t.Errorf("FileGetCbDataAsBoolPointer() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
 // TestCheckFilesContentV6 tests the CheckFilesContentV6 function.
 func TestCheckFilesContentV6(t *testing.T) {
 	t.Parallel()
@@ -699,68 +651,3 @@ func TestCheckIfFileExistsV6(t *testing.T) {
 		})
 	}
 }
-
-// TestCheckIfFileExists tests the CheckIfFileExists function.
-func TestCheckIfFileExists(t *testing.T) {
-	t.Parallel()
-	//nolint
-	type args struct {
-		cbReturn bool
-		cbErr    error
-	}
-	//nolint
-	tests := []struct {
-		name    string
-		args    args
-		wantErr bool
-	}{
-		{
-			name: "cb true and no error",
-			args: args{
-				cbReturn: true,
-				cbErr:    nil,
-			},
-			wantErr: false,
-		},
-		{
-			name: "cb false and no error",
-			args: args{
-				cbReturn: false,
-				cbErr:    nil,
-			},
-			wantErr: false,
-		},
-		{
-			name: "cb error",
-			args: args{
-				cbReturn: true,
-				cbErr:    errors.New("test error"),
-			},
-			wantErr: true,
-		},
-	}
-	for _, tt := range tests {
-		tt := tt // Re-initializing variable so it is not changed while executing the closure below
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			ctrl := gomock.NewController(t)
-			mockRepo := mockrepo.NewMockRepoClient(ctrl)
-			mockRepo.EXPECT().ListFiles(gomock.Any()).Return([]string{"foo"}, nil).AnyTimes()
-			c := checker.CheckRequest{
-				RepoClient: mockRepo,
-			}
-			x := func(path string,
-				dl checker.DetailLogger, data FileCbData) (bool, error) {
-				return tt.args.cbReturn, tt.args.cbErr
-			}
-
-			err := CheckIfFileExists(&c, x, x)
-
-			if (err != nil) != tt.wantErr {
-				t.Errorf("CheckIfFileExists() error = %v, wantErr %v for %v", err, tt.wantErr, tt.name)
-				return
-			}
-		})
-	}
-}
diff --git a/checks/license.go b/checks/license.go
@@ -103,28 +103,28 @@ func testLicenseCheck(name string) bool {
 
 // LicenseCheck runs LicenseCheck check.
 func LicenseCheck(c *checker.CheckRequest) checker.CheckResult {
-	var r bool
-
-	onFile := func(name string, dl checker.DetailLogger, data fileparser.FileCbData) (bool, error) {
-		pdata := fileparser.FileGetCbDataAsBoolPointer(data)
+	var s string
 
+	onFile := func(name string, data fileparser.FileCbData) (bool, error) {
 		if checkLicense(name) {
-			c.Dlogger.Info(&checker.LogMessage{
-				Path:   name,
-				Type:   checker.FileTypeSource,
-				Offset: 1,
-			})
-			*pdata = true
+			if strData, ok := data.(*string); ok && strData != nil {
+				*strData = name
+			}
 			return false, nil
 		}
 		return true, nil
 	}
 
-	err := fileparser.CheckIfFileExists(c, onFile, &r)
+	err := fileparser.CheckIfFileExistsV6(c.RepoClient, onFile, &s)
 	if err != nil {
 		return checker.CreateRuntimeErrorResult(CheckLicense, err)
 	}
-	if r {
+	if s != "" {
+		c.Dlogger.Info(&checker.LogMessage{
+			Path:   s,
+			Type:   checker.FileTypeSource,
+			Offset: 1,
+		})
 		return checker.CreateMaxScoreResult(CheckLicense, "license file detected")
 	}
 	return checker.CreateMinScoreResult(CheckLicense, "license file not detected")

diff --git a/checks/raw/security_policy.go b/checks/raw/security_policy.go
@@ -32,7 +32,7 @@ func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error)
 
 	// Check repository for repository-specific policy.
 	// https://docs.github.com/en/github/building-a-strong-community/creating-a-default-community-health-file.
-	onFile := func(name string, dl checker.DetailLogger, data fileparser.FileCbData) (bool, error) {
+	onFile := func(name string, data fileparser.FileCbData) (bool, error) {
 		pfiles, ok := data.(*[]checker.File)
 		if !ok {
 			// This never happens.
@@ -62,7 +62,7 @@ func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error)
 	}
 
 	files := make([]checker.File, 0)
-	err := fileparser.CheckIfFileExists(c, onFile, &files)
+	err := fileparser.CheckIfFileExistsV6(c.RepoClient, onFile, &files)
 	if err != nil {
 		return checker.SecurityPolicyData{}, err
 	}
@@ -74,18 +74,12 @@ func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error)
 
 	// https://docs.github.com/en/github/building-a-strong-community/creating-a-default-community-health-file.
 	logger := log.NewLogger(log.InfoLevel)
-	dotGitHub := &checker.CheckRequest{
-		Ctx:        c.Ctx,
-		Dlogger:    c.Dlogger,
-		RepoClient: githubrepo.CreateGithubRepoClient(c.Ctx, logger),
-		Repo:       c.Repo.Org(),
-	}
-
-	err = dotGitHub.RepoClient.InitRepo(dotGitHub.Repo, clients.HeadSHA)
+	dotGitHubClient := githubrepo.CreateGithubRepoClient(c.Ctx, logger)
+	err = dotGitHubClient.InitRepo(c.Repo.Org(), clients.HeadSHA)
 	switch {
 	case err == nil:
-		defer dotGitHub.RepoClient.Close()
-		onFile = func(name string, dl checker.DetailLogger, data fileparser.FileCbData) (bool, error) {
+		defer dotGitHubClient.Close()
+		onFile = func(name string, data fileparser.FileCbData) (bool, error) {
 			pfiles, ok := data.(*[]checker.File)
 			if !ok {
 				// This never happens.
@@ -106,7 +100,7 @@ func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error)
 			}
 			return true, nil
 		}
-		err = fileparser.CheckIfFileExists(dotGitHub, onFile, &files)
+		err = fileparser.CheckIfFileExistsV6(dotGitHubClient, onFile, &files)
 		if err != nil {
 			return checker.SecurityPolicyData{}, err
 		}