Add rule for arg / stage name comment descriptions

Signed-off-by: Talon Bowler <[email protected]>

frontend/dockerfile/dockerfile_lint_test.go

@@ -46,8 +46,79 @@ var lintTests = integration.TestFuncs(
 	testInvalidDefaultArgInFrom,
 	testFromPlatformFlagConstDisallowed,
 	testCopyIgnoredFiles,
+	testDefinitionDescription,
 )
 
+func testDefinitionDescription(t *testing.T, sb integration.Sandbox) {
+	dockerfile := []byte(`
+# foo this is the foo
+ARG foo=bar
+
+# base this is the base image
+FROM scratch AS base
+
+# version this is the version number
+ARG version=latest
+
+# baz this is the baz
+ARG foo=baz bar=qux baz=quux
+#
+ARG bit=bat
+
+# comment for something other than ARG or FROM
+COPY Dockerfile .
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
+
+	dockerfile = []byte(`
+# bar this is the bar
+ARG foo=bar
+# BasE this is the BasE image
+FROM scratch AS base
+# definitely a bad comment
+ARG version=latest
+# definitely a bad comment
+ARG foo=baz bar=qux baz=quux
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{
+		Dockerfile: dockerfile,
+		Warnings: []expectedLintWarning{
+			{
+				RuleName:    "InvalidDefinitionDescription",
+				Description: "Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.",
+				URL:         "https://docs.docker.com/go/dockerfile/rule/invalid-definition-description/",
+				Detail:      "Comment for ARG should follow the format: `# foo <description>`",
+				Level:       1,
+				Line:        3,
+			},
+			{
+				RuleName:    "InvalidDefinitionDescription",
+				Description: "Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.",
+				URL:         "https://docs.docker.com/go/dockerfile/rule/invalid-definition-description/",
+				Detail:      "Comment for FROM should follow the format: `# base <description>`",
+				Level:       1,
+				Line:        5,
+			},
+			{
+				RuleName:    "InvalidDefinitionDescription",
+				Description: "Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.",
+				URL:         "https://docs.docker.com/go/dockerfile/rule/invalid-definition-description/",
+				Detail:      "Comment for ARG should follow the format: `# version <description>`",
+				Level:       1,
+				Line:        7,
+			},
+			{
+				RuleName:    "InvalidDefinitionDescription",
+				Description: "Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.",
+				URL:         "https://docs.docker.com/go/dockerfile/rule/invalid-definition-description/",
+				Detail:      "Comment for ARG should follow the format: `# <arg_key> <description>`",
+				Level:       1,
+				Line:        9,
+			},
+		},
+	})
+}
+
 func testCopyIgnoredFiles(t *testing.T, sb integration.Sandbox) {
 	dockerignore := []byte(`
 Dockerfile
@@ -233,30 +304,35 @@ 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 .
 `)
@@ -268,13 +344,14 @@ 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:        2,
+				Line:        3,
 				Level:       1,
 			},
 		},
 	})
 
 	dockerfile = []byte(`#check=skip=ConsistentInstructionCasing;error=true
+#
 FROM scratch as base
 copy Dockerfile .
 `)
@@ -286,7 +363,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:        2,
+				Line:        3,
 				Level:       1,
 			},
 		},
@@ -296,6 +373,7 @@ copy Dockerfile .
 	})
 
 	dockerfile = []byte(`#check=skip=all
+#
 FROM scratch as base
 copy Dockerfile .
 `)
@@ -307,7 +385,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:        2,
+				Line:        3,
 				Level:       1,
 			},
 		},
@@ -320,6 +398,7 @@ copy Dockerfile .
 	})
 
 	dockerfile = []byte(`#check=error=true
+#
 FROM scratch as base
 copy Dockerfile .
 `)
@@ -334,9 +413,11 @@ copy Dockerfile .
 func testStageName(t *testing.T, sb integration.Sandbox) {
 	dockerfile := []byte(`
 # warning: stage name should be lowercase
+#
 FROM scratch AS BadStageName
 
 # warning: 'as' should match 'FROM' cmd casing.
+#
 FROM scratch as base2
 
 FROM scratch AS base3
@@ -349,22 +430,23 @@ FROM scratch AS base3
 				Description: "Stage names should be lowercase",
 				URL:         "https://docs.docker.com/go/dockerfile/rule/stage-name-casing/",
 				Detail:      "Stage name 'BadStageName' should be lowercase",
-				Line:        3,
+				Line:        4,
 				Level:       1,
 			},
 			{
 				RuleName:    "FromAsCasing",
 				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:        6,
+				Line:        8,
 				Level:       1,
 			},
 		},
 	})
 
 	dockerfile = []byte(`
 # warning: 'AS' should match 'from' cmd casing.
+#
 from scratch AS base
 
 from scratch as base2
@@ -378,7 +460,7 @@ from scratch as base2
 				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:        4,
 				Level:       1,
 			},
 		},
@@ -414,6 +496,7 @@ COPY Dockerfile \
 func testConsistentInstructionCasing(t *testing.T, sb integration.Sandbox) {
 	dockerfile := []byte(`
 # warning: 'FROM' should be either lowercased or uppercased
+#
 From scratch as base
 FROM scratch AS base2
 `)
@@ -426,7 +509,7 @@ FROM scratch AS base2
 				URL:         "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/",
 				Detail:      "Command 'From' should match the case of the command majority (uppercase)",
 				Level:       1,
-				Line:        3,
+				Line:        4,
 			},
 		},
 	})
@@ -454,6 +537,7 @@ COPY Dockerfile /bar
 
 	dockerfile = []byte(`
 # warning: 'frOM' should be either lowercased or uppercased
+#
 frOM scratch as base
 from scratch as base2
 `)
@@ -465,7 +549,7 @@ from scratch as base2
 				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 match the case of the command majority (lowercase)",
-				Line:        3,
+				Line:        4,
 				Level:       1,
 			},
 		},
@@ -493,6 +577,7 @@ copy Dockerfile /bar
 
 	dockerfile = []byte(`
 # warning: 'from' should match command majority's casing (uppercase)
+#
 from scratch
 COPY Dockerfile /foo
 COPY Dockerfile /bar
@@ -506,14 +591,15 @@ COPY Dockerfile /baz
 				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 match the case of the command majority (uppercase)",
-				Line:        3,
+				Line:        4,
 				Level:       1,
 			},
 		},
 	})
 
 	dockerfile = []byte(`
 # warning: 'FROM' should match command majority's casing (lowercase)
+#
 FROM scratch
 copy Dockerfile /foo
 copy Dockerfile /bar
@@ -527,7 +613,7 @@ copy Dockerfile /baz
 				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 match the case of the command majority (lowercase)",
-				Line:        3,
+				Line:        4,
 				Level:       1,
 			},
 		},
@@ -1282,6 +1368,12 @@ func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestPara
 			require.Less(t, lintResults.Error.Location.SourceIndex, int32(len(lintResults.Sources)))
 		}
 
+		if len(warnings) != len(lintResults.Warnings) {
+			for _, w := range lintResults.Warnings {
+				t.Logf("Warning Received: %s\n", w.Detail)
+			}
+		}
+
 		require.Equal(t, len(warnings), len(lintResults.Warnings))
 
 		sort.Slice(lintResults.Warnings, func(i, j int) bool {

frontend/dockerfile/docs/rules/_index.md

@@ -100,5 +100,9 @@ $ docker build --check .
       <td><a href="./copy-ignored-file/">CopyIgnoredFile</a></td>
       <td>Attempting to Copy file that is excluded by .dockerignore</td>
     </tr>
+    <tr>
+      <td><a href="./invalid-definition-description/">InvalidDefinitionDescription</a></td>
+      <td>Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.</td>
+    </tr>
   </tbody>
 </table>

frontend/dockerfile/docs/rules/invalid-definition-description.md

@@ -0,0 +1,66 @@
+---
+title: InvalidDefinitionDescription
+description: Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.
+aliases:
+  - /go/dockerfile/rule/invalid-definition-description/
+---
+
+## Output
+
+```text
+Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.
+```
+
+## Description
+
+The [`--call=outline`](https://docs.docker.com/reference/cli/docker/buildx/build/#call-outline)
+and [`--call=targets`](https://docs.docker.com/reference/cli/docker/buildx/build/#call-outline)
+flags for the `docker build` command print descriptions for build targets and arguments.
+The descriptions are generated from [Dockerfile comments](https://docs.docker.com/reference/cli/docker/buildx/build/#descriptions)
+that immediately precede the `FROM` or `ARG` instruction
+and that begin with the name of the build stage or argument.
+For example:
+
+```dockerfile
+# build-cli builds the CLI binary
+FROM alpine AS build-cli
+# VERSION controls the version of the program
+ARG VERSION=1
+```
+
+In cases where preceding comments are not meant to be descriptions,
+add an empty line or comment between the instruction and the preceding comment.
+
+## Examples
+
+❌ Bad: A non-descriptive comment on the line preceding the `FROM` command.
+
+```dockerfile
+# a non-descriptive comment
+FROM scratch AS base
+
+# another non-descriptive comment
+ARG VERSION=1
+```
+
+✅ Good: An empty line separating non-descriptive comments.
+
+```dockerfile
+# a non-descriptive comment
+
+FROM scratch AS base
+
+# another non-descriptive comment
+
+ARG VERSION=1
+```
+
+✅ Good: Comments describing `ARG` keys and stages immediately proceeding the command.
+
+```dockerfile
+# base is a stage for compiling source
+FROM scratch AS base
+# VERSION This is the version number.
+ARG VERSION=1
+```
+