Import static value member resolution#639
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #639 +/- ##
==========================================
+ Coverage 93.68% 93.69% +0.01%
==========================================
Files 29 29
Lines 7221 7235 +14
==========================================
+ Hits 6765 6779 +14
Misses 388 388
Partials 68 68 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request introduces conflict detection and validation for static members. Specifically, it updates NewStaticMember to check for conflicts with existing methods or static members and panic accordingly, and updates NewFuncWith to prevent adding methods that conflict with existing static members. It also adds corresponding tests for these conflict scenarios. The reviewer's feedback suggests adding defensive nil checks for typ and obj in NewStaticMember to prevent nil pointer dereferences, and removing trailing newlines from log.Panicf format strings. Additionally, the reviewer suggests updating the test assertions in TestErrStaticMemberConflict to match the updated panic messages without trailing newlines.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Review: Import static value member resolution
Focused, well-tested change. The allowAccess(method.Pkg(), method.Name()) → allowAccess(obj.Pkg(), obj.Name()) switch in codebuild.go is a genuine correctness fix: the synthetic wrapper method carries the lowercase member name (e.g. name), so access control was previously evaluated against a name that always looks unexported; using the underlying object (e.g. XGos_Game_name) is the right visibility gate. TestImportedStaticMember confirms cross-package access now works. Turning silent duplicate/conflict registration into explicit fail-fast panics is also appropriate for a codegen library, and the conflict permutations are thoroughly covered.
One nit worth addressing:
func_ext.go:263— stale/incomplete doc comment. TheNewStaticMemberdoc still says "other objects resolve as static fields", but this PR standardizes the user-facing term on "static value member" everywhere else (staticMemberKind, the panic messages, theNewFuncWitherror atfunc.go:222, and the tests). Recommend aligning the wording (e.g. "…other objects resolve as static value members."). It would also help to document that this exported function now panics on a conflicting or duplicate registration, since that is a new caller-facing behavior.
Non-blocking observations (no change required):
- The conflict check added to
NewFuncWith(func.go:221) adds an extra O(M) method scan per method declaration on top of an already O(M²) registration loop. Negligible for normal method counts; only matters for pathological types. NewStaticMemberreports conflicts vialog.Panicfwhile the parallel check inNewFuncWithreturns a structuredCodeError. Consistent with existing style, just noting the asymmetry.
No correctness, security, or regression defects found.
4c4fefe to
4bc9d30
Compare
Feature Description
NewStaticMember/TyStaticMemberas the unified static member API for both static methods and static value members.Typ(T).MemberVal(...)and assignable static vars throughMemberRef(...).XGos_Type_namepackage-level symbols as static members on the corresponding named type.types.Named.AddMethoddrops.Implementation Approach
types.Object, so function objects continue to behave as static methods while const/var objects resolve as static fields.TypeTypereceivers.Testing
go test -run 'Test(StaticMethod|ImportedStaticMember|StaticMember|StaticMemberInstantiatedGenericType|ErrStaticMember|ErrStaticMemberConflict)$' -count=1go test -run TestStaticMemberLoadsDelayedNamedType -count=1go test ./...git diff --checkRelated