all: improve code and documentation

HACKING.md

@@ -78,6 +78,14 @@ The rules are mostly sorted in the alphabetical order.
  *  Prefer constants to variables where possible.  Reduce global variables.  Use
     [constant errors] instead of `errors.New`.
 
+ *  Unused arguments in anonymous functions must be called `_`:
+
+    ```go
+    v.onSuccess = func(_ int, msg string) {
+            // …
+    }
+    ```
+
  *  Use linters.
 
  *  Use named returns to improve readability of function signatures.
@@ -106,7 +114,16 @@ The rules are mostly sorted in the alphabetical order.
     ```go
     // Foo implements the Fooer interface for *foo.
     func (f *foo) Foo() {
-        // …
+            // …
+    }
+    ```
+
+    When the implemented interface is unexported:
+
+    ```go
+    // Unwrap implements the hidden wrapper interface for *fooError.
+    func (err *fooError) Unwrap() (unwrapped error) {
+            // …
     }
     ```
 

internal/dnsfilter/dnsfilter.go

@@ -305,7 +305,13 @@ type ResultRule struct {
 	IP net.IP `json:",omitempty"`
 }
 
-// Result holds state of hostname check
+// Result contains the result of a request check.
+//
+// All fields transitively have omitempty tags so that the query log
+// doesn't become too large.
+//
+// TODO(a.garipov): Clarify relationships between fields.  Perhaps
+// replace with a sum type or an interface?
 type Result struct {
 	// IsFiltered is true if the request is filtered.
 	IsFiltered bool `json:",omitempty"`
@@ -334,12 +340,13 @@ type Result struct {
 	ServiceName string `json:",omitempty"`
 }
 
-// Matched can be used to see if any match at all was found, no matter filtered or not
+// Matched returns true if any match at all was found regardless of
+// whether it was filtered or not.
 func (r Reason) Matched() bool {
 	return r != NotFilteredNotFound
 }
 
-// CheckHostRules tries to match the host against filtering rules only
+// CheckHostRules tries to match the host against filtering rules only.
 func (d *DNSFilter) CheckHostRules(host string, qtype uint16, setts *RequestFilteringSettings) (Result, error) {
 	if !setts.FilteringEnabled {
 		return Result{}, nil
@@ -348,8 +355,8 @@ func (d *DNSFilter) CheckHostRules(host string, qtype uint16, setts *RequestFilt
 	return d.matchHost(host, qtype, *setts)
 }
 
-// CheckHost tries to match the host against filtering rules,
-// then safebrowsing and parental if they are enabled
+// CheckHost tries to match the host against filtering rules, then
+// safebrowsing and parental control rules, if they are enabled.
 func (d *DNSFilter) CheckHost(host string, qtype uint16, setts *RequestFilteringSettings) (Result, error) {
 	// sometimes DNS clients will try to resolve ".", which is a request to get root servers
 	if host == "" {

internal/dnsfilter/sbpc.go → internal/dnsfilter/safebrowsing.go

@@ -1,5 +1,3 @@
-// Safe Browsing, Parental Control
-
 package dnsfilter
 
 import (
@@ -22,6 +20,8 @@ import (
 	"golang.org/x/net/publicsuffix"
 )
 
+// Safe browsing and parental control methods.
+
 const (
 	dnsTimeout                = 3 * time.Second
 	defaultSafebrowsingServer = `https://dns-family.adguard.com/dns-query`

internal/dnsfilter/safesearch.go

@@ -102,31 +102,24 @@ func (d *DNSFilter) checkSafeSearch(host string) (Result, error) {
 	}
 
 	// TODO this address should be resolved with upstream that was configured in dnsforward
-	addrs, err := net.LookupIP(safeHost)
+	ips, err := net.LookupIP(safeHost)
 	if err != nil {
 		log.Tracef("SafeSearchDomain for %s was found but failed to lookup for %s cause %s", host, safeHost, err)
 		return Result{}, err
 	}
 
-	found := false
-	for _, i := range addrs {
-		if ipv4 := i.To4(); ipv4 != nil {
+	for _, ip := range ips {
+		if ipv4 := ip.To4(); ipv4 != nil {
 			res.Rules[0].IP = ipv4
-			found = true
 
-			break
-		}
-	}
+			l := d.setCacheResult(gctx.safeSearchCache, host, res)
+			log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, l)
 
-	if !found {
-		return Result{}, fmt.Errorf("no ipv4 addresses in safe search response for %s", safeHost)
+			return res, nil
+		}
 	}
 
-	// Cache result
-	valLen := d.setCacheResult(gctx.safeSearchCache, host, res)
-	log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, valLen)
-
-	return res, nil
+	return Result{}, fmt.Errorf("no ipv4 addresses in safe search response for %s", safeHost)
 }
 
 func (d *DNSFilter) handleSafeSearchEnable(w http.ResponseWriter, r *http.Request) {

internal/querylog/json.go

@@ -35,7 +35,7 @@ func (l *queryLog) getClientIP(clientIP string) string {
 // jobject is a JSON object alias.
 type jobject = map[string]interface{}
 
-// entriesToJSON - converts log entries to JSON
+// entriesToJSON converts query log entries to JSON.
 func (l *queryLog) entriesToJSON(entries []*logEntry, oldest time.Time) (res jobject) {
 	data := []jobject{}
 
@@ -128,10 +128,10 @@ func (l *queryLog) logEntryToJSONEntry(entry *logEntry) (jsonEntry jobject) {
 
 func resultRulesToJSONRules(rules []*dnsfilter.ResultRule) (jsonRules []jobject) {
 	jsonRules = make([]jobject, len(rules))
-	for i := range rules {
+	for i, r := range rules {
 		jsonRules[i] = jobject{
-			"filter_list_id": rules[i].FilterListID,
-			"text":           rules[i].Text,
+			"filter_list_id": r.FilterListID,
+			"text":           r.Text,
 		}
 	}