Skip to content

Commit d9f311d

Browse files
committed
Detect markers separated by blank lines from type declarations
- Modified marker analyzer to check the previous comment group - Added extractOrphanedMarkers helper function - Added test cases for List types and non-List types with blank lines Signed-off-by: Sascha Grunert <[email protected]>
1 parent f87b38e commit d9f311d

File tree

3 files changed

+135
-2
lines changed

3 files changed

+135
-2
lines changed

pkg/analysis/helpers/markers/analyzer.go

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ import (
4545
// is "k8s:item(one: "value", two: "value")=...".
4646
const UnnamedArgument = ""
4747

48+
// maxMarkerSeparationLines is the maximum number of lines that can separate
49+
// a marker comment group from the godoc comment for it to still be considered
50+
// associated with the type declaration.
51+
const maxMarkerSeparationLines = 3
52+
4853
// Markers allows access to markers extracted from the
4954
// go types.
5055
type Markers interface {
@@ -154,7 +159,16 @@ func run(pass *analysis.Pass) (any, error) {
154159
inspect.Preorder(nodeFilter, func(n ast.Node) {
155160
switch typ := n.(type) {
156161
case *ast.GenDecl:
157-
extractGenDeclMarkers(typ, results)
162+
// Find the file this declaration belongs to
163+
var file *ast.File
164+
for _, f := range pass.Files {
165+
if f.Pos() <= typ.Pos() && typ.End() <= f.End() {
166+
file = f
167+
break
168+
}
169+
}
170+
171+
extractGenDeclMarkers(typ, file, pass.Fset, results)
158172
case *ast.Field:
159173
extractFieldMarkers(typ, results)
160174
}
@@ -163,15 +177,20 @@ func run(pass *analysis.Pass) (any, error) {
163177
return results, nil
164178
}
165179

166-
func extractGenDeclMarkers(typ *ast.GenDecl, results *markers) {
180+
func extractGenDeclMarkers(typ *ast.GenDecl, file *ast.File, fset *token.FileSet, results *markers) {
167181
declMarkers := NewMarkerSet()
168182

183+
// Collect markers from the GenDecl's Doc field (comments directly attached to the declaration)
169184
if typ.Doc != nil {
170185
for _, comment := range typ.Doc.List {
171186
if marker := extractMarker(comment); marker.Identifier != "" {
172187
declMarkers.Insert(marker)
173188
}
174189
}
190+
191+
// Also collect markers from the comment group immediately before the godoc comment
192+
// if separated by a blank line.
193+
extractOrphanedMarkers(typ.Doc, file, fset, declMarkers)
175194
}
176195

177196
if len(typ.Specs) == 0 {
@@ -190,6 +209,55 @@ func extractGenDeclMarkers(typ *ast.GenDecl, results *markers) {
190209
}
191210
}
192211

212+
// extractOrphanedMarkers finds markers in the comment group immediately before the godoc comment
213+
// that are separated by a blank line. Only the immediately preceding comment group is checked,
214+
// and it must be within maxMarkerSeparationLines lines of the godoc comment.
215+
//
216+
// This handles cases where markers are separated from type declarations by blank lines, such as:
217+
//
218+
// // +kubebuilder:object:root=true
219+
//
220+
// // MyType does something
221+
// type MyType struct {}
222+
//
223+
// The root marker will be detected even though separated by a blank line from the godoc comment.
224+
func extractOrphanedMarkers(docGroup *ast.CommentGroup, file *ast.File, fset *token.FileSet, declMarkers MarkerSet) {
225+
if file == nil || fset == nil {
226+
return
227+
}
228+
229+
// Find the index of the godoc comment group in the file's comment list
230+
docIndex := -1
231+
232+
for i, cg := range file.Comments {
233+
if cg == docGroup {
234+
docIndex = i
235+
break
236+
}
237+
}
238+
239+
// If there's no previous comment group, we're done
240+
if docIndex <= 0 {
241+
return
242+
}
243+
244+
prevGroup := file.Comments[docIndex-1]
245+
docStartLine := fset.Position(docGroup.Pos()).Line
246+
prevEndLine := fset.Position(prevGroup.End()).Line
247+
248+
// Only consider the previous group if it's within maxMarkerSeparationLines of the godoc comment
249+
if docStartLine-prevEndLine > maxMarkerSeparationLines {
250+
return
251+
}
252+
253+
// Extract markers from the previous comment group
254+
for _, comment := range prevGroup.List {
255+
if marker := extractMarker(comment); marker.Identifier != "" {
256+
declMarkers.Insert(marker)
257+
}
258+
}
259+
}
260+
193261
func extractFieldMarkers(field *ast.Field, results *markers) {
194262
if field == nil || field.Doc == nil {
195263
return

pkg/analysis/statussubresource/testdata/src/a/a.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,20 @@ type ClusterList struct {
7070
Items []Cluster `json:"items"`
7171
}
7272

73+
// Test marker detection with blank line (second level comment bug - issue #53)
74+
// +kubebuilder:object:root=true
75+
76+
// ServiceList contains a list of Service.
77+
type ServiceList struct {
78+
TypeMeta `json:",inline"`
79+
ListMeta `json:"metadata,omitempty"`
80+
Items []Service `json:"items"`
81+
}
82+
83+
type Service struct {
84+
Name string `json:"name"`
85+
}
86+
7387
type TypeMeta struct {
7488
Kind string `json:"kind,omitempty"`
7589
APIVersion string `json:"apiVersion,omitempty"`
@@ -173,3 +187,21 @@ type ReorderedFieldsList struct {
173187
type ReorderedItem struct {
174188
Name string `json:"name"`
175189
}
190+
191+
// Test marker detection with blank line for non-List type (second level comment bug - issue #53)
192+
// This should trigger the linter because it has a status field but no status marker
193+
// +kubebuilder:object:root=true
194+
195+
// BlankLineTest demonstrates marker detection across blank lines.
196+
type BlankLineTest struct { // want "root object type \"BlankLineTest\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
197+
Spec BlankLineTestSpec `json:"spec"`
198+
Status BlankLineTestStatus `json:"status"`
199+
}
200+
201+
type BlankLineTestSpec struct {
202+
Name string `json:"name"`
203+
}
204+
205+
type BlankLineTestStatus struct {
206+
Ready bool `json:"ready"`
207+
}

pkg/analysis/statussubresource/testdata/src/a/a.go.golden

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,20 @@ type ClusterList struct {
7171
Items []Cluster `json:"items"`
7272
}
7373

74+
// Test marker detection with blank line (second level comment bug - issue #53)
75+
// +kubebuilder:object:root=true
76+
77+
// ServiceList contains a list of Service.
78+
type ServiceList struct {
79+
TypeMeta `json:",inline"`
80+
ListMeta `json:"metadata,omitempty"`
81+
Items []Service `json:"items"`
82+
}
83+
84+
type Service struct {
85+
Name string `json:"name"`
86+
}
87+
7488
type TypeMeta struct {
7589
Kind string `json:"kind,omitempty"`
7690
APIVersion string `json:"apiVersion,omitempty"`
@@ -178,3 +192,22 @@ type ReorderedFieldsList struct {
178192
type ReorderedItem struct {
179193
Name string `json:"name"`
180194
}
195+
196+
// Test marker detection with blank line for non-List type (second level comment bug - issue #53)
197+
// This should trigger the linter because it has a status field but no status marker
198+
// +kubebuilder:object:root=true
199+
200+
// BlankLineTest demonstrates marker detection across blank lines.
201+
// +kubebuilder:subresource:status
202+
type BlankLineTest struct { // want "root object type \"BlankLineTest\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
203+
Spec BlankLineTestSpec `json:"spec"`
204+
Status BlankLineTestStatus `json:"status"`
205+
}
206+
207+
type BlankLineTestSpec struct {
208+
Name string `json:"name"`
209+
}
210+
211+
type BlankLineTestStatus struct {
212+
Ready bool `json:"ready"`
213+
}

0 commit comments

Comments
 (0)