Skip to content

Commit 3506cd2

Browse files
yuandrewyuandrew
authored
Fix doclink tool's duplicate docstring generation issue (#1773)
* Removed duplicate "Exposed as" comments, added ability to detect/delete stale aliases. Handle edge case where gofmt reorders the doc comment * fix ci
1 parent 815c648 commit 3506cd2

File tree

4 files changed

+107
-72
lines changed

4 files changed

+107
-72
lines changed

‎internal/cmd/tools/doclink/doclink.go

+100-21
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ type (
4444
}
4545
)
4646

47-
var missing = false
47+
var changesNeeded = false
4848

4949
func main() {
5050
if err := run(); err != nil {
5151
log.Fatal(err)
5252
}
53-
if missing {
54-
log.Fatal("Missing documentation, see previous stdout for which objects. Re-run command with -fix to auto-generate missing docs.")
53+
if changesNeeded {
54+
log.Fatal("Changes needed, see previous stdout for which objects. Re-run command with -fix to auto-generate new docs.")
5555
}
5656
}
5757

@@ -354,48 +354,126 @@ func extractPackageName(file *os.File) (string, error) {
354354
// If mapping is identified, check if doc comment exists for such mapping.
355355
func processInternal(cfg config, file *os.File, pairs map[string]map[string]string) error {
356356
scanner := bufio.NewScanner(file)
357+
nextLine := scanner.Text()
357358
newFile := ""
358359
exposedAs := "// Exposed as: "
360+
var commentBlock string
359361
var inGroup, exposedLinks string
360362
var changesMade, inStruct bool
361363
for scanner.Scan() {
362-
line := scanner.Text()
364+
line := nextLine
365+
nextLine = scanner.Text()
363366
trimmedLine := strings.TrimSpace(line)
364-
if isValidDefinition(trimmedLine, &inGroup, &inStruct) {
367+
trimmedNextLine := strings.TrimSpace(nextLine)
368+
// Keep track of code block, for when we check a valid definition below,
369+
// gofmt will sometimes format links like "[Visibility]: https://sample.url"
370+
// to the bottom of the doc string.
371+
if strings.HasPrefix(trimmedLine, "//") {
372+
commentBlock += trimmedLine + "\n"
373+
} else {
374+
commentBlock = ""
375+
}
376+
377+
// Check for old docs links to remove
378+
if strings.Contains(trimmedNextLine, exposedAs) {
379+
links := strings.Split(strings.TrimPrefix(trimmedNextLine, exposedAs), ", ")
380+
var newLinks []string
381+
for _, link := range links {
382+
staleLink := true
383+
for packageName, pair := range pairs {
384+
for public := range pair {
385+
docLink := fmt.Sprintf("[go.temporal.io/sdk/%s.%s]", packageName, public)
386+
if link == docLink {
387+
staleLink = false
388+
}
389+
}
390+
}
391+
392+
if !staleLink {
393+
newLinks = append(newLinks, link)
394+
} else {
395+
if cfg.fix {
396+
changesMade = true
397+
fmt.Println("Removing stale doc link:", link)
398+
} else {
399+
changesNeeded = true
400+
fmt.Println("Stale doc link:", link)
401+
}
402+
}
403+
}
404+
newTrimmedLine := exposedAs
405+
for i := range newLinks {
406+
newTrimmedLine += newLinks[i] + ", "
407+
}
408+
nextLine = strings.TrimSuffix(newTrimmedLine, ", ")
409+
trimmedNextLine = nextLine
410+
}
411+
412+
// Check for new doc links to add
413+
if isValidDefinition(trimmedNextLine, &inGroup, &inStruct) {
414+
// Find the "Exposed As" line in the doc comment
415+
var lineFromCommentBlock string
416+
comScanner := bufio.NewScanner(strings.NewReader(commentBlock))
417+
for comScanner.Scan() {
418+
tempLine := strings.TrimSpace(comScanner.Text())
419+
if strings.HasPrefix(tempLine, exposedAs) {
420+
lineFromCommentBlock = tempLine
421+
break
422+
}
423+
}
424+
// Check for new doc pairs
365425
for packageName, pair := range pairs {
366426
for public, private := range pair {
367-
if isValidDefinitionWithMatch(trimmedLine, private, inGroup, inStruct) {
427+
if isValidDefinitionWithMatch(trimmedNextLine, private, inGroup, inStruct) {
368428
docLink := fmt.Sprintf("[go.temporal.io/sdk/%s.%s]", packageName, public)
369-
missingDoc := true
370-
if exposedLinks != "" {
371-
if strings.Contains(exposedLinks, docLink) {
372-
missingDoc = false
373-
}
429+
missingDoc := false
430+
if lineFromCommentBlock == "" || !strings.Contains(lineFromCommentBlock, docLink) {
431+
missingDoc = true
374432
}
375-
if missingDoc {
376-
if cfg.fix {
433+
if cfg.fix {
434+
exposedLinks += docLink + ", "
435+
if missingDoc {
377436
changesMade = true
378-
exposedLinks += docLink + ", "
379-
fmt.Printf("Fixed doc in %s for internal:%s to %s:%s\n", file.Name(), private, packageName, public)
380-
} else {
381-
missing = true
437+
fmt.Printf("Added doc in %s for internal:%s to %s:%s\n", file.Name(), private, packageName, public)
438+
}
439+
} else {
440+
if missingDoc {
441+
changesNeeded = true
382442
fmt.Printf("Missing doc in %s for internal:%s to %s:%s\n", file.Name(), private, packageName, public)
383443
}
384444
}
445+
385446
}
386447
}
387448
}
388449
if exposedLinks != "" {
389-
newFile += "//\n" + exposedAs + strings.TrimSuffix(exposedLinks, ", ") + "\n"
450+
updatedLine := exposedAs + strings.TrimSuffix(exposedLinks, ", ")
451+
452+
// If there is an existing "Exposed As" docstring
453+
if lineFromCommentBlock != "" {
454+
// The last line of commentBlock hasn't been written to newFile yet,
455+
// so check if lineFromCommentBlock is that scenario
456+
if lineFromCommentBlock == trimmedLine {
457+
line = updatedLine
458+
} else {
459+
newFile = strings.Replace(newFile, lineFromCommentBlock, updatedLine, 1)
460+
}
461+
} else {
462+
// Last line of existing docstring hasn't been written yet,
463+
// write that line to newFile, then set the updatedLine to
464+
// be the next line to be written to newFile
465+
newFile += line + "\n"
466+
line = "//\n" + updatedLine
467+
}
390468
exposedLinks = ""
469+
391470
}
392-
} else if strings.HasPrefix(trimmedLine, exposedAs) {
393-
exposedLinks = strings.TrimPrefix(trimmedLine, exposedAs)
394471
}
395472
newFile += line + "\n"
396-
397473
}
398474

475+
newFile += nextLine + "\n"
476+
399477
if changesMade {
400478
absPath, err := filepath.Abs(file.Name())
401479
if err != nil {
@@ -469,6 +547,7 @@ func isValidDefinition(line string, inGroup *string, insideStruct *bool) bool {
469547
return false
470548
}
471549

550+
// Checks if `line` is a valid definition, and that definition is for `private`
472551
func isValidDefinitionWithMatch(line, private string, inGroup string, insideStruct bool) bool {
473552
tokens := strings.Fields(line)
474553
if strings.HasPrefix(line, "func "+private+"(") {

‎internal/error.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ func NewApplicationError(msg string, errType string, nonRetryable bool, cause er
386386
)
387387
}
388388

389-
// Exposed as: [go.temporal.io/sdk/temporal.NewApplicationErrorWithOptions], [go.temporal.io/sdk/temporal.NewApplicationErrorWithCause], [go.temporal.io/sdk/temporal.NewApplicationError], [go.temporal.io/sdk/temporal.NewNonRetryableApplicationError]
389+
// Exposed as: [go.temporal.io/sdk/temporal.NewApplicationError], [go.temporal.io/sdk/temporal.NewApplicationErrorWithOptions], [go.temporal.io/sdk/temporal.NewApplicationErrorWithCause], [go.temporal.io/sdk/temporal.NewNonRetryableApplicationError]
390390
func NewApplicationErrorWithOptions(msg string, errType string, options ApplicationErrorOptions) error {
391391
applicationErr := &ApplicationError{
392392
msg: msg,

‎internal/interceptor.go

-44
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ import (
3939
// the interceptor package for more details.
4040
//
4141
// Exposed as: [go.temporal.io/sdk/interceptor.Interceptor]
42-
//
43-
// Exposed as: [go.temporal.io/sdk/interceptor.Interceptor]
4442
type Interceptor interface {
4543
ClientInterceptor
4644
WorkerInterceptor
@@ -50,8 +48,6 @@ type Interceptor interface {
5048
// documentation in the interceptor package for more details.
5149
//
5250
// Exposed as: [go.temporal.io/sdk/interceptor.WorkerInterceptor]
53-
//
54-
// Exposed as: [go.temporal.io/sdk/interceptor.WorkerInterceptor]
5551
type WorkerInterceptor interface {
5652
// InterceptActivity is called before each activity interception needed with
5753
// the next interceptor in the chain.
@@ -69,8 +65,6 @@ type WorkerInterceptor interface {
6965
// details.
7066
//
7167
// Exposed as: [go.temporal.io/sdk/interceptor.ActivityInboundInterceptor]
72-
//
73-
// Exposed as: [go.temporal.io/sdk/interceptor.ActivityInboundInterceptor]
7468
type ActivityInboundInterceptor interface {
7569
// Init is the first call of this interceptor. Implementations can change/wrap
7670
// the outbound interceptor before calling Init on the next interceptor.
@@ -86,8 +80,6 @@ type ActivityInboundInterceptor interface {
8680
// ExecuteActivityInput is the input to ActivityInboundInterceptor.ExecuteActivity.
8781
//
8882
// Exposed as: [go.temporal.io/sdk/interceptor.ExecuteActivityInput]
89-
//
90-
// Exposed as: [go.temporal.io/sdk/interceptor.ExecuteActivityInput]
9183
type ExecuteActivityInput struct {
9284
Args []interface{}
9385
}
@@ -97,8 +89,6 @@ type ExecuteActivityInput struct {
9789
// more details.
9890
//
9991
// Exposed as: [go.temporal.io/sdk/interceptor.ActivityOutboundInterceptor]
100-
//
101-
// Exposed as: [go.temporal.io/sdk/interceptor.ActivityOutboundInterceptor]
10292
type ActivityOutboundInterceptor interface {
10393
// GetInfo intercepts activity.GetInfo.
10494
GetInfo(ctx context.Context) ActivityInfo
@@ -129,8 +119,6 @@ type ActivityOutboundInterceptor interface {
129119
// details.
130120
//
131121
// Exposed as: [go.temporal.io/sdk/interceptor.WorkflowInboundInterceptor]
132-
//
133-
// Exposed as: [go.temporal.io/sdk/interceptor.WorkflowInboundInterceptor]
134122
type WorkflowInboundInterceptor interface {
135123
// Init is the first call of this interceptor. Implementations can change/wrap
136124
// the outbound interceptor before calling Init on the next interceptor.
@@ -168,17 +156,13 @@ type WorkflowInboundInterceptor interface {
168156
// WorkflowInboundInterceptor.ExecuteWorkflow.
169157
//
170158
// Exposed as: [go.temporal.io/sdk/interceptor.ExecuteWorkflowInput]
171-
//
172-
// Exposed as: [go.temporal.io/sdk/interceptor.ExecuteWorkflowInput]
173159
type ExecuteWorkflowInput struct {
174160
Args []interface{}
175161
}
176162

177163
// HandleSignalInput is the input to WorkflowInboundInterceptor.HandleSignal.
178164
//
179165
// Exposed as: [go.temporal.io/sdk/interceptor.HandleSignalInput]
180-
//
181-
// Exposed as: [go.temporal.io/sdk/interceptor.HandleSignalInput]
182166
type HandleSignalInput struct {
183167
SignalName string
184168
// Arg is the signal argument. It is presented as a primitive payload since
@@ -189,8 +173,6 @@ type HandleSignalInput struct {
189173
// UpdateInput carries the name and arguments of a workflow update invocation.
190174
//
191175
// Exposed as: [go.temporal.io/sdk/interceptor.UpdateInput]
192-
//
193-
// Exposed as: [go.temporal.io/sdk/interceptor.UpdateInput]
194176
type UpdateInput struct {
195177
Name string
196178
Args []interface{}
@@ -199,8 +181,6 @@ type UpdateInput struct {
199181
// HandleQueryInput is the input to WorkflowInboundInterceptor.HandleQuery.
200182
//
201183
// Exposed as: [go.temporal.io/sdk/interceptor.HandleQueryInput]
202-
//
203-
// Exposed as: [go.temporal.io/sdk/interceptor.HandleQueryInput]
204184
type HandleQueryInput struct {
205185
QueryType string
206186
Args []interface{}
@@ -211,8 +191,6 @@ type HandleQueryInput struct {
211191
// NOTE: Experimental
212192
//
213193
// Exposed as: [go.temporal.io/sdk/interceptor.ExecuteNexusOperationInput]
214-
//
215-
// Exposed as: [go.temporal.io/sdk/interceptor.ExecuteNexusOperationInput]
216194
type ExecuteNexusOperationInput struct {
217195
// Client to start the operation with.
218196
Client NexusClient
@@ -231,8 +209,6 @@ type ExecuteNexusOperationInput struct {
231209
// NOTE: Experimental
232210
//
233211
// Exposed as: [go.temporal.io/sdk/interceptor.RequestCancelNexusOperationInput]
234-
//
235-
// Exposed as: [go.temporal.io/sdk/interceptor.RequestCancelNexusOperationInput]
236212
type RequestCancelNexusOperationInput struct {
237213
// Client that was used to start the operation.
238214
Client NexusClient
@@ -249,8 +225,6 @@ type RequestCancelNexusOperationInput struct {
249225
// more details.
250226
//
251227
// Exposed as: [go.temporal.io/sdk/interceptor.WorkflowOutboundInterceptor]
252-
//
253-
// Exposed as: [go.temporal.io/sdk/interceptor.WorkflowOutboundInterceptor]
254228
type WorkflowOutboundInterceptor interface {
255229
// Go intercepts workflow.Go.
256230
Go(ctx Context, name string, f func(ctx Context)) Context
@@ -396,8 +370,6 @@ type WorkflowOutboundInterceptor interface {
396370
// interceptor package for more details.
397371
//
398372
// Exposed as: [go.temporal.io/sdk/interceptor.ClientInterceptor]
399-
//
400-
// Exposed as: [go.temporal.io/sdk/interceptor.ClientInterceptor]
401373
type ClientInterceptor interface {
402374
// This is called on client creation if set via client options
403375
InterceptClient(next ClientOutboundInterceptor) ClientOutboundInterceptor
@@ -410,8 +382,6 @@ type ClientInterceptor interface {
410382
// more details.
411383
//
412384
// Exposed as: [go.temporal.io/sdk/interceptor.ClientOutboundInterceptor]
413-
//
414-
// Exposed as: [go.temporal.io/sdk/interceptor.ClientOutboundInterceptor]
415385
type ClientOutboundInterceptor interface {
416386
// ExecuteWorkflow intercepts client.Client.ExecuteWorkflow.
417387
// interceptor.Header will return a non-nil map for this context.
@@ -494,8 +464,6 @@ type ClientPollWorkflowUpdateOutput struct {
494464
// ClientOutboundInterceptor.CreateSchedule.
495465
//
496466
// Exposed as: [go.temporal.io/sdk/interceptor.ScheduleClientCreateInput]
497-
//
498-
// Exposed as: [go.temporal.io/sdk/interceptor.ScheduleClientCreateInput]
499467
type ScheduleClientCreateInput struct {
500468
Options *ScheduleOptions
501469
}
@@ -504,8 +472,6 @@ type ScheduleClientCreateInput struct {
504472
// ClientOutboundInterceptor.ExecuteWorkflow.
505473
//
506474
// Exposed as: [go.temporal.io/sdk/interceptor.ClientExecuteWorkflowInput]
507-
//
508-
// Exposed as: [go.temporal.io/sdk/interceptor.ClientExecuteWorkflowInput]
509475
type ClientExecuteWorkflowInput struct {
510476
Options *StartWorkflowOptions
511477
WorkflowType string
@@ -516,8 +482,6 @@ type ClientExecuteWorkflowInput struct {
516482
// ClientOutboundInterceptor.SignalWorkflow.
517483
//
518484
// Exposed as: [go.temporal.io/sdk/interceptor.ClientSignalWorkflowInput]
519-
//
520-
// Exposed as: [go.temporal.io/sdk/interceptor.ClientSignalWorkflowInput]
521485
type ClientSignalWorkflowInput struct {
522486
WorkflowID string
523487
RunID string
@@ -529,8 +493,6 @@ type ClientSignalWorkflowInput struct {
529493
// ClientOutboundInterceptor.SignalWithStartWorkflow.
530494
//
531495
// Exposed as: [go.temporal.io/sdk/interceptor.ClientSignalWithStartWorkflowInput]
532-
//
533-
// Exposed as: [go.temporal.io/sdk/interceptor.ClientSignalWithStartWorkflowInput]
534496
type ClientSignalWithStartWorkflowInput struct {
535497
SignalName string
536498
SignalArg interface{}
@@ -543,8 +505,6 @@ type ClientSignalWithStartWorkflowInput struct {
543505
// ClientOutboundInterceptor.CancelWorkflow.
544506
//
545507
// Exposed as: [go.temporal.io/sdk/interceptor.ClientCancelWorkflowInput]
546-
//
547-
// Exposed as: [go.temporal.io/sdk/interceptor.ClientCancelWorkflowInput]
548508
type ClientCancelWorkflowInput struct {
549509
WorkflowID string
550510
RunID string
@@ -554,8 +514,6 @@ type ClientCancelWorkflowInput struct {
554514
// ClientOutboundInterceptor.TerminateWorkflow.
555515
//
556516
// Exposed as: [go.temporal.io/sdk/interceptor.ClientTerminateWorkflowInput]
557-
//
558-
// Exposed as: [go.temporal.io/sdk/interceptor.ClientTerminateWorkflowInput]
559517
type ClientTerminateWorkflowInput struct {
560518
WorkflowID string
561519
RunID string
@@ -567,8 +525,6 @@ type ClientTerminateWorkflowInput struct {
567525
// ClientOutboundInterceptor.QueryWorkflow.
568526
//
569527
// Exposed as: [go.temporal.io/sdk/interceptor.ClientQueryWorkflowInput]
570-
//
571-
// Exposed as: [go.temporal.io/sdk/interceptor.ClientQueryWorkflowInput]
572528
type ClientQueryWorkflowInput struct {
573529
WorkflowID string
574530
RunID string

‎internal/nexus_operations.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,12 @@ import (
4242

4343
// NexusOperationContext is an internal only struct that holds fields used by the temporalnexus functions.
4444
type NexusOperationContext struct {
45-
Client Client
46-
Namespace string
47-
TaskQueue string
48-
MetricsHandler metrics.Handler
49-
Log log.Logger
50-
registry *registry
45+
Client Client
46+
Namespace string
47+
TaskQueue string
48+
MetricsHandler metrics.Handler
49+
Log log.Logger
50+
registry *registry
5151
}
5252

5353
func (nc *NexusOperationContext) ResolveWorkflowName(wf any) (string, error) {

0 commit comments

Comments
 (0)