Chore/toolchain independence enhancement#61
Merged
fredbi merged 2 commits intoJul 16, 2026
Conversation
…face Replace the go/types builder tower (NewInterfaceType/NewFunc/NewSignatureType) introduced by go-openapi#60 with mustIfaceFromSource, which type-checks a one-line `type T interface{ MarshalText() ([]byte, error) }` snippet under a nil importer. The snippet imports nothing (its result types []byte and error are Universe types), so the type-checker never consults an importer: same zero-runtime- resolution guarantee as the synthesized interface, but readable and extensible to other stdlib interfaces (json.Marshaler, fmt.Stringer) by editing a string. Guard the scope lookup so a snippet that type-checks but declares no interface T panics wrapping ErrInternal instead of nil-dereferencing, matching the sibling Must* assertions. Cover the helper contract with TestMustIfaceFromSource. Also drop a stray empty doc-comment line before `package godoclink` (gofmt). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Add a bespoke build-once / run-in-a-foreign-environment CI matrix (.github/workflows/toolchain-independence.yml), separate from the shared go-test workflow, that locks the property go-openapi#60 fixed: IsTextMarshaler must not depend on a working build-time toolchain/GOROOT at runtime. - job A builds the resolvers + integration test binaries on `stable`. - job B (pinpoint) runs the resolvers binary under `env -i` — no Go present at all — proving the interface resolution resolves nothing at runtime. - job C (full workflow) runs the integration binary on `oldstable` with the build-time GOROOT removed and GOROOT unset, so the old go/importer path would fail while packages.Load still self-locates on PATH. TestToolchainIndependence_FullScan backs job C: a self-check that the baked runtime.GOROOT() is absent (fails loudly on masking) plus a full-scan symptom assertion that TextMarshaler fields still render as strings. It skips unless CODESCAN_TC_JOBC=1, so a normal `go test ./...` is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Member
Author
|
@KT-Doan Kevin this is the follow-up I've been talking about:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #61 +/- ##
==========================================
+ Coverage 81.60% 81.63% +0.03%
==========================================
Files 84 84
Lines 8513 8528 +15
==========================================
+ Hits 6947 6962 +15
Misses 1146 1146
Partials 420 420 ☔ View full report in Codecov by Harness. |
There was a problem hiding this comment.
Pull request overview
This PR strengthens codescan’s resilience to Go toolchain/build-vs-run environment divergence by making encoding.TextMarshaler detection independent of go/importer and by adding CI + integration coverage to prevent regressions when the runtime toolchain/GOROOT differs from the build-time one.
Changes:
- Replace
encoding.TextMarshalerinterface resolution with a toolchain-independentgo/typestype-check of a synthetic interface snippet. - Add targeted unit tests and a bespoke CI workflow to validate behavior with no Go toolchain and with divergent build/run toolchains.
- Add an integration guard that runs only under the dedicated CI job to validate end-to-end scan output.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/builders/resolvers/assertions.go |
Switches TextMarshaler interface resolution to a synthetic, type-checked interface. |
internal/builders/resolvers/resolvers_test.go |
Adds tests for the new mustIfaceFromSource helper and its panic/error contract. |
internal/integration/coverage_toolchain_independence_test.go |
Adds an integration test that enforces correctness under a build!=run toolchain divergence scenario. |
/.github/workflows/toolchain-independence.yml |
Introduces a dedicated workflow with “no Go present” and “divergent toolchain” jobs. |
internal/builders/godoclink/godoclink.go |
Minor comment formatting change. |
| fset := token.NewFileSet() | ||
| f, err := parser.ParseFile(fset, "iface.go", src, 0) | ||
| if err != nil { | ||
| panic(fmt.Errorf("parsing synthetic interface source %q: %w: %w", src, err, ErrInternal)) |
| // nil importer: the snippet imports nothing, so the type-checker never invokes it. | ||
| pkg, err := new(types.Config).Check("p", fset, []*ast.File{f}, nil) | ||
| if err != nil { | ||
| panic(fmt.Errorf("type-checking synthetic interface source %q: %w: %w", src, err, ErrInternal)) |
Comment on lines
+86
to
+88
| // The importer reads stdlib export data out of the GOROOT the binary was built against: | ||
| // when GOTOOLCHAIN selects a different toolchain at runtime — or the callgin binary simply runs on a machine where | ||
| // Go installation lives at a different path than the build machine's — the lookup fails, and the error is silently |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change type
Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update
Short description
Fixes
Full description
Checklist