refactor: Cleanup and add function clarifications

Author: yongjiajun

cidr_sorter.go

@@ -42,6 +42,7 @@ func (s cidrSorter) Less(i, j int) bool {
 	return p1 < p2 // Smaller prefix (wider network) comes first
 }
 
+// parseCIDR parses a CIDR notation string and returns the IP, prefix length, and whether it's IPv4.
 func parseCIDR(s string) (ip net.IP, prefix int, isV4 bool) {
 	_, ipNet, err := net.ParseCIDR(s)
 	if err != nil {

spf2ip.go

@@ -104,13 +104,11 @@ func (r *SPF2IPResolver) processDomain(ctx context.Context, domain string, depth
 	currentDomainIPs := make(map[string]struct{})
 
 	spfString, err := r.getSPFRecord(ctx, domain)
-	if err != nil {
-		if !errors.Is(err, errIgnorableDNSErr) {
-			log.Printf("Warning: Failed to get SPF record for %s: %v", domain, err)
-			r.resolvedIPsCache[domain] = nil
+	if err != nil && !errors.Is(err, errIgnorableDNSErr) {
+		log.Printf("Warning: Failed to get SPF record for %s: %v", domain, err)
+		r.resolvedIPsCache[domain] = nil
 
-			return nil, err
-		}
+		return nil, err
 	}
 
 	if spfString == "" {
@@ -209,37 +207,37 @@ func (r *SPF2IPResolver) processDomain(ctx context.Context, domain string, depth
 			}
 
 		case "include":
-			if value != "" {
-				includedIPs, includeErr := r.processDomain(ctx, value, depth+1)
-				if includeErr != nil {
-					return nil, fmt.Errorf("include failed for %s (directive in %s): %w", value, domain, includeErr)
-				} else {
-					for ip := range includedIPs {
-						currentDomainIPs[ip] = struct{}{}
-					}
-				}
-			} else {
+			if value == "" {
 				log.Printf("Warning: 'include' modifier without domain in %s", domain)
 				r.resolvedIPsCache[domain] = nil
 
 				return nil, fmt.Errorf("include without domain in %s", domain)
 			}
 
-		case "redirect":
-			if value != "" {
-				r.debugLogPrintf("Debug: Redirecting from %s to %s. Discarding IPs found so far for %s.", domain, value, domain)
+			includedIPs, includeErr := r.processDomain(ctx, value, depth+1)
+			if includeErr != nil {
+				return nil, fmt.Errorf("include failed for %s (directive in %s): %w", value, domain, includeErr)
+			}
+
+			for ip := range includedIPs {
+				currentDomainIPs[ip] = struct{}{}
+			}
 
-				// The result of this domain's processing is now entirely determined by the redirect target.
-				redirectedIPs, redirectErr := r.processDomain(ctx, value, depth+1)
-				r.resolvedIPsCache[domain] = deepCopyMap(redirectedIPs) // Overwrite cache with redirected IPs
+		case "redirect":
+			if value == "" {
+				log.Printf("Warning: 'redirect' modifier without domain in %s", domain)
+				r.resolvedIPsCache[domain] = nil
 
-				return redirectedIPs, redirectErr
+				return nil, fmt.Errorf("redirect without domain in %s", domain)
 			}
 
-			log.Printf("Warning: 'redirect' modifier without domain in %s", domain)
-			r.resolvedIPsCache[domain] = nil
+			r.debugLogPrintf("Debug: Redirecting from %s to %s. Discarding IPs found so far for %s.", domain, value, domain)
+
+			// The result of this domain's processing is now entirely determined by the redirect target.
+			redirectedIPs, redirectErr := r.processDomain(ctx, value, depth+1)
+			r.resolvedIPsCache[domain] = deepCopyMap(redirectedIPs) // Overwrite cache with redirected IPs
 
-			return nil, fmt.Errorf("redirect without domain in %s", domain)
+			return redirectedIPs, redirectErr
 
 		case "exists", "ptr", "all":
 			// These mechanisms don't directly define IPs in the same way, ignore for IP extraction.
@@ -255,6 +253,7 @@ func (r *SPF2IPResolver) processDomain(ctx context.Context, domain string, depth
 	return currentDomainIPs, nil
 }
 
+// lookupIPNetwork returns the appropriate network type for IP lookups based on the resolver's IP version.
 func (r *SPF2IPResolver) lookupIPNetwork() string {
 	switch r.ipVersion {
 	case ipv4:
@@ -266,6 +265,7 @@ func (r *SPF2IPResolver) lookupIPNetwork() string {
 	}
 }
 
+// parseSPFMechanismTargetAndMask extracts the target host and mask suffix from an SPF mechanism value.
 func parseSPFMechanismTargetAndMask(defaultDomain, mechanismValue string) (targetHost, maskSuffix string) {
 	targetHost = defaultDomain
 	maskSuffix = ""
@@ -353,7 +353,8 @@ func (r *SPF2IPResolver) addIPOrCIDRToSet(value string, targetSet map[string]str
 	return fmt.Errorf("value '%s' is not a valid IP address or CIDR block", value)
 }
 
-func (r *SPF2IPResolver) debugLogPrintf(format string, args ...interface{}) {
+// debugLogPrintf logs debug messages if debug logging is enabled.
+func (r *SPF2IPResolver) debugLogPrintf(format string, args ...any) {
 	if r.debugLogging {
 		log.Printf(format, args...)
 	}

spf2ip_test.go

@@ -37,7 +37,7 @@ func TestResolve(t *testing.T) {
 		[]string{"v=spf1 ip4:1.2.3.0/28 include:first.included.com include:second.included.com a mx:mx.example.com -all"}, nil,
 	).Times(1)
 	s.netResolver.EXPECT().LookupTXT(gomock.Any(), "first.included.com").Return(
-		[]string{"v=spf1 ip4:5.6.7.8 ip6:2001:db8::1 include:included.again.com -all"}, nil,
+		[]string{"v=spf1 ip4:5.6.7.8 ip6:2001:db8::1 include:nxdomain.included.com -all"}, nil,
 	).Times(1)
 	s.netResolver.EXPECT().LookupTXT(gomock.Any(), "second.included.com").Return(
 		[]string{"v=spf1 include:first.included.com ip4:1.0.0.0/24 -all"}, nil,
@@ -57,7 +57,7 @@ func TestResolve(t *testing.T) {
 	s.netResolver.EXPECT().LookupIP(gomock.Any(), "ip4", "example.com").Return(
 		[]net.IP{net.ParseIP("8.8.8.8")}, nil,
 	).Times(1)
-	s.netResolver.EXPECT().LookupTXT(gomock.Any(), "included.again.com").Return(
+	s.netResolver.EXPECT().LookupTXT(gomock.Any(), "nxdomain.included.com").Return(
 		nil, &net.DNSError{IsNotFound: true}, // Simulating an ignorable DNS error
 	).Times(1)
 
@@ -304,7 +304,7 @@ func TestResolve_ExceededMaxDepthErr(t *testing.T) {
 		[]string{"v=spf1 include:included0.com -all"}, nil,
 	).Times(1)
 
-	for i := 0; i < maxSPFIncludeDepth; i++ { // excluding the first include
+	for i := range maxSPFIncludeDepth { // excluding the first include
 		s.netResolver.EXPECT().LookupTXT(gomock.Any(), fmt.Sprintf("included%d.com", i)).Return(
 			[]string{fmt.Sprintf("v=spf1 include:included%d.com -all", i+1)}, nil,
 		).Times(1)

utils.go

@@ -4,7 +4,7 @@ import (
 	"maps"
 )
 
-// Helper to copy a map, as maps are reference types.
+// deepCopyMap is a helper for copying a map, as maps are reference types.
 func deepCopyMap(originalMap map[string]struct{}) map[string]struct{} {
 	if originalMap == nil {
 		return nil