fix(search_jobs): fail validation for repo searches (#64300)

This closes a pretty big gap in the query validation for Search Jobs. We
don't support repo search yet and searcher returned errors during
execution, complaining about missing patterns. With this PR we fail
during validation so users cannot even create these kinds of jobs. See
new test cases.

## Test plan
Updated unit tests

Author: stefanhengl

internal/search/job/jobutil/exhaustive_job.go

@@ -93,7 +93,11 @@ func NewExhaustive(inputs *search.Inputs) (Exhaustive, error) {
 				skipPartitioning: true,
 			}
 	} else if resultTypes.Has(result.TypeFile | result.TypePath) {
-		planJob = NewTextSearchJob(b, inputs, resultTypes, repoOptions)
+		var err error
+		planJob, err = NewTextSearchJob(b, inputs, resultTypes, repoOptions)
+		if err != nil {
+			return Exhaustive{}, err
+		}
 	} else {
 		// This should never happen because we checked for supported types above.
 		return Exhaustive{}, errors.Errorf("internal error: unsupported result types %v", resultTypes)

internal/search/job/jobutil/exhaustive_job_test.go

@@ -298,6 +298,32 @@ func TestNewExhaustive(t *testing.T) {
   (numRepos . 1)
   (pathRegexps . ["(?i)search.go"])
   (indexed . false))
+`),
+		},
+		{
+			Name:  "bytes -r:has.path(go.mod)",
+			Query: "index:no bytes -r:has.path(go.mod)",
+			WantPager: autogold.Expect(`
+(REPOPAGER
+  (containsRefGlobs . false)
+  (repoOpts.useIndex . no)
+  (repoOpts.hasFileContent[0].path . go.mod)
+  (repoOpts.hasFileContent[0].negated . true)
+  (PARTIALREPOS
+    (SEARCHERTEXTSEARCH
+      (useFullDeadline . true)
+      (patternInfo . TextPatternInfo{"bytes",filematchlimit:1000000})
+      (numRepos . 0)
+      (pathRegexps . ["(?i)bytes"])
+      (indexed . false))))
+`),
+			WantJob: autogold.Expect(`
+(SEARCHERTEXTSEARCH
+  (useFullDeadline . true)
+  (patternInfo . TextPatternInfo{"bytes",filematchlimit:1000000})
+  (numRepos . 1)
+  (pathRegexps . ["(?i)bytes"])
+  (indexed . false))
 `),
 		},
 	}
@@ -352,6 +378,10 @@ func TestNewExhaustive_negative(t *testing.T) {
 		{query: `index:no type:repo`},
 		{query: `index:no type:symbol`},
 		{query: `index:no foo select:file.owners`},
+		// repo filter without pattern
+		{query: `index:no -repo:has.path(go.mod)`},
+		{query: `index:no repo:has.path(go.mod)`},
+		{query: `index:no repo:foo`},
 	}
 
 	for _, c := range tc {

internal/search/job/jobutil/job.go

@@ -129,7 +129,10 @@ func NewBasicJob(inputs *search.Inputs, b query.Basic) (job.Job, error) {
 					})
 				}
 
-				searcherJob := NewTextSearchJob(b, inputs, resultTypes, repoOptions)
+				searcherJob, err := NewTextSearchJob(b, inputs, resultTypes, repoOptions)
+				if err != nil {
+					return nil, err
+				}
 				addJob(searcherJob)
 			}
 		}
@@ -276,10 +279,13 @@ func NewBasicJob(inputs *search.Inputs, b query.Basic) (job.Job, error) {
 	return basicJob, nil
 }
 
-func NewTextSearchJob(b query.Basic, inputs *search.Inputs, types result.Types, options search.RepoOptions) job.Job {
+func NewTextSearchJob(b query.Basic, inputs *search.Inputs, types result.Types, options search.RepoOptions) (job.Job, error) {
 	// searcher to use full deadline if timeout: set or we are not batch.
 	useFullDeadline := b.GetTimeout() != nil || b.Count() != nil || inputs.Protocol != search.Batch
-	patternInfo := toTextPatternInfo(b, types, inputs.Features, inputs.DefaultLimit())
+	patternInfo, err := toTextPatternInfo(b, types, inputs.Features, inputs.DefaultLimit())
+	if err != nil {
+		return nil, err
+	}
 
 	searcherJob := &searcher.TextSearchJob{
 		PatternInfo:     patternInfo,
@@ -294,7 +300,7 @@ func NewTextSearchJob(b query.Basic, inputs *search.Inputs, types result.Types,
 		child:            &reposPartialJob{searcherJob},
 		repoOpts:         options,
 		containsRefGlobs: query.ContainsRefGlobs(b.ToParseTree()),
-	}
+	}, nil
 }
 
 // orderRacingJobs ensures that searcher and repo search jobs only ever run
@@ -391,7 +397,10 @@ func NewFlatJob(searchInputs *search.Inputs, f query.Flat) (job.Job, error) {
 		}
 
 		if resultTypes.Has(result.TypeStructural) {
-			patternInfo := toTextPatternInfo(f.ToBasic(), resultTypes, searchInputs.Features, searchInputs.DefaultLimit())
+			patternInfo, err := toTextPatternInfo(f.ToBasic(), resultTypes, searchInputs.Features, searchInputs.DefaultLimit())
+			if err != nil {
+				return nil, err
+			}
 			searcherArgs := &search.SearcherParameters{
 				PatternInfo:     patternInfo,
 				UseFullDeadline: useFullDeadline,
@@ -671,7 +680,7 @@ func toSymbolSearchRequest(f query.Flat, feat *search.Features) (*searcher.Symbo
 }
 
 // toTextPatternInfo converts a query to internal values that drive text search.
-func toTextPatternInfo(b query.Basic, resultTypes result.Types, feat *search.Features, defaultLimit int) *search.TextPatternInfo {
+func toTextPatternInfo(b query.Basic, resultTypes result.Types, feat *search.Features, defaultLimit int) (*search.TextPatternInfo, error) {
 	// Handle file: and -file: filters.
 	filesInclude, filesExclude := b.IncludeExcludeValues(query.FieldFile)
 
@@ -691,8 +700,16 @@ func toTextPatternInfo(b query.Basic, resultTypes result.Types, feat *search.Fea
 	selector, _ := filter.SelectPathFromString(b.FindValue(query.FieldSelect)) // Invariant: select is validated
 	count := b.MaxResults(defaultLimit)
 
+	q := protocol.FromJobNode(b.Pattern)
+	if p, ok := q.(*protocol.PatternNode); ok {
+		if p.Value == "" && len(filesExclude) == 0 && len(filesInclude) == 0 &&
+			len(langExclude) == 0 && len(langExclude) == 0 {
+			return nil, errors.New("At least one of pattern and include/exclude patterns must be non-empty")
+		}
+	}
+
 	return &search.TextPatternInfo{
-		Query:                        protocol.FromJobNode(b.Pattern),
+		Query:                        q,
 		IsStructuralPat:              b.IsStructural(),
 		IsCaseSensitive:              b.IsCaseSensitive(),
 		FileMatchLimit:               int32(count),
@@ -707,7 +724,7 @@ func toTextPatternInfo(b query.Basic, resultTypes result.Types, feat *search.Fea
 		CombyRule:                    b.FindValue(query.FieldCombyRule),
 		Index:                        b.Index(),
 		Select:                       selector,
-	}
+	}, nil
 }
 
 func toLangFilters(aliases []string) []string {

internal/search/job/jobutil/job_test.go

@@ -1393,10 +1393,10 @@ func TestToTextPatternInfo(t *testing.T) {
 		output: autogold.Expect(`{"Query":{"Value":"(?://).*?(?:literal).*?(?:slash)","IsNegated":false,"IsRegExp":true},"IsStructuralPat":false,"CombyRule":"","IsCaseSensitive":false,"FileMatchLimit":30,"Index":"yes","Select":[],"IncludePaths":null,"ExcludePaths":"","IncludeLangs":null,"ExcludeLangs":null,"PathPatternsAreCaseSensitive":false,"PatternMatchesContent":true,"PatternMatchesPath":true,"Languages":null}`),
 	}, {
 		input:  `repo:contains.path(Dockerfile)`,
-		output: autogold.Expect(`{"Query":{"Value":"","IsNegated":false,"IsRegExp":true},"IsStructuralPat":false,"CombyRule":"","IsCaseSensitive":false,"FileMatchLimit":30,"Index":"yes","Select":[],"IncludePaths":null,"ExcludePaths":"","IncludeLangs":null,"ExcludeLangs":null,"PathPatternsAreCaseSensitive":false,"PatternMatchesContent":true,"PatternMatchesPath":true,"Languages":null}`),
+		output: autogold.Expect("Error"),
 	}, {
 		input:  `repohasfile:Dockerfile`,
-		output: autogold.Expect(`{"Query":{"Value":"","IsNegated":false,"IsRegExp":true},"IsStructuralPat":false,"CombyRule":"","IsCaseSensitive":false,"FileMatchLimit":30,"Index":"yes","Select":[],"IncludePaths":null,"ExcludePaths":"","IncludeLangs":null,"ExcludeLangs":null,"PathPatternsAreCaseSensitive":false,"PatternMatchesContent":true,"PatternMatchesPath":true,"Languages":null}`),
+		output: autogold.Expect("Error"),
 	}, {
 		input:  `repo:^github\.com/sgtest/go-diff$ make(:[1]) index:only patterntype:structural count:3`,
 		output: autogold.Expect(`{"Query":{"Value":"make(:[1])","IsNegated":false,"IsRegExp":false},"IsStructuralPat":true,"CombyRule":"","IsCaseSensitive":false,"FileMatchLimit":3,"Index":"only","Select":[],"IncludePaths":null,"ExcludePaths":"","IncludeLangs":null,"ExcludeLangs":null,"PathPatternsAreCaseSensitive":false,"PatternMatchesContent":true,"PatternMatchesPath":true,"Languages":null}`),
@@ -1434,7 +1434,10 @@ func TestToTextPatternInfo(t *testing.T) {
 		}
 		b := plan[0]
 		resultTypes := computeResultTypes(b, query.SearchTypeLiteral, defaultResultTypes)
-		p := toTextPatternInfo(b, resultTypes, &feat, limits.DefaultMaxSearchResults)
+		p, err := toTextPatternInfo(b, resultTypes, &feat, limits.DefaultMaxSearchResults)
+		if err != nil {
+			return "Error"
+		}
 		v, _ := json.Marshal(p)
 		return string(v)
 	}