From 8cbb98e4d9a2a52846f91f96a6979bc434fbab11 Mon Sep 17 00:00:00 2001 From: Talon Bowler Date: Mon, 17 Jun 2024 07:24:57 -0700 Subject: [PATCH] Consolidate instruction casing lint rules Signed-off-by: Talon Bowler --- frontend/dockerfile/dockerfile2llb/convert.go | 23 +++---- frontend/dockerfile/dockerfile_lint_test.go | 61 +++++++++---------- frontend/dockerfile/docs/rules/_index.md | 4 -- .../rules/consistent-instruction-casing.md | 2 +- .../rules/file-consistent-command-casing.md | 61 ------------------- .../docs/FileConsistentCommandCasing.md | 53 ---------------- frontend/dockerfile/linter/ruleset.go | 12 +--- 7 files changed, 40 insertions(+), 176 deletions(-) delete mode 100644 frontend/dockerfile/docs/rules/file-consistent-command-casing.md delete mode 100644 frontend/dockerfile/linter/docs/FileConsistentCommandCasing.md diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 4adba7760851..da4085140ecd 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -2162,22 +2162,15 @@ func validateCommandCasing(dockerfile *parser.Result, lint *linter.Linter) { // Here, we check both if the command is consistent per command (ie, "CMD" or "cmd", not "Cmd") // as well as ensuring that the casing is consistent throughout the dockerfile by comparing the // command to the casing of the majority of commands. - if !isSelfConsistentCasing(node.Value) { - msg := linter.RuleConsistentInstructionCasing.Format(node.Value) + var correctCasing string + if isMajorityLower && strings.ToLower(node.Value) != node.Value { + correctCasing = "lowercase" + } else if !isMajorityLower && strings.ToUpper(node.Value) != node.Value { + correctCasing = "uppercase" + } + if correctCasing != "" { + msg := linter.RuleConsistentInstructionCasing.Format(node.Value, correctCasing) lint.Run(&linter.RuleConsistentInstructionCasing, node.Location(), msg) - } else { - var msg string - var needsLintWarn bool - if isMajorityLower && strings.ToUpper(node.Value) == node.Value { - msg = linter.RuleFileConsistentCommandCasing.Format(node.Value, "lowercase") - needsLintWarn = true - } else if !isMajorityLower && strings.ToLower(node.Value) == node.Value { - msg = linter.RuleFileConsistentCommandCasing.Format(node.Value, "uppercase") - needsLintWarn = true - } - if needsLintWarn { - lint.Run(&linter.RuleFileConsistentCommandCasing, node.Location(), msg) - } } } } diff --git a/frontend/dockerfile/dockerfile_lint_test.go b/frontend/dockerfile/dockerfile_lint_test.go index 9a1eb1ec397c..367291e6690e 100644 --- a/frontend/dockerfile/dockerfile_lint_test.go +++ b/frontend/dockerfile/dockerfile_lint_test.go @@ -28,7 +28,6 @@ var lintTests = integration.TestFuncs( testStageName, testNoEmptyContinuation, testConsistentInstructionCasing, - testFileConsistentCommandCasing, testDuplicateStageName, testReservedStageName, testJSONArgsRecommended, @@ -97,19 +96,19 @@ copy Dockerfile . `) checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) - dockerfile = []byte(`#check=skip=FileConsistentCommandCasing,FromAsCasing + dockerfile = []byte(`#check=skip=ConsistentInstructionCasing,FromAsCasing FROM scratch as base copy Dockerfile . `) checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) - dockerfile = []byte(`#check=skip=FileConsistentCommandCasing,FromAsCasing;error=true + dockerfile = []byte(`#check=skip=ConsistentInstructionCasing,FromAsCasing;error=true FROM scratch as base copy Dockerfile . `) checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) - dockerfile = []byte(`#check=skip=FileConsistentCommandCasing + dockerfile = []byte(`#check=skip=ConsistentInstructionCasing FROM scratch as base copy Dockerfile . `) @@ -127,7 +126,7 @@ copy Dockerfile . }, }) - dockerfile = []byte(`#check=skip=FileConsistentCommandCasing;error=true + dockerfile = []byte(`#check=skip=ConsistentInstructionCasing;error=true FROM scratch as base copy Dockerfile . `) @@ -168,7 +167,7 @@ copy Dockerfile . UnmarshalBuildErr: "lint violation found for rules: FromAsCasing", BuildErrLocation: 2, FrontendAttrs: map[string]string{ - "build-arg:BUILDKIT_DOCKERFILE_CHECK": "skip=FileConsistentCommandCasing;error=true", + "build-arg:BUILDKIT_DOCKERFILE_CHECK": "skip=ConsistentInstructionCasing;error=true", }, }) @@ -275,9 +274,9 @@ FROM scratch AS base2 Warnings: []expectedLintWarning{ { RuleName: "ConsistentInstructionCasing", - Description: "Instructions should be in consistent casing (all lower or all upper)", + Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", - Detail: "Command 'From' should be consistently cased", + Detail: "Command 'From' should match the case of the command majority (uppercase)", Level: 1, Line: 3, }, @@ -285,42 +284,40 @@ FROM scratch AS base2 }) dockerfile = []byte(` -# warning: 'FROM' should be either lowercased or uppercased -frOM scratch as base -from scratch as base2 +FROM scratch +# warning: 'copy' should match command majority's casing (uppercase) +copy Dockerfile /foo +COPY Dockerfile /bar `) + checkLinterWarnings(t, sb, &lintTestParams{ Dockerfile: dockerfile, Warnings: []expectedLintWarning{ { RuleName: "ConsistentInstructionCasing", - Description: "Instructions should be in consistent casing (all lower or all upper)", + Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", - Detail: "Command 'frOM' should be consistently cased", - Line: 3, + Detail: "Command 'copy' should match the case of the command majority (uppercase)", + Line: 4, Level: 1, }, }, }) -} -func testFileConsistentCommandCasing(t *testing.T, sb integration.Sandbox) { - dockerfile := []byte(` -FROM scratch -# warning: 'copy' should match command majority's casing (uppercase) -copy Dockerfile /foo -COPY Dockerfile /bar + dockerfile = []byte(` +# warning: 'frOM' should be either lowercased or uppercased +frOM scratch as base +from scratch as base2 `) - checkLinterWarnings(t, sb, &lintTestParams{ Dockerfile: dockerfile, Warnings: []expectedLintWarning{ { - RuleName: "FileConsistentCommandCasing", + RuleName: "ConsistentInstructionCasing", Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", - URL: "https://docs.docker.com/go/dockerfile/rule/file-consistent-command-casing/", - Detail: "Command 'copy' should match the case of the command majority (uppercase)", - Line: 4, + URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", + Detail: "Command 'frOM' should match the case of the command majority (lowercase)", + Line: 3, Level: 1, }, }, @@ -336,9 +333,9 @@ copy Dockerfile /bar Dockerfile: dockerfile, Warnings: []expectedLintWarning{ { - RuleName: "FileConsistentCommandCasing", + RuleName: "ConsistentInstructionCasing", Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", - URL: "https://docs.docker.com/go/dockerfile/rule/file-consistent-command-casing/", + URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", Detail: "Command 'COPY' should match the case of the command majority (lowercase)", Line: 4, Level: 1, @@ -357,9 +354,9 @@ COPY Dockerfile /baz Dockerfile: dockerfile, Warnings: []expectedLintWarning{ { - RuleName: "FileConsistentCommandCasing", + RuleName: "ConsistentInstructionCasing", Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", - URL: "https://docs.docker.com/go/dockerfile/rule/file-consistent-command-casing/", + URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", Detail: "Command 'from' should match the case of the command majority (uppercase)", Line: 3, Level: 1, @@ -378,9 +375,9 @@ copy Dockerfile /baz Dockerfile: dockerfile, Warnings: []expectedLintWarning{ { - RuleName: "FileConsistentCommandCasing", + RuleName: "ConsistentInstructionCasing", Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", - URL: "https://docs.docker.com/go/dockerfile/rule/file-consistent-command-casing/", + URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", Detail: "Command 'FROM' should match the case of the command majority (lowercase)", Line: 3, Level: 1, diff --git a/frontend/dockerfile/docs/rules/_index.md b/frontend/dockerfile/docs/rules/_index.md index 364f849772c3..87a3c5d6f56e 100644 --- a/frontend/dockerfile/docs/rules/_index.md +++ b/frontend/dockerfile/docs/rules/_index.md @@ -42,10 +42,6 @@ $ docker build --check . ConsistentInstructionCasing - Instructions should be in consistent casing (all lower or all upper) - - - FileConsistentCommandCasing All commands within the Dockerfile should use the same casing (either upper or lower) diff --git a/frontend/dockerfile/docs/rules/consistent-instruction-casing.md b/frontend/dockerfile/docs/rules/consistent-instruction-casing.md index f2432bede251..b080490d4736 100644 --- a/frontend/dockerfile/docs/rules/consistent-instruction-casing.md +++ b/frontend/dockerfile/docs/rules/consistent-instruction-casing.md @@ -1,6 +1,6 @@ --- title: ConsistentInstructionCasing -description: Instructions should be in consistent casing (all lower or all upper) +description: All commands within the Dockerfile should use the same casing (either upper or lower) aliases: - /go/dockerfile/rule/consistent-instruction-casing/ --- diff --git a/frontend/dockerfile/docs/rules/file-consistent-command-casing.md b/frontend/dockerfile/docs/rules/file-consistent-command-casing.md deleted file mode 100644 index 7d6188b1f578..000000000000 --- a/frontend/dockerfile/docs/rules/file-consistent-command-casing.md +++ /dev/null @@ -1,61 +0,0 @@ ---- -title: FileConsistentCommandCasing -description: All commands within the Dockerfile should use the same casing (either upper or lower) -aliases: - - /go/dockerfile/rule/file-consistent-command-casing/ ---- - -## Output - -Example warning: - -```text -Command 'foo' should match the case of the command majority (uppercase) -``` - -## Description - -Instructions within a Dockerfile should have consistent casing through out the -entire files. Instructions are not case-sensitive, but the convention is to use -uppercase for instruction keywords to make it easier to distinguish keywords -from arguments. - -Whether you prefer instructions to be uppercase or lowercase, you should make -sure you use consistent casing to help improve readability of the Dockerfile. - -## Examples - -❌ Bad: mixed uppercase and lowercase. - -```dockerfile -FROM alpine:latest AS builder -run apk --no-cache add build-base - -FROM builder AS build1 -copy source1.cpp source.cpp -``` - -✅ Good: all uppercase. - -```dockerfile -FROM alpine:latest AS builder -RUN apk --no-cache add build-base - -FROM builder AS build1 -COPY source1.cpp source.cpp -``` - -✅ Good: all lowercase. - -```dockerfile -from alpine:latest as builder -run apk --no-cache add build-base - -from builder as build1 -copy source1.cpp source.cpp -``` - -## Related errors - -- [`FromAsCasing`](./from-as-casing.md) - diff --git a/frontend/dockerfile/linter/docs/FileConsistentCommandCasing.md b/frontend/dockerfile/linter/docs/FileConsistentCommandCasing.md deleted file mode 100644 index 18872ffcb3a3..000000000000 --- a/frontend/dockerfile/linter/docs/FileConsistentCommandCasing.md +++ /dev/null @@ -1,53 +0,0 @@ -## Output - -Example warning: - -```text -Command 'foo' should match the case of the command majority (uppercase) -``` - -## Description - -Instructions within a Dockerfile should have consistent casing through out the -entire files. Instructions are not case-sensitive, but the convention is to use -uppercase for instruction keywords to make it easier to distinguish keywords -from arguments. - -Whether you prefer instructions to be uppercase or lowercase, you should make -sure you use consistent casing to help improve readability of the Dockerfile. - -## Examples - -❌ Bad: mixed uppercase and lowercase. - -```dockerfile -FROM alpine:latest AS builder -run apk --no-cache add build-base - -FROM builder AS build1 -copy source1.cpp source.cpp -``` - -✅ Good: all uppercase. - -```dockerfile -FROM alpine:latest AS builder -RUN apk --no-cache add build-base - -FROM builder AS build1 -COPY source1.cpp source.cpp -``` - -✅ Good: all lowercase. - -```dockerfile -from alpine:latest as builder -run apk --no-cache add build-base - -from builder as build1 -copy source1.cpp source.cpp -``` - -## Related errors - -- [`FromAsCasing`](./from-as-casing.md) diff --git a/frontend/dockerfile/linter/ruleset.go b/frontend/dockerfile/linter/ruleset.go index 40b3b643cf39..6c70c8ee0c2e 100644 --- a/frontend/dockerfile/linter/ruleset.go +++ b/frontend/dockerfile/linter/ruleset.go @@ -29,18 +29,10 @@ var ( return "Empty continuation line" }, } - RuleConsistentInstructionCasing = LinterRule[func(string) string]{ + RuleConsistentInstructionCasing = LinterRule[func(string, string) string]{ Name: "ConsistentInstructionCasing", - Description: "Instructions should be in consistent casing (all lower or all upper)", - URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", - Format: func(command string) string { - return fmt.Sprintf("Command '%s' should be consistently cased", command) - }, - } - RuleFileConsistentCommandCasing = LinterRule[func(string, string) string]{ - Name: "FileConsistentCommandCasing", Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", - URL: "https://docs.docker.com/go/dockerfile/rule/file-consistent-command-casing/", + URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", Format: func(violatingCommand, correctCasing string) string { return fmt.Sprintf("Command '%s' should match the case of the command majority (%s)", violatingCommand, correctCasing) },