decoder.go (3)

83-93: Possible collision when inserting file paths into src.
Currently, the code overwrites entries in src with an empty slice whenever a key from multipartFiles already exists in src. This has potential to discard existing data if there's a parameter collision.

 for path := range multipartFiles {
-    src[path] = []string{""}
+    if _, exists := src[path]; !exists {
+        src[path] = []string{""}
+    }
 }

---

104-104: Remove or replace debug print.
fmt.Println(parts) appears to be a leftover debugging statement. Consider removing or using structured logging.

- fmt.Println(parts)
+ // fmt.Println(parts) // remove or replace with logger if needed

---

105-115: Discourage printing errors directly.
Using fmt.Print(err) in production code could clutter output. Prefer proper logging or returning the error in a structured way.

} else if !d.ignoreUnknownKeys {
-    fmt.Print(err)
+    // log.Printf("parsePath error: %v", err)
    errors[path] = UnknownKeyError{Key: path}
}

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 107-108: decoder.go#L107-L108
Added lines #L107 - L108 were not covered by tests

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

decoder_test.go (3)

2844-2898: Improve error handling in benchmark.

Move the error check inside the benchmark loop to catch errors in intermediate iterations.

 for i := 0; i < b.N; i++ {
-    err = decoder.Decode(&s, data, fileHeaders)
+    if err := decoder.Decode(&s, data, fileHeaders); err != nil {
+        b.Fatalf("Failed to decode at iteration %d: %v", i, err)
+    }
 }
-
-if err != nil {
-    b.Fatalf("Failed to decode: %v", err)
-}

---

2900-2943: Add descriptive names to test cases.

Consider adding descriptive names to test cases to improve readability and debugging.

 tc := []struct {
+    name     string
     typ      reflect.Type
     input    map[string][]string
     expected bool
 }{
     {
+        name:     "plain string type",
         typ:      reflect.TypeOf(string("")),
         expected: false,
     },
     {
+        name:     "string slice type",
         typ:      reflect.TypeOf([]string{}),
         expected: false,
     },
     // ... rest of test cases
 }

-for _, tt := range tc {
+for _, tt := range tc {
+    t.Run(tt.name, func(t *testing.T) {
         if isMultipartField(tt.typ) != tt.expected {
             t.Errorf("Expected %v, got %v", tt.expected, isMultipartField(tt.typ))
         }
+    })
 }

---

2981-3055: Restructure test into sub-tests for better organization.

Consider using sub-tests to group related test cases and improve test output.

 func TestHandleMultipartField(t *testing.T) {
     t.Parallel()

     // ... setup code ...

-    ok := handleMultipartField(rv.FieldByName("F"), files["f"])
-    if !ok {
-        t.Error("Expected handleMultipartField to return true")
-    }
-    // ... more tests ...

+    tests := []struct {
+        name     string
+        field    string
+        wantOk   bool
+        validate func(*testing.T, S)
+    }{
+        {
+            name:   "single file pointer",
+            field:  "F",
+            wantOk: true,
+            validate: func(t *testing.T, s S) {
+                if s.F == nil {
+                    t.Error("Expected F to be a file header, got nil")
+                }
+                if s.F.Filename != "test.txt" {
+                    t.Errorf("Expected F.Filename to be 'test.txt', got %s", s.F.Filename)
+                }
+            },
+        },
+        // ... more test cases
+    }
+
+    for _, tt := range tests {
+        t.Run(tt.name, func(t *testing.T) {
+            s := S{}
+            rv := reflect.ValueOf(&s).Elem()
+            if got := handleMultipartField(rv.FieldByName(tt.field), files["f"]); got != tt.wantOk {
+                t.Errorf("handleMultipartField() = %v, want %v", got, tt.wantOk)
+            }
+            tt.validate(t, s)
+        })
+    }

decoder.go (3)

304-307: Consider using const declarations for type safety.

The type declarations could be made more type-safe using const declarations.

-var (
+const (
     multipartFileHeaderPointerType      = reflect.TypeOf(&multipart.FileHeader{})
     sliceMultipartFileHeaderPointerType = reflect.TypeOf([]*multipart.FileHeader{})
-)

---

311-349: Simplify function structure with early returns.

The function could be simplified by using early returns to reduce nesting.

 func handleMultipartField(field reflect.Value, files []*multipart.FileHeader) bool {
     fieldType := field.Type()
     if !isMultipartField(fieldType) {
         return false
     }

     // Skip if files are empty and field is multipart
     if len(files) == 0 {
         return true
     }

     // Check for *multipart.FileHeader
     if fieldType == multipartFileHeaderPointerType {
         field.Set(reflect.ValueOf(files[0]))
         return true
     }

     // Check for []*multipart.FileHeader
     if fieldType == sliceMultipartFileHeaderPointerType {
         field.Set(reflect.ValueOf(files))
         return true
     }

     // Check for *[]*multipart.FileHeader
     if fieldType.Kind() != reflect.Pointer {
         return false
     }
     
     fieldType = fieldType.Elem()
     if fieldType != sliceMultipartFileHeaderPointerType {
         return false
     }

     if field.IsNil() {
         field.Set(reflect.New(fieldType))
     }
     
     field.Elem().Set(reflect.ValueOf(files))
     return true
 }

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 348-348: decoder.go#L348
Added line #L348 was not covered by tests

---

83-93: Add input validation for files parameter.

Consider validating the files parameter to ensure consistent behavior.

 func (d *Decoder) Decode(dst interface{}, src map[string][]string, files ...map[string][]*multipart.FileHeader) error {
+    if src == nil {
+        return errors.New("schema: source map is nil")
+    }
+
     var multipartFiles map[string][]*multipart.FileHeader
 
     if len(files) > 0 {
+        if files[0] == nil {
+            return errors.New("schema: files map is nil")
+        }
         multipartFiles = files[0]
     }
 
     // Add files as empty string values to src in order to make path parsing work easily
     for path := range multipartFiles {
+        if path == "" {
+            return errors.New("schema: empty path in files map")
+        }
         src[path] = []string{""}
     }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro