chore: In gno doc, use JSONDocumentation (like from vm/qdoc)

[jefft0]

PR https://github.com/gnolang/gno/pull/3851 makes gnoweb use vm/qdoc which returns a JSONDocumentation. This PR makes gno doc use a JSONDocumentation to print the docs. Currently, this is obtained from the local gno code, but this opens to way for gno doc to make a remote call to vm/qdoc when needed.

In general, this PR separates two processes which are currently intertwined: Figuring out what information needs to be printed and extracting that information from the go/doc AST. Now, we first extract all information from the go/doc AST into a JSONDocumentation, and then we figure out what needs to be printed.

As a follow-on to PR https://github.com/gnolang/gno/pull/3459, add more information in JSONDocumentation:

• In JSONDocumentation, add Bugs
• In JSONType, change Signature to Type (breaking change)
• In JSONType, add Alias for type A = B
• In JSONType, add Kind to handle struct, interface, etc.
• In JSONType, add InterElems and Fields to include information about interface elements (methods, embedded types) and struct fields
• In JSONField, add Doc which is useful for struct fields
• In JSONFunc, add Crossing
• Update tests accordingly

Make gno doc use a JSONDocumentation:

• In WriteJSONDocumentation, add a param for WriteDocumentationOptions so that we can set Source
• In pkgPrinter, change the type of doc to JSONDocumentation. Get it from WriteJSONDocumentation
• In pkgPrinter, remove unused fields pkg, file and fs
• In pkgPrinter, change typedValue and constructor to use the symbol strings instead of the specific go/doc AST types (which are not in JSONDocumentation)
• Change pkgPrinter methods to use JSONDocumentation instead of the go/doc AST
• In pkgPrinter.ToText, use a default Parser and Printer so that we don't need the go/doc package AST. Make it a non-method function
• In pkgPrinter.symbolDoc, remove the unused bool return

For example. gno doc valopers prints:

package valopers // import "valopers"

Package valopers is designed around the permissionless lifecycle of valoper
profiles.

const MonikerMaxLength
var ErrValoperExists
func AddToAuthList(cur realm, address_XXX std.Address, member std.Address)
func Auth() *authz.Authorizer
func DeleteFromAuthList(cur realm, address_XXX std.Address, member std.Address)
func NewInstructionsProposalCallback(newInstructions string) func() error
func NewMinFeeProposalCallback(newMinFee int64) func() error
func Register(cur realm, moniker string, description string, address_XXX std.Address, pubKey string)
func Render(fullPath string) string
func UpdateDescription(cur realm, address_XXX std.Address, description string)
func UpdateKeepRunning(cur realm, address_XXX std.Address, keepRunning bool)
func UpdateMoniker(cur realm, address_XXX std.Address, moniker string)
type Valoper struct{ ... }
    func GetByAddr(address_XXX std.Address) Valoper

BREAKING CHANGE: In JSONType, changed Signature to Type. The signature can be easily constructed with "type " + Name + " " + Type. (Should be minimal impact. This doesn't affect gnoweb which doesn't show types.)

[Gno2D2]

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):

• [ ] IGNORE the bot requirements for this PR (force green CI check)

Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info) 🟢 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:

1. Fix any issues flagged by automated checks.
2. Follow the Contributor Checklist to ensure your PR is ready for review.
   • Add new tests, or document why they are unnecessary.
   • Provide clear examples/screenshots, if necessary.
   • Update documentation, if required.
   • Ensure no breaking changes, or include BREAKING CHANGE notes.
   • Link related issues/PRs, where applicable.

☑️ Reviewer Actions:

1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.

📚 Resources:

• Report a bug with the bot.
• View the bot’s configuration file.

Debug

Automated Checks

Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: jefft0/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🟢 Requirement satisfied
└── 🟢 If
    ├── 🟢 Condition
    │   └── 🟢 Or
    │       ├── 🟢 At least 1 user(s) of the organization reviewed the pull request (with state "APPROVED")
    │       ├── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🟢 Then
        └── 🟢 And
            ├── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)
            └── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request

Manual Checks

**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

• Any user with comment edit permission

[codecov[bot]]

Codecov Report

:x: Patch coverage is 76.72584% with 118 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
gnovm/pkg/doc/print.go	68.21%	84 Missing and 12 partials :warning:
gnovm/pkg/doc/json_doc.go	88.41%	13 Missing and 6 partials :warning:
gnovm/pkg/doc/doc.go	91.89%	3 Missing :warning:

:loudspeaker: Thoughts on this report? Let us know!

[thehowl]

• If JSONType is an interface, it doesn't include the methods. We can add to the Fields where Name is the method name and Type is a func. For example type Test interface { Meth(int x) string} is a JSONField where Name is Meth and Type is func (int x) string . Does that sound good?

I would separate Fields and Methods, for structs and interfaces respectively, rather than overloading them. We can use omitempty to only include them for one. (We could use omitzero to print an empty JSON array using a non-nil slice, so show Fields OR Methods even when they're empty, but it's available only in 1.24 which is released in August :cry: ).

• Should JSONDocumentation include the package BUGS?

If it's trivial, yes.

[jefft0]

I would separate Fields and Methods, for structs and interfaces respectively

That would work. Alternatively, in JSONDocumentation, we can keep Types []*JSONType only for struct types, and add Interfaces []*JSONInterface where JSONInterface has Methods (and no Fields). What do you think?

[thehowl]

That would work. Alternatively, in JSONDocumentation, we can keep Types []*JSONType only for struct types, and add Interfaces []*JSONInterface where JSONInterface has Methods (and no Fields). What do you think?

No, I think it makes sense to keep them together - Types can actually have not just struct and interfaces, but all kinds of declared types, no? Like type A int. So maybe instead of "interface": true/false we need a "kind" field, like reflect.Kind. Seeing as we work on the AST, though, it should be different.

Here's how I propose we make it:

type JSONType struct {
	Name    string      `json:"name"`             // JSONType
	Type    string      `json:"type"`             // struct { ... }
	Doc     string      `json:"doc"`              // godoc documentation...
	Alias   bool        `json:"alias"`            // true if Assign != 0; ie. if an alias like `type A = B`
	Kind    string      `json:"kind"`             // struct | interface | array | slice | map | channel | func | pointer | ident
	Methods []JSONField `json:"methods,omitzero"` // interface methods (kind == "struct")
	Fields  []JSONField `json:"fields,omitzero"`  // struct fields (kind == "interface")
}

• Remove Signature in favor of Type (it can be easily constructed with "type " + Name + " " + Type)
• Add Alias to indicate type aliases
• Add Kind; allows a reader to quickly "peek" into the kind of the type. We may extend this to put this in other places, but let's start with JSONType. This should be based on a simple AST type switch on the value of TypeSpec.Type; SelectorExpr and Ident should both be considered idents, ParenExpr should be de-parenthesized before generating the JSONType.
• Methods and Fields have omitzero; use omitempty for now and add a comment noting to switch when possible.

[jefft0]

Here's how I propose we make it

Sounds good. A detail question about struct field comments. Consider the struct

type MyStruct struct {
  // Above comment
  X int // line comment
}

In the AST Field type, the above comment and line comment are separated as Doc and Comment . In JSONField, should we combine these into one Doc string? (Same question for interface methods.)

[thehowl]

In the AST Field type, the above comment and line comment are separated as Doc and Comment . In JSONField, should we combine these into one Doc string? (Same question for interface methods.)

Sounds good.

[jefft0]

@thehowl , in JSONType, you suggest Methods []JSONField. Instead, I suggest to use JSONFunc which will allow us to separate out parameters and return types. Sound good?

[jefft0]

• Remove Signature in favor of Type (it can be easily constructed with "type " + Name + " " + Type)
• Add Alias to indicate type aliases
• Add Kind; allows a reader to quickly "peek" into the kind of the type. We may extend this to put this in other places, but let's start with JSONType. This should be based on a simple AST type switch on the value of TypeSpec.Type; SelectorExpr and Ident should both be considered idents, ParenExpr should be de-parenthesized before generating the JSONType.
• Methods and Fields have omitzero; use omitempty for now and add a comment noting to switch when possible.

Hi @thehowl . These changes are complete. Ready for review again.

[thehowl]

On some thought, let's merge this after #4316.

The syntax to cross will be different again, so we'll have to update how this works regardless.

[thehowl]

@jefft0 xform2 is merged. Can you update this so that cur realm as a first parameter is identified as crossing, rather than using the crossing() statement?

I think we can still call it a "crossing function", so only the detection system should change.

[jefft0]

@jefft0 xform2 is merged. Can you update this so that cur realm as a first parameter is identified as crossing, rather than using the crossing() statement?

Hi @thehowl . Done. https://github.com/gnolang/gno/pull/3931/commits/977b1d6bb27a74ef75a1403226c3677d2a9641a9

[jefft0]

Overall, it looks good. @jefft0 Just before approval, what is the point of detecting and retaining the BUG keyword? I understand its relevance to the current package, but once we reach the mainnet, I don't think we should encourage people to retain functionality with BUG. Since the contract cannot be updated, I don't see the value in deploying a buggy feature.

I included BUGS because @thehowl asked me to. https://github.com/gnolang/gno/pull/3931#issuecomment-2810044808

[thehowl]

Overall, it looks good. @jefft0 Just before approval, what is the point of detecting and retaining the BUG keyword? I understand its relevance to the current package, but once we reach the mainnet, I don't think we should encourage people to retain functionality with BUG. Since the contract cannot be updated, I don't see the value in deploying a buggy feature.

// BUG can be used to mention even rare bug conditions: https://pkg.go.dev/reflect#pkg-note-BUG

Gno realms specifically will likely have some form of evolution, and bugs may be kept in packages for many reasons, chief among which backwards compatibility, so I would say it's sometimes desirable to have them, and having a mechanism to clearly mark bugs is better IMO than not having one.

We can also remove it as less important, but even Go does ship packages with bugs. Admittedly, this bug feature is more useful if package documentation can be updated (even if the rest of the program can't), but I don't think this is a wrong feature to have itself