Skip to content

Commit ac66121

Browse files
authored
refactor(sidekick): use string for PathSegment.Literal (#5871)
This change converts the Literal field in the PathSegment struct from `*string` to just `string`. This simplifies object construction and eliminates the need for safe pointer handling (nil checks and dereferences) throughout the codebase.
1 parent dfc3348 commit ac66121

19 files changed

Lines changed: 65 additions & 77 deletions

internal/sidekick/api/model.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -741,8 +741,8 @@ func (template *PathTemplate) FlatPath() string {
741741
sep := ""
742742
for _, segment := range template.Segments {
743743
buffer.WriteString(sep)
744-
if segment.Literal != nil {
745-
buffer.WriteString(*segment.Literal)
744+
if segment.Literal != "" {
745+
buffer.WriteString(segment.Literal)
746746
} else if segment.Variable != nil {
747747
fmt.Fprintf(&buffer, "{%s}", strings.Join(segment.Variable.FieldPath, "."))
748748
}
@@ -753,7 +753,7 @@ func (template *PathTemplate) FlatPath() string {
753753

754754
// PathSegment is a segment of a path.
755755
type PathSegment struct {
756-
Literal *string
756+
Literal string
757757
Variable *PathVariable
758758
}
759759

@@ -773,7 +773,7 @@ func NewPathVariable(fields ...string) *PathVariable {
773773

774774
// WithLiteral adds a literal to the path template.
775775
func (p *PathTemplate) WithLiteral(l string) *PathTemplate {
776-
p.Segments = append(p.Segments, PathSegment{Literal: &l})
776+
p.Segments = append(p.Segments, PathSegment{Literal: l})
777777
return p
778778
}
779779

@@ -822,7 +822,7 @@ func (v *PathVariable) WithAllowReserved() *PathVariable {
822822

823823
// WithLiteral adds a literal to the path segment.
824824
func (s *PathSegment) WithLiteral(l string) *PathSegment {
825-
s.Literal = &l
825+
s.Literal = l
826826
return s
827827
}
828828

internal/sidekick/api/model_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,11 @@ func TestPathTemplateBuilder(t *testing.T) {
206206
WithMatchRecursive()).
207207
WithVariableNamed("v2", "field").
208208
WithVerb("verb")
209-
name := "v1"
210209
verb := "verb"
211210
want := &PathTemplate{
212211
Segments: []PathSegment{
213212
{
214-
Literal: &name,
213+
Literal: "v1",
215214
},
216215
{
217216
Variable: &PathVariable{

internal/sidekick/api/resource_identification.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,11 @@ func identifyHeuristicTarget(method *Method, binding *PathBinding, vocabulary ma
8585
// Iterate backwards over segments
8686
for i := len(tmpl.Segments) - 1; i >= 0; i-- {
8787
seg := tmpl.Segments[i]
88-
if seg.Variable == nil || i == 0 || tmpl.Segments[i-1].Literal == nil {
88+
if seg.Variable == nil || i == 0 || tmpl.Segments[i-1].Literal == "" {
8989
continue
9090
}
9191

92-
token := *tmpl.Segments[i-1].Literal
92+
token := tmpl.Segments[i-1].Literal
9393
if !vocabulary[token] && !isVersionString(token) {
9494
continue // continue scanning backward if not in vocabulary
9595
}
@@ -105,8 +105,8 @@ func identifyHeuristicTarget(method *Method, binding *PathBinding, vocabulary ma
105105
prevSeg := tmpl.Segments[firstIndex-1]
106106

107107
// Case 1: The preceding segment is a standalone literal (e.g., "global")
108-
if prevSeg.Literal != nil {
109-
if isVersionString(*prevSeg.Literal) {
108+
if prevSeg.Literal != "" {
109+
if isVersionString(prevSeg.Literal) {
110110
break // Stop at version strings (e.g., "v1")
111111
}
112112
firstIndex--
@@ -115,11 +115,11 @@ func identifyHeuristicTarget(method *Method, binding *PathBinding, vocabulary ma
115115

116116
// Case 2: The preceding segment is a variable. It must be paired with a literal.
117117
if prevSeg.Variable != nil {
118-
if firstIndex < 2 || tmpl.Segments[firstIndex-2].Literal == nil {
118+
if firstIndex < 2 || tmpl.Segments[firstIndex-2].Literal == "" {
119119
break // Variable near the beginning, cannot form a pair
120120
}
121121

122-
prevLiteralVal := *tmpl.Segments[firstIndex-2].Literal
122+
prevLiteralVal := tmpl.Segments[firstIndex-2].Literal
123123

124124
// If the literal is a known collection, include the pair and continue
125125
if vocabulary[prevLiteralVal] {
@@ -257,15 +257,15 @@ func constructTemplate(method *Method, segments []PathSegment) ([]PathSegment, e
257257

258258
var result []PathSegment
259259
h := "//" + host
260-
result = append(result, PathSegment{Literal: &h})
260+
result = append(result, PathSegment{Literal: h})
261261

262262
for _, seg := range segments {
263-
if seg.Literal != nil {
264-
l := *seg.Literal
263+
if seg.Literal != "" {
264+
l := seg.Literal
265265
if isVersionString(l) {
266266
continue
267267
}
268-
result = append(result, PathSegment{Literal: &l})
268+
result = append(result, PathSegment{Literal: l})
269269
} else if seg.Variable != nil {
270270
result = append(result, PathSegment{Variable: &PathVariable{
271271
FieldPath: seg.Variable.FieldPath,

internal/sidekick/api/resource_name_heuristic.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ func BuildHeuristicVocabulary(model *API) map[string]bool {
8888
for i := len(tmpl.Segments) - 1; i >= 0; i-- {
8989
seg := tmpl.Segments[i]
9090
if seg.Variable != nil {
91-
if i > 0 && tmpl.Segments[i-1].Literal != nil {
92-
token := *tmpl.Segments[i-1].Literal
91+
if i > 0 && tmpl.Segments[i-1].Literal != "" {
92+
token := tmpl.Segments[i-1].Literal
9393
// Do not add API version strings (e.g., v1, v1beta1) to the vocabulary
9494
if isVersionString(token) {
9595
continue

internal/sidekick/api/test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -284,15 +284,14 @@ func ParseTemplateForTest(template string) []PathSegment {
284284
if strings.HasPrefix(template, "//") {
285285
host = "//" + parts[0]
286286
}
287-
segments = append(segments, PathSegment{Literal: &host})
287+
segments = append(segments, PathSegment{Literal: host})
288288

289289
for _, part := range parts[1:] {
290290
if strings.HasPrefix(part, "{") && strings.HasSuffix(part, "}") {
291291
fieldPath := strings.Split(part[1:len(part)-1], ".")
292292
segments = append(segments, PathSegment{Variable: &PathVariable{FieldPath: fieldPath}})
293293
} else {
294-
l := part
295-
segments = append(segments, PathSegment{Literal: &l})
294+
segments = append(segments, PathSegment{Literal: part})
296295
}
297296
}
298297
return segments

internal/sidekick/api/xref.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -411,12 +411,11 @@ func toResourceNamePattern(pattern ResourcePattern, skipLast bool) *ResourceName
411411
var segments []ResourceNameSegment
412412
for i := 0; i < len(pattern); i++ {
413413
s := pattern[i]
414-
if s.Literal != nil && strings.HasPrefix(*s.Literal, "//") {
414+
if strings.HasPrefix(s.Literal, "//") {
415415
continue
416416
}
417-
seg := ResourceNameSegment{}
418-
if s.Literal != nil {
419-
seg.Literal = *s.Literal
417+
seg := ResourceNameSegment{
418+
Literal: s.Literal,
420419
}
421420
if s.Variable != nil && len(s.Variable.FieldPath) > 0 {
422421
// The parser used for parsing resource name patterns is the same used for
@@ -429,7 +428,7 @@ func toResourceNamePattern(pattern ResourcePattern, skipLast bool) *ResourceName
429428
}
430429

431430
// Try to combine with next segment if this is a literal and next is variable
432-
if s.Literal != nil && i+1 < len(pattern) && pattern[i+1].Variable != nil {
431+
if s.Literal != "" && i+1 < len(pattern) && pattern[i+1].Variable != nil {
433432
if len(pattern[i+1].Variable.FieldPath) > 0 {
434433
seg.Variable = pattern[i+1].Variable.FieldPath[0]
435434
}

internal/sidekick/dart/dart.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,9 @@ func httpPathFmt(pathInfo *api.PathInfo) string {
170170
t := pathInfo.Bindings[0].PathTemplate
171171
for _, segment := range t.Segments {
172172
switch {
173-
case segment.Literal != nil:
173+
case segment.Literal != "":
174174
builder.WriteString("/")
175-
builder.WriteString(*segment.Literal)
175+
builder.WriteString(segment.Literal)
176176
case segment.Variable != nil:
177177
// Form '${request.foo!.bar!.baz}'.
178178
builder.WriteString("/${request")

internal/sidekick/gcloud/path.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ func pathFormatFromSegments(segments []api.PathSegment) string {
108108
var parts []string
109109
for _, seg := range segments {
110110
switch {
111-
case seg.Literal != nil:
112-
parts = append(parts, *seg.Literal)
111+
case seg.Literal != "":
112+
parts = append(parts, seg.Literal)
113113
case seg.Variable != nil && len(seg.Variable.FieldPath) > 0:
114114
parts = append(parts, "%s")
115115
hasVar = true

internal/sidekick/parser/discovery/uritemplate.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func ParseUriTemplate(uriTemplate string) (*api.PathTemplate, error) {
7070
return nil, err
7171
}
7272
pos += width
73-
template.Verb = literal.Literal
73+
template.Verb = &literal.Literal
7474
}
7575
if pos != len(uriTemplate) {
7676
return nil, fmt.Errorf("trailing data (%q) cannot be parsed as a URI template", uriTemplate[pos:])
@@ -150,5 +150,5 @@ func parseLiteral(input string) (*api.PathSegment, int, error) {
150150
if tail != "" && tail[0] != slash {
151151
return nil, index, fmt.Errorf("found unexpected character %v in literal %q, stopped at position %v", tail[0], input, index)
152152
}
153-
return &api.PathSegment{Literal: &literal}, width, nil
153+
return &api.PathSegment{Literal: literal}, width, nil
154154
}

internal/sidekick/parser/discovery/uritemplate_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ func TestParseLiteral(t *testing.T) {
141141
t.Errorf("expected a successful parse with input=%s, err=%v", test.input, err)
142142
continue
143143
}
144-
if diff := cmp.Diff(&api.PathSegment{Literal: &test.want}, gotSegment); diff != "" {
144+
if diff := cmp.Diff(&api.PathSegment{Literal: test.want}, gotSegment); diff != "" {
145145
t.Errorf("mismatch [%s] (-want, +got):\n%s", test.input, diff)
146146
}
147147
if len(test.want) != gotWidth {

0 commit comments

Comments
 (0)