Skip to content

Commit 86a4c86

Browse files
committed
Fix Go extractor incorrectly excluding packages with .. in relative paths
The Go extractor was incorrectly excluding valid packages when their relative path from a wantedRoot contained '..' (parent directory references). This occurred because: 1. Each package is checked against all wantedRoots (which can number in the hundreds for large projects) 2. Most sibling package directories result in relative paths like ../../package/path 3. The exclusion regex .*(^|/)(\.\.|vendor)($|/).* excludes paths with .. 4. Cross-module dependencies had their ModDir excluded from wantedRoots This fix: 1. Adds all package ModDirs (including cross-module dependencies) to wantedRoots 2. Prioritizes checking module root and package directory first before checking unrelated sibling package directories This ensures packages are evaluated against the most relevant directories first, avoiding false positives from the '..' exclusion pattern. Testing: - Added integration test at go/ql/integration-tests/package-exclusion-fix/ - Test demonstrates cross-module dependency extraction - Verified without fix: 2 files extracted (config.go missing) - Verified with fix: 4 files extracted (config.go present)
1 parent 5d2ddbf commit 86a4c86

File tree

11 files changed

+146
-5
lines changed

11 files changed

+146
-5
lines changed

go/extractor/extractor.go

Lines changed: 79 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,12 @@ func ExtractWithFlags(buildFlags []string, patterns []string, extractTests bool)
224224
}
225225

226226
log.Printf("Done extracting types for package %s.", pkg.PkgPath)
227+
228+
// Add ModDir to wantedRoots for all packages (including cross-module dependencies)
229+
// This ensures dependencies from other modules can be extracted
230+
if pkgInfo, ok := pkgInfos[pkg.PkgPath]; ok && pkgInfo.ModDir != "" {
231+
wantedRoots[pkgInfo.ModDir] = true
232+
}
227233
})
228234

229235
if len(pkgsNotFound) > 0 {
@@ -273,14 +279,33 @@ func ExtractWithFlags(buildFlags []string, patterns []string, extractTests bool)
273279
return
274280
}
275281

276-
for root := range wantedRoots {
277-
pkgInfo := pkgInfos[pkg.PkgPath]
282+
pkgInfo := pkgInfos[pkg.PkgPath]
283+
matched := false
284+
285+
// Prioritize checking against module root and package directory.
286+
// This avoids incorrectly excluding packages due to relative paths containing ".."
287+
// when checking against unrelated sibling package directories.
288+
priorityRoots := []string{}
289+
if pkgInfo.ModDir != "" {
290+
priorityRoots = append(priorityRoots, pkgInfo.ModDir)
291+
}
292+
priorityRoots = append(priorityRoots, pkgInfo.PkgDir)
293+
294+
for _, root := range priorityRoots {
295+
if !wantedRoots[root] {
296+
continue
297+
}
298+
278299
relDir, err := filepath.Rel(root, pkgInfo.PkgDir)
279-
if err != nil || noExtractRe.MatchString(relDir) {
280-
// if the path can't be made relative or matches the noExtract regexp skip it
300+
if err != nil {
281301
continue
282302
}
283303

304+
if relDir != "." && noExtractRe.MatchString(relDir) {
305+
continue
306+
}
307+
308+
matched = true
284309
extraction.extractPackage(pkg)
285310

286311
modDir := pkgInfo.ModDir
@@ -306,7 +331,56 @@ func ExtractWithFlags(buildFlags []string, patterns []string, extractTests bool)
306331
return
307332
}
308333

309-
log.Printf("Skipping dependency package %s.", pkg.PkgPath)
334+
// If priority roots didn't match, fall back to checking all other roots
335+
if !matched {
336+
for root := range wantedRoots {
337+
// Skip priority roots we already checked
338+
isPriorityRoot := false
339+
for _, pr := range priorityRoots {
340+
if root == pr {
341+
isPriorityRoot = true
342+
break
343+
}
344+
}
345+
if isPriorityRoot {
346+
continue
347+
}
348+
349+
relDir, err := filepath.Rel(root, pkgInfo.PkgDir)
350+
if err != nil || noExtractRe.MatchString(relDir) {
351+
continue
352+
}
353+
354+
matched = true
355+
extraction.extractPackage(pkg)
356+
357+
modDir := pkgInfo.ModDir
358+
if modDir == "" {
359+
modDir = pkgInfo.PkgDir
360+
}
361+
if modDir != "" {
362+
modPath := filepath.Join(modDir, "go.mod")
363+
if util.FileExists(modPath) {
364+
log.Printf("Extracting %s", modPath)
365+
start := time.Now()
366+
367+
err := extraction.extractGoMod(modPath)
368+
if err != nil {
369+
log.Printf("Failed to extract go.mod: %s", err.Error())
370+
}
371+
372+
end := time.Since(start)
373+
log.Printf("Done extracting %s (%dms)", modPath, end.Nanoseconds()/1000000)
374+
}
375+
}
376+
377+
return
378+
}
379+
}
380+
381+
if !matched {
382+
log.Printf("Skipping dependency package %s.", pkg.PkgPath)
383+
}
310384
})
311385

312386
extraction.WaitGroup.Wait()
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"configuration" : {
3+
"go" : { }
4+
}
5+
}

go/ql/integration-tests/package-exclusion-fix/diagnostics.expected

Whitespace-only changes.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package config
2+
3+
type Settings struct {
4+
Value string
5+
}
6+
7+
func Value() string {
8+
return Settings{Value: "ok"}.Value
9+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module example.com/configmodule
2+
3+
go 1.20
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
go 1.20
2+
3+
use ./mainmodule
4+
use ./configmodule
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package worker
2+
3+
import "example.com/configmodule/config"
4+
5+
func Use() string {
6+
return config.Value()
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
module example.com/mainmodule
2+
3+
go 1.20
4+
5+
require example.com/configmodule v0.0.0
6+
7+
replace example.com/configmodule => ../configmodule
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
extractedFiles
2+
| src/configmodule/config/config.go:0:0:0:0 | src/configmodule/config/config.go |
3+
| src/configmodule/go.mod:0:0:0:0 | src/configmodule/go.mod |
4+
| src/mainmodule/app/jobs/worker/worker.go:0:0:0:0 | src/mainmodule/app/jobs/worker/worker.go |
5+
| src/mainmodule/go.mod:0:0:0:0 | src/mainmodule/go.mod |
6+
#select
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# Test for the package exclusion bug fix.
2+
#
3+
# This test reproduces the scenario where a dependency package in a separate module
4+
# was incorrectly excluded due to relative paths containing ".." when checked against
5+
# wantedRoots from the main module.
6+
#
7+
# Structure:
8+
# - configmodule/config/ (separate module with config package)
9+
# - mainmodule/app/jobs/worker/ (main package that depends on config)
10+
#
11+
# Bug scenario (old code):
12+
# When building just mainmodule packages, wantedRoots contains mainmodule directories
13+
# but NOT configmodule's ModDir. Checking config against mainmodule/app/jobs/worker
14+
# produces ../../configmodule/config (contains ".."), causing incorrect exclusion.
15+
#
16+
# Fix: Adds all dependency ModDirs to wantedRoots and prioritizes checking them first.
17+
def test(codeql, go):
18+
codeql.database.create(source_root="src")

0 commit comments

Comments
 (0)