From 4270ddf3cb7004fbd674aba69c78d7e239cdbb96 Mon Sep 17 00:00:00 2001 From: Talon Bowler Date: Thu, 19 Sep 2024 08:21:33 -0700 Subject: [PATCH] remove the directive from the comments in the AST Signed-off-by: Talon Bowler --- frontend/dockerfile/dockerfile_lint_test.go | 14 +++-------- frontend/dockerfile/parser/parser.go | 26 ++++++++++++++------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/frontend/dockerfile/dockerfile_lint_test.go b/frontend/dockerfile/dockerfile_lint_test.go index b52056fbcb7c..efb3ff7842cf 100644 --- a/frontend/dockerfile/dockerfile_lint_test.go +++ b/frontend/dockerfile/dockerfile_lint_test.go @@ -338,35 +338,30 @@ COPY $bar . func testRuleCheckOption(t *testing.T, sb integration.Sandbox) { dockerfile := []byte(`#check=skip=all -# FROM scratch as base copy Dockerfile . `) checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) dockerfile = []byte(`#check=skip=all;error=true -# FROM scratch as base copy Dockerfile . `) checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) dockerfile = []byte(`#check=skip=ConsistentInstructionCasing,FromAsCasing -# FROM scratch as base copy Dockerfile . `) checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) dockerfile = []byte(`#check=skip=ConsistentInstructionCasing,FromAsCasing;error=true -# FROM scratch as base copy Dockerfile . `) checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) dockerfile = []byte(`#check=skip=ConsistentInstructionCasing -# FROM scratch as base copy Dockerfile . `) @@ -378,14 +373,13 @@ copy Dockerfile . Description: "The 'as' keyword should match the case of the 'from' keyword", URL: "https://docs.docker.com/go/dockerfile/rule/from-as-casing/", Detail: "'as' and 'FROM' keywords' casing do not match", - Line: 3, + Line: 2, Level: 1, }, }, }) dockerfile = []byte(`#check=skip=ConsistentInstructionCasing;error=true -# FROM scratch as base copy Dockerfile . `) @@ -397,7 +391,7 @@ copy Dockerfile . Description: "The 'as' keyword should match the case of the 'from' keyword", URL: "https://docs.docker.com/go/dockerfile/rule/from-as-casing/", Detail: "'as' and 'FROM' keywords' casing do not match", - Line: 3, + Line: 2, Level: 1, }, }, @@ -407,7 +401,6 @@ copy Dockerfile . }) dockerfile = []byte(`#check=skip=all -# FROM scratch as base copy Dockerfile . `) @@ -419,7 +412,7 @@ copy Dockerfile . Description: "The 'as' keyword should match the case of the 'from' keyword", URL: "https://docs.docker.com/go/dockerfile/rule/from-as-casing/", Detail: "'as' and 'FROM' keywords' casing do not match", - Line: 3, + Line: 2, Level: 1, }, }, @@ -432,7 +425,6 @@ copy Dockerfile . }) dockerfile = []byte(`#check=error=true -# FROM scratch as base copy Dockerfile . `) diff --git a/frontend/dockerfile/parser/parser.go b/frontend/dockerfile/parser/parser.go index c70236ff7d56..740f03b708f3 100644 --- a/frontend/dockerfile/parser/parser.go +++ b/frontend/dockerfile/parser/parser.go @@ -167,16 +167,17 @@ func (d *directives) setEscapeToken(s string) error { // possibleParserDirective looks for parser directives, eg '# escapeToken='. // Parser directives must precede any builder instruction or other comments, -// and cannot be repeated. -func (d *directives) possibleParserDirective(line []byte) error { +// and cannot be repeated. Returns true if a parser directive was found. +func (d *directives) possibleParserDirective(line []byte) (bool, error) { directive, err := d.parser.ParseLine(line) if err != nil { - return err + return false, err } if directive != nil && directive.Name == keyEscape { - return d.setEscapeToken(directive.Value) + err := d.setEscapeToken(directive.Value) + return err == nil, err } - return nil + return directive != nil, nil } // newDefaultDirectives returns a new directives structure with the default escapeToken token @@ -300,7 +301,13 @@ func Parse(rwc io.Reader) (*Result, error) { comments = append(comments, comment) } } - bytesRead, err = processLine(d, bytesRead, true) + var directiveOk bool + bytesRead, directiveOk, err = processLine(d, bytesRead, true) + // If the line is a directive, strip it from the comments + // so it doesn't get added to the AST. + if directiveOk { + comments = comments[:len(comments)-1] + } if err != nil { return nil, withLocation(err, currentLine, 0) } @@ -316,7 +323,7 @@ func Parse(rwc io.Reader) (*Result, error) { var hasEmptyContinuationLine bool for !isEndOfLine && scanner.Scan() { - bytesRead, err := processLine(d, scanner.Bytes(), false) + bytesRead, _, err := processLine(d, scanner.Bytes(), false) if err != nil { return nil, withLocation(err, currentLine, 0) } @@ -527,12 +534,13 @@ func trimContinuationCharacter(line []byte, d *directives) ([]byte, bool) { // TODO: remove stripLeftWhitespace after deprecation period. It seems silly // to preserve whitespace on continuation lines. Why is that done? -func processLine(d *directives, token []byte, stripLeftWhitespace bool) ([]byte, error) { +func processLine(d *directives, token []byte, stripLeftWhitespace bool) ([]byte, bool, error) { token = trimNewline(token) if stripLeftWhitespace { token = trimLeadingWhitespace(token) } - return trimComments(token), d.possibleParserDirective(token) + directiveOk, err := d.possibleParserDirective(token) + return trimComments(token), directiveOk, err } // Variation of bufio.ScanLines that preserves the line endings