Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit d82c7fd

Browse files
jtibshiranimmanela
andauthored
Search: fix case-sensitive queries with lang filters (#62157)
This PR fixes a regression where language filters might not match when case sensitivity is enabled. Before, we just appended `(?i)` to the file filters to indicate case insensitivity, but still passed a case-sensitive file query to Zoekt (called `case_file_regex`). Now, we make sure that the Zoekt queries are also case insensitive so there is no conflict in how to interpret them. Co-authored-by: Matthew Manela <[email protected]>
1 parent 0896934 commit d82c7fd

File tree

3 files changed

+64
-20
lines changed

3 files changed

+64
-20
lines changed

dev/gqltest/search_test.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,28 @@ func testSearchClient(t *testing.T, client searchClient) {
254254
}
255255
// Make sure we only got .go files
256256
for _, r := range results.Results {
257-
if !strings.Contains(r.File.Name, ".go") {
257+
if !strings.Contains(strings.ToLower(r.File.Name), ".go") {
258+
t.Fatalf("Found file name does not end with .go: %s", r.File.Name)
259+
}
260+
}
261+
})
262+
263+
t.Run("lang: filter with case sensitivity", func(t *testing.T) {
264+
// Guard against a previous regression where case sensitivity broke lang filters. This search
265+
// query closely mimics the one the web client ues for search-based code navigation.
266+
results, err := client.SearchFiles("type:symbol ^readLine$ lang:go case:yes patterntype:regexp")
267+
if err != nil {
268+
t.Fatal(err)
269+
}
270+
271+
// Ensure some results are returned
272+
if len(results.Results) == 0 {
273+
t.Fatal("Want non-zero results but got none")
274+
}
275+
276+
// Make sure we only got .go files
277+
for _, r := range results.Results {
278+
if !strings.Contains(strings.ToLower(r.File.Name), ".go") {
258279
t.Fatalf("Found file name does not end with .go: %s", r.File.Name)
259280
}
260281
}

internal/search/zoekt/query.go

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -52,27 +52,23 @@ func QueryToZoektQuery(b query.Basic, resultTypes result.Types, feat *search.Fea
5252
and = append(and, &zoekt.Not{Child: filter})
5353
}
5454
} else {
55-
filesInclude = append(filesInclude, mapSlice(langInclude, query.LangToFileRegexp)...)
56-
filesExclude = append(filesExclude, mapSlice(langExclude, query.LangToFileRegexp)...)
57-
}
58-
59-
// zoekt also uses regular expressions for file paths
60-
// TODO PathPatternsAreCaseSensitive
61-
// TODO whitespace in file path patterns?
62-
for _, i := range filesInclude {
63-
q, err := zoektquery.FileRe(i, isCaseSensitive)
55+
// By default, convert the language filters to file regexp patterns. Note: these
56+
// are always case-insensitive, to match broadly on extensions like file.C.
57+
langFilters, err := toFileFilters(
58+
mapSlice(langInclude, query.LangToFileRegexp),
59+
mapSlice(langExclude, query.LangToFileRegexp),
60+
false)
6461
if err != nil {
6562
return nil, err
6663
}
67-
and = append(and, q)
64+
and = append(and, langFilters...)
6865
}
69-
if len(filesExclude) > 0 {
70-
q, err := zoektquery.FileRe(query.UnionRegExps(filesExclude), isCaseSensitive)
71-
if err != nil {
72-
return nil, err
73-
}
74-
and = append(and, &zoekt.Not{Child: q})
66+
67+
fileFilters, err := toFileFilters(filesInclude, filesExclude, isCaseSensitive)
68+
if err != nil {
69+
return nil, err
7570
}
71+
and = append(and, fileFilters...)
7672

7773
var repoHasFilters []zoekt.Q
7874
for _, filter := range b.RepoHasFileContent() {
@@ -90,6 +86,27 @@ func toLangFilter(lang string) zoekt.Q {
9086
return &zoekt.Language{Language: lang}
9187
}
9288

89+
// toFileFilters converts a list of file regexp patterns to Zoekt file filters.
90+
func toFileFilters(filesInclude []string, filesExclude []string, isCaseSensitive bool) ([]zoekt.Q, error) {
91+
var filters []zoekt.Q
92+
// TODO whitespace in file path patterns?
93+
for _, i := range filesInclude {
94+
q, err := zoektquery.FileRe(i, isCaseSensitive)
95+
if err != nil {
96+
return nil, err
97+
}
98+
filters = append(filters, q)
99+
}
100+
if len(filesExclude) > 0 {
101+
q, err := zoektquery.FileRe(query.UnionRegExps(filesExclude), isCaseSensitive)
102+
if err != nil {
103+
return nil, err
104+
}
105+
filters = append(filters, &zoekt.Not{Child: q})
106+
}
107+
return filters, nil
108+
}
109+
93110
func QueryForFileContentArgs(opt query.RepoHasFileContentArgs, caseSensitive bool) zoekt.Q {
94111
var children []zoekt.Q
95112
if opt.Path != "" {

internal/search/zoekt/query_test.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,19 @@ func TestQueryToZoektQuery(t *testing.T) {
8282
Name: "Languages get passed as file filter",
8383
Type: search.TextRequest,
8484
Query: `file:\.go$ lang:go`,
85-
WantZoektOutput: autogold.Expect(`(and file_regex:"\\.go(?m:$)" file_regex:"(?i:\\.GO)(?m:$)")`),
85+
WantZoektOutput: autogold.Expect(`(and file_regex:"(?i:\\.GO)(?m:$)" file_regex:"\\.go(?m:$)")`),
8686
},
8787
{
88-
Name: "Languages still use case_insensitive in case sensitivity mode",
88+
Name: "Languages still use case_insensitive in case sensitivity mode (Go)",
8989
Type: search.TextRequest,
9090
Query: `file:\.go$ lang:go case:true`,
91-
WantZoektOutput: autogold.Expect(`(and case_file_regex:"\\.go(?m:$)" case_file_regex:"(?i:\\.GO)(?m:$)")`),
91+
WantZoektOutput: autogold.Expect(`(and file_regex:"(?i:\\.GO)(?m:$)" case_file_regex:"\\.go(?m:$)")`),
92+
},
93+
{
94+
Name: "Languages still use case_insensitive in case sensitivity mode (Typescript)",
95+
Type: search.TextRequest,
96+
Query: `lang:typescript case:true`,
97+
WantZoektOutput: autogold.Expect(`file_regex:"(?i:\\.)(?:(?i:TS)(?m:$)|(?i:CTS)(?m:$)|(?i:MTS)(?m:$))"`),
9298
},
9399
{
94100
Name: "Language get passed as lang: query",

0 commit comments

Comments
 (0)