Merge pull request #179 from atc0005/i176-fix-goconst-linting-errors

Fix golangci-lint "goconst" errors (WORKAROUND)

config/config.go

@@ -410,16 +410,16 @@ func (c *Config) String() string {
 	// If the AppMetadata fields are empty, note that to aid in
 	// troubleshooting elsewhere in the codebase
 	if c.AppName == "" {
-		c.AppName = "NotSet"
+		c.AppName = fieldValueNotSet
 	}
 	if c.AppDescription == "" {
-		c.AppDescription = "NotSet"
+		c.AppDescription = fieldValueNotSet
 	}
 	if c.AppVersion == "" {
-		c.AppVersion = "NotSet"
+		c.AppVersion = fieldValueNotSet
 	}
 	if c.AppURL == "" {
-		c.AppURL = "NotSet"
+		c.AppURL = fieldValueNotSet
 	}
 
 	return fmt.Sprintf("AppName=%q, AppDescription=%q, AppVersion=%q, AppURL=%q, FilePattern=%q, FileExtensions=%q, Paths=%v, RecursiveSearch=%t, FileAge=%d, NumFilesToKeep=%d, KeepOldest=%t, Remove=%t, IgnoreErrors=%t, LogFormat=%q, LogFilePath=%q, ConfigFile=%q, ConsoleOutput=%q, LogLevel=%q, UseSyslog=%t, logger=%v, flagParser=%v,  logFileHandle=%v",

config/config_test.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"path"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/atc0005/elbow/units"
@@ -49,9 +50,9 @@ func TestNewConfigFlagsOnly(t *testing.T) {
 	defer func() { os.Args = oldArgs }()
 
 	// TODO: A useful way to automate retrieving the app name?
-	appName := "elbow"
-	if runtime.GOOS == "windows" {
-		appName += ".exe"
+	appName := strings.ToLower(defaultAppName)
+	if runtime.GOOS == windowsOSName {
+		appName += windowsAppSuffix
 	}
 
 	// Note to self: Don't add/escape double-quotes here. The shell strips

config/constants.go

@@ -0,0 +1,34 @@
+// Copyright 2019 Adam Chalkley
+//
+// https://github.com/atc0005/elbow
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package config
+
+// NOTE: This file was created as a way of hotfixing the goconst linting errors
+// as part of resolving GH-176. This file is also intended to help
+// start a more thorough exploration of whether Go has a similar concept to
+// collecting constants in header files like C/C++ does.
+//
+// TODO: Evaluate use of constants throughout the entire codebase
+
+// Workarounds for golantci-lint errors:
+// string `STRING` has N occurrences, make it a constant (goconst)
+const fakeValue = "fakeValue"
+const defaultAppName string = "Elbow"
+const fieldValueNotSet string = "NotSet"
+const windowsOSName string = "windows"
+const logFormatText string = "text"
+const logFormatJSON string = "json"
+const windowsAppSuffix string = ".exe"

config/get.go

@@ -27,7 +27,7 @@ import (
 // otherwise
 func (c *Config) GetAppName() string {
 	if c == nil || c.AppName == "" {
-		return "Elbow"
+		return defaultAppName
 	}
 	return c.AppName
 }
@@ -161,7 +161,7 @@ func (c *Config) GetLogLevel() string {
 // otherwise
 func (c *Config) GetLogFormat() string {
 	if c == nil || c.LogFormat == nil {
-		return "text"
+		return logFormatText
 	}
 	return *c.LogFormat
 }

config/merge_complete_configs_test.go

@@ -20,12 +20,15 @@ import (
 	"bytes"
 	"os"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/alexflint/go-arg"
 	"github.com/sirupsen/logrus"
 )
 
+// TODO: Evaluate replacing bare strings with constants (see constants.go)
+
 // TestMergeConfigUsingCompleteConfigObjects creates multiple Config structs
 // and merges them in sequence, verifying that after each MergeConfig
 // operation that the initial config struct has been updated to reflect the
@@ -189,6 +192,7 @@ func TestMergeConfigUsingCompleteConfigObjects(t *testing.T) {
 		logger: logrus.New(),
 	}
 
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	envVarTables := []struct {
 		envVar string
 		value  string
@@ -293,13 +297,14 @@ func TestMergeConfigUsingCompleteConfigObjects(t *testing.T) {
 	}
 
 	// TODO: A useful way to automate retrieving the app name?
-	appName := "elbow"
-	if runtime.GOOS == "windows" {
-		appName += ".exe"
+	appName := strings.ToLower(defaultAppName)
+	if runtime.GOOS == windowsOSName {
+		appName += windowsAppSuffix
 	}
 
 	// Note to self: Don't add/escape double-quotes here. The shell strips
 	// them away and the application never sees them.
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	os.Args = []string{
 		appName,
 		"--paths", "/tmp/elbow/path4",

config/merge_incomplete_configs_test.go

@@ -20,11 +20,14 @@ import (
 	"bytes"
 	"os"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/alexflint/go-arg"
 )
 
+// TODO: Evaluate replacing bare strings with constants (see constants.go)
+
 // TestMergeConfigUsingIncompleteConfigObjects creates multiple Config structs
 // and merges them in sequence, verifying that after each MergeConfig
 // operation that the initial config struct has been updated to reflect the
@@ -122,13 +125,14 @@ func TestMergeConfigUsingIncompleteConfigObjects(t *testing.T) {
 	// settings that we are not overriding
 	// NOTE: Paths and FileExtensions are set below after config struct is
 	// instantiated
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	expectedPathsAfterFileMerge := []string{"/tmp/elbow/path1"}
 	expectedFileExtensionsAfterFileMerge := []string{".war"}
 	expectedFileAgeAfterFileMerge := 90
 	expectedNumFilesToKeepAfterFileMerge := 1
 	expectedRecursiveSearchAfterFileMerge := true
 	expectedLogLevelAfterFileMerge := "notice"
-	expectedLogFormatAfterFileMerge := "text"
+	expectedLogFormatAfterFileMerge := logFormatText
 	expectedUseSyslogAfterFileMerge := true
 
 	expectedKeepOldestAfterFileMerge := baseConfig.GetKeepOldest()
@@ -205,6 +209,7 @@ func TestMergeConfigUsingIncompleteConfigObjects(t *testing.T) {
 	// Setup subset of total environment variables for parsing with
 	// alexflint/go-arg package. These values should override baseConfig
 	// settings
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	envVarTables := []struct {
 		envVar string
 		value  string
@@ -266,14 +271,15 @@ func TestMergeConfigUsingIncompleteConfigObjects(t *testing.T) {
 	// settings that we are not overriding
 	// NOTE: Paths and FileExtensions are set below after config struct is
 	// instantiated
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	expectedPathsAfterEnvVarsMerge := []string{"/tmp/elbow/path3"}
 	expectedFileExtensionsAfterEnvVarsMerge := []string{".docx", ".pptx"}
 	expectedFilePatternAfterEnvVarsMerge := "reach-masterqa-"
 	expectedFileAgeAfterEnvVarsMerge := 3
 	expectedNumFilesToKeepAfterEnvVarsMerge := 4
 	expectedKeepOldestAfterEnvVarsMerge := false
 	expectedRemoveAfterEnvVarsMerge := true
-	expectedLogFormatAfterEnvVarsMerge := "text"
+	expectedLogFormatAfterEnvVarsMerge := logFormatText
 	expectedLogFilePathAfterEnvVarsMerge := "/var/log/elbow/env.log"
 
 	expectedRecursiveSearchAfterEnvVarsMerge := baseConfig.GetRecursiveSearch()
@@ -356,13 +362,14 @@ func TestMergeConfigUsingIncompleteConfigObjects(t *testing.T) {
 	flagsConfig.AppDescription = baseConfig.GetAppDescription()
 
 	// TODO: A useful way to automate retrieving the app name?
-	appName := "elbow"
-	if runtime.GOOS == "windows" {
-		appName += ".exe"
+	appName := strings.ToLower(defaultAppName)
+	if runtime.GOOS == windowsOSName {
+		appName += windowsAppSuffix
 	}
 
 	// Note to self: Don't add/escape double-quotes here. The shell strips
 	// them away and the application never sees them.
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	os.Args = []string{
 		appName,
 		"--pattern", "reach-master-",
@@ -402,12 +409,13 @@ func TestMergeConfigUsingIncompleteConfigObjects(t *testing.T) {
 	// settings that we are not overriding
 	// NOTE: Paths and FileExtensions are set below after config struct is
 	// instantiated
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	expectedFileExtensionsAfterFlagsMerge := []string{".java", ".class"}
 	expectedFilePatternAfterFlagsMerge := "reach-master-"
 	expectedFileAgeAfterFlagsMerge := 5
 	expectedNumFilesToKeepAfterFlagsMerge := 6
 	expectedRemoveAfterFlagsMerge := true
-	expectedLogFormatAfterFlagsMerge := "json"
+	expectedLogFormatAfterFlagsMerge := logFormatJSON
 	expectedLogLevelAfterFlagsMerge := "panic"
 	expectedConsoleOutputAfterFlagsMerge := "stderr"
 

config/validate.go

@@ -99,8 +99,8 @@ func (c Config) Validate() error {
 	switch {
 	case c.LogFormat == nil:
 		return fmt.Errorf("field LogFormat not configured")
-	case *c.LogFormat == "text":
-	case *c.LogFormat == "json":
+	case *c.LogFormat == logFormatText:
+	case *c.LogFormat == logFormatJSON:
 	default:
 		return fmt.Errorf("invalid option %q provided for log format", *c.LogFormat)
 	}
@@ -114,6 +114,7 @@ func (c Config) Validate() error {
 	switch {
 	case c.ConsoleOutput == nil:
 		return fmt.Errorf("field ConsoleOutput not configured")
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	case *c.ConsoleOutput == "stdout":
 	case *c.ConsoleOutput == "stderr":
 	default:
@@ -123,6 +124,7 @@ func (c Config) Validate() error {
 	switch {
 	case c.LogLevel == nil:
 		return fmt.Errorf("field LogLevel not configured")
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	case *c.LogLevel == "emergency":
 	case *c.LogLevel == "alert":
 	case *c.LogLevel == "critical":

config/validate_test.go

@@ -20,9 +20,7 @@ import (
 	"testing"
 )
 
-// Fix linting error
-// string `fakeValue` has 3 occurrences, make it a constant (goconst)
-const fakeValue = "fakeValue"
+// TODO: Evaluate replacing bare strings with constants (see constants.go)
 
 // This is *mostly* a default config struct with the addition of config.Paths
 // and config.FileExtensions fields set to fill out the set.

logging/logging.go

@@ -106,6 +106,7 @@ func (lb LogBuffer) Flush(logger *logrus.Logger) error {
 // logger object.
 func SetLoggerFormatter(logger *logrus.Logger, format string) error {
 	switch format {
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	case "text":
 		logger.SetFormatter(&logrus.TextFormatter{})
 	case "json":
@@ -123,6 +124,8 @@ func SetLoggerFormatter(logger *logrus.Logger, format string) error {
 func SetLoggerConsoleOutput(logger *logrus.Logger, consoleOutput string) error {
 
 	switch {
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
+	// TODO: Update switch statement to switch on consoleOutput
 	case consoleOutput == "stdout":
 		logger.SetOutput(os.Stdout)
 	case consoleOutput == "stderr":
@@ -163,6 +166,8 @@ func SetLoggerLogFile(logger *logrus.Logger, logFilePath string) (*os.File, erro
 // with a lower level than the one configured.
 func SetLoggerLevel(logger *logrus.Logger, logLevel string) error {
 
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
+
 	// https://godoc.org/github.com/sirupsen/logrus#Level
 	// https://golang.org/pkg/log/syslog/#Priority
 	// https://en.wikipedia.org/wiki/Syslog#Severity_level

logging/logging_test.go

@@ -123,6 +123,7 @@ func TestSetLoggerLevelShouldSucceed(t *testing.T) {
 		loggerLevel logrus.Level
 	}
 
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	tests := []test{
 		test{logLevel: "emerg", loggerLevel: logrus.PanicLevel},
 		test{logLevel: "panic", loggerLevel: logrus.PanicLevel},
@@ -181,6 +182,7 @@ func TestSetLoggerFormatterShouldSucceed(t *testing.T) {
 
 	logger := logrus.New()
 
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	tests := []test{
 		test{format: "text", result: nil},
 		test{format: "json", result: nil},
@@ -217,6 +219,7 @@ func TestSetLoggerConsoleOutputShouldSucceed(t *testing.T) {
 
 	logger := logrus.New()
 
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	tests := []test{
 		test{consoleOutput: "stdout", result: nil},
 		test{consoleOutput: "stderr", result: nil},

logging/logging_unix.go

@@ -44,6 +44,7 @@ func EnableSyslogLogging(logger *logrus.Logger, logBuffer *LogBuffer, logLevel s
 	var syslogLogLevel syslog.Priority
 
 	switch logLevel {
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	case "emerg", "panic":
 		// syslog: System is unusable; a panic condition.
 		// logrus: calls panic

main.go

@@ -36,6 +36,7 @@ var version = "x.y.z"
 
 // TODO: Move this elsewhere to a dedicated location.
 // TODO: Create a metadata subpackage?
+// TODO: Evaluate replacing bare strings with constants (see constants.go)
 var defaultAppName = "Elbow"
 
 func main() {

main_test.go

@@ -42,6 +42,7 @@ func TestMain(t *testing.T) {
 
 	// Note to self: Don't add/escape double-quotes here. The shell strips
 	// them away and the application never sees them.
+	// TODO: Evaluate replacing bare strings with constants (see constants.go)
 	os.Args = []string{
 		appName,
 		"--paths", "/tmp/elbow/path1",