internal/buildattr: implement @ignore attributes

This adds support for an `@ignore()` attribute to enable a file to be
unconditionally ignored. Unlike `@if` build attributes, a file with an
`@ignore()` attribute is always ignored when calculating dependencies,
for example when running `cue mod tidy`.

We decided against following Go's lead here, which would have been
`@if(ignore)` because there is no existing convention like Go had, and
because this is sufficiently different from regular build tags that it
seems like it justifies another attribute. Also because `@ignore()`
seems to read a bit better than `@if(ignore)`.

Fixes #2962.

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I7ee84cea2ffce6765bd7667feb78c4d44e0254ea
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1198249
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

Author: rogpeppe

cmd/cue/cmd/help.go

@@ -581,7 +581,8 @@ Injecting files
 A "build" attribute defines a boolean expression that causes a file
 to only be included in a build if its expression evaluates to true.
 There may only be a single @if attribute per file and it must
-appear before a package clause.
+appear before a package clause, or before any CUE declarations
+if there is no package clause.
 
 The expression is a subset of CUE consisting only of identifiers
 and the operators &&, ||, !, where identifiers refer to tags
@@ -595,6 +596,19 @@ if the user includes the flag "-t prod" on the command line.
 
    package foo
 
+Ignoring files
+
+An "ignore" attribute causes a file to be unconditionally excluded
+from a build. The @ignore attribute must appear before a package
+clause or before any other CUE syntax if there is no package clause.
+
+For example:
+
+    @ignore()
+
+    // This file will be excluded for all purposes.
+
+    package foo
 
 Injecting values
 

cmd/cue/cmd/testdata/script/modtidy_with_build_attrs.txtar

@@ -41,32 +41,15 @@ deps: {
 		v:       "v0.0.1"
 		default: true
 	}
-	"test.example/d7@v0": {
-		v:       "v0.0.1"
-		default: true
-	}
-	"test.example/d8@v0": {
-		v:       "v0.0.1"
-		default: true
-	}
 }
 -- want-stdout-1 --
 prod: false
-ignore: {
-    self: "test.example/d7"
-}
 x: {
     self: "test.example/d2"
 }
 -- want-stdout-2 --
 prod: true
-ignore: {
-    self: "test.example/d7"
-}
 x: {
-    ignore: {
-        self: "test.example/d8"
-    }
     self:        "test.example/d1"
     prodenabled: false
     y: {

cue/load/tags.go

@@ -358,9 +358,12 @@ func shouldBuildFile(f *ast.File, tagIsSet func(key string) bool) errors.Error {
 	if err != nil {
 		return err
 	}
-	if !ok {
-		_, body := attr.Split()
+	if ok {
+		return nil
+	}
+	if key, body := attr.Split(); key == "if" {
 		return excludeError{errors.Newf(attr.Pos(), "@if(%s) did not match", body)}
+	} else {
+		return excludeError{errors.Newf(attr.Pos(), "@ignore() attribute found")}
 	}
-	return nil
 }

internal/buildattr/buildattr.go

@@ -9,14 +9,21 @@ import (
 	"cuelang.org/go/cue/token"
 )
 
+// ShouldIgnoreFile reports whether a File contains an @ignore() file-level
+// attribute and hence should be ignored.
+func ShouldIgnoreFile(f *ast.File) bool {
+	ignore, _, _ := getBuildAttr(f)
+	return ignore
+}
+
 // ShouldBuildFile reports whether a File should be included based on its
 // attributes. It uses tagIsSet to determine whether a given attribute
 // key should be treated as set.
 //
 // It also returns the build attribute if one was found.
 func ShouldBuildFile(f *ast.File, tagIsSet func(key string) bool) (bool, *ast.Attribute, errors.Error) {
-	a, err := getBuildAttr(f)
-	if err != nil {
+	ignore, a, err := getBuildAttr(f)
+	if ignore || err != nil {
 		return false, a, err
 	}
 	if a == nil {
@@ -37,28 +44,33 @@ func ShouldBuildFile(f *ast.File, tagIsSet func(key string) bool) (bool, *ast.At
 	return include, a, nil
 }
 
-func getBuildAttr(f *ast.File) (*ast.Attribute, errors.Error) {
-	var a *ast.Attribute
+func getBuildAttr(f *ast.File) (ignore bool, a *ast.Attribute, err errors.Error) {
 	for _, d := range f.Decls {
 		switch x := d.(type) {
 		case *ast.Attribute:
-			key, _ := x.Split()
-			if key != "if" {
-				continue
+			switch key, _ := x.Split(); key {
+			case "ignore":
+				return true, x, nil
+			case "if":
+				if a != nil {
+					err := errors.Newf(d.Pos(), "multiple @if attributes")
+					err = errors.Append(err,
+						errors.Newf(a.Pos(), "previous declaration here"))
+					return false, a, err
+				}
+				a = x
 			}
-			if a != nil {
-				err := errors.Newf(d.Pos(), "multiple @if attributes")
-				err = errors.Append(err,
-					errors.Newf(a.Pos(), "previous declaration here"))
-				return a, err
-			}
-			a = x
-
 		case *ast.Package:
-			return a, nil
+			return false, a, nil
+		case *ast.CommentGroup:
+		default:
+			// If it's anything else, then we know we won't see a package
+			// clause so avoid scanning more than we need to (this
+			// could be a large file with no package clause)
+			return false, a, nil
 		}
 	}
-	return a, nil
+	return false, a, nil
 }
 
 func shouldInclude(expr ast.Expr, tagIsSet func(key string) bool) (bool, errors.Error) {

internal/buildattr/buildattr_test.go

@@ -265,7 +265,8 @@ package something
 
 package something
 `,
-	wantOK: true,
+	wantOK:   false,
+	wantAttr: "@ignore()",
 }, {
 	testName: "IgnoreWithBuildAttrs",
 	syntax: `
@@ -274,12 +275,14 @@ package something
 
 package something
 `,
-	wantOK: false,
-	wantTagCalls: map[string]bool{
-		"blah": true,
-	},
-	wantAttr: "@if(blah)",
+	wantOK:   false,
+	wantAttr: "@ignore()",
 }, {
+	// It's arguable whether multiple @if attributes
+	// should be an error when there's an @ignore
+	// attribute, but it's easily worked around by
+	// putting the @ignore attribute first, which should
+	// be fairly intuitive.
 	testName: "IgnoreWithMultipleEarlierIfs",
 	syntax: `
 @if(foo)
@@ -304,31 +307,23 @@ multiple @if attributes:
 
 package something
 `,
-	wantOK: false,
-	wantError: `previous declaration here:
-    testfile.cue:3:1
-multiple @if attributes:
-    testfile.cue:4:1
-`,
-	wantAttr: "@if(foo)",
+	wantOK:   false,
+	wantAttr: "@ignore()",
 }, {
 	testName: "IgnoreWithoutPackageClause",
 	syntax: `
 @ignore()
 a: 5
 `,
-	wantOK: true,
+	wantOK:   false,
+	wantAttr: "@ignore()",
 }, {
 	testName: "IfAfterDeclaration",
 	syntax: `
 a: 1
 @if(foo)
 `,
-	wantOK: false,
-	wantTagCalls: map[string]bool{
-		"foo": true,
-	},
-	wantAttr: "@if(foo)",
+	wantOK: true,
 }}
 
 func TestShouldBuildFile(t *testing.T) {

internal/mod/modload/tidy.go

@@ -66,10 +66,10 @@ func tidy(ctx context.Context, fsys fs.FS, modRoot string, reg Registry, checkTi
 	}
 	// TODO check that module path is well formed etc
 	origRs := modrequirements.NewRequirements(mf.QualifiedModule(), reg, mf.DepVersions(), mf.DefaultMajorVersions())
-	// Note: we can just ignore build tags and the fact that we might
+	// Note: we can ignore build tags and the fact that we might
 	// have _tool.cue and _test.cue files, because we want to include
-	// all of them.
-	rootPkgPaths, err := modimports.AllImports(modimports.AllModuleFiles(fsys, modRoot))
+	// all of those, but we do need to consider @ignore() attributes.
+	rootPkgPaths, err := modimports.AllImports(withoutIgnoredFiles(modimports.AllModuleFiles(fsys, modRoot)))
 	if err != nil {
 		return nil, err
 	}
@@ -175,7 +175,11 @@ func modfileFromRequirements(old *modfile.File, rs *modrequirements.Requirements
 //
 // In general a file should always be considered unless it's a _tool.cue file
 // that's not in the main module.
-func (ld *loader) shouldIncludePkgFile(pkgPath string, mod module.Version, fsys fs.FS, mf modimports.ModuleFile) bool {
+func (ld *loader) shouldIncludePkgFile(pkgPath string, mod module.Version, fsys fs.FS, mf modimports.ModuleFile) (_ok bool) {
+	if buildattr.ShouldIgnoreFile(mf.Syntax) {
+		// The file is marked to be explicitly ignored.
+		return false
+	}
 	if mod.Path() == ld.mainModule.Path() {
 		// All files in the main module are considered.
 		return true
@@ -716,6 +720,18 @@ func (ld *loader) spotCheckRoots(ctx context.Context, rs *modrequirements.Requir
 	return true
 }
 
+func withoutIgnoredFiles(iter func(func(modimports.ModuleFile, error) bool)) func(func(modimports.ModuleFile, error) bool) {
+	return func(yield func(modimports.ModuleFile, error) bool) {
+		// TODO for mf, err := range iter {
+		iter(func(mf modimports.ModuleFile, err error) bool {
+			if err == nil && buildattr.ShouldIgnoreFile(mf.Syntax) {
+				return true
+			}
+			return yield(mf, err)
+		})
+	}
+}
+
 func logf(f string, a ...any) {
 	if logging {
 		log.Printf(f, a...)