Clean up code and strengthening (slack) (#519)

Changes include:
- Refactor of strings into package-wide constants.
- Predeclaration of regexps to be instantiated at package load time.
- Checking of unchecked errors.
- Structural changes:
  - Adding verifications to type-casting code.
  - Remove unnecessary 'len(X) > 0' checks before iterating over X.
  - Remove unnecessary 'else' clause after 'if' with 'return'.
  - Unexporting of public fields of Bridge struct.
- Formatting:
  - One-field-per-line struct definitions.

1 files changed, 35 insertions, 38 deletions

diff --git a/bridge/slack/helpers.go b/bridge/slack/helpers.go
index 273cf7c6..9af5dfac 100644
--- a/bridge/slack/helpers.go
+++ b/bridge/slack/helpers.go
@@ -8,8 +8,8 @@ import (
 	"github.com/nlopes/slack"
 )
 
-func (b *Bslack) userName(id string) string {
-	for _, u := range b.Users {
+func (b *Bslack) getUsername(id string) string {
+	for _, u := range b.users {
 		if u.ID == id {
 			if u.Profile.DisplayName != "" {
 				return u.Profile.DisplayName
@@ -17,22 +17,26 @@ func (b *Bslack) userName(id string) string {
 			return u.Name
 		}
 	}
+	b.Log.Warnf("Could not find user with ID '%s'", id)
 	return ""
 }
 
 func (b *Bslack) getAvatar(userid string) string {
-	var avatar string
-	if b.Users != nil {
-		for _, u := range b.Users {
-			if userid == u.ID {
-				return u.Profile.Image48
-			}
+	for _, u := range b.users {
+		if userid == u.ID {
+			return u.Profile.Image48
 		}
 	}
-	return avatar
+	return ""
+}
+
+func (b *Bslack) getChannel(channel string) (*slack.Channel, error) {
+	if strings.HasPrefix(channel, "ID:") {
+		return b.getChannelByID(strings.TrimPrefix(channel, "ID:"))
+	}
+	return b.getChannelByName(channel)
 }
 
-/*
 func (b *Bslack) getChannelByName(name string) (*slack.Channel, error) {
 	if b.channels == nil {
 		return nil, fmt.Errorf("%s: channel %s not found (no channels found)", b.Account, name)
@@ -44,7 +48,6 @@ func (b *Bslack) getChannelByName(name string) (*slack.Channel, error) {
 	}
 	return nil, fmt.Errorf("%s: channel %s not found", b.Account, name)
 }
-*/
 
 func (b *Bslack) getChannelByID(ID string) (*slack.Channel, error) {
 	if b.channels == nil {
@@ -58,45 +61,40 @@ func (b *Bslack) getChannelByID(ID string) (*slack.Channel, error) {
 	return nil, fmt.Errorf("%s: channel %s not found", b.Account, ID)
 }
 
-func (b *Bslack) getChannelID(name string) string {
-	idcheck := strings.Split(name, "ID:")
-	if len(idcheck) > 1 {
-		return idcheck[1]
-	}
-	for _, channel := range b.channels {
-		if channel.Name == name {
-			return channel.ID
-		}
-	}
-	return ""
-}
+var (
+	mentionRE  = regexp.MustCompile(`<@([a-zA-Z0-9]+)>`)
+	channelRE  = regexp.MustCompile(`<#[a-zA-Z0-9]+\|(.+?)>`)
+	variableRE = regexp.MustCompile(`<!((?:subteam\^)?[a-zA-Z0-9]+)(?:\|@?(.+?))?>`)
+	urlRE      = regexp.MustCompile(`<(.*?)(\|.*?)?>`)
+)
 
 // @see https://api.slack.com/docs/message-formatting#linking_to_channels_and_users
 func (b *Bslack) replaceMention(text string) string {
-	results := regexp.MustCompile(`<@([a-zA-Z0-9]+)>`).FindAllStringSubmatch(text, -1)
-	for _, r := range results {
-		text = strings.Replace(text, "<@"+r[1]+">", "@"+b.userName(r[1]), -1)
+	replaceFunc := func(match string) string {
+		userID := strings.Trim(match, "@<>")
+		if username := b.getUsername(userID); userID != "" {
+			return "@" + username
+		}
+		return match
 	}
-	return text
+	return mentionRE.ReplaceAllStringFunc(text, replaceFunc)
 }
 
 // @see https://api.slack.com/docs/message-formatting#linking_to_channels_and_users
 func (b *Bslack) replaceChannel(text string) string {
-	results := regexp.MustCompile(`<#[a-zA-Z0-9]+\|(.+?)>`).FindAllStringSubmatch(text, -1)
-	for _, r := range results {
-		text = strings.Replace(text, r[0], "#"+r[1], -1)
+	for _, r := range channelRE.FindAllStringSubmatch(text, -1) {
+		text = strings.Replace(text, r[0], "#"+r[1], 1)
 	}
 	return text
 }
 
 // @see https://api.slack.com/docs/message-formatting#variables
 func (b *Bslack) replaceVariable(text string) string {
-	results := regexp.MustCompile(`<!((?:subteam\^)?[a-zA-Z0-9]+)(?:\|@?(.+?))?>`).FindAllStringSubmatch(text, -1)
-	for _, r := range results {
+	for _, r := range variableRE.FindAllStringSubmatch(text, -1) {
 		if r[2] != "" {
-			text = strings.Replace(text, r[0], "@"+r[2], -1)
+			text = strings.Replace(text, r[0], "@"+r[2], 1)
 		} else {
-			text = strings.Replace(text, r[0], "@"+r[1], -1)
+			text = strings.Replace(text, r[0], "@"+r[1], 1)
 		}
 	}
 	return text
@@ -104,12 +102,11 @@ func (b *Bslack) replaceVariable(text string) string {
 
 // @see https://api.slack.com/docs/message-formatting#linking_to_urls
 func (b *Bslack) replaceURL(text string) string {
-	results := regexp.MustCompile(`<(.*?)(\|.*?)?>`).FindAllStringSubmatch(text, -1)
-	for _, r := range results {
+	for _, r := range urlRE.FindAllStringSubmatch(text, -1) {
 		if len(strings.TrimSpace(r[2])) == 1 { // A display text separator was found, but the text was blank
-			text = strings.Replace(text, r[0], "", -1)
+			text = strings.Replace(text, r[0], "", 1)
 		} else {
-			text = strings.Replace(text, r[0], r[1], -1)
+			text = strings.Replace(text, r[0], r[1], 1)
 		}
 	}
 	return text