Pull request:* all: fix all staticcheck simplification and unused warnings

Merge in DNS/adguard-home from 2270-fix-s-u-warnings to master Squashed commit of the following: commit 03e0f78bd471057007c2d4042ee26eda2bbc9b29 Merge: 50dc3ef5c 7e16fda57 Author: Eugene Burkov <e.burkov@adguard.com> Date: Fri Nov 6 11:54:09 2020 +0300 Merge branch 'master' into 2270-fix-s-u-warnings commit 50dc3ef5c44a5fdc941794c26784b0c44d7b5aa0 Author: Eugene Burkov <e.burkov@adguard.com> Date: Thu Nov 5 19:48:54 2020 +0300 * all: improve code quality commit d6d804f759ce3e47154a389b427550e72c4b9090 Author: Eugene Burkov <e.burkov@adguard.com> Date: Thu Nov 5 18:03:35 2020 +0300 * all: fix all staticcheck simplification and unused warnings Closes #2270.
2020-11-06 12:15:08 +03:00
parent 7e16fda57b
commit 3e1f922252
24 changed files with 159 additions and 210 deletions
--- a/internal/dnsfilter/dnsfilter.go
+++ b/internal/dnsfilter/dnsfilter.go
@@ -199,7 +199,7 @@ func (d *Dnsfilter) WriteDiskConfig(c *Config) {
 // SetFilters - set new filters (synchronously or asynchronously)
 // When filters are set asynchronously, the old filters continue working until the new filters are ready.
 //  In this case the caller must ensure that the old filter files are intact.
-func (d *Dnsfilter) SetFilters(blockFilters []Filter, allowFilters []Filter, async bool) error {
+func (d *Dnsfilter) SetFilters(blockFilters, allowFilters []Filter, async bool) error {
 	if async {
 		params := filtersInitializerParams{
 			allowFilters: allowFilters,
@@ -481,13 +481,10 @@ func matchBlockedServicesRules(host string, svcs []ServiceEntry) Result {
 // Adding rule and matching against the rules
 //

-// Return TRUE if file exists
+// fileExists returns true if file exists.
 func fileExists(fn string) bool {
 	_, err := os.Stat(fn)
-	if err != nil {
-		return false
-	}
-	return true
+	return err == nil
 }

 func createFilteringEngine(filters []Filter) (*filterlist.RuleStorage, *urlfilter.DNSEngine, error) {
@@ -508,7 +505,8 @@ func createFilteringEngine(filters []Filter) (*filterlist.RuleStorage, *urlfilte
 			}
 		} else if runtime.GOOS == "windows" {
 			// On Windows we don't pass a file to urlfilter because
-			//  it's difficult to update this file while it's being used.
+			// it's difficult to update this file while it's being
+			// used.
 			data, err := ioutil.ReadFile(f.FilePath)
 			if err != nil {
 				return nil, nil, fmt.Errorf("ioutil.ReadFile(): %s: %w", f.FilePath, err)
@@ -537,7 +535,7 @@ func createFilteringEngine(filters []Filter) (*filterlist.RuleStorage, *urlfilte
 	return rulesStorage, filteringEngine, nil
 }

-// Initialize urlfilter objects
+// Initialize urlfilter objects.
 func (d *Dnsfilter) initFiltering(allowFilters, blockFilters []Filter) error {
 	rulesStorage, filteringEngine, err := createFilteringEngine(blockFilters)
 	if err != nil {
@@ -563,7 +561,8 @@ func (d *Dnsfilter) initFiltering(allowFilters, blockFilters []Filter) error {
 	return nil
 }

-// matchHost is a low-level way to check only if hostname is filtered by rules, skipping expensive safebrowsing and parental lookups
+// matchHost is a low-level way to check only if hostname is filtered by rules,
+// skipping expensive safebrowsing and parental lookups.
 func (d *Dnsfilter) matchHost(host string, qtype uint16, setts RequestFilteringSettings) (Result, error) {
 	d.engineLock.RLock()
 	// Keep in mind that this lock must be held no just when calling Match()
@@ -664,20 +663,18 @@ func makeResult(rule rules.Rule, reason Reason) Result {
 	return res
 }

-// InitModule() - manually initialize blocked services map
+// InitModule manually initializes blocked services map.
 func InitModule() {
 	initBlockedServices()
 }

-// New creates properly initialized DNS Filter that is ready to be used
+// New creates properly initialized DNS Filter that is ready to be used.
 func New(c *Config, blockFilters []Filter) *Dnsfilter {
 	if c != nil {
 		cacheConf := cache.Config{
 			EnableLRU: true,
 		}

-		// initialize objects only once
-
 		if gctx.safebrowsingCache == nil {
 			cacheConf.MaxSize = c.SafeBrowsingCacheSize
 			gctx.safebrowsingCache = cache.New(cacheConf)
@@ -747,7 +744,7 @@ func (d *Dnsfilter) Start() {
 // stats
 //

-// GetStats return dns filtering stats since startup
+// GetStats return dns filtering stats since startup.
 func (d *Dnsfilter) GetStats() Stats {
 	return gctx.stats
 }
--- a/internal/dnsfilter/dnsfilter_test.go
+++ b/internal/dnsfilter/dnsfilter_test.go
@@ -3,9 +3,6 @@ package dnsfilter
 import (
 	"fmt"
 	"net"
-	"os"
-	"path"
-	"runtime"
 	"testing"

 	"github.com/AdguardTeam/urlfilter/rules"
@@ -36,13 +33,6 @@ func purgeCaches() {
 	}
 }

-func _Func() string {
-	pc := make([]uintptr, 10) // at least 1 entry needed
-	runtime.Callers(2, pc)
-	f := runtime.FuncForPC(pc[0])
-	return path.Base(f.Name())
-}
-
 func NewForTest(c *Config, filters []Filter) *Dnsfilter {
 	setts = RequestFilteringSettings{}
 	setts.FilteringEnabled = true
@@ -71,7 +61,7 @@ func (d *Dnsfilter) checkMatch(t *testing.T, hostname string) {
 	}
 }

-func (d *Dnsfilter) checkMatchIP(t *testing.T, hostname string, ip string, qtype uint16) {
+func (d *Dnsfilter) checkMatchIP(t *testing.T, hostname, ip string, qtype uint16) {
 	t.Helper()
 	ret, err := d.CheckHost(hostname, qtype, &setts)
 	if err != nil {
@@ -107,7 +97,7 @@ func TestEtcHostsMatching(t *testing.T) {
 ::1 host2
 `,
 		addr, addr6)
-	filters := []Filter{Filter{
+	filters := []Filter{{
 		ID: 0, Data: []byte(text),
 	}}
 	d := NewForTest(nil, filters)
@@ -351,11 +341,15 @@ func TestParentalControl(t *testing.T) {

 // FILTERING

-var blockingRules = "||example.org^\n"
-var whitelistRules = "||example.org^\n@@||test.example.org\n"
-var importantRules = "@@||example.org^\n||test.example.org^$important\n"
-var regexRules = "/example\\.org/\n@@||test.example.org^\n"
-var maskRules = "test*.example.org^\nexam*.com\n"
+const nl = "\n"
+
+const (
+	blockingRules  = `||example.org^` + nl
+	whitelistRules = `||example.org^` + nl + `@@||test.example.org` + nl
+	importantRules = `@@||example.org^` + nl + `||test.example.org^$important` + nl
+	regexRules     = `/example\.org/` + nl + `@@||test.example.org^` + nl
+	maskRules      = `test*.example.org^` + nl + `exam*.com` + nl
+)

 var tests = []struct {
 	testname   string
@@ -406,7 +400,7 @@ var tests = []struct {
 func TestMatching(t *testing.T) {
 	for _, test := range tests {
 		t.Run(fmt.Sprintf("%s-%s", test.testname, test.hostname), func(t *testing.T) {
-			filters := []Filter{Filter{
+			filters := []Filter{{
 				ID: 0, Data: []byte(test.rules),
 			}}
 			d := NewForTest(nil, filters)
@@ -430,14 +424,14 @@ func TestWhitelist(t *testing.T) {
 	rules := `||host1^
 ||host2^
 `
-	filters := []Filter{Filter{
+	filters := []Filter{{
 		ID: 0, Data: []byte(rules),
 	}}

 	whiteRules := `||host1^
 ||host3^
 `
-	whiteFilters := []Filter{Filter{
+	whiteFilters := []Filter{{
 		ID: 0, Data: []byte(whiteRules),
 	}}
 	d := NewForTest(nil, filters)
@@ -455,7 +449,6 @@ func TestWhitelist(t *testing.T) {
 	assert.True(t, err == nil)
 	assert.True(t, ret.IsFiltered && ret.Reason == FilteredBlackList)
 	assert.True(t, ret.Rule == "||host2^")
-
 }

 // CLIENT SETTINGS
@@ -476,7 +469,7 @@ func applyClientSettings(setts *RequestFilteringSettings) {
 //  then apply per-client settings and check behaviour once again
 func TestClientSettings(t *testing.T) {
 	var r Result
-	filters := []Filter{Filter{
+	filters := []Filter{{
 		ID: 0, Data: []byte("||example.org^\n"),
 	}}
 	d := NewForTest(&Config{ParentalEnabled: true, SafeBrowsingEnabled: false}, filters)
@@ -532,13 +525,6 @@ func TestClientSettings(t *testing.T) {
 	assert.True(t, r.IsFiltered && r.Reason == FilteredBlockedService)
 }

-func prepareTestDir() string {
-	const dir = "./agh-test"
-	_ = os.RemoveAll(dir)
-	_ = os.MkdirAll(dir, 0755)
-	return dir
-}
-
 // BENCHMARKS

 func BenchmarkSafeBrowsing(b *testing.B) {
--- a/internal/dnsfilter/safe_search.go
+++ b/internal/dnsfilter/safe_search.go
@@ -22,8 +22,7 @@ func (d *Dnsfilter) setCacheResult(cache cache.Cache, host string, res Result) i
 	var buf bytes.Buffer

 	expire := uint(time.Now().Unix()) + d.Config.CacheTime*60
-	var exp []byte
-	exp = make([]byte, 4)
+	exp := make([]byte, 4)
 	binary.BigEndian.PutUint32(exp, uint32(expire))
 	_, _ = buf.Write(exp)

--- a/internal/dnsfilter/sb_pc.go
+++ b/internal/dnsfilter/sb_pc.go
@@ -22,11 +22,13 @@ import (
 	"golang.org/x/net/publicsuffix"
 )

-const dnsTimeout = 3 * time.Second
-const defaultSafebrowsingServer = "https://dns-family.adguard.com/dns-query"
-const defaultParentalServer = "https://dns-family.adguard.com/dns-query"
-const sbTXTSuffix = "sb.dns.adguard.com."
-const pcTXTSuffix = "pc.dns.adguard.com."
+const (
+	dnsTimeout                = 3 * time.Second
+	defaultSafebrowsingServer = `https://dns-family.adguard.com/dns-query`
+	defaultParentalServer     = `https://dns-family.adguard.com/dns-query`
+	sbTXTSuffix               = `sb.dns.adguard.com.`
+	pcTXTSuffix               = `pc.dns.adguard.com.`
+)

 func (d *Dnsfilter) initSecurityServices() error {
 	var err error
@@ -60,7 +62,7 @@ expire byte[4]
 hash byte[32]
 ...
 */
-func (c *sbCtx) setCache(prefix []byte, hashes []byte) {
+func (c *sbCtx) setCache(prefix, hashes []byte) {
 	d := make([]byte, 4+len(hashes))
 	expire := uint(time.Now().Unix()) + c.cacheTime*60
 	binary.BigEndian.PutUint32(d[:4], uint32(expire))
@@ -158,16 +160,28 @@ func hostnameToHashes(host string) map[[32]byte]string {

 // convert hash array to string
 func (c *sbCtx) getQuestion() string {
-	q := ""
+	b := &strings.Builder{}
+	encoder := hex.NewEncoder(b)
+
 	for hash := range c.hashToHost {
-		q += fmt.Sprintf("%s.", hex.EncodeToString(hash[0:2]))
+		// Ignore errors, since strings.(*Buffer).Write never returns
+		// errors.
+		//
+		// TODO(e.burkov, a.garipov): Find out and document why exactly
+		// this slice.
+		_, _ = encoder.Write(hash[0:2])
+		_, _ = b.WriteRune('.')
 	}
+
 	if c.svc == "SafeBrowsing" {
-		q += sbTXTSuffix
-	} else {
-		q += pcTXTSuffix
+		// See comment above.
+		_, _ = b.WriteString(sbTXTSuffix)
+		return b.String()
 	}
-	return q
+
+	// See comment above.
+	_, _ = b.WriteString(pcTXTSuffix)
+	return b.String()
 }

 // Find the target hash in TXT response
--- a/internal/dnsfilter/sb_pc_test.go
+++ b/internal/dnsfilter/sb_pc_test.go
@@ -23,16 +23,16 @@ func TestSafeBrowsingHash(t *testing.T) {
 	assert.False(t, ok)

 	c := &sbCtx{
-		svc: "SafeBrowsing",
+		svc:        "SafeBrowsing",
+		hashToHost: hashes,
 	}

-	// test getQuestion()
-	c.hashToHost = hashes
 	q := c.getQuestion()
-	assert.True(t, strings.Index(q, "7a1b.") >= 0)
-	assert.True(t, strings.Index(q, "af5a.") >= 0)
-	assert.True(t, strings.Index(q, "eb11.") >= 0)
-	assert.True(t, strings.Index(q, "sb.dns.adguard.com.") > 0)
+
+	assert.True(t, strings.Contains(q, "7a1b."))
+	assert.True(t, strings.Contains(q, "af5a."))
+	assert.True(t, strings.Contains(q, "eb11."))
+	assert.True(t, strings.HasSuffix(q, "sb.dns.adguard.com."))
 }

 func TestSafeBrowsingCache(t *testing.T) {