build: ignore out of bound lines from ctags (#694)

universal-ctags sometimes returns lines that are out of bounds. In
practice it seems to only do an off by one. We haven't noticed the
linenum error until a recent change of mine which didn't append an extra
entry to NLS if the file was terminated by "\n". In practice this would
end up being filtered out later on. So we update to just continue rather
than error here.

An example is
https://github.com/sourcegraph/sourcegraph/blob/v5.2.2/client/web-sveltekit/.storybook/main.ts

  $ universal-ctags '--fields=*' --output-format=json main.ts | grep 22
  {"_type": "tag", "name": "config", "path": "main.ts", "pattern": "/^export default config$/", "language": "TypeScript", "line": 22, "kind": "constant", "roles": "def"}

  $ wc -l main.ts
  21 main.ts

  $ tail -n1 main.ts
  export default config

Test Plan: added a unit test

build/ctags.go

@@ -134,7 +134,8 @@ func (t *tagsToSections) Convert(content []byte, tags []*ctags.Entry) ([]zoekt.D
 		}
 		lineIdx := t.Line - 1
 		if lineIdx >= len(nls) {
-			return nil, nil, fmt.Errorf("linenum for entry out of range %v", t)
+			// Observed this with a .TS file.
+			continue
 		}
 
 		lineOff := uint32(0)

build/ctags_test.go

@@ -116,16 +116,38 @@ func TestTagsToSectionsEOF(t *testing.T) {
 			Name: "bar",
 			Line: 2,
 		},
-	}
 
-	secs, _, err := (&tagsToSections{}).Convert(c, tags)
-	if err != nil {
-		t.Fatal("tagsToSections", err)
+		// We have seen ctags do this on a JS file
+		{
+			Name: "wat",
+			Line: -1,
+		},
+
+		// We have seen ctags return out of bounds lines
+		{
+			Name: "goliath",
+			Line: 3,
+		},
 	}
 
-	if len(secs) != 1 || secs[0].Start != 17 || secs[0].End != 20 {
-		t.Fatalf("got %#v, want 1 section (17,20)", secs)
+	// We run this test twice. Once with a final \n and without.
+	do := func(t *testing.T, doc []byte) {
+		secs, _, err := (&tagsToSections{}).Convert(doc, tags)
+		if err != nil {
+			t.Fatal("tagsToSections", err)
+		}
+
+		if len(secs) != 1 || secs[0].Start != 17 || secs[0].End != 20 {
+			t.Fatalf("got %#v, want 1 section (17,20)", secs)
+		}
 	}
+
+	t.Run("no final newline", func(t *testing.T) {
+		do(t, c)
+	})
+	t.Run("trailing newline", func(t *testing.T) {
+		do(t, append(c, '\n'))
+	})
 }
 
 func TestOverlaps(t *testing.T) {
@@ -232,9 +254,7 @@ func TestOverlaps(t *testing.T) {
 }
 
 func BenchmarkTagsToSections(b *testing.B) {
-	if checkCTags() == "" {
-		b.Skip("ctags not available")
-	}
+	requireCTags(b)
 
 	file, err := os.ReadFile("./testdata/large_file.cc")
 	parser, err := ctags.NewParser(ctags.UniversalCTags, "universal-ctags")
@@ -268,3 +288,18 @@ func BenchmarkTagsToSections(b *testing.B) {
 		}
 	}
 }
+
+func requireCTags(tb testing.TB) {
+	tb.Helper()
+
+	if checkCTags() != "" {
+		return
+	}
+
+	// On CI we require ctags to be available. Otherwise we skip
+	if os.Getenv("CI") != "" {
+		tb.Fatal("universal-ctags is missing")
+	} else {
+		tb.Skip("universal-ctags is missing")
+	}
+}

build/scoring_test.go

@@ -27,8 +27,8 @@ import (
 )
 
 type scoreCase struct {
-	fileName           string
-	content            []byte
+	fileName  string
+	content   []byte
 	query     query.Q
 	language  string
 	wantScore float64
@@ -58,7 +58,7 @@ func TestFileNameMatch(t *testing.T) {
 			wantScore: 510,
 		},
 	}
-	
+
 	for _, c := range cases {
 		checkScoring(t, c, ctags.UniversalCTags)
 	}
@@ -165,7 +165,7 @@ func TestJava(t *testing.T) {
 	}
 
 	for _, c := range cases {
-        checkScoring(t, c, ctags.UniversalCTags)
+		checkScoring(t, c, ctags.UniversalCTags)
 	}
 }
 
@@ -174,7 +174,7 @@ func TestKotlin(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	
+
 	cases := []scoreCase{
 		{
 			fileName: "example.kt",
@@ -239,7 +239,7 @@ func TestCpp(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	
+
 	cases := []scoreCase{
 		{
 			fileName: "example.cc",
@@ -489,9 +489,7 @@ func skipIfCTagsUnavailable(t *testing.T, parserType ctags.CTagsParserType) {
 
 	switch parserType {
 	case ctags.UniversalCTags:
-		if checkCTags() == "" {
-			t.Skip("ctags not available")
-		}
+		requireCTags(t)
 	case ctags.ScipCTags:
 		if checkScipCTags() == "" {
 			t.Skip("scip-ctags not available")
@@ -560,9 +558,7 @@ func checkScoring(t *testing.T, c scoreCase, parserType ctags.CTagsParserType) {
 }
 
 func TestDocumentRanks(t *testing.T) {
-	if os.Getenv("CI") == "" && checkCTags() == "" {
-		t.Skip("ctags not available")
-	}
+	requireCTags(t)
 	dir := t.TempDir()
 
 	opts := Options{
@@ -649,9 +645,7 @@ func TestDocumentRanks(t *testing.T) {
 }
 
 func TestRepoRanks(t *testing.T) {
-	if os.Getenv("CI") == "" && checkCTags() == "" {
-		t.Skip("ctags not available")
-	}
+	requireCTags(t)
 	dir := t.TempDir()
 
 	opts := Options{