detached 5b5bc74

Apply Task 1 review fixes: error wrapping, validation, tests, go.mod fix, package rename

commit
5b5bc74aaa9e67e0c376dd4786e214d26acc9af6
author
brice · · 2026-04-09 05:12 UTC
parent
c274f18
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
6 files · +93 -5
Unified
UnifiedSplit
M cmd/pipewiregen/internal/parser/parser.go +44 -1
@@ -2,6 +2,7 @@ package parser
22
33 import (
44 "encoding/json"
5+ "fmt"
56 "os"
67
78 "github.com/bnema/purego-pipewire/cmd/pipewiregen/internal/model"
@@ -11,11 +12,53 @@ import (
1112 func Load(path string) (*model.Model, error) {
1213 b, err := os.ReadFile(path)
1314 if err != nil {
14- return nil, err
15+ return nil, fmt.Errorf("failed to read file %s: %w", path, err)
1516 }
1617 var m model.Model
1718 if err := json.Unmarshal(b, &m); err != nil {
19+ return nil, fmt.Errorf("failed to unmarshal JSON from %s: %w", path, err)
20+ }
21+ if err := validate(&m); err != nil {
1822 return nil, err
1923 }
2024 return &m, nil
2125 }
26+
27+func validate(m *model.Model) error {
28+ // Build set of valid library names
29+ libNames := make(map[string]bool)
30+ for i, lib := range m.Libraries {
31+ if lib.Name == "" {
32+ return fmt.Errorf("library at index %d has empty name", i)
33+ }
34+ if lib.SOName == "" {
35+ return fmt.Errorf("library %q has empty soname", lib.Name)
36+ }
37+ libNames[lib.Name] = true
38+ }
39+
40+ // Validate groups
41+ for i, group := range m.Groups {
42+ if group.Name == "" {
43+ return fmt.Errorf("group at index %d has empty name", i)
44+ }
45+ }
46+
47+ // Validate symbols
48+ for i, sym := range m.Symbols {
49+ if sym.Name == "" {
50+ return fmt.Errorf("symbol at index %d has empty name", i)
51+ }
52+ if sym.Library == "" {
53+ return fmt.Errorf("symbol %q has empty library", sym.Name)
54+ }
55+ if sym.Signature == "" {
56+ return fmt.Errorf("symbol %q has empty signature", sym.Name)
57+ }
58+ if !libNames[sym.Library] {
59+ return fmt.Errorf("symbol %q references unknown library %q", sym.Name, sym.Library)
60+ }
61+ }
62+
63+ return nil
64+}
@@ -2,6 +2,7 @@ package parser
2 2
3 import (3 import (
4 "encoding/json"4 "encoding/json"
5+ "fmt"
5 "os"6 "os"
6 7
7 "github.com/bnema/purego-pipewire/cmd/pipewiregen/internal/model"8 "github.com/bnema/purego-pipewire/cmd/pipewiregen/internal/model"
@@ -11,11 +12,53 @@ import (
11 func Load(path string) (*model.Model, error) {12 func Load(path string) (*model.Model, error) {
12 b, err := os.ReadFile(path)13 b, err := os.ReadFile(path)
13 if err != nil {14 if err != nil {
14- return nil, err15+ return nil, fmt.Errorf("failed to read file %s: %w", path, err)
15 }16 }
16 var m model.Model17 var m model.Model
17 if err := json.Unmarshal(b, &m); err != nil {18 if err := json.Unmarshal(b, &m); err != nil {
19+ return nil, fmt.Errorf("failed to unmarshal JSON from %s: %w", path, err)
20+ }
21+ if err := validate(&m); err != nil {
18 return nil, err22 return nil, err
19 }23 }
20 return &m, nil24 return &m, nil
21 }25 }
26+
27+func validate(m *model.Model) error {
28+ // Build set of valid library names
29+ libNames := make(map[string]bool)
30+ for i, lib := range m.Libraries {
31+ if lib.Name == "" {
32+ return fmt.Errorf("library at index %d has empty name", i)
33+ }
34+ if lib.SOName == "" {
35+ return fmt.Errorf("library %q has empty soname", lib.Name)
36+ }
37+ libNames[lib.Name] = true
38+ }
39+
40+ // Validate groups
41+ for i, group := range m.Groups {
42+ if group.Name == "" {
43+ return fmt.Errorf("group at index %d has empty name", i)
44+ }
45+ }
46+
47+ // Validate symbols
48+ for i, sym := range m.Symbols {
49+ if sym.Name == "" {
50+ return fmt.Errorf("symbol at index %d has empty name", i)
51+ }
52+ if sym.Library == "" {
53+ return fmt.Errorf("symbol %q has empty library", sym.Name)
54+ }
55+ if sym.Signature == "" {
56+ return fmt.Errorf("symbol %q has empty signature", sym.Name)
57+ }
58+ if !libNames[sym.Library] {
59+ return fmt.Errorf("symbol %q references unknown library %q", sym.Name, sym.Library)
60+ }
61+ }
62+
63+ return nil
64+}
M cmd/pipewiregen/internal/parser/parser_test.go +34 -1
@@ -1,6 +1,9 @@
11 package parser
22
3-import "testing"
3+import (
4+ "strings"
5+ "testing"
6+)
47
58 func TestLoadParsesLibraryAndSymbolGroups(t *testing.T) {
69 model, err := Load("testdata/minimal_pipewire.json")
@@ -14,3 +17,33 @@ func TestLoadParsesLibraryAndSymbolGroups(t *testing.T) {
1417 t.Fatalf("group=%q want %q", got, want)
1518 }
1619 }
20+
21+func TestLoadMissingFileReturnsError(t *testing.T) {
22+ _, err := Load("testdata/nonexistent_file.json")
23+ if err == nil {
24+ t.Fatal("expected error for missing file, got nil")
25+ }
26+ if !strings.Contains(err.Error(), "failed to read file") {
27+ t.Fatalf("error message should contain 'failed to read file', got: %v", err)
28+ }
29+}
30+
31+func TestLoadInvalidJSONReturnsError(t *testing.T) {
32+ _, err := Load("testdata/invalid.json")
33+ if err == nil {
34+ t.Fatal("expected error for invalid JSON, got nil")
35+ }
36+ if !strings.Contains(err.Error(), "failed to unmarshal JSON") {
37+ t.Fatalf("error message should contain 'failed to unmarshal JSON', got: %v", err)
38+ }
39+}
40+
41+func TestLoadSymbolWithUnknownLibraryReturnsError(t *testing.T) {
42+ _, err := Load("testdata/unknown_library.json")
43+ if err == nil {
44+ t.Fatal("expected error for symbol with unknown library, got nil")
45+ }
46+ if !strings.Contains(err.Error(), "unknown library") {
47+ t.Fatalf("error message should contain 'unknown library', got: %v", err)
48+ }
49+}
@@ -1,6 +1,9 @@
1 package parser1 package parser
2 2
3-import "testing"3+import (
4+ "strings"
5+ "testing"
6+)
4 7
5 func TestLoadParsesLibraryAndSymbolGroups(t *testing.T) {8 func TestLoadParsesLibraryAndSymbolGroups(t *testing.T) {
6 model, err := Load("testdata/minimal_pipewire.json")9 model, err := Load("testdata/minimal_pipewire.json")
@@ -14,3 +17,33 @@ func TestLoadParsesLibraryAndSymbolGroups(t *testing.T) {
14 t.Fatalf("group=%q want %q", got, want)17 t.Fatalf("group=%q want %q", got, want)
15 }18 }
16 }19 }
20+
21+func TestLoadMissingFileReturnsError(t *testing.T) {
22+ _, err := Load("testdata/nonexistent_file.json")
23+ if err == nil {
24+ t.Fatal("expected error for missing file, got nil")
25+ }
26+ if !strings.Contains(err.Error(), "failed to read file") {
27+ t.Fatalf("error message should contain 'failed to read file', got: %v", err)
28+ }
29+}
30+
31+func TestLoadInvalidJSONReturnsError(t *testing.T) {
32+ _, err := Load("testdata/invalid.json")
33+ if err == nil {
34+ t.Fatal("expected error for invalid JSON, got nil")
35+ }
36+ if !strings.Contains(err.Error(), "failed to unmarshal JSON") {
37+ t.Fatalf("error message should contain 'failed to unmarshal JSON', got: %v", err)
38+ }
39+}
40+
41+func TestLoadSymbolWithUnknownLibraryReturnsError(t *testing.T) {
42+ _, err := Load("testdata/unknown_library.json")
43+ if err == nil {
44+ t.Fatal("expected error for symbol with unknown library, got nil")
45+ }
46+ if !strings.Contains(err.Error(), "unknown library") {
47+ t.Fatalf("error message should contain 'unknown library', got: %v", err)
48+ }
49+}
A cmd/pipewiregen/internal/parser/testdata/invalid.json +1 -0
@@ -0,0 +1,1 @@
1+{ this is not valid json }⏎̸
@@ -0,0 +1,1 @@
1+{ this is not valid json }⏎̸
A cmd/pipewiregen/internal/parser/testdata/unknown_library.json +11 -0
@@ -0,0 +1,11 @@
1+{
2+ "libraries": [
3+ {"name": "pipewire", "soname": "libpipewire-0.3.so.0"}
4+ ],
5+ "groups": [
6+ {"name": "init", "symbols": ["pw_init"]}
7+ ],
8+ "symbols": [
9+ {"name": "pw_init", "library": "nonexistent", "signature": "func()"}
10+ ]
11+}
@@ -0,0 +1,11 @@
1+{
2+ "libraries": [
3+ {"name": "pipewire", "soname": "libpipewire-0.3.so.0"}
4+ ],
5+ "groups": [
6+ {"name": "init", "symbols": ["pw_init"]}
7+ ],
8+ "symbols": [
9+ {"name": "pw_init", "library": "nonexistent", "signature": "func()"}
10+ ]
11+}
M cmd/pipewiregen/internal/pipewiregen/normalize.go +2 -2
@@ -1,3 +1,3 @@
1-// Package pipewiregen contains PipeWire-specific naming and grouping rules
1+// Package normalize contains PipeWire-specific naming and grouping rules
22 // for the code generator.
3-package pipewiregen
3+package normalize
@@ -1,3 +1,3 @@
1-// Package pipewiregen contains PipeWire-specific naming and grouping rules1+// Package normalize contains PipeWire-specific naming and grouping rules
2 // for the code generator.2 // for the code generator.
3-package pipewiregen3+package normalize
M go.mod +1 -1
@@ -1,3 +1,3 @@
11 module github.com/bnema/purego-pipewire
22
3-go 1.24.0
3+go 1.23
@@ -1,3 +1,3 @@
1 module github.com/bnema/purego-pipewire1 module github.com/bnema/purego-pipewire
2 2
3-go 1.24.03+go 1.23

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 1

Tags 8

↑↓ moveenter openesc close