Skip to content

proposal: go/types: add HasTypeName interface #66890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
adonovan opened this issue Apr 18, 2024 · 16 comments
Closed

proposal: go/types: add HasTypeName interface #66890

adonovan opened this issue Apr 18, 2024 · 16 comments
Assignees
Labels
Milestone

Comments

@adonovan
Copy link
Member

During the types.Alias work, there was a recurring need (7 times in x/tools; @findleyr is adding an eighth) for this interface:

package types // import "go/types"

// HasTypeName abstracts the three kinds of types that have declared names:
// aliases ([*Alias]), defined types ([*Named]), and type parameters ([*TypeParam]).
//
// Note that the Go spec considers built-in types such as string and int to
// be defined types, but this package represents them as [*Basic],
// since they do not have a declaration or [TypeName].
type HasTypeName interface {
    Obj() *TypeName
}

Of course users can easily define it for themselves, but adding it to go/types provides a good place to hang additional documentation.

Related:

@findleyr @griesemer

@gopherbot gopherbot added this to the Proposal milestone Apr 18, 2024
@adonovan adonovan moved this to Incoming in Proposals Apr 18, 2024
@adonovan adonovan self-assigned this Apr 18, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/604476 mentions this issue: go/types: clarify Named, Alias, TypeName, Object

gopherbot pushed a commit that referenced this issue Sep 20, 2024
Updates #65855
Updates #66890

Change-Id: I167c9de818049cae02f0d99f8e0fb4017e07bea9
Reviewed-on: https://go-review.googlesource.com/c/go/+/604476
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
@griesemer
Copy link
Contributor

I note that we have an unexported function

// hasName reports whether t has a name. This includes
// predeclared types, defined types, and type parameters.
// hasName may be called with types that are not fully set up.
func hasName(t Type) bool {
	switch Unalias(t).(type) {
	case *Basic, *Named, *TypeParam:
		return true
	}
	return false
}

Per the spec: "Predeclared types, defined types, and type parameters are called named types."

Maybe the interface should just be called "HasName". Having theObj method is not quite the same as being a named type (aliases have Obj but may denote type literals - one of the mistakes we made when introducing aliases).

@adonovan
Copy link
Member Author

In the branch where I was playing with this I ended up using the name TypeWithName to make clear that the interface is a restriction of Type; otherwise it's a bit unclear what kind of thing it is.

@findleyr
Copy link
Member

My 2 cents: types.HasName seems better than types.TypeWithName

@griesemer
Copy link
Contributor

In the spec, a named type has a very specific meaning; specifically, an alias type may not be a named type. This interface is about whether a type has an Obj method which returns a TypeName. I think HasTypeName or HasObj seem ok names.

@aclements aclements moved this from Incoming to Active in Proposals May 8, 2025
@aclements
Copy link
Member

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— aclements for the proposal review group

@aclements
Copy link
Member

@griesemer notes that this is almost but not quite a "named type" from the spec (an alias is only a named type if it aliases a named type). We'll need to rewrite the comment to be careful about that. The fact that it doesn't correspond exactly to the spec seems to be okay. Given that, HasTypeName is good.

@aclements
Copy link
Member

aclements commented May 12, 2025

@adonovan to post an updated version that accounts for #71886 (but the API will stay the same)

@aclements
Copy link
Member

Based on the discussion above, this proposal seems like a likely accept.
— aclements for the proposal review group

The proposal is to add the following to package go/types:

// HasTypeName abstracts the four kinds of types that have declared names:
// basic types ([*Basic]), defined types ([*Named]), aliases ([*Alias]), and
// type parameters ([*TypeParam]),
//
// Note that the Go spec considers all of these "named types", with the exception
// of alias types, which are only "named types" if their target is a named type.
type HasTypeName interface {
    Obj() *TypeName
}

@aclements aclements moved this from Active to Likely Accept in Proposals May 13, 2025
@adonovan
Copy link
Member Author

Unfortunately I just discovered that #71886 is infeasible, so we need to revert the comment to the original wording to exclude *Basic.

@griesemer
Copy link
Contributor

Maybe this should be a function instead?

func TypeNameFor(Type) *TypeName

@aclements aclements moved this from Likely Accept to Active in Proposals May 14, 2025
@aclements
Copy link
Member

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— aclements for the proposal review group

@adonovan
Copy link
Member Author

Maybe this should be a function instead?
func TypeNameFor(Type) *TypeName

Yeah, that would work. I looked at x/tools and 5 out of 7 of the places where we define this interface would be satisfied with this helper.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/672975 mentions this issue: internal/typesinternal: add TypeNameFor helper

@adonovan
Copy link
Member Author

I added func TypeNameFor(Type) *TypeName to gopls, shown below, but I don't think it's a simple enough abstraction to go in go/types. So I retract this proposal too.

// TypeNameFor returns the type name symbol for the specified type, if
// it is a [*types.Alias], [*types.Named], [*types.TypeParam], or a
// [*types.Basic] representing a type.
//
// For all other types, and for Basic types representing a builtin,
// constant, or nil, it returns nil. Be careful not to convert the
// resulting nil pointer to a [types.Object]!
//
// If t is the type of a constant, it may be an "untyped" type, which
// has no TypeName. To access the name of such types (e.g. "untyped
// int"), use [types.Basic.Name].
func TypeNameFor(t types.Type) *types.TypeName {
	switch t := t.(type) {
	case *types.Alias:
		return t.Obj()
	case *types.Named:
		return t.Obj()
	case *types.TypeParam:
		return t.Obj()
	case *types.Basic:
		// See issues #71886 and #66890 for some history.
		if tname, ok := types.Universe.Lookup(t.Name()).(*types.TypeName); ok {
			return tname
		}
	}
	return nil
}

gopherbot pushed a commit to golang/tools that referenced this issue May 15, 2025
This function eliminates most of the places where we had
reinvented this wheel:

   type HasTypeName interface { Obj() *TypeName }

Updates golang/go#66890
Updates golang/go#71886

Change-Id: I70d3c8141b20efb1ba0e02fc846d8b38b8f3a23c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/672975
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Commit-Queue: Alan Donovan <[email protected]>
@aclements
Copy link
Member

This proposal has been declined as retracted.
— aclements for the proposal review group

@aclements aclements moved this from Active to Declined in Proposals May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Declined
Development

No branches or pull requests

5 participants