refactored opt.Warn to simplify and updated warning data

Signed-off-by: Talon Bowler <[email protected]>

frontend/dockerfile/builder/build.go

@@ -74,8 +74,8 @@ func Build(ctx context.Context, c client.Client) (_ *client.Result, err error) {
 		Client:       bc,
 		SourceMap:    src.SourceMap,
 		MetaResolver: c,
-		Warn: func(msg, url string, detail [][]byte, location []parser.Range) {
-			src.Warn(ctx, msg, warnOpts(location, detail, url))
+		Warn: func(rulename, description, url, msg string, location []parser.Range) {
+			src.Warn(ctx, msg, warnOpts(location, [][]byte{[]byte(description)}, url))
 		},
 	}
 

frontend/dockerfile/dockerfile2llb/convert.go

@@ -64,7 +64,7 @@ type ConvertOpt struct {
 	TargetPlatform *ocispecs.Platform
 	MetaResolver   llb.ImageMetaResolver
 	LLBCaps        *apicaps.CapSet
-	Warn           func(short, url string, detail [][]byte, location []parser.Range)
+	Warn           linter.LintWarnFunc
 }
 
 type SBOMTargets struct {
@@ -113,31 +113,9 @@ func Dockefile2Outline(ctx context.Context, dt []byte, opt ConvertOpt) (*outline
 
 func DockerfileLint(ctx context.Context, dt []byte, opt ConvertOpt) (*lint.LintResults, error) {
 	results := &lint.LintResults{}
-	opt.Warn = func(short, url string, detail [][]byte, location []parser.Range) {
-		var ruleName, ruleDetail string
-		if strings.HasPrefix(short, "Lint Rule ") {
-			ruleName = strings.TrimPrefix(short, "Lint Rule ")
-			ruleParts := strings.Split(ruleName, ":")
-			ruleName = strings.Trim(ruleParts[0], "'")
-			ruleDetail = strings.TrimSpace(ruleParts[1])
-		} else {
-			return
-		}
-		sourceIndex := results.AddSource(opt.SourceMap.Filename, "Dockerfile", opt.SourceMap.Data)
-		sourceLocation := []lint.Range{}
-		for _, loc := range location {
-			sourceLocation = append(sourceLocation, lint.Range{
-				Start: lint.Position{
-					Line:   loc.Start.Line,
-					Column: loc.Start.Character,
-				},
-				End: lint.Position{
-					Line:   loc.End.Line,
-					Column: loc.End.Character,
-				},
-			})
-		}
-		results.AddWarning(ruleName, ruleDetail, sourceIndex, sourceLocation)
+	sourceIndex := results.AddSource(opt.SourceMap)
+	opt.Warn = func(rulename, description, url, fmtmsg string, location []parser.Range) {
+		results.AddWarning(rulename, description, url, fmtmsg, sourceIndex, location)
 	}
 	_, err := toDispatchState(ctx, dt, opt)
 	if err != nil {
@@ -198,7 +176,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 	}
 
 	if opt.Warn == nil {
-		opt.Warn = func(string, string, [][]byte, []parser.Range) {}
+		opt.Warn = func(string, string, string, string, []parser.Range) {}
 	}
 
 	if opt.Client != nil && opt.LLBCaps == nil {
@@ -1982,7 +1960,7 @@ func isSelfConsistentCasing(s string) bool {
 	return s == strings.ToLower(s) || s == strings.ToUpper(s)
 }
 
-func validateCommandCasing(dockerfile *parser.Result, warn func(short, url string, detail [][]byte, location []parser.Range)) {
+func validateCommandCasing(dockerfile *parser.Result, warn linter.LintWarnFunc) {
 	var lowerCount, upperCount int
 	for _, node := range dockerfile.AST.Children {
 		if isSelfConsistentCasing(node.Value) {

frontend/dockerfile/dockerfile_lint_test.go

@@ -3,14 +3,14 @@ package dockerfile
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"os"
 	"sort"
 	"testing"
 	"time"
 
 	"github.com/containerd/continuity/fs/fstest"
 	"github.com/moby/buildkit/client"
+	"github.com/moby/buildkit/frontend/dockerfile/linter"
 	"github.com/moby/buildkit/frontend/dockerui"
 	gateway "github.com/moby/buildkit/frontend/gateway/client"
 
@@ -39,16 +39,16 @@ FROM scratch AS base3
 	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
 		{
 			RuleName:    "StageNameCasing",
-			Description: "Stage name 'BadStageName' should be lowercase",
+			Description: "Stage names should be lowercase",
+			Detail:      "Stage name 'BadStageName' should be lowercase",
 			Line:        3,
-			Detail:      "Stage names should be lowercase",
 			Level:       1,
 		},
 		{
 			RuleName:    "FromAsCasing",
-			Description: "'as' and 'FROM' keywords' casing do not match",
+			Description: "The 'as' keyword should match the case of the 'from' keyword",
+			Detail:      "'as' and 'FROM' keywords' casing do not match",
 			Line:        6,
-			Detail:      "The 'as' keyword should match the case of the 'from' keyword",
 			Level:       1,
 		},
 	})
@@ -62,10 +62,10 @@ from scratch as base2
 	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
 		{
 			RuleName:    "FromAsCasing",
-			Description: "'AS' and 'from' keywords' casing do not match",
-			Line:        3,
-			Detail:      "The 'as' keyword should match the case of the 'from' keyword",
+			Description: "The 'as' keyword should match the case of the 'from' keyword",
+			Detail:      "'AS' and 'from' keywords' casing do not match",
 			Level:       1,
+			Line:        3,
 		},
 	})
 }
@@ -84,11 +84,11 @@ COPY Dockerfile \
 	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
 		{
 			RuleName:    "NoEmptyContinuations",
-			Description: "Empty continuation line",
+			Description: "Empty continuation lines will become errors in a future release",
+			Detail:      "Empty continuation line",
+			Level:       1,
 			Line:        6,
-			Detail:      "Empty continuation lines will become errors in a future release",
 			URL:         "https://github.com/moby/moby/pull/33719",
-			Level:       1,
 		},
 	})
 }
@@ -102,10 +102,10 @@ FROM scratch AS base2
 	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
 		{
 			RuleName:    "SelfConsistentCommandCasing",
-			Description: "Command 'From' should be consistently cased",
-			Line:        3,
-			Detail:      "Commands should be in consistent casing (all lower or all upper)",
+			Description: "Commands should be in consistent casing (all lower or all upper)",
+			Detail:      "Command 'From' should be consistently cased",
 			Level:       1,
+			Line:        3,
 		},
 	})
 	dockerfile = []byte(`
@@ -116,9 +116,9 @@ from scratch as base2
 	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
 		{
 			RuleName:    "SelfConsistentCommandCasing",
-			Description: "Command 'frOM' should be consistently cased",
+			Description: "Commands should be in consistent casing (all lower or all upper)",
+			Detail:      "Command 'frOM' should be consistently cased",
 			Line:        3,
-			Detail:      "Commands should be in consistent casing (all lower or all upper)",
 			Level:       1,
 		},
 	})
@@ -134,9 +134,9 @@ COPY Dockerfile /bar
 	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
 		{
 			RuleName:    "FileConsistentCommandCasing",
-			Description: "Command 'copy' should match the case of the command majority (uppercase)",
+			Description: "All commands within the Dockerfile should use the same casing (either upper or lower)",
+			Detail:      "Command 'copy' should match the case of the command majority (uppercase)",
 			Line:        4,
-			Detail:      "All commands within the Dockerfile should use the same casing (either upper or lower)",
 			Level:       1,
 		},
 	})
@@ -150,9 +150,9 @@ copy Dockerfile /bar
 	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
 		{
 			RuleName:    "FileConsistentCommandCasing",
-			Description: "Command 'COPY' should match the case of the command majority (lowercase)",
+			Description: "All commands within the Dockerfile should use the same casing (either upper or lower)",
+			Detail:      "Command 'COPY' should match the case of the command majority (lowercase)",
 			Line:        4,
-			Detail:      "All commands within the Dockerfile should use the same casing (either upper or lower)",
 			Level:       1,
 		},
 	})
@@ -167,9 +167,9 @@ COPY Dockerfile /baz
 	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
 		{
 			RuleName:    "FileConsistentCommandCasing",
-			Description: "Command 'from' should match the case of the command majority (uppercase)",
+			Description: "All commands within the Dockerfile should use the same casing (either upper or lower)",
+			Detail:      "Command 'from' should match the case of the command majority (uppercase)",
 			Line:        3,
-			Detail:      "All commands within the Dockerfile should use the same casing (either upper or lower)",
 			Level:       1,
 		},
 	})
@@ -184,9 +184,9 @@ copy Dockerfile /baz
 	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
 		{
 			RuleName:    "FileConsistentCommandCasing",
-			Description: "Command 'FROM' should match the case of the command majority (lowercase)",
+			Description: "All commands within the Dockerfile should use the same casing (either upper or lower)",
+			Detail:      "Command 'FROM' should match the case of the command majority (lowercase)",
 			Line:        3,
-			Detail:      "All commands within the Dockerfile should use the same casing (either upper or lower)",
 			Level:       1,
 		},
 	})
@@ -229,8 +229,8 @@ func checkUnmarshal(t *testing.T, sb integration.Sandbox, c *client.Client, f fr
 		require.Equal(t, len(expected), len(lintResults.Warnings))
 		sort.Slice(lintResults.Warnings, func(i, j int) bool {
 			// sort by line number in ascending order
-			firstRange := lintResults.Warnings[i].Location[0]
-			secondRange := lintResults.Warnings[j].Location[0]
+			firstRange := lintResults.Warnings[i].Location.Ranges[0]
+			secondRange := lintResults.Warnings[j].Location.Ranges[0]
 			return firstRange.Start.Line < secondRange.Start.Line
 		})
 		// Compare expectedLintWarning with actual lint results
@@ -323,22 +323,28 @@ func checkLinterWarnings(t *testing.T, sb integration.Sandbox, dockerfile []byte
 	require.NoError(t, err)
 	defer c.Close()
 
-	checkProgressStream(t, sb, c, f, dir, dockerfile, expected)
-	checkUnmarshal(t, sb, c, f, dir, dockerfile, expected)
+	t.Run("warntype=progress", func(t *testing.T) {
+		checkProgressStream(t, sb, c, f, dir, dockerfile, expected)
+	})
+	t.Run("warntype=unmarshal", func(t *testing.T) {
+		checkUnmarshal(t, sb, c, f, dir, dockerfile, expected)
+	})
 }
 
 func checkVertexWarning(t *testing.T, warning *client.VertexWarning, expected expectedLintWarning) {
-	short := fmt.Sprintf("Lint Rule '%s': %s (line %d)", expected.RuleName, expected.Description, expected.Line)
+	short := linter.LintFormatShort(expected.RuleName, expected.Detail, expected.Line)
 	require.Equal(t, short, string(warning.Short))
-	require.Equal(t, expected.Detail, string(warning.Detail[0]))
+	require.Equal(t, expected.Description, string(warning.Detail[0]))
 	require.Equal(t, expected.URL, warning.URL)
 	require.Equal(t, expected.Level, warning.Level)
 }
 
 func checkLintWarning(t *testing.T, warning lint.Warning, expected expectedLintWarning) {
+	short := linter.LintFormatShort(expected.RuleName, expected.Detail, expected.Line)
 	require.Equal(t, expected.RuleName, warning.RuleName)
-	detail := fmt.Sprintf("%s (line %d)", expected.Description, expected.Line)
-	require.Equal(t, detail, warning.Detail)
+	require.Equal(t, expected.Description, warning.Description)
+	require.Equal(t, expected.URL, warning.URL)
+	require.Equal(t, short, warning.Detail)
 }
 
 func unmarshalLintResults(res *gateway.Result) (*lint.LintResults, error) {

frontend/dockerfile/linter/linter.go

@@ -22,9 +22,12 @@ func (rule LinterRule[F]) Run(warn LintWarnFunc, location []parser.Range, txt ..
 	if len(txt) == 0 {
 		txt = []string{rule.Description}
 	}
-	short := strings.Join(txt, " ")
-	short = fmt.Sprintf("Lint Rule '%s': %s (line %d)", rule.Name, short, startLine)
-	warn(short, rule.URL, [][]byte{[]byte(rule.Description)}, location)
+	short := LintFormatShort(rule.Name, strings.Join(txt, " "), startLine)
+	warn(rule.Name, rule.Description, rule.URL, short, location)
 }
 
-type LintWarnFunc func(short, url string, detail [][]byte, location []parser.Range)
+func LintFormatShort(rulename, msg string, startLine int) string {
+	return fmt.Sprintf("Lint Rule '%s': %s (line %d)", rulename, msg, startLine)
+}
+
+type LintWarnFunc func(rulename, description, url, fmtmsg string, location []parser.Range)