Add GetInvalidPaths method to FieldMask for better validation feedback (Golang)

[s4s7]

Problem

The current FieldMask.IsValid(m proto.Message) bool method only returns a boolean indicating whether all paths are valid, but provides no information about which specific paths are invalid. This makes it difficult for developers to:

1. Debug invalid field masks - developers have to manually inspect each path to determine which ones are causing validation failures
2. Provide meaningful error messages - applications cannot give users specific feedback about which fields are problematic
3. Implement incremental fixes - when working with complex field masks, it's hard to identify and fix invalid paths one by one

Proposed Solution

Add a new method GetInvalidPaths(m proto.Message) []string to the FieldMask type that returns a list of invalid paths.

// GetInvalidPaths returns a slice of paths that are invalid for the given message type.
// Returns an empty slice if all paths are valid.
func (x *FieldMask) GetInvalidPaths(m proto.Message) []string

Use Cases

API Validation with Detailed Error Messages

Current Problem - No way to identify which specific paths are invalid:

req.Msg.FieldMask.Normalize()
if !req.Msg.FieldMask.IsValid(req.Msg) {
    // ❌ Can only return generic error - no way to tell user which paths are wrong
    return status.Error(codes.InvalidArgument, "invalid field mask")
}

With GetInvalidPaths - Provide specific feedback:

req.Msg.FieldMask.Normalize()
if invalidPaths := req.Msg.FieldMask.GetInvalidPaths(req.Msg); len(invalidPaths) > 0 {
    // ✅ Tell user exactly which paths are problematic
    return status.Errorf(codes.InvalidArgument, 
        "invalid field mask paths: %v", invalidPaths)
    // Error: "invalid field mask paths: [user.invalid_field, metadata.bad_path]"
}

Implementation

The implementation would reuse the existing numValidPaths function internally:

func (x *FieldMask) GetInvalidPaths(m proto.Message) []string {
    if x == nil {
        return nil
    }
    
    paths := x.GetPaths()
    numValid := numValidPaths(m, paths)
    
    if numValid == len(paths) {
        return nil // all paths are valid
    }
    
    // Return the invalid paths (from numValid index onwards)
    invalidPaths := make([]string, len(paths)-numValid)
    copy(invalidPaths, paths[numValid:])
    return invalidPaths
}

Benefits

1. Zero Breaking Changes - This is a pure addition that doesn't modify existing behavior
2. Consistent with Existing Patterns - Follows the same validation logic as IsValid and Append
3. Minimal Performance Impact - Reuses existing validation logic, no additional reflection
4. Developer Experience - Significantly improves debugging and error reporting capabilities
5. Simple API - Clean, intuitive method signature that's easy to use

Alternatives Considered

1. Modify IsValid to return detailed information - Would break existing API
2. Add a separate validation result struct - More complex API for the common use case
3. Use Append with empty paths - Workaround that's not intuitive and doesn't provide all invalid paths

Backward Compatibility

This change is fully backward compatible as it only adds a new method without modifying existing functionality.

[puellanivis]

🤔 Does numValidPaths reorder the paths input? It seems it just does return i when it finds an invalid path.

If it doesn’t reorder the paths, then I don’t think the logic of your proposed implementation would actually hold? In that, it could return additional paths that are valid, but since they’re after an invalid path, you end up reporting them as if they were invalid.

P.S.: And if it does reorder the paths, then that would be quite the astonishment.

[stapelberg]

(FWIW, this issue reads a lot like it was generated by an LLM, so I won’t spend time on this until the author replies in their own human voice to @puellanivis’s question.)

[s4s7]

Thank you for your feedback!

Here's the corrected implementation that properly checks each path individually.

func (x *FieldMask) GetInvalidPaths(m proto.Message) []string {
    if x == nil {
        return nil
    }
    paths := x.GetPaths()
    invalidPaths := make([]string, 0, len(paths))
    for _, path := range paths {
        if numValidPaths(m, []string{path}) == 0 {
            invalidPaths = append(invalidPaths, path)
        }
    }
    return invalidPaths
}

However, this approach has performance implications due to nested loops and multiple descriptor retrievals.
Here's a more efficient version.

func (x *FieldMask) GetInvalidPaths(m proto.Message) []string {
	if x == nil {
		return nil
	}
	paths := x.GetPaths()
	if len(paths) == 0 {
		return nil
	}

	md0 := m.ProtoReflect().Descriptor()
        invalidPaths := make([]string, 0, len(paths))
	for _, path := range paths {
		md := md0
		isValid := rangeFields(path, func(field string) bool {

                        // ----------------- same logic as numValidPaths -----------------

			// Search the field within the message.
			if md == nil {
				return false // not within a message
			}
			fd := md.Fields().ByName(protoreflect.Name(field))
			// The real field name of a group is the message name.
			if fd == nil {
				gd := md.Fields().ByName(protoreflect.Name(strings.ToLower(field)))
				if gd != nil && gd.Kind() == protoreflect.GroupKind && string(gd.Message().Name()) == field {
					fd = gd
				}
			} else if fd.Kind() == protoreflect.GroupKind && string(fd.Message().Name()) != field {
				fd = nil
			}
			if fd == nil {
				return false // message does not have this field
			}
			// Identify the next message to search within.
			md = fd.Message() // may be nil
			// Repeated fields are only allowed at the last position.
			if fd.IsList() || fd.IsMap() {
				md = nil
			}
			return true

                        // ------------------------------------------------------------------ 


		})
		if !isValid {
			invalidPaths = append(invalidPaths, path)
		}
	}
	return invalidPaths
}

I can certainly refactor and add test code.
What do you think of this approach?