Merge pull request moby#5046 from daghack/consolidate-case-checks

Consolidate instruction casing lint rules

Author: tonistiigi

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)
-			}
 		}
 	}
 }

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,52 +274,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,
 			},
 		},
@@ -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,

frontend/dockerfile/docs/rules/_index.md

@@ -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>

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/
 ---

frontend/dockerfile/docs/rules/file-consistent-command-casing.md

@@ -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)
 		},