Skip to content

Fix ApplyInterface interface source resolution#1443

Open
qqxhb wants to merge 3 commits intomasterfrom
fix/issue-954-applyinterface-packages
Open

Fix ApplyInterface interface source resolution#1443
qqxhb wants to merge 3 commits intomasterfrom
fix/issue-954-applyinterface-packages

Conversation

@qqxhb
Copy link
Member

@qqxhb qqxhb commented Feb 28, 2026

Summary

  • make ApplyInterface interface source resolution more robust by using go list -json to locate the package directory and its GoFiles
  • avoids relying on go/build.Default.Import (can be sensitive to unusual GOROOT/toolchain setups)
  • avoids pulling in golang.org/x/tools/go/packages to reduce toolchain-compat issues in some linters

Tests

  • go test ./...

@propel-code-bot
Copy link

propel-code-bot bot commented Feb 28, 2026

It also adds parser fixtures and a new test to validate that GetInterfacePath correctly locates interface files.

Affected Areas

internal/parser/export.go
generator.go
internal/parser/export_test.go
internal/parser/fixtures/ifaces/ifaces.go

This summary was automatically generated by @propel-code-bot

propel-code-bot[bot]
propel-code-bot bot previously approved these changes Feb 28, 2026
Copy link

@propel-code-bot propel-code-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, ship it! 🚢

Details
Why was this auto-approved?
APPROVED: No issues detected.

}
skip++
}
cmd := exec.Command("go", "list", "-json", ".")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory

[Documentation] The PR intent/summary says GetInterfacePath now uses golang.org/x/tools/go/packages, but the implementation here shells out to go list. If the switch to go list is intentional, please update the PR description to avoid confusion; otherwise, consider using packages.Load (e.g., with packages.NeedFiles) to align with the stated goal.

Context for Agents
The PR intent/summary says `GetInterfacePath` now uses `golang.org/x/tools/go/packages`, but the implementation here shells out to `go list`. If the switch to `go list` is intentional, please update the PR description to avoid confusion; otherwise, consider using `packages.Load` (e.g., with `packages.NeedFiles`) to align with the stated goal.

File: internal/parser/export.go
Line: 59

}
skip++
}
cmd := exec.Command("go", "list", "-json", ".")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

[Logic] If the runtime.Caller scan never finds a non-gen file, file stays empty and filepath.Dir(file) becomes ".", so go list runs in the current working directory. That can resolve the wrong package and make interface discovery fail silently. Guard against file == "" before running go list to surface a clear error.

Suggested change
cmd := exec.Command("go", "list", "-json", ".")
if file == "" {
return nil, fmt.Errorf("go list package %s fail: failed to locate caller file", arg.PkgPath())
}
cmd := exec.Command("go", "list", "-json", ".")
Context for Agents
If the `runtime.Caller` scan never finds a non-gen file, `file` stays empty and `filepath.Dir(file)` becomes ".", so `go list` runs in the current working directory. That can resolve the wrong package and make interface discovery fail silently. Guard against `file == ""` before running `go list` to surface a clear error.

```suggestion
			if file == "" {
				return nil, fmt.Errorf("go list package %s fail: failed to locate caller file", arg.PkgPath())
			}
			cmd := exec.Command("go", "list", "-json", ".")
```

File: internal/parser/export.go
Line: 59

@propel-code-bot propel-code-bot bot dismissed their stale review February 28, 2026 09:37

Dismissing previous approval: 1 new comment(s) require attention based on Default policy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant