Repository: openllb/hlb
Pull request #206 (2020-11-11 23:04:24)

Deprecate groups, introduce pipelines
- The name `group` and `parallel` has been confusing to explain to newcomers. After the last few months of usage, it seems more intuitive to be called `pipeline` and `stage`.
- Introduces linter rules and auto fix for the deprecation.

![group-pipeline](https://user-images.githubusercontent.com/6493975/98874466-2ce1f200-242f-11eb-9be9-27c73c83bed9.png)


Patch:
diff --git a/build.hlb b/build.hlb
index 7b9f1e7..74c4ae4 100644
--- a/build.hlb
+++ b/build.hlb
@@ -16,8 +16,8 @@ fs lint() {
 	go.lint src
 }
 
-group gen() {
-	parallel fs {
+pipeline gen() {
+	stage fs {
 		mkdocs.generatedBuiltin
 		download "./builtin/."
 	} fs {
diff --git a/builtin/builtin.go b/builtin/builtin.go
index 6ca83b2..bd58df7 100644
--- a/builtin/builtin.go
+++ b/builtin/builtin.go
@@ -57,7 +57,8 @@ var (
 			"localEnv":  codegen.LocalEnv{},
 			"localRun":  codegen.LocalRun{},
 		},
-		parser.Group: map[string]parser.Callable{
+		parser.Pipeline: map[string]parser.Callable{
+			"stage":    codegen.Stage{},
 			"parallel": codegen.Stage{},
 		},
 		"option::image": map[string]parser.Callable{
diff --git a/builtin/lookup.go b/builtin/lookup.go
index ef160d9..d9002ae 100644
--- a/builtin/lookup.go
+++ b/builtin/lookup.go
@@ -195,16 +195,6 @@ var (
 					},
 				},
 			},
-			"group": LookupByKind{
-				Func: map[string]FuncLookup{
-					"parallel": FuncLookup{
-						Params: []*parser.Field{
-							parser.NewField("group", "groups", true),
-						},
-						Effects: []*parser.Field{},
-					},
-				},
-			},
 			"option::copy": LookupByKind{
 				Func: map[string]FuncLookup{
 					"followSymlinks": FuncLookup{
@@ -593,6 +583,16 @@ var (
 					},
 				},
 			},
+			"pipeline": LookupByKind{
+				Func: map[string]FuncLookup{
+					"stage": FuncLookup{
+						Params: []*parser.Field{
+							parser.NewField("pipeline", "pipelines", true),
+						},
+						Effects: []*parser.Field{},
+					},
+				},
+			},
 			parser.String: LookupByKind{
 				Func: map[string]FuncLookup{
 					"format": FuncLookup{
@@ -1304,12 +1304,12 @@ string template(string text)
 # @return an option to add a field to the template.
 option::template stringField(string name, string value)
 
-# Executes group or filesystem target(s). Multiple targets specified within
+# Executes pipeline or filesystem target(s). Multiple targets specified within
 # a stage is executed in parallel. 
 #
-# @param groups the targets to run in parallel.
-# @return a group that returns when all its targets have finished.
-group parallel(variadic group groups)
+# @param pipelines the targets to run in parallel.
+# @return a pipeline that returns when all its targets have finished.
+pipeline stage(variadic pipeline pipelines)
 
 `
 )
diff --git a/checker/checker.go b/checker/checker.go
index 8eabb90..1b2eecc 100644
--- a/checker/checker.go
+++ b/checker/checker.go
@@ -376,7 +376,7 @@ func (c *checker) checkCall(scope *parser.Scope, kset *parser.KindSet, ie *parse
 }
 
 func (c *checker) checkExpr(scope *parser.Scope, kset *parser.KindSet, expr *parser.Expr) error {
-	if kset.Has(parser.Group) {
+	if kset.Has(parser.Pipeline) {
 		kset = parser.NewKindSet(append(kset.Kinds(), parser.Filesystem)...)
 	}
 	switch {
diff --git a/cmd/hlb/command/lint.go b/cmd/hlb/command/lint.go
index a62cd60..6083ace 100644
--- a/cmd/hlb/command/lint.go
+++ b/cmd/hlb/command/lint.go
@@ -61,24 +61,28 @@ func Lint(ctx context.Context, r io.Reader, info LintInfo) error {
 	if err != nil {
 		spans := diagnostic.Spans(err)
 		for _, span := range spans {
-			fmt.Fprintf(os.Stderr, "%s\n", span.Pretty(ctx))
+			if !info.Fix {
+				fmt.Fprintf(os.Stderr, "%s\n", span.Pretty(ctx))
+				continue
+			}
 
-			if info.Fix {
-				var em *errdefs.ErrModule
-				if errors.As(span, &em) {
-					filename := em.Module.Pos.Filename
-					info, err := os.Stat(filename)
-					if err != nil {
-						return err
-					}
+			var em *errdefs.ErrModule
+			if errors.As(span, &em) {
+				filename := em.Module.Pos.Filename
+				info, err := os.Stat(filename)
+				if err != nil {
+					return err
+				}
 
-					err = ioutil.WriteFile(filename, []byte(em.Module.String()), info.Mode())
-					if err != nil {
-						return err
-					}
+				err = ioutil.WriteFile(filename, []byte(em.Module.String()), info.Mode())
+				if err != nil {
+					return err
 				}
 			}
 		}
+		if info.Fix {
+			return nil
+		}
 
 		color := diagnostic.Color(ctx)
 		fmt.Fprint(os.Stderr, color.Sprintf(
@@ -86,9 +90,6 @@ func Lint(ctx context.Context, r io.Reader, info LintInfo) error {
 			color.Green(fmt.Sprintf("`hlb lint --fix %s`", mod.Pos.Filename)),
 		))
 
-		if info.Fix {
-			return nil
-		}
 		return errdefs.WithAbort(err, len(spans))
 	}
 
diff --git a/codegen/value.go b/codegen/value.go
index 0b7193f..87d44f3 100644
--- a/codegen/value.go
+++ b/codegen/value.go
@@ -281,7 +281,7 @@ type reqValue struct {
 }
 
 func (v *reqValue) Kind() parser.Kind {
-	return parser.Group
+	return parser.Pipeline
 }
 
 func (v *reqValue) Request() (solver.Request, error) {
diff --git a/language/builtin.hlb b/language/builtin.hlb
index e3dbc31..f7792d8 100644
--- a/language/builtin.hlb
+++ b/language/builtin.hlb
@@ -654,9 +654,9 @@ string template(string text)
 # @return an option to add a field to the template.
 option::template stringField(string name, string value)
 
-# Executes group or filesystem target(s). Multiple targets specified within
+# Executes pipeline or filesystem target(s). Multiple targets specified within
 # a stage is executed in parallel. 
 #
-# @param groups the targets to run in parallel.
-# @return a group that returns when all its targets have finished.
-group parallel(variadic group groups)
+# @param pipelines the targets to run in parallel.
+# @return a pipeline that returns when all its targets have finished.
+pipeline stage(variadic pipeline pipelines)
diff --git a/linter/linter.go b/linter/linter.go
index 5467b42..f2efad5 100644
--- a/linter/linter.go
+++ b/linter/linter.go
@@ -55,6 +55,24 @@ func (l *Linter) Lint(ctx context.Context, mod *parser.Module) {
 				l.LintRecursive(ctx, mod, id.Expr)
 			}
 		},
+		func(t *parser.Type) {
+			if string(t.Kind) == "group" {
+				l.errs = append(l.errs, errdefs.WithDeprecated(
+					mod, t,
+					"type `group` is deprecated, use `pipeline` instead",
+				))
+				t.Kind = parser.Pipeline
+			}
+		},
+		func(call *parser.CallStmt) {
+			if call.Name != nil && call.Name.Ident.Text == "parallel" {
+				l.errs = append(l.errs, errdefs.WithDeprecated(
+					mod, call.Name,
+					"function `parallel` is deprecated, use `stage` instead",
+				))
+				call.Name.Ident.Text = "stage"
+			}
+		},
 	)
 }
 
diff --git a/parser/cst.go b/parser/cst.go
index 1f1d952..8e5eb36 100644
--- a/parser/cst.go
+++ b/parser/cst.go
@@ -131,7 +131,7 @@ const (
 	Int        Kind = "int"
 	Bool       Kind = "bool"
 	Filesystem Kind = "fs"
-	Group      Kind = "group"
+	Pipeline   Kind = "pipeline"
 	Option     Kind = "option"
 )
 


Test patch:
diff --git a/checker/checker_test.go b/checker/checker_test.go
index da52b7e..75413ef 100644
--- a/checker/checker_test.go
+++ b/checker/checker_test.go
@@ -320,31 +320,31 @@ func TestChecker_Check(t *testing.T) {
 			)
 		},
 	}, {
-		"basic group support",
+		"basic pipeline support",
 		`
-		group default() {
-			parallel groupA fs { image "b"; }
-			groupC
+		pipeline default() {
+			stage pipelineA fs { image "b"; }
+			pipelineC
 		}
-		group groupA() {
-			parallel fs { image "a"; }
+		pipeline pipelineA() {
+			stage fs { image "a"; }
 		}
-		group groupC() {
-			parallel fs { image "c"; }
+		pipeline pipelineC() {
+			stage fs { image "c"; }
 		}
 		`,
 		nil,
 	}, {
-		"errors when fs statement is called in a group block",
+		"errors when fs statement is called in a pipeline block",
 		`
-		group badGroup() {
+		pipeline badGroup() {
 			image "alpine"
 		}
 		`,
 		func(mod *parser.Module) error {
 			return errdefs.WithWrongType(
 				parser.Find(mod, "image"),
-				[]parser.Kind{parser.Group},
+				[]parser.Kind{parser.Pipeline},
 				parser.Filesystem,
 				errdefs.Defined(parser.Find(builtin.Module, "image")),
 			)