fix(cycle-detection): drop type-spec analysis and add method-call support#191
Merged
Conversation
…port The cycle-detection rule used to walk every TypeSpec/ValueSpec identifier and emit "function call cycle" errors on mutually recursive struct types via pointer/slice indirection. Every legal Go recursive type goes through such indirection (direct embedding is a compile error), so the analysis only produced false positives — most recently breaking gno-ibc CI on a Schema/Field tree type. Drop analyzeTypeSpec and analyzeValueSpec entirely. The rule now analyzes only function calls, matching the message it already emits. Two latent gaps in the call-graph builder are fixed in the same change because dropping the type-spec edges without them would silently regress the rule's canonical use case: - analyzeFuncDecl filtered call-site idents to ident.Name == name, so mutual function recursion (a calls b calls a) was never recorded — only type-spec pollution surfaced those cycles. - Method calls are now resolved when the selector receiver matches the enclosing method's receiver param, so T.foo calling T.bar and back is detected. Cross-type calls and calls through variables still need go/types resolution and remain out of scope. Tests cover the gno-ibc Field/Schema reproducer plus a battery of shapes that must not trigger (self-pointer linked list, recursive map values, function-typed field, embedded mutual via pointers, slice-of-pointer-to-self, pointer var initializers, type alias chains) and positive same-type method recursion cases. testdata/cycle/types.gno only existed to assert the now-deprecated type-cycle behavior and is removed; the engine integration test points at the existing testdata/cycle0.gno function-cycle fixture.
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
1 participant
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.
Description
The cycle-detection rule used to walk every TypeSpec/ValueSpec identifier and emit "function call cycle" errors on mutually recursive struct types via pointer/slice indirection. Every legal Go recursive type goes through such indirection (direct embedding is a compile error), so the analysis only produced false positives.
Drop analyzeTypeSpec and analyzeValueSpec entirely. The rule now analyzes only function calls, matching the message it already emits.
Two latent gaps in the call-graph builder are fixed in the same change because dropping the type-spec edges without them would silently regress the rule's canonical use case:
testdata/cycle/types.gno only existed to assert the now-deprecated type-cycle behavior and is removed; the engine integration test points at the existing testdata/cycle0.gno function-cycle fixture.