Skip to content

Commit

Permalink
Consolidate instruction casing lint rules
Browse files Browse the repository at this point in the history
Signed-off-by: Talon Bowler <talon.bowler@docker.com>
(cherry picked from commit 8cbb98e)
  • Loading branch information
daghack authored and tonistiigi committed Jun 18, 2024
1 parent 050e3b6 commit e34c21a
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 176 deletions.
23 changes: 8 additions & 15 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -2163,22 +2163,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)
}
}
}
}
Expand Down
61 changes: 29 additions & 32 deletions frontend/dockerfile/dockerfile_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ var lintTests = integration.TestFuncs(
testStageName,
testNoEmptyContinuation,
testConsistentInstructionCasing,
testFileConsistentCommandCasing,
testDuplicateStageName,
testReservedStageName,
testJSONArgsRecommended,
Expand Down Expand Up @@ -96,19 +95,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 .
`)
Expand All @@ -126,7 +125,7 @@ copy Dockerfile .
},
})

dockerfile = []byte(`#check=skip=FileConsistentCommandCasing;error=true
dockerfile = []byte(`#check=skip=ConsistentInstructionCasing;error=true
FROM scratch as base
copy Dockerfile .
`)
Expand Down Expand Up @@ -167,7 +166,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",
},
})

Expand Down Expand Up @@ -274,52 +273,50 @@ 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,
},
},
})

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,
},
},
Expand All @@ -335,9 +332,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,
Expand All @@ -356,9 +353,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,
Expand All @@ -377,9 +374,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,
Expand Down
4 changes: 0 additions & 4 deletions frontend/dockerfile/docs/rules/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ $ docker build --check .
</tr>
<tr>
<td><a href="./consistent-instruction-casing/">ConsistentInstructionCasing</a></td>
<td>Instructions should be in consistent casing (all lower or all upper)</td>
</tr>
<tr>
<td><a href="./file-consistent-command-casing/">FileConsistentCommandCasing</a></td>
<td>All commands within the Dockerfile should use the same casing (either upper or lower)</td>
</tr>
<tr>
Expand Down
Original file line number Diff line number Diff line change
@@ -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/
---
Expand Down
61 changes: 0 additions & 61 deletions frontend/dockerfile/docs/rules/file-consistent-command-casing.md

This file was deleted.

53 changes: 0 additions & 53 deletions frontend/dockerfile/linter/docs/FileConsistentCommandCasing.md

This file was deleted.

12 changes: 2 additions & 10 deletions frontend/dockerfile/linter/ruleset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
Expand Down

0 comments on commit e34c21a

Please sign in to comment.