Merge pull request moby#5243 from daghack/heredoc-case-consistent

frontend: command casing rule check after parsing the ast

Author: tonistiigi

frontend/dockerfile/dockerfile2llb/convert.go

@@ -251,8 +251,6 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 		}
 	}
 
-	validateCommandCasing(dockerfile, lint)
-
 	proxyEnv := proxyEnvFromBuildArgs(opt.BuildArgs)
 
 	stages, metaArgs, err := instructions.Parse(dockerfile.AST, lint)
@@ -263,6 +261,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 		return nil, errors.New("dockerfile contains no stages to build")
 	}
 	validateStageNames(stages, lint)
+	validateCommandCasing(stages, lint)
 
 	shlex := shell.NewLex(dockerfile.EscapeToken)
 	outline := newOutlineCapture()
@@ -2199,32 +2198,49 @@ func isSelfConsistentCasing(s string) bool {
 	return s == strings.ToLower(s) || s == strings.ToUpper(s)
 }
 
-func validateCommandCasing(dockerfile *parser.Result, lint *linter.Linter) {
+func validateCaseMatch(name string, isMajorityLower bool, location []parser.Range, lint *linter.Linter) {
+	var correctCasing string
+	if isMajorityLower && strings.ToLower(name) != name {
+		correctCasing = "lowercase"
+	} else if !isMajorityLower && strings.ToUpper(name) != name {
+		correctCasing = "uppercase"
+	}
+	if correctCasing != "" {
+		msg := linter.RuleConsistentInstructionCasing.Format(name, correctCasing)
+		lint.Run(&linter.RuleConsistentInstructionCasing, location, msg)
+	}
+}
+
+func validateCommandCasing(stages []instructions.Stage, lint *linter.Linter) {
 	var lowerCount, upperCount int
-	for _, node := range dockerfile.AST.Children {
-		if isSelfConsistentCasing(node.Value) {
-			if strings.ToLower(node.Value) == node.Value {
+	for _, stage := range stages {
+		if isSelfConsistentCasing(stage.OrigCmd) {
+			if strings.ToLower(stage.OrigCmd) == stage.OrigCmd {
 				lowerCount++
 			} else {
 				upperCount++
 			}
 		}
+		for _, cmd := range stage.Commands {
+			cmdName := cmd.Name()
+			if isSelfConsistentCasing(cmdName) {
+				if strings.ToLower(cmdName) == cmdName {
+					lowerCount++
+				} else {
+					upperCount++
+				}
+			}
+		}
 	}
 
 	isMajorityLower := lowerCount > upperCount
-	for _, node := range dockerfile.AST.Children {
+	for _, stage := range stages {
 		// 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.
-		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)
+		validateCaseMatch(stage.OrigCmd, isMajorityLower, stage.Location, lint)
+		for _, cmd := range stage.Commands {
+			validateCaseMatch(cmd.Name(), isMajorityLower, cmd.Location(), lint)
 		}
 	}
 }

frontend/dockerfile/dockerfile_lint_test.go

@@ -578,6 +578,14 @@ copy Dockerfile /bar
 FROM scratch
 COPY Dockerfile /foo
 COPY Dockerfile /bar
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
+
+	dockerfile = []byte(`
+FROM alpine
+RUN <<'EOT'
+env
+EOT
 `)
 	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
 }

frontend/dockerfile/instructions/commands.go

@@ -506,6 +506,7 @@ type ShellCommand struct {
 type Stage struct {
 	Name     string    // name of the stage
 	Commands []Command // commands contained within the stage
+	OrigCmd  string    // original FROM command, used for rule checks
 	BaseName string    // name of the base stage or source
 	Platform string    // platform of base source to use
 

frontend/dockerfile/instructions/parse.go

@@ -398,6 +398,7 @@ func parseFrom(req parseRequest) (*Stage, error) {
 	code := strings.TrimSpace(req.original)
 	return &Stage{
 		BaseName:   req.args[0],
+		OrigCmd:    req.command,
 		Name:       stageName,
 		SourceCode: code,
 		Commands:   []Command{},