From b3d39cffc19ed93ebe7624e91c4abfbf648562b6 Mon Sep 17 00:00:00 2001 From: Sherif Abdel-Naby Date: Sat, 10 Apr 2021 11:07:02 +0200 Subject: [PATCH] Add Linters and Lint Code Signed-off-by: Sherif Abdel-Naby --- .editorconfig | 18 ++++++ .gitignore | 25 +++++++- .golangci.yml | 160 ++++++++++++++++++++++++++++++++++++++++++++++++++ go.sum | 2 + job/job.go | 3 +- jobmap.go | 11 ++-- log.go | 2 +- metric.go | 3 +- options.go | 3 +- schedule.go | 33 ++++++----- scheduler.go | 22 ++++--- timer.go | 7 ++- 12 files changed, 247 insertions(+), 42 deletions(-) create mode 100755 .editorconfig create mode 100644 .golangci.yml diff --git a/.editorconfig b/.editorconfig new file mode 100755 index 0000000..e9a14ca --- /dev/null +++ b/.editorconfig @@ -0,0 +1,18 @@ +root = true + +[*] +charset = utf-8 +end_of_line = lf +indent_size = 4 +indent_style = space +insert_final_newline = true +trim_trailing_whitespace = true + +[*.go] +indent_style = tab + +[{Makefile, *.mk}] +indent_style = tab + +[*.proto] +indent_size = 2 diff --git a/.gitignore b/.gitignore index 66fd13c..7005456 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,24 @@ +### General +bin +.env + +### JetBrains template +# User-specific stuff +.idea + + +### Windows template +# Windows thumbnail cache files +Thumbs.db + +### macOS template +# General +.DS_Store + +# Thumbnails +._* + +### Go template # Binaries for programs and plugins *.exe *.exe~ @@ -11,5 +32,5 @@ # Output of the go coverage tool, specifically when used with LiteIDE *.out -# Dependency directories (remove the comment below to include it) -# vendor/ +### VisualStudioCode template +.vscode/* diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..14bb505 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,160 @@ +# options for analysis running +run: + # default concurrency is a available CPU number + concurrency: 4 + + # timeout for analysis, e.g. 30s, 5m, default is 1m + deadline: 20m + + # exit code when at least one issue was found, default is 1 + issues-exit-code: 1 + + # include test files or not, default is true + tests: false + +# output configuration options +output: + # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number" + format: tab + + # print lines of code with issue, default is true + print-issued-lines: true + + # print linter name in the end of issue text, default is true + print-linter-name: true + +# all available settings of specific linters +linters-settings: + errcheck: + # report about not checking of errors in type assetions: `a := b.(MyStruct)`; + # default is false: such cases aren't reported by default. + check-type-assertions: true + + # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`; + # default is false: such cases aren't reported by default. + check-blank: false + + # [deprecated] comma-separated list of pairs of the form pkg:regex + # the regex is used to ignore names within pkg. (default "fmt:.*"). + # see https://github.com/kisielk/errcheck#the-deprecated-method for details + ignore: fmt:.*,io/ioutil:^Read.* + + # path to a file containing a list of functions to exclude from checking + # see https://github.com/kisielk/errcheck#excluding-functions for details + # exclude: /path/to/file.txt + govet: + # report about shadowed variables + check-shadowing: true + + # settings per analyzer + settings: + printf: # analyzer name, run `go tool vet help` to see all analyzers + funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf + golint: + # minimal confidence for issues, default is 0.8 + min-confidence: 0.8 + gofmt: + # simplify code: gofmt with `-s` option, true by default + simplify: true + goimports: + # put imports beginning with prefix after 3rd-party packages; + # it's a comma-separated list of prefixes + local-prefixes: github.com/palletone/go-palletone + gocyclo: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 10 + maligned: + # print struct with more effective memory layout or not, false by default + suggest-new: true + dupl: + # tokens count to trigger issue, 150 by default + threshold: 150 + goconst: + # minimal length of string constant, 3 by default + min-len: 3 + # minimal occurrences count to trigger, 3 by default + min-occurrences: 3 + depguard: + list-type: blacklist + include-go-root: false + packages: + - github.com/davecgh/go-spew/spew + misspell: + # Correct spellings using locale preferences for US or UK. + # Default is to use a neutral variety of English. + # Setting locale to US will correct the British spelling of 'colour' to 'color'. + locale: US + ignore-words: + - someword + lll: + # max line length, lines longer will be reported. Default is 120. + # '\t' is counted as 1 character by default, and can be changed with the tab-width option + line-length: 120 + # tab width in spaces. Default to 1. + tab-width: 1 + unused: + # treat code as a program (not a library) and report unused exported identifiers; default is false. + # XXX: if you enable this setting, unused will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find funcs usages. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false + unparam: + # Inspect exported functions, default is false. Set to true if no external program/library imports your code. + # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find external interfaces. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false + nakedret: + # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 + max-func-lines: 30 + prealloc: + # XXX: we don't recommend using this linter before doing performance profiling. + # For most programs usage of prealloc will be a premature optimization. + + # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them. + # True by default. + simple: true + range-loops: true # Report preallocation suggestions on range loops, true by default + for-loops: false # Report preallocation suggestions on for loops, false by default + +linters: + enable: + - deadcode + - govet + - ineffassign + - staticcheck + - structcheck + - typecheck + - unused + - varcheck + - bodyclose + - depguard + - dupl + - govet + - gochecknoinits + - unused + - whitespace + - goconst + - gofmt + - goimports + - gosec + - misspell + - nakedret + - prealloc + - unconvert + - wrapcheck + - govet + - gomodguard + - goconst + - gocyclo + # - unparam + enable-all: false + disable-all: true + # presets: + # - bugs + # - unused + fast: false diff --git a/go.sum b/go.sum index 8949aeb..2a64889 100644 --- a/go.sum +++ b/go.sum @@ -8,6 +8,7 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw= github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= +github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.2.0 h1:qJYtXnJRWmpe7m/3XlyhrsLrEURqHRM2kxzoxXqyUDs= @@ -65,6 +66,7 @@ golang.org/x/tools v0.0.0-20191029041327-9cc4af7d6b2c/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20191029190741-b9c20aec41a5 h1:hKsoRgsbwY1NafxrwTs+k64bikrLBkAgPir1TNCj3Zs= golang.org/x/tools v0.0.0-20191029190741-b9c20aec41a5/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0 h1:bxAC2xTBsZGibn2RTntX0oH50xLsqy1OxA9tTL3p/lk= diff --git a/job/job.go b/job/job.go index ac1d6e1..08fecfc 100644 --- a/job/job.go +++ b/job/job.go @@ -2,9 +2,10 @@ package job import ( "fmt" - "github.com/google/uuid" "sync" "time" + + "github.com/google/uuid" ) type Job struct { diff --git a/jobmap.go b/jobmap.go index 39f5ec3..b65aaef 100644 --- a/jobmap.go +++ b/jobmap.go @@ -1,8 +1,9 @@ package sched import ( - "github.com/sherifabdlnaby/sched/job" "sync" + + "github.com/sherifabdlnaby/sched/job" ) type jobMap struct { @@ -16,16 +17,16 @@ func newJobMap() *jobMap { } } -func (jm *jobMap) add(job *job.Job) { +func (jm *jobMap) add(j *job.Job) { jm.mx.Lock() defer jm.mx.Unlock() - jm.jobs[job.ID()] = job + jm.jobs[j.ID()] = j } -func (jm *jobMap) delete(job *job.Job) { +func (jm *jobMap) delete(j *job.Job) { jm.mx.Lock() defer jm.mx.Unlock() - delete(jm.jobs, job.ID()) + delete(jm.jobs, j.ID()) } func (jm *jobMap) len() int { diff --git a/log.go b/log.go index e593e01..2926da5 100644 --- a/log.go +++ b/log.go @@ -4,7 +4,7 @@ import ( "go.uber.org/zap" ) -//Logger Sched logging interface similar to uber-go/zap, while keeping the option to change the logging implementation +// Logger Sched logging interface similar to uber-go/zap, while keeping the option to change the logging implementation // It is a sub-interface of uber-go/zap SugaredLogger. type Logger interface { Debugw(msg string, keysAndValues ...interface{}) diff --git a/metric.go b/metric.go index 20c6aeb..e6642a0 100644 --- a/metric.go +++ b/metric.go @@ -2,8 +2,9 @@ package sched import ( "fmt" - "github.com/uber-go/tally" "time" + + "github.com/uber-go/tally" ) type metrics struct { diff --git a/options.go b/options.go index d249bb9..eea6738 100644 --- a/options.go +++ b/options.go @@ -1,8 +1,9 @@ package sched import ( - "github.com/uber-go/tally" "time" + + "github.com/uber-go/tally" ) type options struct { diff --git a/schedule.go b/schedule.go index 1814126..8834f84 100644 --- a/schedule.go +++ b/schedule.go @@ -1,10 +1,11 @@ package sched import ( - "github.com/sherifabdlnaby/sched/job" - "github.com/uber-go/tally" "sync" "time" + + "github.com/sherifabdlnaby/sched/job" + "github.com/uber-go/tally" ) type Schedule struct { @@ -39,7 +40,7 @@ type Schedule struct { } // NewSchedule NewSchedule -func NewSchedule(ID string, timer Timer, jobFunc func(), opts ...Option) *Schedule { +func NewSchedule(id string, timer Timer, jobFunc func(), opts ...Option) *Schedule { var options = defaultOptions() // Apply Options @@ -47,11 +48,8 @@ func NewSchedule(ID string, timer Timer, jobFunc func(), opts ...Option) *Schedu option.apply(options) } - // Set ID - id := ID - // Set Logger - logger := options.logger.With("ID", id) + logger := options.logger.With("id", id) // Set Metrics // // Init Default Scope if true, ignore io.closer on purpose. @@ -60,7 +58,7 @@ func NewSchedule(ID string, timer Timer, jobFunc func(), opts ...Option) *Schedu Reporter: newConsoleStatsReporter(logger.Named("metrics")), }, options.defaultScopePrintEvery) } - metrics := *newMetrics(ID, options.metricsScope) + metrics := *newMetrics(id, options.metricsScope) return &Schedule{ ID: id, @@ -118,11 +116,10 @@ func (s *Schedule) Stop() { s.state = STOPPED s.logger.Infow("Job Schedule Stopped") s.metrics.up.Update(0) - s.logger.Sync() + _ = s.logger.Sync() } func (s *Schedule) Finish() { - // Stop First s.Stop() @@ -137,7 +134,7 @@ func (s *Schedule) Finish() { s.logger.Infow("Job Schedule Finished") } -//scheduleLoop scheduler control loop +// scheduleLoop scheduler control loop func (s *Schedule) scheduleLoop() { // Main Loop for { @@ -146,7 +143,8 @@ func (s *Schedule) scheduleLoop() { s.logger.Infow("No more Jobs will be scheduled") break } - nextRunDuration := zeroNegativeDuration(nextRun.Sub(time.Now())) + nextRunDuration := time.Until(nextRun) + nextRunDuration = negativeToZero(nextRunDuration) nextRunChan := time.After(nextRunDuration) s.logger.Infow("Job Next Run Scheduled", "After", nextRunDuration.Round(1*time.Second).String(), "At", nextRun.Format(time.RFC3339)) select { @@ -174,7 +172,6 @@ func (s *Schedule) runJobInstance() { defer s.activeJobs.delete(jobInstance) s.logger.Infow("Job Run Starting", "Instance", jobInstance.ID()) - s.logger.Sync() s.metrics.runs.Inc(1) if s.activeJobs.len() > 1 { s.metrics.overlappingCount.Inc(1) @@ -184,15 +181,19 @@ func (s *Schedule) runJobInstance() { err := jobInstance.Run() if err != nil { - s.logger.Errorw("Job Error", "Instance", jobInstance.ID(), "Duration", jobInstance.ActualElapsed().Round(1*time.Millisecond), "State", jobInstance.State().String(), "error", err.Error()) + s.logger.Errorw("Job Error", "Instance", jobInstance.ID(), + "Duration", jobInstance.ActualElapsed().Round(1*time.Millisecond), + "State", jobInstance.State().String(), "error", err.Error()) s.metrics.runErrors.Inc(1) } - s.logger.Infow("Job Finished", "Instance", jobInstance.ID(), "Duration", jobInstance.ActualElapsed().Round(1*time.Millisecond), "State", jobInstance.State().String()) + s.logger.Infow("Job Finished", "Instance", jobInstance.ID(), + "Duration", jobInstance.ActualElapsed().Round(1*time.Millisecond), + "State", jobInstance.State().String()) s.metrics.runActualElapsed.Record(jobInstance.ActualElapsed()) s.metrics.runTotalElapsed.Record(jobInstance.TotalElapsed()) } -func zeroNegativeDuration(nextRunDuration time.Duration) time.Duration { +func negativeToZero(nextRunDuration time.Duration) time.Duration { if nextRunDuration < 0 { nextRunDuration = 0 } diff --git a/scheduler.go b/scheduler.go index 418b8d8..b30c22f 100644 --- a/scheduler.go +++ b/scheduler.go @@ -18,25 +18,25 @@ func NewScheduler(opts ...Option) *Scheduler { } } -func (s *Scheduler) Add(ID string, timer Timer, job func()) { +func (s *Scheduler) Add(id string, timer Timer, job func()) { s.mx.Lock() defer s.mx.Unlock() // Create schedule - schedule := NewSchedule(ID, timer, job, s.scheduleOpts...) + schedule := NewSchedule(id, timer, job, s.scheduleOpts...) // Add to managed schedules - s.schedules[ID] = schedule + s.schedules[id] = schedule } -func (s *Scheduler) Start(ID string) error { +func (s *Scheduler) Start(id string) error { s.mx.Lock() defer s.mx.Unlock() - // Find Schedule by ID - schedule, found := s.schedules[ID] + // Find Schedule by id + schedule, found := s.schedules[id] if !found { - return fmt.Errorf("schdule with this ID does not exit") + return fmt.Errorf("schdule with this id does not exit") } // Start it ¯\_(ツ)_/¯ @@ -51,15 +51,14 @@ func (s *Scheduler) StartAll() { for _, schedule := range s.schedules { schedule.Start() } - return } -func (s *Scheduler) Stop(ID string) error { +func (s *Scheduler) Stop(id string) error { s.mx.Lock() defer s.mx.Unlock() - schedule, found := s.schedules[ID] + schedule, found := s.schedules[id] if !found { - return fmt.Errorf("schdule with this ID does not exit") + return fmt.Errorf("schdule with this id does not exit") } schedule.Stop() return nil @@ -77,5 +76,4 @@ func (s *Scheduler) StopAll() { }(schedule) } wg.Wait() - return } diff --git a/timer.go b/timer.go index ae06dd5..af7384a 100644 --- a/timer.go +++ b/timer.go @@ -2,8 +2,9 @@ package sched import ( "fmt" - "github.com/gorhill/cronexpr" "time" + + "github.com/gorhill/cronexpr" ) type Timer interface { @@ -25,7 +26,7 @@ func NewOnce(delay time.Duration) (*Once, error) { } func NewOnceTime(t time.Time) (*Once, error) { - remaining := t.Sub(time.Now()) + remaining := time.Until(t) if remaining < 0 { return &Once{ delay: remaining, @@ -75,7 +76,7 @@ type Cron struct { func NewCron(cronExpression string) (*Cron, error) { expression, err := cronexpr.Parse(cronExpression) if err != nil { - return nil, err + return nil, fmt.Errorf("cron expression invalid: %w", err) } return &Cron{expression: *expression}, nil }