dockerfile: add hint suggestions to UndeclaredArgInFrom

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
moby · May 14, 2024 · 1f3fce4 · 1f3fce4
1 parent 05782a7
commit 1f3fce4
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 21 deletions.
diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go
@@ -261,7 +261,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 	// set base state for every image
 	for i, st := range stages {
 		nameMatch, err := shlex.ProcessWordWithMatches(st.BaseName, metaArgsToMap(optMetaArgs))
-		reportUnusedFromArgs(nameMatch.Unmatched, st.Location, opt.Warn)
+		reportUnusedFromArgs(metaArgsKeys(optMetaArgs), nameMatch.Unmatched, st.Location, opt.Warn)
 		used := nameMatch.Matched
 
 		if err != nil {
@@ -285,7 +285,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 
 		if v := st.Platform; v != "" {
 			platMatch, err := shlex.ProcessWordWithMatches(v, metaArgsToMap(optMetaArgs))
-			reportUnusedFromArgs(platMatch.Unmatched, st.Location, opt.Warn)
+			reportUnusedFromArgs(metaArgsKeys(optMetaArgs), platMatch.Unmatched, st.Location, opt.Warn)
 
 			if err != nil {
 				return nil, parser.WithLocation(errors.Wrapf(err, "failed to process arguments for platform %s", platMatch.Result), st.Location)
@@ -2169,9 +2169,10 @@ func toPBLocation(sourceIndex int, location []parser.Range) pb.Location {
 	}
 }
 
-func reportUnusedFromArgs(unmatched map[string]struct{}, location []parser.Range, warn linter.LintWarnFunc) {
+func reportUnusedFromArgs(values []string, unmatched map[string]struct{}, location []parser.Range, warn linter.LintWarnFunc) {
 	for arg := range unmatched {
-		msg := linter.RuleUndeclaredArgInFrom.Format(arg)
+		suggest, _ := suggest.Search(arg, values, true)
+		msg := linter.RuleUndeclaredArgInFrom.Format(arg, suggest)
 		linter.RuleUndeclaredArgInFrom.Run(warn, location, msg)
 	}
 }

diff --git a/frontend/dockerfile/dockerfile_lint_test.go b/frontend/dockerfile/dockerfile_lint_test.go
@@ -462,7 +462,7 @@ COPY Dockerfile .
 			{
 				RuleName:    "UndeclaredArgInFrom",
 				Description: "FROM command must use declared ARGs",
-				Detail:      "FROM argument 'BULIDPLATFORM' is not declared",
+				Detail:      "FROM argument 'BULIDPLATFORM' is not declared (did you mean BUILDPLATFORM?)",
 				Level:       1,
 				Line:        2,
 			},
@@ -484,7 +484,7 @@ COPY Dockerfile .
 			{
 				RuleName:    "UndeclaredArgInFrom",
 				Description: "FROM command must use declared ARGs",
-				Detail:      "FROM argument 'MYARCH' is not declared",
+				Detail:      "FROM argument 'MYARCH' is not declared (did you mean MY_ARCH?)",
 				Level:       1,
 				Line:        4,
 			},

diff --git a/frontend/dockerfile/linter/ruleset.go b/frontend/dockerfile/linter/ruleset.go
@@ -70,11 +70,15 @@ var (
 			return "Maintainer instruction is deprecated in favor of using label"
 		},
 	}
-	RuleUndeclaredArgInFrom = LinterRule[func(string) string]{
+	RuleUndeclaredArgInFrom = LinterRule[func(string, string) string]{
 		Name:        "UndeclaredArgInFrom",
 		Description: "FROM command must use declared ARGs",
-		Format: func(baseArg string) string {
-			return fmt.Sprintf("FROM argument '%s' is not declared", baseArg)
+		Format: func(baseArg, suggest string) string {
+			out := fmt.Sprintf("FROM argument '%s' is not declared", baseArg)
+			if suggest != "" {
+				out += fmt.Sprintf(" (did you mean %s?)", suggest)
+			}
+			return out
 		},
 	}
 	RuleWorkdirRelativePath = LinterRule[func(workdir string) string]{

diff --git a/util/suggest/error.go b/util/suggest/error.go
@@ -6,16 +6,7 @@ import (
 	"github.com/agext/levenshtein"
 )
 
-// WrapError wraps error with a suggestion for fixing it
-func WrapError(err error, val string, options []string, caseSensitive bool) error {
-	err, _ = WrapErrorMaybe(err, val, options, caseSensitive)
-	return err
-}
-
-func WrapErrorMaybe(err error, val string, options []string, caseSensitive bool) (error, bool) {
-	if err == nil {
-		return nil, false
-	}
+func Search(val string, options []string, caseSensitive bool) (string, bool) {
 	orig := val
 	if !caseSensitive {
 		val = strings.ToLower(val)
@@ -28,7 +19,7 @@ func WrapErrorMaybe(err error, val string, options []string, caseSensitive bool)
 		}
 		if val == opt {
 			// exact match means error was unrelated to the value
-			return err, false
+			return "", false
 		}
 		dist := levenshtein.Distance(val, opt, nil)
 		if dist < mindist {
@@ -40,8 +31,21 @@ func WrapErrorMaybe(err error, val string, options []string, caseSensitive bool)
 			mindist = dist
 		}
 	}
+	return match, match != ""
+}
 
-	if match == "" {
+// WrapError wraps error with a suggestion for fixing it
+func WrapError(err error, val string, options []string, caseSensitive bool) error {
+	err, _ = WrapErrorMaybe(err, val, options, caseSensitive)
+	return err
+}
+
+func WrapErrorMaybe(err error, val string, options []string, caseSensitive bool) (error, bool) {
+	if err == nil {
+		return nil, false
+	}
+	match, ok := Search(val, options, caseSensitive)
+	if match == "" || !ok {
 		return err, false
 	}