output: add colored-tab (#3729)

Author: ldez

.golangci.reference.yml

@@ -76,7 +76,7 @@ run:
 
 # output configuration options
 output:
-  # Format: colored-line-number|line-number|json|tab|checkstyle|code-climate|junit-xml|github-actions|teamcity
+  # Format: colored-line-number|line-number|json|colored-tab|tab|checkstyle|code-climate|junit-xml|github-actions|teamcity
   #
   # Multiple can be specified by separating them by comma, output can be provided
   # for each of them by separating format name and path by colon symbol.

.golangci.yml

@@ -10,7 +10,7 @@ linters-settings:
   dupl:
     threshold: 100
   funlen:
-    lines: 100
+    lines: -1 # the number of lines (code + empty lines) is not a right metric and leads to code without empty line or one-liner.
     statements: 50
   goconst:
     min-len: 2

pkg/commands/run.go

@@ -475,8 +475,10 @@ func (e *Executor) createPrinter(format string, w io.Writer) (printers.Printer,
 		p = printers.NewText(e.cfg.Output.PrintIssuedLine,
 			format == config.OutFormatColoredLineNumber, e.cfg.Output.PrintLinterName,
 			e.log.Child(logutils.DebugKeyTextPrinter), w)
-	case config.OutFormatTab:
-		p = printers.NewTab(e.cfg.Output.PrintLinterName, e.log.Child(logutils.DebugKeyTabPrinter), w)
+	case config.OutFormatTab, config.OutFormatColoredTab:
+		p = printers.NewTab(e.cfg.Output.PrintLinterName,
+			format == config.OutFormatColoredTab,
+			e.log.Child(logutils.DebugKeyTabPrinter), w)
 	case config.OutFormatCheckstyle:
 		p = printers.NewCheckstyle(w)
 	case config.OutFormatCodeClimate:

pkg/config/output.go

@@ -5,6 +5,7 @@ const (
 	OutFormatLineNumber        = "line-number"
 	OutFormatColoredLineNumber = "colored-line-number"
 	OutFormatTab               = "tab"
+	OutFormatColoredTab        = "colored-tab"
 	OutFormatCheckstyle        = "checkstyle"
 	OutFormatCodeClimate       = "code-climate"
 	OutFormatHTML              = "html"
@@ -28,11 +29,13 @@ var OutFormats = []string{
 
 type Output struct {
 	Format              string
-	Color               string
 	PrintIssuedLine     bool   `mapstructure:"print-issued-lines"`
 	PrintLinterName     bool   `mapstructure:"print-linter-name"`
 	UniqByLine          bool   `mapstructure:"uniq-by-line"`
 	SortResults         bool   `mapstructure:"sort-results"`
 	PrintWelcomeMessage bool   `mapstructure:"print-welcome"`
 	PathPrefix          string `mapstructure:"path-prefix"`
+
+	// only work with CLI flags because the setup of logs is done before the config file parsing.
+	Color string
 }

pkg/golinters/nolintlint/nolintlint_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/golangci/golangci-lint/pkg/result"
 )
 
-//nolint:funlen
 func TestLinter_Run(t *testing.T) {
 	type issueWithReplacement struct {
 		issue       string

pkg/lint/lintersdb/enabled_set_test.go

@@ -10,7 +10,6 @@ import (
 	"github.com/golangci/golangci-lint/pkg/lint/linter"
 )
 
-//nolint:funlen
 func TestGetEnabledLintersSet(t *testing.T) {
 	type cs struct {
 		cfg  config.Linters

pkg/printers/tab.go

@@ -14,20 +14,28 @@ import (
 
 type Tab struct {
 	printLinterName bool
-	log             logutils.Log
-	w               io.Writer
+	useColors       bool
+
+	log logutils.Log
+	w   io.Writer
 }
 
-func NewTab(printLinterName bool, log logutils.Log, w io.Writer) *Tab {
+func NewTab(printLinterName, useColors bool, log logutils.Log, w io.Writer) *Tab {
 	return &Tab{
 		printLinterName: printLinterName,
+		useColors:       useColors,
 		log:             log,
 		w:               w,
 	}
 }
 
 func (p *Tab) SprintfColored(ca color.Attribute, format string, args ...any) string {
 	c := color.New(ca)
+
+	if !p.useColors {
+		c.DisableColor()
+	}
+
 	return c.Sprintf(format, args...)
 }
 

pkg/printers/tab_test.go

@@ -6,6 +6,7 @@ import (
 	"go/token"
 	"testing"
 
+	"github.com/fatih/color"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -14,6 +15,13 @@ import (
 )
 
 func TestTab_Print(t *testing.T) {
+	// force color globally
+	backup := color.NoColor
+	t.Cleanup(func() {
+		color.NoColor = backup
+	})
+	color.NoColor = false
+
 	issues := []result.Issue{
 		{
 			FromLinter: "linter-a",
@@ -44,16 +52,50 @@ func TestTab_Print(t *testing.T) {
 		},
 	}
 
-	buf := new(bytes.Buffer)
+	testCases := []struct {
+		desc            string
+		printLinterName bool
+		useColors       bool
+		expected        string
+	}{
+		{
+			desc:            "with linter name",
+			printLinterName: true,
+			useColors:       false,
+			expected: `path/to/filea.go:10:4   linter-a  some issue
+path/to/fileb.go:300:9  linter-b  another issue
+`,
+		},
+		{
+			desc:            "disable all options",
+			printLinterName: false,
+			useColors:       false,
+			expected: `path/to/filea.go:10:4   some issue
+path/to/fileb.go:300:9  another issue
+`,
+		},
+		{
+			desc:            "enable all options",
+			printLinterName: true,
+			useColors:       true,
+			//nolint:lll // color characters must be in a simple string.
+			expected: "\x1b[1mpath/to/filea.go:10\x1b[0m:4   linter-a  \x1b[31msome issue\x1b[0m\n\x1b[1mpath/to/fileb.go:300\x1b[0m:9  linter-b  \x1b[31manother issue\x1b[0m\n",
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
 
-	printer := NewTab(true, logutils.NewStderrLog(logutils.DebugKeyEmpty), buf)
+			buf := new(bytes.Buffer)
 
-	err := printer.Print(context.Background(), issues)
-	require.NoError(t, err)
+			printer := NewTab(test.printLinterName, test.useColors, logutils.NewStderrLog(logutils.DebugKeyEmpty), buf)
 
-	expected := `path/to/filea.go:10:4   linter-a  some issue
-path/to/fileb.go:300:9  linter-b  another issue
-`
+			err := printer.Print(context.Background(), issues)
+			require.NoError(t, err)
 
-	assert.Equal(t, expected, buf.String())
+			assert.Equal(t, test.expected, buf.String())
+		})
+	}
 }

pkg/printers/text.go

@@ -14,8 +14,8 @@ import (
 
 type Text struct {
 	printIssuedLine bool
-	useColors       bool
 	printLinterName bool
+	useColors       bool
 
 	log logutils.Log
 	w   io.Writer
@@ -24,19 +24,20 @@ type Text struct {
 func NewText(printIssuedLine, useColors, printLinterName bool, log logutils.Log, w io.Writer) *Text {
 	return &Text{
 		printIssuedLine: printIssuedLine,
-		useColors:       useColors,
 		printLinterName: printLinterName,
+		useColors:       useColors,
 		log:             log,
 		w:               w,
 	}
 }
 
 func (p *Text) SprintfColored(ca color.Attribute, format string, args ...any) string {
+	c := color.New(ca)
+
 	if !p.useColors {
-		return fmt.Sprintf(format, args...)
+		c.DisableColor()
 	}
 
-	c := color.New(ca)
 	return c.Sprintf(format, args...)
 }
 
@@ -73,7 +74,7 @@ func (p *Text) printSourceCode(i *result.Issue) {
 	}
 }
 
-func (p Text) printUnderLinePointer(i *result.Issue) {
+func (p *Text) printUnderLinePointer(i *result.Issue) {
 	// if column == 0 it means column is unknown (e.g. for gosec)
 	if len(i.SourceLines) != 1 || i.Pos.Column == 0 {
 		return

pkg/printers/text_test.go

@@ -6,6 +6,7 @@ import (
 	"go/token"
 	"testing"
 
+	"github.com/fatih/color"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -14,6 +15,13 @@ import (
 )
 
 func TestText_Print(t *testing.T) {
+	// force color globally
+	backup := color.NoColor
+	t.Cleanup(func() {
+		color.NoColor = backup
+	})
+	color.NoColor = false
+
 	issues := []result.Issue{
 		{
 			FromLinter: "linter-a",
@@ -44,19 +52,78 @@ func TestText_Print(t *testing.T) {
 		},
 	}
 
-	buf := new(bytes.Buffer)
-
-	printer := NewText(true, false, true, logutils.NewStderrLog(logutils.DebugKeyEmpty), buf)
-
-	err := printer.Print(context.Background(), issues)
-	require.NoError(t, err)
-
-	expected := `path/to/filea.go:10:4: some issue (linter-a)
+	testCases := []struct {
+		desc            string
+		printIssuedLine bool
+		printLinterName bool
+		useColors       bool
+		expected        string
+	}{
+		{
+			desc:            "printIssuedLine and printLinterName",
+			printIssuedLine: true,
+			printLinterName: true,
+			useColors:       false,
+			expected: `path/to/filea.go:10:4: some issue (linter-a)
+path/to/fileb.go:300:9: another issue (linter-b)
+func foo() {
+	fmt.Println("bar")
+}
+`,
+		},
+		{
+			desc:            "printLinterName only",
+			printIssuedLine: false,
+			printLinterName: true,
+			useColors:       false,
+			expected: `path/to/filea.go:10:4: some issue (linter-a)
 path/to/fileb.go:300:9: another issue (linter-b)
+`,
+		},
+		{
+			desc:            "printIssuedLine only",
+			printIssuedLine: true,
+			printLinterName: false,
+			useColors:       false,
+			expected: `path/to/filea.go:10:4: some issue
+path/to/fileb.go:300:9: another issue
 func foo() {
 	fmt.Println("bar")
 }
-`
+`,
+		},
+		{
+			desc:            "enable all options",
+			printIssuedLine: true,
+			printLinterName: true,
+			useColors:       true,
+			//nolint:lll // color characters must be in a simple string.
+			expected: "\x1b[1mpath/to/filea.go:10\x1b[0m:4: \x1b[31msome issue\x1b[0m (linter-a)\n\x1b[1mpath/to/fileb.go:300\x1b[0m:9: \x1b[31manother issue\x1b[0m (linter-b)\nfunc foo() {\n\tfmt.Println(\"bar\")\n}\n",
+		},
+		{
+			desc:            "disable all options",
+			printIssuedLine: false,
+			printLinterName: false,
+			useColors:       false,
+			expected: `path/to/filea.go:10:4: some issue
+path/to/fileb.go:300:9: another issue
+`,
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			buf := new(bytes.Buffer)
 
-	assert.Equal(t, expected, buf.String())
+			printer := NewText(test.printIssuedLine, test.useColors, test.printLinterName, logutils.NewStderrLog(logutils.DebugKeyEmpty), buf)
+
+			err := printer.Print(context.Background(), issues)
+			require.NoError(t, err)
+
+			assert.Equal(t, test.expected, buf.String())
+		})
+	}
 }