-
Notifications
You must be signed in to change notification settings - Fork 297
Add missing documentation for exposed structs in internal package #2232
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
faffd7c
6e2a41a
c50b3ec
247edac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,14 +18,17 @@ type ( | |
| // | ||
| // Exposed as: [go.temporal.io/sdk/activity.Type] | ||
| ActivityType struct { | ||
| // Name is the name of the activity type. | ||
| Name string | ||
| } | ||
|
|
||
| // ActivityInfo contains information about a currently executing activity. | ||
| // | ||
| // Exposed as: [go.temporal.io/sdk/activity.Info] | ||
| ActivityInfo struct { | ||
| // TaskToken is the token that identifies the activity task. | ||
| TaskToken []byte | ||
| // WorkflowType is the type of the workflow that started this activity. | ||
| WorkflowType *WorkflowType | ||
| // Namespace of the workflow that started this activity. Empty if this activity was not started by a workflow. | ||
| // If present, the value is always the same as Namespace since workflows can only run activities in their own | ||
|
|
@@ -36,9 +39,13 @@ type ( | |
| // Execution details of the workflow that started this activity. All fields are empty if this activity was not | ||
| // started by a workflow. | ||
| WorkflowExecution WorkflowExecution | ||
| // ActivityID is the ID of the activity. | ||
| ActivityID string | ||
| // ActivityRunID is the run ID of the activity. Empty if the activity was started by a workflow. | ||
| ActivityRunID string // Run ID of the activity. Empty if the activity was started by a workflow. | ||
| // ActivityType is the type of the activity. | ||
| ActivityType ActivityType | ||
| // TaskQueue is the name of the task queue that the activity needs to be scheduled on. | ||
| TaskQueue string | ||
| Namespace string // Namespace of this activity. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like some of these got missed due to having inline comments. Can we make sure the tool flags these to have doc comments, even if they have existing inline comments? |
||
| HeartbeatTimeout time.Duration // Maximum time between heartbeats. 0 means no heartbeat needed. | ||
|
|
@@ -73,6 +80,7 @@ type ( | |
| // ambiguity between string names and function references. Also users should | ||
| // always use this string name when executing this activity. | ||
| Name string | ||
| // DisableAlreadyRegisteredCheck disables the check for already registered activities. | ||
| DisableAlreadyRegisteredCheck bool | ||
|
|
||
| // When registering a struct with activities, skip functions that are not valid activities. If false, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,6 +127,11 @@ func run() error { | |
| if err != nil { | ||
| return fmt.Errorf("error while parsing internal files: %v", err) | ||
| } | ||
|
|
||
| err = checkInternalDocs(path, file, publicToInternal) | ||
| if err != nil { | ||
| return fmt.Errorf("error while checking internal docs: %v", err) | ||
| } | ||
| } | ||
| return nil | ||
| }) | ||
|
|
@@ -587,3 +592,84 @@ func isValidDefinitionWithMatch(line, private string, inGroup string, insideStru | |
| } | ||
| return false | ||
| } | ||
|
|
||
| func checkInternalDocs(path string, file *os.File, pairs map[string]map[string]string) error { | ||
| fs := token.NewFileSet() | ||
|
|
||
| // Reset file pointer to start | ||
| _, err := file.Seek(0, 0) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to seek: %v", err) | ||
| } | ||
|
|
||
| node, err := parser.ParseFile(fs, path, file, parser.ParseComments) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to parse file %s: %v", path, err) | ||
| } | ||
|
|
||
| exposedTypes := make(map[string]bool) | ||
| for _, pair := range pairs { | ||
| for _, private := range pair { | ||
| exposedTypes[private] = true | ||
| } | ||
| } | ||
|
|
||
| ast.Inspect(node, func(n ast.Node) bool { | ||
| if genDecl, ok := n.(*ast.GenDecl); ok && genDecl.Tok == token.TYPE { | ||
| for _, spec := range genDecl.Specs { | ||
| if typeSpec, ok := spec.(*ast.TypeSpec); ok { | ||
| if !exposedTypes[typeSpec.Name.Name] { | ||
| continue | ||
| } | ||
| if structType, ok := typeSpec.Type.(*ast.StructType); ok { | ||
| for _, field := range structType.Fields.List { | ||
| isExported := false | ||
| var fieldName string | ||
| if len(field.Names) > 0 { | ||
| for _, name := range field.Names { | ||
| if ast.IsExported(name.Name) { | ||
| isExported = true | ||
| fieldName = name.Name | ||
| break | ||
| } | ||
| } | ||
| } else { | ||
| // Check anonymous field | ||
|
brucearctor marked this conversation as resolved.
Outdated
|
||
| if ident, ok := field.Type.(*ast.Ident); ok { | ||
| if ast.IsExported(ident.Name) { | ||
| isExported = true | ||
| fieldName = ident.Name | ||
| } | ||
| } else if sel, ok := field.Type.(*ast.SelectorExpr); ok { | ||
| if ast.IsExported(sel.Sel.Name) { | ||
| isExported = true | ||
| fieldName = sel.Sel.Name | ||
| } | ||
| } else if star, ok := field.Type.(*ast.StarExpr); ok { | ||
| if ident, ok := star.X.(*ast.Ident); ok { | ||
| if ast.IsExported(ident.Name) { | ||
| isExported = true | ||
| fieldName = ident.Name | ||
| } | ||
| } else if sel, ok := star.X.(*ast.SelectorExpr); ok { | ||
| if ast.IsExported(sel.Sel.Name) { | ||
| isExported = true | ||
| fieldName = sel.Sel.Name | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if isExported && field.Doc == nil && field.Comment == nil { | ||
| changesNeeded = true | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| fmt.Printf("Missing doc for exposed struct %s field %s in %s\n", typeSpec.Name.Name, fieldName, path) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return true | ||
| }) | ||
| return nil | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.