Skip to content

Commit a01fa97

Browse files
committed
Address copilot review comments
Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech>
1 parent 1d381f2 commit a01fa97

5 files changed

Lines changed: 52 additions & 29 deletions

File tree

go/fn/internal/docs/render.go

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,29 +40,42 @@ type DocOutput struct {
4040
// RenderHelp writes formatted help text to w.
4141
// If sections are empty and metadata is zero-value, writes a minimal
4242
// "no documentation available" message.
43-
func RenderHelp(w io.Writer, sections Sections, meta Metadata) {
43+
// Returns the first write error encountered.
44+
func RenderHelp(w io.Writer, sections Sections, meta Metadata) error {
4445
if sections.Short == "" && sections.Long == "" && sections.Examples == "" && isMetadataEmpty(meta) {
45-
_, _ = fmt.Fprint(w, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n")
46-
return
46+
_, err := fmt.Fprint(w, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n")
47+
return err
4748
}
4849

4950
if sections.Short != "" {
50-
_, _ = fmt.Fprintf(w, "%s\n", sections.Short)
51+
if _, err := fmt.Fprintf(w, "%s\n", sections.Short); err != nil {
52+
return err
53+
}
5154
}
5255

5356
if sections.Long != "" {
5457
if sections.Short != "" {
55-
_, _ = fmt.Fprint(w, "\n")
58+
if _, err := fmt.Fprint(w, "\n"); err != nil {
59+
return err
60+
}
61+
}
62+
if _, err := fmt.Fprintf(w, "%s\n", sections.Long); err != nil {
63+
return err
5664
}
57-
_, _ = fmt.Fprintf(w, "%s\n", sections.Long)
5865
}
5966

6067
if sections.Examples != "" {
6168
if sections.Short != "" || sections.Long != "" {
62-
_, _ = fmt.Fprint(w, "\n")
69+
if _, err := fmt.Fprint(w, "\n"); err != nil {
70+
return err
71+
}
72+
}
73+
if _, err := fmt.Fprintf(w, "Examples:\n%s\n", sections.Examples); err != nil {
74+
return err
6375
}
64-
_, _ = fmt.Fprintf(w, "Examples:\n%s\n", sections.Examples)
6576
}
77+
78+
return nil
6679
}
6780

6881
// isMetadataEmpty reports whether all fields of meta are zero-value.

go/fn/internal/docs/render_test.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,9 @@ func TestProperty4_HelpOutputExcludesCobraBoilerplate(t *testing.T) {
8484

8585
// Render help output.
8686
var buf bytes.Buffer
87-
RenderHelp(&buf, sections, meta)
87+
if err := RenderHelp(&buf, sections, meta); err != nil {
88+
t.Errorf("RenderHelp failed: %v", err)
89+
}
8890
output := buf.String()
8991

9092
// Assert that the help output does NOT contain cobra-style boilerplate.
@@ -218,7 +220,9 @@ func TestProperty3_HelpOutputContainsParsedSections(t *testing.T) {
218220

219221
// Render help output.
220222
var buf bytes.Buffer
221-
RenderHelp(&buf, sections, Metadata{})
223+
if err := RenderHelp(&buf, sections, Metadata{}); err != nil {
224+
t.Errorf("RenderHelp failed: %v", err)
225+
}
222226
output := buf.String()
223227

224228
// Assert that the help output contains each parsed section.
@@ -250,7 +254,9 @@ func TestRenderHelp_FullSectionsAndMetadata(t *testing.T) {
250254
}
251255

252256
var buf bytes.Buffer
253-
RenderHelp(&buf, sections, meta)
257+
if err := RenderHelp(&buf, sections, meta); err != nil {
258+
t.Errorf("RenderHelp failed: %v", err)
259+
}
254260
output := buf.String()
255261

256262
// Verify output contains the Short description.
@@ -283,7 +289,9 @@ func TestRenderHelp_EmptySections(t *testing.T) {
283289
meta := Metadata{}
284290

285291
var buf bytes.Buffer
286-
RenderHelp(&buf, sections, meta)
292+
if err := RenderHelp(&buf, sections, meta); err != nil {
293+
t.Errorf("RenderHelp failed: %v", err)
294+
}
287295
output := buf.String()
288296

289297
expected := "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n"

go/fn/run.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,8 @@ func AsMain(input any, opts ...Option) error {
142142
// handleHelp renders help text to STDOUT based on registered docs.
143143
func handleHelp(cfg *mainConfig) error {
144144
if cfg.readme == nil && cfg.metadata == nil {
145-
_, _ = fmt.Fprint(os.Stdout, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n")
146-
return nil
145+
_, err := fmt.Fprint(os.Stdout, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n")
146+
return err
147147
}
148148

149149
sections := docs.ParseMarkers(cfg.readme)
@@ -153,15 +153,14 @@ func handleHelp(cfg *mainConfig) error {
153153
meta = docs.Metadata{}
154154
}
155155

156-
docs.RenderHelp(os.Stdout, sections, meta)
157-
return nil
156+
return docs.RenderHelp(os.Stdout, sections, meta)
158157
}
159158

160159
// handleDoc renders JSON documentation to STDOUT based on registered docs.
161160
func handleDoc(cfg *mainConfig) error {
162161
if cfg.readme == nil && cfg.metadata == nil {
163-
_, _ = fmt.Fprint(os.Stdout, "{}")
164-
return nil
162+
_, err := fmt.Fprint(os.Stdout, "{}")
163+
return err
165164
}
166165

167166
sections := docs.ParseMarkers(cfg.readme)
@@ -184,20 +183,21 @@ func readFilesAsResourceList(paths []string) (*ResourceList, error) {
184183
FunctionConfig: NewEmptyKubeObject(),
185184
}
186185
for _, path := range paths {
187-
data, err := os.ReadFile(filepath.Clean(path))
186+
cleanPath := filepath.Clean(path)
187+
data, err := os.ReadFile(cleanPath)
188188
if err != nil {
189189
if os.IsNotExist(err) {
190-
return nil, fmt.Errorf("file not found: %s", path)
190+
return nil, fmt.Errorf("file not found: %s", cleanPath)
191191
}
192-
return nil, fmt.Errorf("failed to read file %s: %v", path, err)
192+
return nil, fmt.Errorf("failed to read file %s: %v", cleanPath, err)
193193
}
194194
// Empty files are valid — proceed with no items from this file.
195195
if len(strings.TrimSpace(string(data))) == 0 {
196196
continue
197197
}
198198
objects, err := ParseKubeObjects(data)
199199
if err != nil {
200-
return nil, fmt.Errorf("failed to parse KRM resources from %s: %v", path, err)
200+
return nil, fmt.Errorf("failed to parse KRM resources from %s: %v", cleanPath, err)
201201
}
202202
for _, obj := range objects {
203203
rl.Items = append(rl.Items, obj)

go/fn/run_filemode_property_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ func genKRMResource() *rapid.Generator[string] {
3838
for i := range numEntries {
3939
key := rapid.StringMatching(`[a-z][a-z0-9]{1,8}`).Draw(t, fmt.Sprintf("key%d", i))
4040
value := rapid.StringMatching(`[a-zA-Z0-9]{1,15}`).Draw(t, fmt.Sprintf("value%d", i))
41-
fmt.Fprintf(&dataLines, " %s: %s\n", key, value)
41+
if _, err := fmt.Fprintf(&dataLines, " %s: %s\n", key, value); err != nil {
42+
t.Fatalf("failed to write data line: %v", err)
43+
}
4244
}
4345
return fmt.Sprintf(`apiVersion: v1
4446
kind: ConfigMap

go/fn/run_flags_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,13 @@ func captureStdout(t *testing.T, fn func()) string {
4444

4545
fn()
4646

47-
_ = w.Close()
47+
require.NoError(t, w.Close())
4848
os.Stdout = origStdout
4949

5050
var buf bytes.Buffer
5151
_, err = io.Copy(&buf, r)
5252
require.NoError(t, err)
53-
_ = r.Close()
53+
require.NoError(t, r.Close())
5454

5555
return buf.String()
5656
}
@@ -66,13 +66,13 @@ func captureStderr(t *testing.T, fn func()) string {
6666

6767
fn()
6868

69-
_ = w.Close()
69+
require.NoError(t, w.Close())
7070
os.Stderr = origStderr
7171

7272
var buf bytes.Buffer
7373
_, err = io.Copy(&buf, r)
7474
require.NoError(t, err)
75-
_ = r.Close()
75+
require.NoError(t, r.Close())
7676

7777
return buf.String()
7878
}
@@ -96,11 +96,11 @@ func TestAsMain_HelpFlag_ExitsZero(t *testing.T) {
9696
origStdin := os.Stdin
9797
r, w, err := os.Pipe()
9898
require.NoError(t, err)
99-
_ = w.Close() // Close write end immediately — reading would get EOF
99+
require.NoError(t, w.Close()) // Close write end immediately — reading would get EOF
100100
os.Stdin = r
101101
t.Cleanup(func() {
102102
os.Stdin = origStdin
103-
_ = r.Close()
103+
require.NoError(t, r.Close())
104104
})
105105

106106
output := captureStdout(t, func() {

0 commit comments

Comments
 (0)