From b8e53a10361ea3d8f67ab3abbb3448c0fdf8b276 Mon Sep 17 00:00:00 2001
From: Juris Bune <jbune@evolution.com>
Date: Mon, 25 Mar 2024 10:38:00 +0200
Subject: [PATCH] Enable additional linters and fix issues

Enabling following linters in golangci config:

- ireturn
- prealloc
- reassign

As part of linter issues some interfaces such as Commander and Measurer
were removed since their use is limited.

Also some minor spellings are fixed.
---
 .golangci.yaml                 | 14 ++++++++++++--
 bitrate.go                     |  7 ++-----
 common.go                      |  5 -----
 config.go                      |  5 +----
 config_test.go                 |  4 +---
 ease_test.go                   |  4 +---
 internal/analysis/plot.go      |  6 +++---
 internal/analysis/plot_test.go |  2 +-
 internal/vqm/vqm.go            | 22 +++++++---------------
 internal/vqm/vqm_test.go       | 11 +++--------
 run.go                         | 13 +++++--------
 vqmplot.go                     |  7 ++-----
 12 files changed, 38 insertions(+), 62 deletions(-)

diff --git a/.golangci.yaml b/.golangci.yaml
index 4ffa1ef..0124be9 100644
--- a/.golangci.yaml
+++ b/.golangci.yaml
@@ -7,7 +7,8 @@ linters-settings:
     min-len: 2
     min-occurrences: 3
   govet:
-    check-shadowing: true
+    shadow:
+      strict: true
   nolintlint:
     require-explanation: true
     require-specific: true
@@ -21,7 +22,13 @@ linters-settings:
       Copyright ©{{CPY_YEAR}} {{COMPANY}}. All rights reserved.
       Use of this source code is governed by a MIT-style
       license that can be found in the LICENSE file.
-
+  ireturn:
+    allow:
+      - anon
+      - error
+      - empty
+      - stdlib
+      - generic
 
 linters:
   disable-all: true
@@ -44,3 +51,6 @@ linters:
     - goheader
     - gosec
     - exportloopref
+    - ireturn
+    - prealloc
+    - reassign
diff --git a/bitrate.go b/bitrate.go
index 15b575d..1c9af4d 100644
--- a/bitrate.go
+++ b/bitrate.go
@@ -21,9 +21,6 @@ import (
 	"gonum.org/v1/plot/vg/vgimg"
 )
 
-// Make sure BitrateApp implements Commander interface.
-var _ Commander = (*BitrateApp)(nil)
-
 // BitrateApp is bitrate subcommand context that implements Commander interface.
 type BitrateApp struct {
 	// Configuration object
@@ -38,8 +35,8 @@ type BitrateApp struct {
 	gf globalFlags
 }
 
-// CreateBitrateCommand will create Commander instance from BitrateApp.
-func CreateBitrateCommand() Commander {
+// CreateBitrateCommand will instance of BitrateApp.
+func CreateBitrateCommand() *BitrateApp {
 	longHelp := `Subcommand "bitrate" will create bitrate plot for given video file.`
 	app := &BitrateApp{
 		fs: flag.NewFlagSet("bitrate", flag.ContinueOnError),
diff --git a/common.go b/common.go
index 55f7ee8..2fc6d9f 100644
--- a/common.go
+++ b/common.go
@@ -17,11 +17,6 @@ import (
 	"github.com/evolution-gaming/ease/internal/logging"
 )
 
-// Commander interface should be implemented by commands and sub-commands.
-type Commander interface {
-	Run([]string) error
-}
-
 // AppError a custom error returned from CLI application.
 //
 // AppError is handy error type envisioned to be used in CLI's main.
diff --git a/config.go b/config.go
index d619236..09e845e 100644
--- a/config.go
+++ b/config.go
@@ -237,7 +237,7 @@ func (o ConfigVal[T]) MarshalJSON() ([]byte, error) {
 	return json.Marshal(o.Value())
 }
 
-func CreateDumpConfCommand() Commander {
+func CreateDumpConfCommand() *DumpConfApp {
 	longHelp := `Command "dump-conf" will print actual application configuration taking into account
 configuration file provided and default configuration values.
 
@@ -261,9 +261,6 @@ Examples:
 
 // Also define command "dump-conf" here.
 
-// Make sure App implements Commander interface.
-var _ Commander = (*DumpConfApp)(nil)
-
 // DumpConfApp is subcommand application context that implements Commander interface.
 // Although this is very simple application, but for consistency sake is is implemented in
 // similar style as other subcommands.
diff --git a/config_test.go b/config_test.go
index c7949b2..e3f1b43 100644
--- a/config_test.go
+++ b/config_test.go
@@ -166,9 +166,7 @@ func Test_DumpConfApp_Run(t *testing.T) {
 	cmd := CreateDumpConfCommand()
 
 	// Redirect output to buffer
-	if c, ok := cmd.(*DumpConfApp); ok {
-		c.out = commandOutput
-	}
+	cmd.out = commandOutput
 
 	err := cmd.Run([]string{"-conf", confFile})
 	assert.NoError(t, err, "Unexpected error running encode")
diff --git a/ease_test.go b/ease_test.go
index cf9214c..af03084 100644
--- a/ease_test.go
+++ b/ease_test.go
@@ -102,9 +102,7 @@ func Test_RunApp_Run_FlagErrors(t *testing.T) {
 			cmd := CreateRunCommand()
 			// Discard usage output so that during test execution test output is
 			// not flooded with command Usage/Help stuff.
-			if c, ok := cmd.(*App); ok {
-				c.fs.SetOutput(io.Discard)
-			}
+			cmd.fs.SetOutput(io.Discard)
 			gotErr := cmd.Run(tc.givenArgs)
 			assert.ErrorContains(t, gotErr, tc.want)
 		})
diff --git a/internal/analysis/plot.go b/internal/analysis/plot.go
index e2f8b0c..7e8e36f 100644
--- a/internal/analysis/plot.go
+++ b/internal/analysis/plot.go
@@ -69,7 +69,7 @@ func CreateCDFPlot(values []float64, name string) (*plot.Plot, error) {
 	p.Y.Min = 0
 
 	// We are going to mutate values slice, so make a copy to avoid mangling
-	// underlying array and creating unexpected sideffect in caller's scope.
+	// underlying array and creating unexpected side-effect in caller's scope.
 	lValues := make([]float64, len(values))
 	copy(lValues, values)
 	// Make sure values are sorted
@@ -102,7 +102,7 @@ func CreateHistogramPlot(values []float64, name string) (*plot.Plot, error) {
 	p.Y.Label.Text = "N"
 
 	// We are going to mutate values slice, so make a copy to avoid mangling
-	// underlying array and creating unexpected sideffect in caller's scope.
+	// underlying array and creating unexpected side-effect in caller's scope.
 	lValues := make([]float64, len(values))
 	copy(lValues, values)
 
@@ -505,7 +505,7 @@ func horizontalLineWithLabel(y, xMin, xMax float64, label string) (*plotter.Line
 
 // createQuantileLines is helper to create vertical Quantile lines.
 func createQuantileLines(p *plot.Plot, values []float64, quantiles ...float64) []plot.Plotter {
-	var plotters []plot.Plotter
+	plotters := make([]plot.Plotter, 0, len(quantiles))
 	colorCount := len(ColorPalette)
 	for i, q := range quantiles {
 		qVal := stat.Quantile(q, stat.Empirical, values, nil)
diff --git a/internal/analysis/plot_test.go b/internal/analysis/plot_test.go
index 1ab70ef..279f795 100644
--- a/internal/analysis/plot_test.go
+++ b/internal/analysis/plot_test.go
@@ -22,7 +22,6 @@ var frameMetricsFile = "../../testdata/vqm/frame_metrics.json"
 
 // getVmafValues fixture provides slice of VMAF metrics.
 func getVmafValues(t *testing.T) []float64 {
-	var values []float64
 	var metrics vqm.FrameMetrics
 
 	j, err := os.Open(frameMetricsFile)
@@ -32,6 +31,7 @@ func getVmafValues(t *testing.T) []float64 {
 	err2 := json.NewDecoder(j).Decode(&metrics)
 	require.NoError(t, err2, "Error Unmarshaling metrics")
 
+	values := make([]float64, 0, len(metrics))
 	for _, v := range metrics {
 		values = append(values, v.VMAF)
 	}
diff --git a/internal/vqm/vqm.go b/internal/vqm/vqm.go
index 74dbdd1..554f4b0 100644
--- a/internal/vqm/vqm.go
+++ b/internal/vqm/vqm.go
@@ -28,14 +28,6 @@ var DefaultFfmpegVMAFTemplate = "-hide_banner -i {{.CompressedFile}} -i {{.Sourc
 	"-lavfi libvmaf=n_subsample=1:log_path={{.ResultFile}}:feature=name=psnr:" +
 	"log_fmt=json:model=path={{.ModelPath}}:n_threads={{.NThreads}} -f null -"
 
-// Measurer is an interface that must be implemented by VQM tool which is capable of
-// calculating Vide Quality Metrics.
-type Measurer interface {
-	// Measure should run actual VQM measuring process
-	Measure() error
-	GetMetrics() (*AggregateMetric, error)
-}
-
 // FfmpegVMAFConfig exposes parameters for ffmpegVMAF creation.
 type FfmpegVMAFConfig struct {
 	FfmpegPath         string
@@ -45,8 +37,8 @@ type FfmpegVMAFConfig struct {
 }
 
 // NewFfmpegVMAF will initialize VQM Measurer based on ffmpeg and libvmaf.
-func NewFfmpegVMAF(cfg *FfmpegVMAFConfig, compressedFile, sourceFile string) (Measurer, error) {
-	var vqt *ffmpegVMAF
+func NewFfmpegVMAF(cfg *FfmpegVMAFConfig, compressedFile, sourceFile string) (*FfmpegVMAF, error) {
+	var vqt *FfmpegVMAF
 
 	// Too much CPU threads are also bad. This was an issue on 128 threaded AMD
 	// EPYC, ffmpeg was deadlocking at some point during VMAF calculations.
@@ -82,7 +74,7 @@ func NewFfmpegVMAF(cfg *FfmpegVMAFConfig, compressedFile, sourceFile string) (Me
 		return vqt, fmt.Errorf("NewFfmpegVMAF() prepare command: %w", err)
 	}
 
-	vqt = &ffmpegVMAF{
+	vqt = &FfmpegVMAF{
 		exePath:        cfg.FfmpegPath,
 		ffmpegArgs:     ffmpegArgs,
 		sourceFile:     sourceFile,
@@ -95,8 +87,8 @@ func NewFfmpegVMAF(cfg *FfmpegVMAFConfig, compressedFile, sourceFile string) (Me
 	return vqt, nil
 }
 
-// ffmpegVMAF defines VQM tool and implements Measurer interface.
-type ffmpegVMAF struct {
+// FfmpegVMAF defines VQM tool and implements Measurer interface.
+type FfmpegVMAF struct {
 	// Path to ffmpeg executable
 	exePath string
 	// ffmpeg command arguments
@@ -111,7 +103,7 @@ type ffmpegVMAF struct {
 	measured   bool
 }
 
-func (f *ffmpegVMAF) Measure() error {
+func (f *FfmpegVMAF) Measure() error {
 	var err error
 
 	if f.measured {
@@ -160,7 +152,7 @@ type Metric struct {
 	Variance     float64
 }
 
-func (f *ffmpegVMAF) GetMetrics() (*AggregateMetric, error) {
+func (f *FfmpegVMAF) GetMetrics() (*AggregateMetric, error) {
 	if !f.measured {
 		return nil, errors.New("GetMetrics() depends on Measure() called first")
 	}
diff --git a/internal/vqm/vqm_test.go b/internal/vqm/vqm_test.go
index 1dd428f..706935b 100644
--- a/internal/vqm/vqm_test.go
+++ b/internal/vqm/vqm_test.go
@@ -24,13 +24,8 @@ import (
 //	go test -run ^TestFfmpegVMAF ./internal/vqm -save-result
 var saveResultFile = flag.Bool("save-result", false, "Save result file")
 
-func TestFfmpegVMAFImplementsMeasurer(t *testing.T) {
-	// Test that tool implements Measurer interface.
-	var _ Measurer = &ffmpegVMAF{}
-}
-
 func TestFfmpegVMAF(t *testing.T) {
-	var tool Measurer // tool under test
+	var tool *FfmpegVMAF // tool under test
 	var aggMetrics *AggregateMetric
 
 	wrkDir := t.TempDir()
@@ -106,7 +101,7 @@ func TestFfmpegVMAF_Negative(t *testing.T) {
 	libvmafModelPath, _ := tools.FindLibvmafModel()
 
 	// Valid tool fixture.
-	getValidTool := func() Measurer {
+	getValidTool := func() *FfmpegVMAF {
 		srcFile := "../../testdata/video/testsrc01.mp4"
 		compressedFile := "../../testdata/video/testsrc01.mp4"
 		resultFile := t.TempDir() + "/result.json"
@@ -123,7 +118,7 @@ func TestFfmpegVMAF_Negative(t *testing.T) {
 	}
 
 	// Invalid tool fixture.
-	getInvalidTool := func() Measurer {
+	getInvalidTool := func() *FfmpegVMAF {
 		srcFile := "nonexistent-source"
 		compressedFile := "non-existent-compressed"
 		resultFile := t.TempDir() + "/result.json"
diff --git a/run.go b/run.go
index 000f1f2..cacca44 100644
--- a/run.go
+++ b/run.go
@@ -24,8 +24,8 @@ import (
 	"github.com/jszwec/csvutil"
 )
 
-// CreateRunCommand will create Commander instance from App.
-func CreateRunCommand() Commander {
+// CreateRunCommand will create instance of App.
+func CreateRunCommand() *App {
 	longHelp := `Subcommand "run" will execute encoding plan according to definition in file
 provided as parameter to -plan flag and will calculate and report VQM
 metrics. This flag is mandatory.
@@ -50,9 +50,6 @@ Examples:
 	return app
 }
 
-// Make sure App implements Commander interface.
-var _ Commander = (*App)(nil)
-
 // App is subcommand application context that implements Commander interface.
 type App struct {
 	// Configuration object
@@ -324,9 +321,9 @@ func (a *App) analyse() error {
 
 // saveReport writes recorded metrics to report file.
 func (a *App) saveReport() error {
-	var report []metric.Record
-
-	for _, id := range a.mStore.GetIDs() {
+	ids := a.mStore.GetIDs()
+	report := make([]metric.Record, 0, len(ids))
+	for _, id := range ids {
 		r, err := a.mStore.Get(id)
 		if err != nil {
 			return fmt.Errorf("getting record (id=%v) from metric store: %w", id, err)
diff --git a/vqmplot.go b/vqmplot.go
index 4a24aac..35f196f 100644
--- a/vqmplot.go
+++ b/vqmplot.go
@@ -20,8 +20,8 @@ import (
 // Support these metrics for plotting.
 var supportedMetrics = "VMAF, PSNR, MS-SSIM"
 
-// CreateVQMPlotCommand will create Commander instance from VQMPlotApp.
-func CreateVQMPlotCommand() Commander {
+// CreateVQMPlotCommand will create instance of VQMPlotApp.
+func CreateVQMPlotCommand() *VQMPlotApp {
 	longHelp := `Subcommand "vqmplot" will create plot for given metric from JSON report as
 generated by libvmaf.
 
@@ -46,9 +46,6 @@ Examples:
 	return app
 }
 
-// Make sure VQMPlotApp implements Commander interface.
-var _ Commander = (*VQMPlotApp)(nil)
-
 // VQMPlotApp is vqmplot subcommand context that implements Commander interface.
 type VQMPlotApp struct {
 	// FlagSet instance