Repository: grpc/grpc-go
File: internal/xds/matcher/string_matcher.go (Go)

Code before:
		if matcherProto.GetSuffix() == "" {
			return StringMatcher{}, errors.New("empty suffix is not allowed in StringMatcher")
		}
		matcher.suffixMatch = newStrPtr(&mt.Suffix, matcher.ignoreCase)
	case *v3matcherpb.StringMatcher_SafeRegex:
		regex := matcherProto.GetSafeRegex().GetRegex()
		re, err := regexp.Compile(regex)
		if err != nil {
			return StringMatcher{}, fmt.Errorf("safe_regex matcher %q is invalid", regex)
		}
		matcher.regexMatch = re
	case *v3matcherpb.StringMatcher_Contains:
		if matcherProto.GetContains() == "" {
			return StringMatcher{}, errors.New("empty contains is not allowed in StringMatcher")
		}
		matcher.containsMatch = newStrPtr(&mt.Contains, matcher.ignoreCase)
	default:
		return StringMatcher{}, fmt.Errorf("unrecognized string matcher: %+v", matcherProto)
	}
	return matcher, nil
}

// NewStringMatcher is a helper function to create a StringMatcher based
// on the given arguments. At most one of exact, prefix, suffix, contains and
// regex should be non-nil.
func NewStringMatcher(exact, prefix, suffix, contains *string, regex *regexp.Regexp, ignoreCase bool) StringMatcher {
	return StringMatcher{
		exactMatch:    newStrPtr(exact, ignoreCase),
		prefixMatch:   newStrPtr(prefix, ignoreCase),
		suffixMatch:   newStrPtr(suffix, ignoreCase),
		containsMatch: newStrPtr(contains, ignoreCase),
		regexMatch:    regex,
		ignoreCase:    ignoreCase,
	}
}

// ExactMatch returns the value of the configured exact match or an empty string
// if exact match criteria was not specified.
func (sm StringMatcher) ExactMatch() string {
	if sm.exactMatch != nil {
		return *sm.exactMatch
	}
	return ""
}

// Equal returns true if other and sm are equivalent to each other.
func (sm StringMatcher) Equal(other StringMatcher) bool {
	if sm.ignoreCase != other.ignoreCase {
		return false
	}


Review comment (suggestion):
This constructor relies on the caller to read the documentation to avoid mistakes. It violates the principle of "Make Invalid States Unrepresentable."

Though it is a little more code, providing separate constructors for each matcher type can avoid this problem. It will also make the call sites more readable since there won't be extra `nils`.
```go
// NewExactMatcher creates a matcher that requires exact equality.
func NewExactMatcher(pattern string, ignoreCase bool) StringMatcher {
    return StringMatcher{
        exactMatch: newStrPtr(pattern, ignoreCase),
        ignoreCase: ignoreCase,
    }
}

// NewPrefixMatcher creates a matcher checking for a prefix.
func NewPrefixMatcher(prefix string, ignoreCase bool) StringMatcher {
    return StringMatcher{
        prefixMatch: newStrPtr(prefix, ignoreCase),
        ignoreCase:  ignoreCase,
    }
}

// NewRegexMatcher creates a matcher using a regular expression.
func NewRegexMatcher(regex *regexp.Regexp) StringMatcher {
    // Regex usually ignores the generic 'ignoreCase' flag 
    // as it is often embedded in the regex compilation itself.
    return StringMatcher{
        regexMatch: regex,
    }
}
```

Code after:
		if matcherProto.GetSuffix() == "" {
			return StringMatcher{}, errors.New("empty suffix is not allowed in StringMatcher")
		}
		matcher.suffixMatch = newStrPtr(&mt.Suffix, matcher.ignoreCase)
	case *v3matcherpb.StringMatcher_SafeRegex:
		regex := matcherProto.GetSafeRegex().GetRegex()
		re, err := regexp.Compile(regex)
		if err != nil {
			return StringMatcher{}, fmt.Errorf("safe_regex matcher %q is invalid", regex)
		}
		matcher.regexMatch = re
	case *v3matcherpb.StringMatcher_Contains:
		if matcherProto.GetContains() == "" {
			return StringMatcher{}, errors.New("empty contains is not allowed in StringMatcher")
		}
		matcher.containsMatch = newStrPtr(&mt.Contains, matcher.ignoreCase)
	default:
		return StringMatcher{}, fmt.Errorf("unrecognized string matcher: %+v", matcherProto)
	}
	return matcher, nil
}

// NewExactStringMatcher creates a string matcher that requires the input string
// to exactly match the pattern specified here. The match will be case
// insensitive if ignore_case is true.
func NewExactStringMatcher(pattern string, ignoreCase bool) StringMatcher {
	return StringMatcher{
		exactMatch: newStrPtr(&pattern, ignoreCase),
		ignoreCase: ignoreCase,
	}
}

// NewPrefixStringMatcher creates a string matcher that requires the input
// string to contain the prefix specified here. The match will be case
// insensitive if ignore_case is true.
func NewPrefixStringMatcher(prefix string, ignoreCase bool) StringMatcher {
	return StringMatcher{
		prefixMatch: newStrPtr(&prefix, ignoreCase),
		ignoreCase:  ignoreCase,
	}
}

// NewSuffixStringMatcher creates a string matcher that requires the input
// string to contain the suffix specified here. The match will be case
// insensitive if ignore_case is true.
func NewSuffixStringMatcher(suffix string, ignoreCase bool) StringMatcher {
	return StringMatcher{
		suffixMatch: newStrPtr(&suffix, ignoreCase),
		ignoreCase:  ignoreCase,
	}
}