Minor code aesthetic improvements, based on vet+fmt+lint

This patch is the result of running go vet, go fmt -s and the linter,
and fixing some of the things they noted/suggested.

There shouldn't be any significant logic changes, it's mostly
readability improvements.

chasquid.go

@@ -235,6 +235,7 @@ func mustReadDir(path string) []os.FileInfo {
 // We keep them distinct, as policies can differ between them.
 type SocketMode string
 
+// Valid socket modes.
 const (
 	ModeSMTP       SocketMode = "SMTP"
 	ModeSubmission SocketMode = "Submission"

cmd/smtp-check/smtp-check.go

@@ -28,7 +28,7 @@ func main() {
 
 	domain, err := idna.ToASCII(domain)
 	if err != nil {
-		log.Fatal("IDNA conversion failed: %v", err)
+		log.Fatalf("IDNA conversion failed: %v", err)
 	}
 
 	mxs, err := net.LookupMX(domain)

internal/aliases/aliases.go

@@ -74,6 +74,7 @@ type Recipient struct {
 
 type RType string
 
+// Valid recipient types.
 const (
 	EMAIL RType = "(email)"
 	PIPE  RType = "(pipe)"
@@ -148,7 +149,7 @@ func (v *Resolver) resolve(rcount int, addr string) ([]Recipient, error) {
 
 	rcpts := v.aliases[addr]
 	if len(rcpts) == 0 {
-		return []Recipient{Recipient{addr, EMAIL}}, nil
+		return []Recipient{{addr, EMAIL}}, nil
 	}
 
 	ret := []Recipient{}
@@ -285,7 +286,7 @@ func parseFile(domain, path string) (map[string][]Recipient, error) {
 
 		if rawalias[0] == '|' {
 			cmd := strings.TrimSpace(rawalias[1:])
-			aliases[addr] = []Recipient{Recipient{cmd, PIPE}}
+			aliases[addr] = []Recipient{{cmd, PIPE}}
 		} else {
 			rs := []Recipient{}
 			for _, a := range strings.Split(rawalias, ",") {

internal/queue/queue.go

@@ -380,32 +380,32 @@ func (item *Item) deliver(q *Queue, rcpt *Recipient) (err error, permanent bool)
 		cmd := exec.CommandContext(ctx, c[0], c[1:]...)
 		cmd.Stdin = bytes.NewReader(item.Data)
 		return cmd.Run(), true
+	}
 
+	// Recipient type is EMAIL.
+	if envelope.DomainIn(rcpt.Address, q.localDomains) {
+		deliverAttempts.Add("email:local", 1)
+		return q.localC.Deliver(item.From, rcpt.Address, item.Data)
 	} else {
-		if envelope.DomainIn(rcpt.Address, q.localDomains) {
+		deliverAttempts.Add("email:remote", 1)
-			deliverAttempts.Add("email:local", 1)
+		from := item.From
-			return q.localC.Deliver(item.From, rcpt.Address, item.Data)
+		if !envelope.DomainIn(item.From, q.localDomains) {
-		} else {
+			// We're sending from a non-local to a non-local. This should
-			deliverAttempts.Add("email:remote", 1)
+			// happen only when there's an alias to forward email to a
-			from := item.From
+			// non-local domain.  In this case, using the original From is
-			if !envelope.DomainIn(item.From, q.localDomains) {
+			// problematic, as we may not be an authorized sender for this.
-				// We're sending from a non-local to a non-local. This should
+			// Some MTAs (like Exim) will do it anyway, others (like
-				// happen only when there's an alias to forward email to a
+			// gmail) will construct a special address based on the
-				// non-local domain.  In this case, using the original From is
+			// original address.  We go with the latter.
-				// problematic, as we may not be an authorized sender for this.
+			// Note this assumes "+" is an alias suffix separator.
-				// Some MTAs (like Exim) will do it anyway, others (like
+			// We use the IDNA version of the domain if possible, because
-				// gmail) will construct a special address based on the
+			// we can't know if the other side will support SMTPUTF8.
-				// original address.  We go with the latter.
+			from = fmt.Sprintf("%s+fwd_from=%s@%s",
-				// Note this assumes "+" is an alias suffix separator.
+				envelope.UserOf(rcpt.OriginalAddress),
-				// We use the IDNA version of the domain if possible, because
+				strings.Replace(from, "@", "=", -1),
-				// we can't know if the other side will support SMTPUTF8.
+				mustIDNAToASCII(envelope.DomainOf(rcpt.OriginalAddress)))
-				from = fmt.Sprintf("%s+fwd_from=%s@%s",
-					envelope.UserOf(rcpt.OriginalAddress),
-					strings.Replace(from, "@", "=", -1),
-					mustIDNAToASCII(envelope.DomainOf(rcpt.OriginalAddress)))
-			}
-			return q.remoteC.Deliver(from, rcpt.Address, item.Data)
 		}
+		return q.remoteC.Deliver(from, rcpt.Address, item.Data)
 	}
 }
 

internal/smtp/smtp.go

@@ -47,16 +47,16 @@ func (c *Client) cmd(expectCode int, format string, args ...interface{}) (int, s
 // It will check the addresses, decide if SMTPUTF8 is needed, and apply the
 // necessary transformations.
 func (c *Client) MailAndRcpt(from string, to string) error {
-	from, from_needs, err := c.prepareForSMTPUTF8(from)
+	from, fromNeeds, err := c.prepareForSMTPUTF8(from)
 	if err != nil {
 		return err
 	}
 
-	to, to_needs, err := c.prepareForSMTPUTF8(to)
+	to, toNeeds, err := c.prepareForSMTPUTF8(to)
 	if err != nil {
 		return err
 	}
-	smtputf8Needed := from_needs || to_needs
+	smtputf8Needed := fromNeeds || toNeeds
 
 	cmdStr := "MAIL FROM:<%s>"
 	if ok, _ := c.Extension("8BITMIME"); ok {
@@ -102,8 +102,8 @@ func (c *Client) prepareForSMTPUTF8(addr string) (string, bool, error) {
 	user, domain := envelope.Split(addr)
 
 	if !isASCII(user) {
-		return addr, true, &textproto.Error{599,
+		return addr, true, &textproto.Error{Code: 599,
-			"local part is not ASCII but server does not support SMTPUTF8"}
+			Msg: "local part is not ASCII but server does not support SMTPUTF8"}
 	}
 
 	// If it's only the domain, convert to IDNA and move on.
@@ -112,8 +112,8 @@ func (c *Client) prepareForSMTPUTF8(addr string) (string, bool, error) {
 		// The domain is not IDNA compliant, which is odd.
 		// Fail with a permanent error, not ideal but this should not
 		// happen.
-		return addr, true, &textproto.Error{599,
+		return addr, true, &textproto.Error{
-			"non-ASCII domain is not IDNA safe"}
+			Code: 599, Msg: "non-ASCII domain is not IDNA safe"}
 	}
 
 	return user + "@" + domain, false, nil

internal/smtp/smtp_test.go

@@ -17,10 +17,10 @@ func TestIsPermanent(t *testing.T) {
 		err       error
 		permanent bool
 	}{
-		{&textproto.Error{499, ""}, false},
+		{&textproto.Error{Code: 499, Msg: ""}, false},
-		{&textproto.Error{500, ""}, true},
+		{&textproto.Error{Code: 500, Msg: ""}, true},
-		{&textproto.Error{599, ""}, true},
+		{&textproto.Error{Code: 599, Msg: ""}, true},
-		{&textproto.Error{600, ""}, false},
+		{&textproto.Error{Code: 600, Msg: ""}, false},
 		{fmt.Errorf("something"), false},
 	}
 	for _, c := range cases {

internal/spf/spf.go

@@ -30,9 +30,9 @@ import (
 
 // Functions that we can override for testing purposes.
 var (
-	lookupTXT func(domain string) (txts []string, err error) = net.LookupTXT
+	lookupTXT = net.LookupTXT
-	lookupMX  func(domain string) (mxs []*net.MX, err error) = net.LookupMX
+	lookupMX  = net.LookupMX
-	lookupIP  func(host string) (ips []net.IP, err error)    = net.LookupIP
+	lookupIP  = net.LookupIP
 )
 
 // Results and Errors. Note the values have meaning, we use them in headers.