dockerfile: improve error messages and add suggest to flag parsing

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,15 +285,24 @@ 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)
 			}
 
+			if platMatch.Result == "" {
+				err := errors.Errorf("empty platform value from expression %s", v)
+				err = parser.WithLocation(err, st.Location)
+				err = wrapSuggestAny(err, platMatch.Unmatched, metaArgsKeys(optMetaArgs))
+				return nil, err
+			}
+
 			p, err := platforms.Parse(platMatch.Result)
 			if err != nil {
-				return nil, parser.WithLocation(errors.Wrapf(err, "failed to parse platform %s", platMatch.Result), st.Location)
+				err = parser.WithLocation(err, st.Location)
+				err = wrapSuggestAny(err, platMatch.Unmatched, metaArgsKeys(optMetaArgs))
+				return nil, parser.WithLocation(errors.Wrapf(err, "failed to parse platform %s", v), st.Location)
 			}
 
 			for k := range platMatch.Matched {
@@ -687,6 +696,14 @@ func metaArgsToMap(metaArgs []instructions.KeyValuePairOptional) map[string]stri
 	return m
 }
 
+func metaArgsKeys(metaArgs []instructions.KeyValuePairOptional) []string {
+	s := make([]string, 0, len(metaArgs))
+	for _, arg := range metaArgs {
+		s = append(s, arg.Key)
+	}
+	return s
+}
+
 func toCommand(ic instructions.Command, allDispatchStates *dispatchStates) (command, error) {
 	cmd := command{Command: ic}
 	if c, ok := ic.(*instructions.CopyCommand); ok {
@@ -749,7 +766,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
 			}
 
 			newword, unmatched, err := opt.shlex.ProcessWord(word, env)
-			reportUnmatchedVariables(cmd, d.buildArgs, unmatched, &opt)
+			reportUnmatchedVariables(cmd, d.buildArgs, env, unmatched, &opt)
 			return newword, err
 		})
 		if err != nil {
@@ -765,7 +782,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
 			lex := shell.NewLex('\\')
 			lex.SkipProcessQuotes = true
 			newword, unmatched, err := lex.ProcessWord(word, env)
-			reportUnmatchedVariables(cmd, d.buildArgs, unmatched, &opt)
+			reportUnmatchedVariables(cmd, d.buildArgs, env, unmatched, &opt)
 			return newword, err
 		})
 		if err != nil {
@@ -2086,19 +2103,25 @@ func validateStageNames(stages []instructions.Stage, warn linter.LintWarnFunc) {
 	}
 }
 
-func reportUnmatchedVariables(cmd instructions.Command, buildArgs []instructions.KeyValuePairOptional, unmatched map[string]struct{}, opt *dispatchOpt) {
+func reportUnmatchedVariables(cmd instructions.Command, buildArgs []instructions.KeyValuePairOptional, env []string, unmatched map[string]struct{}, opt *dispatchOpt) {
+	for _, buildArg := range buildArgs {
+		delete(unmatched, buildArg.Key)
+	}
 	if len(unmatched) == 0 {
 		return
 	}
-	for _, buildArg := range buildArgs {
-		delete(unmatched, buildArg.Key)
+	options := metaArgsKeys(opt.metaArgs)
+	for _, envVar := range env {
+		key, _ := parseKeyValue(envVar)
+		options = append(options, key)
 	}
 	for cmdVar := range unmatched {
-		_, nonEnvOk := nonEnvArgs[cmdVar]
-		if !nonEnvOk {
-			msg := linter.RuleUndefinedVar.Format(cmdVar)
-			linter.RuleUndefinedVar.Run(opt.lintWarn, cmd.Location(), msg)
+		if _, nonEnvOk := nonEnvArgs[cmdVar]; nonEnvOk {
+			continue
 		}
+		match, _ := suggest.Search(cmdVar, options, true)
+		msg := linter.RuleUndefinedVar.Format(cmdVar, match)
+		linter.RuleUndefinedVar.Run(opt.lintWarn, cmd.Location(), msg)
 	}
 }
 
@@ -2152,9 +2175,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)
 	}
 }
@@ -2178,3 +2202,14 @@ func validateUsedOnce(c instructions.Command, loc *instructionTracker, warn lint
 	}
 	loc.MarkUsed(c.Location())
 }
+
+func wrapSuggestAny(err error, keys map[string]struct{}, options []string) error {
+	for k := range keys {
+		var ok bool
+		ok, err = suggest.WrapErrorMaybe(err, k, options, true)
+		if ok {
+			break
+		}
+	}
+	return err
+}

frontend/dockerfile/dockerfile_lint_test.go

@@ -462,16 +462,38 @@ 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,
 			},
 		},
-		StreamBuildErr:    "failed to solve: failed to parse platform : \"\" is an invalid component of \"\": platform specifier component must match \"^[A-Za-z0-9_-]+$\": invalid argument",
-		UnmarshalBuildErr: "failed to parse platform : \"\" is an invalid component of \"\": platform specifier component must match \"^[A-Za-z0-9_-]+$\": invalid argument",
+		StreamBuildErr:    "failed to solve: empty platform value from expression $BULIDPLATFORM (did you mean BUILDPLATFORM?)",
+		UnmarshalBuildErr: "empty platform value from expression $BULIDPLATFORM (did you mean BUILDPLATFORM?)",
 		BuildErrLocation:  2,
 	})
 
+	dockerfile = []byte(`
+ARG MY_OS=linux
+ARG MY_ARCH=amd64
+FROM --platform=linux/${MYARCH} busybox
+COPY Dockerfile .
+	`)
+	checkLinterWarnings(t, sb, &lintTestParams{
+		Dockerfile: dockerfile,
+		Warnings: []expectedLintWarning{
+			{
+				RuleName:    "UndeclaredArgInFrom",
+				Description: "FROM command must use declared ARGs",
+				Detail:      "FROM argument 'MYARCH' is not declared (did you mean MY_ARCH?)",
+				Level:       1,
+				Line:        4,
+			},
+		},
+		StreamBuildErr:    "failed to solve: failed to parse platform linux/${MYARCH}: \"\" is an invalid component of \"linux/\": platform specifier component must match \"^[A-Za-z0-9_-]+$\": invalid argument (did you mean MY_ARCH?)",
+		UnmarshalBuildErr: "failed to parse platform linux/${MYARCH}: \"\" is an invalid component of \"linux/\": platform specifier component must match \"^[A-Za-z0-9_-]+$\": invalid argument (did you mean MY_ARCH?)",
+		BuildErrLocation:  4,
+	})
+
 	dockerfile = []byte(`
 ARG tag=latest
 FROM busybox:${tag}${version} AS b
@@ -560,6 +582,43 @@ RUN echo $foo
 			},
 		},
 	})
+
+	dockerfile = []byte(`
+FROM alpine
+ARG DIR_BINARIES=binaries/
+ARG DIR_ASSETS=assets/
+ARG DIR_CONFIG=config/
+COPY $DIR_ASSET .
+	`)
+	checkLinterWarnings(t, sb, &lintTestParams{
+		Dockerfile: dockerfile,
+		Warnings: []expectedLintWarning{
+			{
+				RuleName:    "UndefinedVar",
+				Description: "Variables should be defined before their use",
+				Detail:      "Usage of undefined variable '$DIR_ASSET' (did you mean $DIR_ASSETS?)",
+				Level:       1,
+				Line:        6,
+			},
+		},
+	})
+
+	dockerfile = []byte(`
+FROM alpine
+ENV PATH=$PAHT:/tmp/bin
+		`)
+	checkLinterWarnings(t, sb, &lintTestParams{
+		Dockerfile: dockerfile,
+		Warnings: []expectedLintWarning{
+			{
+				RuleName:    "UndefinedVar",
+				Description: "Variables should be defined before their use",
+				Detail:      "Usage of undefined variable '$PAHT' (did you mean $PATH?)",
+				Level:       1,
+				Line:        3,
+			},
+		},
+	})
 }
 
 func testMultipleInstructionsDisallowed(t *testing.T, sb integration.Sandbox) {

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]{
@@ -91,11 +95,15 @@ var (
 			return fmt.Sprintf("Usage of undefined variable '$%s'", arg)
 		},
 	}
-	RuleUndefinedVar = LinterRule[func(string) string]{
+	RuleUndefinedVar = LinterRule[func(string, string) string]{
 		Name:        "UndefinedVar",
 		Description: "Variables should be defined before their use",
-		Format: func(arg string) string {
-			return fmt.Sprintf("Usage of undefined variable '$%s'", arg)
+		Format: func(arg, suggest string) string {
+			out := fmt.Sprintf("Usage of undefined variable '$%s'", arg)
+			if suggest != "" {
+				out += fmt.Sprintf(" (did you mean $%s?)", suggest)
+			}
+			return out
 		},
 	}
 	RuleMultipleInstructionsDisallowed = LinterRule[func(instructionName string) string]{

util/suggest/error.go

@@ -6,11 +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 {
-	if err == nil {
-		return nil
-	}
+func Search(val string, options []string, caseSensitive bool) (string, bool) {
 	orig := val
 	if !caseSensitive {
 		val = strings.ToLower(val)
@@ -23,7 +19,7 @@ func WrapError(err error, val string, options []string, caseSensitive bool) erro
 		}
 		if val == opt {
 			// exact match means error was unrelated to the value
-			return err
+			return "", false
 		}
 		dist := levenshtein.Distance(val, opt, nil)
 		if dist < mindist {
@@ -35,12 +31,25 @@ func WrapError(err error, val string, options []string, caseSensitive bool) erro
 			mindist = dist
 		}
 	}
+	return match, match != ""
+}
 
-	if match == "" {
-		return err
+// 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) (bool, error) {
+	if err == nil {
+		return false, nil
+	}
+	match, ok := Search(val, options, caseSensitive)
+	if match == "" || !ok {
+		return false, err
 	}
 
-	return &suggestError{
+	return true, &suggestError{
 		err:   err,
 		match: match,
 	}