Skip to content

dev: isolate printer code #4435

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

Merged
merged 7 commits into from
Mar 3, 2024
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
103 changes: 14 additions & 89 deletions pkg/commands/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ import (
"github.com/golangci/golangci-lint/pkg/timeutils"
)

const defaultFileMode = 0644

const defaultTimeout = time.Minute

const (
Expand Down Expand Up @@ -82,6 +80,8 @@ type runCommand struct {

dbManager *lintersdb.Manager

printer *printers.Printer

log logutils.Log
debugf logutils.DebugFunc
reportData *report.Data
Expand Down Expand Up @@ -178,6 +178,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()
Expand Down Expand Up @@ -314,20 +321,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)
Expand Down Expand Up @@ -391,80 +390,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
Expand Down Expand Up @@ -682,7 +607,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)
Expand Down
10 changes: 5 additions & 5 deletions pkg/printers/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions pkg/printers/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand All @@ -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",
Expand All @@ -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")
}
Expand Down
134 changes: 133 additions & 1 deletion pkg/printers/printer.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading