Skip to content

Commit

Permalink
feat: new output.formats file configuration syntax (#4521)
Browse files Browse the repository at this point in the history
  • Loading branch information
ldez committed Mar 18, 2024
1 parent b91c194 commit e3ed3ba
Show file tree
Hide file tree
Showing 8 changed files with 258 additions and 51 deletions.
24 changes: 17 additions & 7 deletions .golangci.next.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,25 @@ run:

# output configuration options
output:
# 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.
# The formats used to render issues.
# Format: `colored-line-number`, `line-number`, `json`, `colored-tab`, `tab`, `checkstyle`, `code-climate`, `junit-xml`, `github-actions`, `teamcity`
# Output path can be either `stdout`, `stderr` or path to the file to write to.
# Example: "checkstyle:report.xml,json:stdout,colored-line-number"
#
# Default: colored-line-number
format: json
# For the CLI flag (`--out-format`), multiple formats can be specified by separating them by comma.
# The output can be specified for each of them by separating format name and path by colon symbol.
# Example: "--out-format=checkstyle:report.xml,json:stdout,colored-line-number"
# The CLI flag (`--out-format`) override the configuration file.
#
# Default:
# formats:
# - format: colored-line-number
# path: stdout
formats:
- format: json
path: stderr
- format: checkstyle
path: report.xml
- format: colored-line-number

# Print lines of code with issue.
# Default: true
Expand Down
45 changes: 36 additions & 9 deletions jsonschema/golangci.next.jsonschema.json
Original file line number Diff line number Diff line change
Expand Up @@ -451,15 +451,42 @@
"type": "object",
"additionalProperties": false,
"properties": {
"format": {
"description": "Output format to use.",
"pattern": "^(,?(colored-line-number|line-number|json|colored-tab|tab|checkstyle|code-climate|junit-xml|github-actions|teamcity)(:[^,]+)?)+$",
"default": "colored-line-number",
"examples": [
"colored-line-number",
"checkstyle:report.json,colored-line-number",
"line-number:golangci-lint.out,colored-line-number:stdout"
]
"formats": {
"description": "Output formats to use.",
"type": "array",
"items": {
"type": "object",
"additionalProperties": false,
"properties": {
"path": {
"default": "stdout",
"anyOf": [
{
"enum": [ "stdout", "stderr" ]
},
{
"type": "string"
}
]
},
"format": {
"default": "colored-line-number",
"enum": [
"colored-line-number",
"line-number",
"json",
"colored-tab",
"tab",
"checkstyle",
"code-climate",
"junit-xml",
"github-actions",
"teamcity"
]
}
},
"required": ["format"]
}
},
"print-issued-lines": {
"description": "Print lines of code with issue.",
Expand Down
4 changes: 2 additions & 2 deletions pkg/commands/flagsets.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ func setupRunFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
}

func setupOutputFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
internal.AddFlagAndBind(v, fs, fs.String, "out-format", "output.format", config.OutFormatColoredLineNumber,
color.GreenString(fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|"))))
internal.AddFlagAndBind(v, fs, fs.String, "out-format", "output.formats", config.OutFormatColoredLineNumber,
color.GreenString(fmt.Sprintf("Formats of output: %s", strings.Join(config.AllOutputFormats, "|"))))
internal.AddFlagAndBind(v, fs, fs.Bool, "print-issued-lines", "output.print-issued-lines", true,
color.GreenString("Print lines of code with issue"))
internal.AddFlagAndBind(v, fs, fs.Bool, "print-linter-name", "output.print-linter-name", true,
Expand Down
34 changes: 28 additions & 6 deletions pkg/config/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ func (l *Loader) Load() error {

l.handleGoVersion()

l.handleDeprecation()
err = l.handleDeprecation()
if err != nil {
return err
}

err = l.handleEnableOnlyOption()
if err != nil {
Expand Down Expand Up @@ -164,7 +167,7 @@ func (l *Loader) parseConfig() error {
var configFileNotFoundError viper.ConfigFileNotFoundError
if errors.As(err, &configFileNotFoundError) {
// Load configuration from flags only.
err = l.viper.Unmarshal(l.cfg)
err = l.viper.Unmarshal(l.cfg, customDecoderHook())
if err != nil {
return fmt.Errorf("can't unmarshal config by viper (flags): %w", err)
}
Expand All @@ -181,7 +184,7 @@ func (l *Loader) parseConfig() error {
}

// Load configuration from all sources (flags, file).
if err := l.viper.Unmarshal(l.cfg, fileDecoderHook()); err != nil {
if err := l.viper.Unmarshal(l.cfg, customDecoderHook()); err != nil {
return fmt.Errorf("can't unmarshal config by viper (flags, file): %w", err)
}

Expand Down Expand Up @@ -279,28 +282,47 @@ func (l *Loader) handleGoVersion() {
}
}

func (l *Loader) handleDeprecation() {
func (l *Loader) handleDeprecation() error {
// Deprecated since v1.57.0
if len(l.cfg.Run.SkipFiles) > 0 {
l.warn("The configuration option `run.skip-files` is deprecated, please use `issues.exclude-files`.")
l.cfg.Issues.ExcludeFiles = l.cfg.Run.SkipFiles
}

// Deprecated since v1.57.0
if len(l.cfg.Run.SkipDirs) > 0 {
l.warn("The configuration option `run.skip-dirs` is deprecated, please use `issues.exclude-dirs`.")
l.cfg.Issues.ExcludeDirs = l.cfg.Run.SkipDirs
}

// The 2 options are true by default.
// Deprecated since v1.57.0
if !l.cfg.Run.UseDefaultSkipDirs {
l.warn("The configuration option `run.skip-dirs-use-default` is deprecated, please use `issues.exclude-dirs-use-default`.")
}
l.cfg.Issues.UseDefaultExcludeDirs = l.cfg.Run.UseDefaultSkipDirs && l.cfg.Issues.UseDefaultExcludeDirs

// The 2 options are false by default.
// Deprecated since v1.57.0
if l.cfg.Run.ShowStats {
l.warn("The configuration option `run.show-stats` is deprecated, please use `output.show-stats`")
}
l.cfg.Output.ShowStats = l.cfg.Run.ShowStats || l.cfg.Output.ShowStats

// Deprecated since v1.57.0
if l.cfg.Output.Format != "" {
l.warn("The configuration option `output.format` is deprecated, please use `output.formats`")

var f OutputFormats
err := f.UnmarshalText([]byte(l.cfg.Output.Format))
if err != nil {
return fmt.Errorf("unmarshal output format: %w", err)
}

l.cfg.Output.Formats = f
}

return nil
}

func (l *Loader) handleEnableOnlyOption() error {
Expand Down Expand Up @@ -332,13 +354,13 @@ func (l *Loader) warn(format string) {
l.log.Warnf(format)
}

func fileDecoderHook() viper.DecoderConfigOption {
func customDecoderHook() viper.DecoderConfigOption {
return viper.DecodeHook(mapstructure.ComposeDecodeHookFunc(
// Default hooks (https://github.com/spf13/viper/blob/518241257478c557633ab36e474dfcaeb9a3c623/viper.go#L135-L138).
mapstructure.StringToTimeDurationHookFunc(),
mapstructure.StringToSliceHookFunc(","),

// Needed for forbidigo.
// Needed for forbidigo, and output.formats.
mapstructure.TextUnmarshallerHookFunc(),
))
}
Expand Down
62 changes: 53 additions & 9 deletions pkg/config/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const (
OutFormatTeamCity = "teamcity"
)

var OutFormats = []string{
var AllOutputFormats = []string{
OutFormatColoredLineNumber,
OutFormatLineNumber,
OutFormatJSON,
Expand All @@ -35,14 +35,17 @@ var OutFormats = []string{
}

type Output struct {
Format string `mapstructure:"format"`
PrintIssuedLine bool `mapstructure:"print-issued-lines"`
PrintLinterName bool `mapstructure:"print-linter-name"`
UniqByLine bool `mapstructure:"uniq-by-line"`
SortResults bool `mapstructure:"sort-results"`
SortOrder []string `mapstructure:"sort-order"`
PathPrefix string `mapstructure:"path-prefix"`
ShowStats bool `mapstructure:"show-stats"`
Formats OutputFormats `mapstructure:"formats"`
PrintIssuedLine bool `mapstructure:"print-issued-lines"`
PrintLinterName bool `mapstructure:"print-linter-name"`
UniqByLine bool `mapstructure:"uniq-by-line"`
SortResults bool `mapstructure:"sort-results"`
SortOrder []string `mapstructure:"sort-order"`
PathPrefix string `mapstructure:"path-prefix"`
ShowStats bool `mapstructure:"show-stats"`

// Deprecated: use Formats instead.
Format string `mapstructure:"format"`
}

func (o *Output) Validate() error {
Expand All @@ -64,5 +67,46 @@ func (o *Output) Validate() error {
}
}

for _, format := range o.Formats {
err := format.Validate()
if err != nil {
return err
}
}

return nil
}

type OutputFormat struct {
Format string `mapstructure:"format"`
Path string `mapstructure:"path"`
}

func (o *OutputFormat) Validate() error {
if o.Format == "" {
return errors.New("the format is required")
}

if !slices.Contains(AllOutputFormats, o.Format) {
return fmt.Errorf("unsupported output format %q", o.Format)
}

return nil
}

type OutputFormats []OutputFormat

func (p *OutputFormats) UnmarshalText(text []byte) error {
formats := strings.Split(string(text), ",")

for _, item := range formats {
format, path, _ := strings.Cut(item, ":")

*p = append(*p, OutputFormat{
Path: path,
Format: format,
})
}

return nil
}
80 changes: 80 additions & 0 deletions pkg/config/output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,86 @@ func TestOutput_Validate_error(t *testing.T) {
},
expected: `the sort-order name "linter" is repeated several times`,
},
{
desc: "unsupported format",
settings: &Output{
Formats: []OutputFormat{
{
Format: "test",
},
},
},
expected: `unsupported output format "test"`,
},
}

for _, test := range testCases {
test := test
t.Run(test.desc, func(t *testing.T) {
t.Parallel()

err := test.settings.Validate()
require.EqualError(t, err, test.expected)
})
}
}

func TestOutputFormat_Validate(t *testing.T) {
testCases := []struct {
desc string
settings *OutputFormat
}{
{
desc: "only format",
settings: &OutputFormat{
Format: "json",
},
},
{
desc: "format and path (relative)",
settings: &OutputFormat{
Format: "json",
Path: "./example.json",
},
},
{
desc: "format and path (absolute)",
settings: &OutputFormat{
Format: "json",
Path: "/tmp/example.json",
},
},
}

for _, test := range testCases {
test := test
t.Run(test.desc, func(t *testing.T) {
t.Parallel()

err := test.settings.Validate()
require.NoError(t, err)
})
}
}

func TestOutputFormat_Validate_error(t *testing.T) {
testCases := []struct {
desc string
settings *OutputFormat
expected string
}{
{
desc: "empty",
settings: &OutputFormat{},
expected: "the format is required",
},
{
desc: "unsupported format",
settings: &OutputFormat{
Format: "test",
},
expected: `unsupported output format "test"`,
},
}

for _, test := range testCases {
Expand Down
Loading

0 comments on commit e3ed3ba

Please sign in to comment.