dev: isolate printer code (#4435)

Author: ldez

pkg/commands/run.go

@@ -44,8 +44,6 @@ import (
 	"github.com/golangci/golangci-lint/pkg/timeutils"
 )
 
-const defaultFileMode = 0644
-
 const defaultTimeout = time.Minute
 
 const (
@@ -83,6 +81,8 @@ type runCommand struct {
 
 	dbManager *lintersdb.Manager
 
+	printer *printers.Printer
+
 	log        logutils.Log
 	debugf     logutils.DebugFunc
 	reportData *report.Data
@@ -184,6 +184,13 @@ func (c *runCommand) preRunE(_ *cobra.Command, _ []string) error {
 
 	c.dbManager = dbManager
 
+	printer, err := printers.NewPrinter(c.log, c.cfg, c.reportData)
+	if err != nil {
+		return err
+	}
+
+	c.printer = printer
+
 	c.goenv = goutil.NewEnv(c.log.Child(logutils.DebugKeyGoEnv))
 
 	c.fileCache = fsutils.NewFileCache()
@@ -320,20 +327,12 @@ func (c *runCommand) runAndPrint(ctx context.Context, args []string) error {
 
 	issues, err := c.runAnalysis(ctx, args)
 	if err != nil {
-		return err // XXX: don't loose type
+		return err // XXX: don't lose type
 	}
 
-	formats := strings.Split(c.cfg.Output.Format, ",")
-	for _, format := range formats {
-		out := strings.SplitN(format, ":", 2)
-		if len(out) < 2 {
-			out = append(out, "")
-		}
-
-		err := c.printReports(issues, out[1], out[0])
-		if err != nil {
-			return err
-		}
+	err = c.printer.Print(issues)
+	if err != nil {
+		return err
 	}
 
 	c.printStats(issues)
@@ -397,80 +396,6 @@ func (c *runCommand) setExitCodeIfIssuesFound(issues []result.Issue) {
 	}
 }
 
-func (c *runCommand) printReports(issues []result.Issue, path, format string) error {
-	w, shouldClose, err := c.createWriter(path)
-	if err != nil {
-		return fmt.Errorf("can't create output for %s: %w", path, err)
-	}
-
-	p, err := c.createPrinter(format, w)
-	if err != nil {
-		if file, ok := w.(io.Closer); shouldClose && ok {
-			_ = file.Close()
-		}
-		return err
-	}
-
-	if err = p.Print(issues); err != nil {
-		if file, ok := w.(io.Closer); shouldClose && ok {
-			_ = file.Close()
-		}
-		return fmt.Errorf("can't print %d issues: %w", len(issues), err)
-	}
-
-	if file, ok := w.(io.Closer); shouldClose && ok {
-		_ = file.Close()
-	}
-
-	return nil
-}
-
-func (c *runCommand) createWriter(path string) (io.Writer, bool, error) {
-	if path == "" || path == "stdout" {
-		return logutils.StdOut, false, nil
-	}
-	if path == "stderr" {
-		return logutils.StdErr, false, nil
-	}
-	f, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, defaultFileMode)
-	if err != nil {
-		return nil, false, err
-	}
-	return f, true, nil
-}
-
-func (c *runCommand) createPrinter(format string, w io.Writer) (printers.Printer, error) {
-	var p printers.Printer
-	switch format {
-	case config.OutFormatJSON:
-		p = printers.NewJSON(c.reportData, w)
-	case config.OutFormatColoredLineNumber, config.OutFormatLineNumber:
-		p = printers.NewText(c.cfg.Output.PrintIssuedLine,
-			format == config.OutFormatColoredLineNumber, c.cfg.Output.PrintLinterName,
-			c.log.Child(logutils.DebugKeyTextPrinter), w)
-	case config.OutFormatTab, config.OutFormatColoredTab:
-		p = printers.NewTab(c.cfg.Output.PrintLinterName,
-			format == config.OutFormatColoredTab,
-			c.log.Child(logutils.DebugKeyTabPrinter), w)
-	case config.OutFormatCheckstyle:
-		p = printers.NewCheckstyle(w)
-	case config.OutFormatCodeClimate:
-		p = printers.NewCodeClimate(w)
-	case config.OutFormatHTML:
-		p = printers.NewHTML(w)
-	case config.OutFormatJunitXML:
-		p = printers.NewJunitXML(w)
-	case config.OutFormatGithubActions:
-		p = printers.NewGithub(w)
-	case config.OutFormatTeamCity:
-		p = printers.NewTeamCity(w)
-	default:
-		return nil, fmt.Errorf("unknown output format %s", format)
-	}
-
-	return p, nil
-}
-
 func (c *runCommand) printStats(issues []result.Issue) {
 	if !c.cfg.Run.ShowStats {
 		return
@@ -688,7 +613,7 @@ func formatMemory(memBytes uint64) string {
 	return fmt.Sprintf("%dmb", memBytes/Mb)
 }
 
-// --- Related to cache.
+// Related to cache.
 
 func initHashSalt(version string, cfg *config.Config) error {
 	binSalt, err := computeBinarySalt(version)

pkg/printers/github.go

@@ -8,16 +8,16 @@ import (
 	"github.com/golangci/golangci-lint/pkg/result"
 )
 
-type github struct {
+type GitHub struct {
 	w io.Writer
 }
 
 const defaultGithubSeverity = "error"
 
-// NewGithub output format outputs issues according to GitHub actions format:
+// NewGitHub output format outputs issues according to GitHub actions format:
 // https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message
-func NewGithub(w io.Writer) Printer {
-	return &github{w: w}
+func NewGitHub(w io.Writer) *GitHub {
+	return &GitHub{w: w}
 }
 
 // print each line as: ::error file=app.js,line=10,col=15::Something went wrong
@@ -41,7 +41,7 @@ func formatIssueAsGithub(issue *result.Issue) string {
 	return ret
 }
 
-func (p *github) Print(issues []result.Issue) error {
+func (p *GitHub) Print(issues []result.Issue) error {
 	for ind := range issues {
 		_, err := fmt.Fprintln(p.w, formatIssueAsGithub(&issues[ind]))
 		if err != nil {

pkg/printers/github_test.go

@@ -13,7 +13,7 @@ import (
 	"github.com/golangci/golangci-lint/pkg/result"
 )
 
-func TestGithub_Print(t *testing.T) {
+func TestGitHub_Print(t *testing.T) {
 	issues := []result.Issue{
 		{
 			FromLinter: "linter-a",
@@ -45,7 +45,7 @@ func TestGithub_Print(t *testing.T) {
 	}
 
 	buf := new(bytes.Buffer)
-	printer := NewGithub(buf)
+	printer := NewGitHub(buf)
 
 	err := printer.Print(issues)
 	require.NoError(t, err)
@@ -57,7 +57,7 @@ func TestGithub_Print(t *testing.T) {
 	assert.Equal(t, expected, buf.String())
 }
 
-func TestFormatGithubIssue(t *testing.T) {
+func Test_formatIssueAsGithub(t *testing.T) {
 	sampleIssue := result.Issue{
 		FromLinter: "sample-linter",
 		Text:       "some issue",
@@ -74,7 +74,7 @@ func TestFormatGithubIssue(t *testing.T) {
 	require.Equal(t, "::error file=path/to/file.go,line=10::some issue (sample-linter)", formatIssueAsGithub(&sampleIssue))
 }
 
-func TestFormatGithubIssueWindows(t *testing.T) {
+func Test_formatIssueAsGithub_Windows(t *testing.T) {
 	if runtime.GOOS != "windows" {
 		t.Skip("Skipping test on non Windows")
 	}

pkg/printers/printer.go

@@ -1,9 +1,141 @@
 package printers
 
 import (
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+
+	"github.com/golangci/golangci-lint/pkg/config"
+	"github.com/golangci/golangci-lint/pkg/logutils"
+	"github.com/golangci/golangci-lint/pkg/report"
 	"github.com/golangci/golangci-lint/pkg/result"
 )
 
-type Printer interface {
+const defaultFileMode = 0644
+
+type issuePrinter interface {
 	Print(issues []result.Issue) error
 }
+
+// Printer prints issues
+type Printer struct {
+	cfg        *config.Config
+	reportData *report.Data
+
+	log logutils.Log
+
+	stdOut io.Writer
+	stdErr io.Writer
+}
+
+// NewPrinter creates a new Printer.
+func NewPrinter(log logutils.Log, cfg *config.Config, reportData *report.Data) (*Printer, error) {
+	if log == nil {
+		return nil, errors.New("missing log argument in constructor")
+	}
+	if cfg == nil {
+		return nil, errors.New("missing config argument in constructor")
+	}
+	if reportData == nil {
+		return nil, errors.New("missing reportData argument in constructor")
+	}
+
+	return &Printer{
+		cfg:        cfg,
+		reportData: reportData,
+		log:        log,
+		stdOut:     logutils.StdOut,
+		stdErr:     logutils.StdErr,
+	}, nil
+}
+
+// Print prints issues based on the formats defined
+func (c *Printer) Print(issues []result.Issue) error {
+	formats := strings.Split(c.cfg.Output.Format, ",")
+
+	for _, item := range formats {
+		format, path, _ := strings.Cut(item, ":")
+		err := c.printReports(issues, path, format)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func (c *Printer) printReports(issues []result.Issue, path, format string) error {
+	w, shouldClose, err := c.createWriter(path)
+	if err != nil {
+		return fmt.Errorf("can't create output for %s: %w", path, err)
+	}
+
+	defer func() {
+		if file, ok := w.(io.Closer); shouldClose && ok {
+			_ = file.Close()
+		}
+	}()
+
+	p, err := c.createPrinter(format, w)
+	if err != nil {
+		return err
+	}
+
+	if err = p.Print(issues); err != nil {
+		return fmt.Errorf("can't print %d issues: %w", len(issues), err)
+	}
+
+	return nil
+}
+
+func (c *Printer) createWriter(path string) (io.Writer, bool, error) {
+	if path == "" || path == "stdout" {
+		return c.stdOut, false, nil
+	}
+
+	if path == "stderr" {
+		return c.stdErr, false, nil
+	}
+
+	f, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, defaultFileMode)
+	if err != nil {
+		return nil, false, err
+	}
+
+	return f, true, nil
+}
+
+func (c *Printer) createPrinter(format string, w io.Writer) (issuePrinter, error) {
+	var p issuePrinter
+
+	switch format {
+	case config.OutFormatJSON:
+		p = NewJSON(c.reportData, w)
+	case config.OutFormatColoredLineNumber, config.OutFormatLineNumber:
+		p = NewText(c.cfg.Output.PrintIssuedLine,
+			format == config.OutFormatColoredLineNumber, c.cfg.Output.PrintLinterName,
+			c.log.Child(logutils.DebugKeyTextPrinter), w)
+	case config.OutFormatTab, config.OutFormatColoredTab:
+		p = NewTab(c.cfg.Output.PrintLinterName,
+			format == config.OutFormatColoredTab,
+			c.log.Child(logutils.DebugKeyTabPrinter), w)
+	case config.OutFormatCheckstyle:
+		p = NewCheckstyle(w)
+	case config.OutFormatCodeClimate:
+		p = NewCodeClimate(w)
+	case config.OutFormatHTML:
+		p = NewHTML(w)
+	case config.OutFormatJunitXML:
+		p = NewJunitXML(w)
+	case config.OutFormatGithubActions:
+		p = NewGitHub(w)
+	case config.OutFormatTeamCity:
+		p = NewTeamCity(w)
+	default:
+		return nil, fmt.Errorf("unknown output format %s", format)
+	}
+
+	return p, nil
+}