Skip to content
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

sort lint warning output and make consistent #4852

Merged
merged 3 commits into from
Apr 17, 2024
Merged
Changes from 2 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
114 changes: 61 additions & 53 deletions frontend/subrequests/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import (
"fmt"
"io"
"sort"
"text/tabwriter"

"github.com/moby/buildkit/client/llb"
"github.com/moby/buildkit/frontend/dockerfile/parser"
"github.com/moby/buildkit/frontend/gateway/client"
"github.com/moby/buildkit/frontend/subrequests"
"github.com/moby/buildkit/solver/errdefs"
"github.com/moby/buildkit/solver/pb"
)

Expand All @@ -30,13 +30,6 @@ var SubrequestLintDefinition = subrequests.Request{
},
}

type Source struct {
Filename string `json:"fileName"`
Language string `json:"language"`
Definition *pb.Definition `json:"definition"`
Data []byte `json:"data"`
}

type Warning struct {
RuleName string `json:"ruleName"`
Description string `json:"description,omitempty"`
Expand All @@ -46,19 +39,19 @@ type Warning struct {
}

type LintResults struct {
Warnings []Warning `json:"warnings"`
Sources []Source `json:"sources"`
Warnings []Warning `json:"warnings"`
Sources []*pb.SourceInfo `json:"sources"`
}

func (results *LintResults) AddSource(sourceMap *llb.SourceMap) int {
newSource := Source{
newSource := &pb.SourceInfo{
Filename: sourceMap.Filename,
Language: sourceMap.Language,
Definition: sourceMap.Definition.ToPB(),
Data: sourceMap.Data,
}
for i, source := range results.Sources {
if sourceEqual(source, newSource) {
if sourceInfoEqual(source, newSource) {
return i
}
}
Expand Down Expand Up @@ -93,13 +86,6 @@ func (results *LintResults) AddWarning(rulename, description, url, fmtmsg string
})
}

func sourceEqual(a, b Source) bool {
if a.Filename != b.Filename || a.Language != b.Language {
return false
}
return bytes.Equal(a.Data, b.Data)
}

func (results *LintResults) ToResult() (*client.Result, error) {
res := client.NewResult()
dt, err := json.MarshalIndent(results, "", " ")
Expand All @@ -124,47 +110,69 @@ func (results *LintResults) ToResult() (*client.Result, error) {
return res, nil
}

func (results *LintResults) validateWarnings() error {
for _, warning := range results.Warnings {
if warning.Location.SourceIndex < 0 {
return fmt.Errorf("sourceIndex is required")
daghack marked this conversation as resolved.
Show resolved Hide resolved
}
if int(warning.Location.SourceIndex) >= len(results.Sources) {
return fmt.Errorf("sourceIndex is out of range")
daghack marked this conversation as resolved.
Show resolved Hide resolved
}
warningSource := results.Sources[warning.Location.SourceIndex]
daghack marked this conversation as resolved.
Show resolved Hide resolved
if warningSource == nil {
return fmt.Errorf("sourceIndex points to nil source")
}
if warningSource.Definition == nil {
return fmt.Errorf("sourceIndex points to source with nil definition")
}
if len(warning.Location.Ranges) == 0 {
return fmt.Errorf("ranges is required")
}
}
return nil
}

func PrintLintViolations(dt []byte, w io.Writer) error {
var warnings LintResults
var results LintResults

if err := json.Unmarshal(dt, &warnings); err != nil {
if err := json.Unmarshal(dt, &results); err != nil {
return err
}

// Here, we're grouping the warnings by rule name
lintWarnings := make(map[string][]Warning)
lintWarningRules := []string{}
for _, warning := range warnings.Warnings {
if _, ok := lintWarnings[warning.RuleName]; !ok {
lintWarningRules = append(lintWarningRules, warning.RuleName)
lintWarnings[warning.RuleName] = []Warning{}
}
lintWarnings[warning.RuleName] = append(lintWarnings[warning.RuleName], warning)
if err := results.validateWarnings(); err != nil {
return err
}
sort.Strings(lintWarningRules)

tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0)
for _, rule := range lintWarningRules {
fmt.Fprintf(tw, "Lint Rule %s\n", rule)
for _, warning := range lintWarnings[rule] {
source := warnings.Sources[warning.Location.SourceIndex]
sourceData := bytes.Split(source.Data, []byte("\n"))
firstRange := warning.Location.Ranges[0]
if firstRange.Start.Line != firstRange.End.Line {
fmt.Fprintf(tw, "\t%s:%d-%d\n", source.Filename, firstRange.Start.Line, firstRange.End.Line)
} else {
fmt.Fprintf(tw, "\t%s:%d\n", source.Filename, firstRange.Start.Line)
}
fmt.Fprintf(tw, "\t%s\n", warning.Detail)
for _, r := range warning.Location.Ranges {
for i := r.Start.Line; i <= r.End.Line; i++ {
fmt.Fprintf(tw, "\t%d\t|\t%s\n", i, sourceData[i-1])
}
}
fmt.Fprintln(tw)

sort.Slice(results.Warnings, func(i, j int) bool {
sourceInfoI := results.Sources[results.Warnings[i].Location.SourceIndex]
daghack marked this conversation as resolved.
Show resolved Hide resolved
sourceInfoJ := results.Sources[results.Warnings[j].Location.SourceIndex]
if sourceInfoI.Filename != sourceInfoJ.Filename {
return sourceInfoI.Filename < sourceInfoJ.Filename
}
return results.Warnings[i].Location.Ranges[0].Start.Line < results.Warnings[j].Location.Ranges[0].Start.Line
daghack marked this conversation as resolved.
Show resolved Hide resolved
})

for _, warning := range results.Warnings {
fmt.Fprintf(w, "\n- %s\n%s\n", warning.Detail, warning.Description)
if warning.URL != "" {
fmt.Fprintf(w, "URL: %s\n", warning.URL)
}
sourceInfo := results.Sources[warning.Location.SourceIndex]
tonistiigi marked this conversation as resolved.
Show resolved Hide resolved
source := errdefs.Source{
Info: sourceInfo,
Ranges: warning.Location.Ranges,
}
err := source.Print(w)
if err != nil {
return err
}
fmt.Fprintln(tw)
}
return nil
}

return tw.Flush()
func sourceInfoEqual(a, b *pb.SourceInfo) bool {
if a.Filename != b.Filename || a.Language != b.Language {
return false
}
return bytes.Equal(a.Data, b.Data)
}
Loading