From 1f3fce41534b99bdfb631efabe8fe2ce7c37458d Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Mon, 13 May 2024 23:16:53 -0700 Subject: [PATCH] dockerfile: add hint suggestions to UndeclaredArgInFrom Signed-off-by: Tonis Tiigi --- frontend/dockerfile/dockerfile2llb/convert.go | 9 +++--- frontend/dockerfile/dockerfile_lint_test.go | 4 +-- frontend/dockerfile/linter/ruleset.go | 10 +++++-- util/suggest/error.go | 28 +++++++++++-------- 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 301de585b27da..a24759ece8adc 100644 --- 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 index c3efe9174ba47..7343b8300c1ec 100644 --- 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 index 3c15dccab12d9..ba96fc7444857 100644 --- 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 index 1c3d41e6f9654..2b206998dd8a5 100644 --- 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 }