Skip to content
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

proposal: add annotations #70178

Closed
Azer0s opened this issue Nov 3, 2024 · 10 comments
Closed

proposal: add annotations #70178

Azer0s opened this issue Nov 3, 2024 · 10 comments
Labels
Milestone

Comments

@Azer0s
Copy link

Azer0s commented Nov 3, 2024

Proposal Details

Background & Motivation

Right now, we have many cases where we basically already use comments as annotations.
All compiler directives and the build directives are written in comments when in reality they are part of the code. As a comment they are easily overseen and very much "magic".

For example: this is pure magic. Of course, we could look up the documentation for go:linkname but we should be able to figure these things out by looking into the source code.

package ex

import _ "unsafe"
import _ "embed"

//go:embed hello.txt
var s string

//go:linkname myFuncWithBody example/ex.MyFunc
func myFuncWithBody() string {
   return s
}

func MyFunc() string

Comments should, in my opinion, not contain source code (which is what that is in most cases) and the status quo is definitely not an elegant solution. Therefore I propose a C++ style annotation syntax.

Proposal

My proposal would tie in C++ annotation syntax with an idiomatic Go-style solution. I propose to add a new Annotate interface which structs that can be used as annotations should implement. The Annotate.Annotate method simply returns the scope on which the annotation can function.

The annotation will then be picked up by the compiler (like magic comments are, right now) or with the reflect package when using custom annotations.

So your annotation would look like so

package compile

type LinkName struct {
    To string
}

func (l *LinkName) Annotate() annotate.Target {
    return annotate.TargetFunc
}

type Embed struct {
    File string
}

func (e * Embed) Annotate() annotate.Target {
    return annotate.TargetVar
}

An you could then annotate like this:

package ex

import (
    "unsafe",
    "compile"
)

[[compile.Embed{ File: "hello.txt" }]]
var s string

[[compile.LinkName{ To: "example/ex.MyFunc" }]]
func myFuncWithBody() string {
   return s
}

func MyFunc() string

But this, of course, could also be useful in many other cases when using custom annotations. HTTP routes could be annotated but we could also phase out tags for something more elegant. Especially because custom field tags are, to my knowledge, not type safe in any way.

[[http.Get{Path: "/tasks"}]]
func GetAllTasks(w http.ResponseWriter, r *http.Request) {
    tasks, err := taskService.GetAllTasks(ctx.Background())
    // ...
    w.Write(taskJson)
}
type TaskDto struct {
    [[json.Ignore]]
    Id string
    //...
   
    [[json.As{Name: "shortName"}]]
    VeryVeryLongName int
}

Discussion

There are a couple of problems with this proposal. The biggest one being: how would one access these annotations. I would suggest via the reflect package.

One point of discussion is also how the annotation itself should look. I won't lie, the C++ style is a bit...fugly. I think this is not really that important of a decision. I personally would prefer the C++ style as I think it looks "right" in the context of the Go language but @ or <<>> could also look interesting.

type TaskDto struct {
    @json.Ignore
    Id string
    //...
   
    @json.As{Name: "shortName"}
    VeryVeryLongName int
}
type TaskDto struct {
    <<json.Ignore>>
    Id string
    //...
   
    <<json.As{Name: "shortName"}>>
    VeryVeryLongName int
}

Another problem is still having the "old way" of doing things around. I would suggest a simple warning that you are using an old style and should consider rewriting.

I know this also goes a bit against not providing multiple ways of doing the same thing. At the same time, this isn't really the same thing. This is a new thing which just happens to also come with an improvement of an already existing thing. This sometimes happens and should probably not be a reason to stop innovation.

@Azer0s Azer0s added the Proposal label Nov 3, 2024
@gopherbot gopherbot added this to the Proposal milestone Nov 3, 2024
@gabyhelp
Copy link

gabyhelp commented Nov 3, 2024

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@seankhliao
Copy link
Member

Duplicate of #36669

@seankhliao seankhliao marked this as a duplicate of #36669 Nov 3, 2024
@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Nov 3, 2024
@seankhliao
Copy link
Member

see also #23637 for tags specifically

@Azer0s
Copy link
Author

Azer0s commented Nov 3, 2024

Duplicate of #36669

Hi! Not really, this addresses mostly Go compiler directives.

@Azer0s
Copy link
Author

Azer0s commented Nov 3, 2024

@seankhliao neither #36669 nor #23637 address compiler directives. My solution aims to unify the solution to these problems. I don't really see why this was closed.

@seankhliao
Copy link
Member

[[http.Get{Path: "/tasks"}]] isn't a compiler directive, that's a decorator.

@Azer0s
Copy link
Author

Azer0s commented Nov 3, 2024

@seankhliao I specifically wrote "this could also be used" for this. This isn't the main focus. I address this in the background section. Please read these before you close without asking.

@seankhliao
Copy link
Member

Struct tags aren't compiler directives either.
Making them more code like is of limited use when they aren't actual code either, you can't pass variables or references to them.
See also the embed proposal for why compiler directives were chosen over a more structured approach https://go.googlesource.com/proposal/+/master/design/draft-embed.md#approach

@Azer0s
Copy link
Author

Azer0s commented Nov 3, 2024

Struct tags aren't compiler directives either. Making them more code like is of limited use when they aren't actual code either, you can't pass variables or references to them. See also the embed proposal for why compiler directives were chosen over a more structured approach https://go.googlesource.com/proposal/+/master/design/draft-embed.md#approach

Again...please read the Background and Motivation section. I never said that they are, just that this solution could also solve a common problem many people have with tags. Also, the link doesn't discuss anything like the solution I am proposing.

@ianlancetaylor
Copy link
Member

@Azer0s Thanks for the proposal, but we aren't going to use types defined in Go as compile-time annotations. Compiler directives are imperfect, but using Go types is a huge jump in complexity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants