Add rule for arg / stage name comment descriptions

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

frontend/dockerfile/dockerfile_lint_test.go

@@ -46,8 +46,71 @@ 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
+`)
+	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 stage/arg should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line/comment between them.",
+				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 stage/arg should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line/comment between them.",
+				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 stage/arg should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line/comment between them.",
+				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 stage/arg should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line/comment between them.",
+				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

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 stage/arg should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line/comment between them.</td>
+    </tr>
   </tbody>
 </table>

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

@@ -0,0 +1,50 @@
+---
+title: InvalidDefinitionDescription
+description: Comment for stage/arg should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line/comment between them.
+aliases:
+  - /go/dockerfile/rule/invalid-definition-description/
+---
+
+## Output
+
+```text
+Comment for stage/arg should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line/comment between them.
+```
+
+## Description
+
+When using `docker build` in conjunction with the `--check=outline` or `--check=targets` arguments, `buildx` can parse comments to show descriptions for arguments and stages. This functionality relies on the descriptive comments being particularly formatted. In the cases where comments immediately preceding the `FROM` or `ARG` commands are not meant to be descriptions, add an empty line / comment between them.
+
+## 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 This is the base stage.
+FROM scratch AS base
+# VERSION This is the version number.
+ARG VERSION=1
+```
+

frontend/dockerfile/instructions/parse.go

@@ -100,7 +100,12 @@ func ParseInstructionWithLinter(node *parser.Node, lint *linter.Linter) (v inter
 			msg := linter.RuleFromAsCasing.Format(req.command, req.args[1])
 			lint.Run(&linter.RuleFromAsCasing, node.Location(), msg)
 		}
-		return parseFrom(req)
+		fromCmd, err := parseFrom(req)
+		if err != nil {
+			return nil, err
+		}
+		validateDefinitionDescription("FROM", []string{fromCmd.Name}, node.PrevComment, node.Location(), lint)
+		return fromCmd, nil
 	case command.Onbuild:
 		return parseOnBuild(req)
 	case command.Workdir:
@@ -122,7 +127,16 @@ func ParseInstructionWithLinter(node *parser.Node, lint *linter.Linter) (v inter
 	case command.StopSignal:
 		return parseStopSignal(req)
 	case command.Arg:
-		return parseArg(req)
+		argCmd, err := parseArg(req)
+		if err != nil {
+			return nil, err
+		}
+		argKeys := []string{}
+		for _, arg := range argCmd.Args {
+			argKeys = append(argKeys, arg.Key)
+		}
+		validateDefinitionDescription("ARG", argKeys, node.PrevComment, node.Location(), lint)
+		return argCmd, nil
 	case command.Shell:
 		return parseShell(req)
 	}
@@ -857,3 +871,22 @@ func doesFromCaseMatchAsCase(req parseRequest) bool {
 	}
 	return req.args[1] == strings.ToUpper(req.args[1])
 }
+
+func validateDefinitionDescription(instruction string, argKeys []string, descComments []string, location []parser.Range, lint *linter.Linter) {
+	if len(descComments) == 0 || len(argKeys) == 0 {
+		return
+	}
+	descCommentParts := strings.Split(descComments[len(descComments)-1], " ")
+	for _, key := range argKeys {
+		if key == descCommentParts[0] {
+			return
+		}
+	}
+	exampleKey := argKeys[0]
+	if len(argKeys) > 1 {
+		exampleKey = "<arg_key>"
+	}
+
+	msg := linter.RuleInvalidDefinitionDescription.Format(instruction, exampleKey)
+	lint.Run(&linter.RuleInvalidDefinitionDescription, location, msg)
+}

frontend/dockerfile/linter/docs/InvalidDefinitionDescription.md

@@ -0,0 +1,42 @@
+## Output
+
+```text
+Comment for stage/arg should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line/comment between them.
+```
+
+## Description
+
+When using `docker build` in conjunction with the `--check=outline` or `--check=targets` arguments, `buildx` can parse comments to show descriptions for arguments and stages. This functionality relies on the descriptive comments being particularly formatted. In the cases where comments immediately preceding the `FROM` or `ARG` commands are not meant to be descriptions, add an empty line / comment between them.
+
+## 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 This is the base stage.
+FROM scratch AS base
+# VERSION This is the version number.
+ARG VERSION=1
+```

frontend/dockerfile/linter/ruleset.go

@@ -164,4 +164,12 @@ var (
 			return fmt.Sprintf("Attempting to %s file %q that is excluded by .dockerignore", cmd, file)
 		},
 	}
+	RuleInvalidDefinitionDescription = LinterRule[func(string, string) string]{
+		Name:        "InvalidDefinitionDescription",
+		Description: "Comment for stage/arg should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line/comment between them.",
+		URL:         "https://docs.docker.com/go/dockerfile/rule/invalid-definition-description/",
+		Format: func(instruction, defName string) string {
+			return fmt.Sprintf("Comment for %s should follow the format: `# %s <description>`", instruction, defName)
+		},
+	}
 )