Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix golangci-lint "goconst" errors (WORKAROUND) #179

Merged
merged 1 commit into from
Dec 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 4 additions & 3 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"os"
"path"
"runtime"
"strings"
"testing"

"github.com/atc0005/elbow/units"
Expand Down Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions config/constants.go
Original file line number Diff line number Diff line change
@@ -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"
4 changes: 2 additions & 2 deletions config/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
// otherwise
func (c *Config) GetAppName() string {
if c == nil || c.AppName == "" {
return "Elbow"
return defaultAppName
}
return c.AppName
}
Expand Down Expand Up @@ -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
}
Expand Down
11 changes: 8 additions & 3 deletions config/merge_complete_configs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
20 changes: 14 additions & 6 deletions config/merge_incomplete_configs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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-",
Expand Down Expand Up @@ -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"

Expand Down
6 changes: 4 additions & 2 deletions config/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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:
Expand All @@ -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":
Expand Down
4 changes: 1 addition & 3 deletions config/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions logging/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand All @@ -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":
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions logging/logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down
1 change: 1 addition & 0 deletions logging/logging_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down