fix: Fixing negation logic (#5368)

* chore: Adding repro red test

* fix: Fixing logic for naked negation

* fix: Fixing stack discovery negation

* fix: Fixing typo in bug report template

* fix: Avoiding usage of `slices.DeleteFunc` to prevent zeroing

Author: yhakbar

.github/ISSUE_TEMPLATE/01-bug_report.md

@@ -20,7 +20,7 @@ Without this information, it is much less likely that maintainers will invest ti
 The exceptions to requiring steps to reproduce are:
 
 1. You are reporting a bug that you don't know how to reproduce, but you are reporting it so that others in the community are aware of it.
-2. You are willing to fix the bug yourself, and you accept the reponsibility of ensuring that the bug is valid, and that the fix is well tested.
+2. You are willing to fix the bug yourself, and you accept the responsibility of ensuring that the bug is valid, and that the fix is well tested.
 
 How to provide steps for reproduction:
 

internal/filter/ast.go

@@ -21,6 +21,8 @@ type Expression interface {
 	RequiresParse() (Expression, bool)
 	// IsRestrictedToStacks returns true if the expression is restricted to stacks.
 	IsRestrictedToStacks() bool
+	// Negated returns the equivalent expression with negation flipped.
+	Negated() Expression
 }
 
 // Expressions is a slice of expressions.
@@ -57,6 +59,7 @@ func (p *PathExpression) String() string                        { return p.Value
 func (p *PathExpression) RequiresDiscovery() (Expression, bool) { return p, false }
 func (p *PathExpression) RequiresParse() (Expression, bool)     { return p, false }
 func (p *PathExpression) IsRestrictedToStacks() bool            { return false }
+func (p *PathExpression) Negated() Expression                   { return NewPrefixExpression("!", p) }
 
 // AttributeExpression represents a key-value attribute filter (e.g., "name=my-app").
 type AttributeExpression struct {
@@ -119,6 +122,9 @@ func (a *AttributeExpression) RequiresParse() (Expression, bool) {
 func (a *AttributeExpression) IsRestrictedToStacks() bool {
 	return a.Key == "type" && a.Value == "stack"
 }
+func (a *AttributeExpression) Negated() Expression {
+	return NewPrefixExpression("!", a)
+}
 
 // PrefixExpression represents a prefix operator expression (e.g., "!name=foo").
 type PrefixExpression struct {
@@ -157,6 +163,14 @@ func (p *PrefixExpression) IsRestrictedToStacks() bool {
 		return false
 	}
 }
+func (p *PrefixExpression) Negated() Expression {
+	switch p.Operator {
+	case "!":
+		return p.Right
+	default:
+		return NewPrefixExpression("!", p.Right)
+	}
+}
 
 // InfixExpression represents an infix operator expression (e.g., "./apps/* | name=bar").
 type InfixExpression struct {
@@ -204,6 +218,14 @@ func (i *InfixExpression) IsRestrictedToStacks() bool {
 		return false
 	}
 }
+func (i *InfixExpression) Negated() Expression {
+	switch i.Operator {
+	case "|":
+		return NewInfixExpression(i.Left.Negated(), i.Operator, i.Right)
+	default:
+		return NewInfixExpression(i.Left.Negated(), i.Operator, i.Right)
+	}
+}
 
 // GraphExpression represents a graph traversal expression (e.g., "...foo", "foo...", "...foo...", "^foo").
 type GraphExpression struct {
@@ -255,6 +277,9 @@ func (g *GraphExpression) RequiresParse() (Expression, bool) {
 	return g, true
 }
 func (g *GraphExpression) IsRestrictedToStacks() bool { return false }
+func (g *GraphExpression) Negated() Expression {
+	return NewPrefixExpression("!", g)
+}
 
 // GitExpression represents a Git-based filter expression (e.g., "[main...HEAD]" or "[main]").
 // It filters components based on changes between Git references.
@@ -280,6 +305,9 @@ func (g *GitExpression) RequiresParse() (Expression, bool) {
 	return nil, false
 }
 func (g *GitExpression) IsRestrictedToStacks() bool { return false }
+func (g *GitExpression) Negated() Expression {
+	return NewPrefixExpression("!", g)
+}
 
 // GitExpressions is a slice of Git expressions.
 type GitExpressions []*GitExpression

internal/filter/evaluator.go

@@ -195,20 +195,23 @@ func evaluatePrefixExpression(l log.Logger, expr *PrefixExpression, components c
 		return nil, err
 	}
 
-	excludeSet := make(map[string]struct{}, len(toExclude))
-	for _, c := range toExclude {
-		excludeSet[c.Path()] = struct{}{}
+	if len(toExclude) == 0 {
+		return components, nil
 	}
 
-	var result component.Components
+	// We don't use slices.DeleteFunc here because we don't want the members of the original components slice to be
+	// zeroed.
+	results := make(component.Components, 0, len(components)-len(toExclude))
 
 	for _, c := range components {
-		if _, ok := excludeSet[c.Path()]; !ok {
-			result = append(result, c)
+		if slices.Contains(toExclude, c) {
+			continue
 		}
+
+		results = append(results, c)
 	}
 
-	return result, nil
+	return results, nil
 }
 
 // evaluateInfixExpression evaluates an infix expression (intersection).

internal/filter/filter.go

@@ -55,6 +55,27 @@ func (f *Filter) RequiresParse() (Expression, bool) {
 	return f.expr.RequiresParse()
 }
 
+// Negated returns the equivalent filter with negation flipped.
+//
+// If the filter is already negated, it will return the non-negated filter.
+func (f *Filter) Negated() *Filter {
+	switch node := f.expr.(type) {
+	case *PrefixExpression:
+		return NewFilter(node.Right, f.originalQuery)
+	case *InfixExpression:
+		return NewFilter(
+			NewInfixExpression(
+				node.Left.Negated(),
+				node.Operator,
+				node.Right,
+			),
+			f.originalQuery,
+		)
+	default:
+		return f
+	}
+}
+
 // Apply is a convenience function that parses and evaluates a filter in one step.
 // It's equivalent to calling Parse followed by Evaluate.
 func Apply(l log.Logger, filterString string, components component.Components) (component.Components, error) {

internal/filter/filters.go

@@ -286,19 +286,44 @@ func (f Filters) Evaluate(l log.Logger, components component.Components) (compon
 		return nil, err
 	}
 
-	// Phase 2: Apply negative filters to remove components
-	for _, filter := range negativeFilters {
-		var result component.Components
+	if len(negativeFilters) == 0 {
+		return combined, nil
+	}
+
+	// Phase 2: Apply negative filters to find components to remove
+	toRemove := make(component.Components, 0, len(combined))
 
-		result, err = filter.Evaluate(l, combined)
+	for _, filter := range negativeFilters {
+		removed, err := filter.Negated().Evaluate(l, combined)
 		if err != nil {
 			return nil, err
 		}
 
-		combined = result
+		for _, c := range removed {
+			if !slices.Contains(toRemove, c) {
+				toRemove = append(toRemove, c)
+			}
+		}
+	}
+
+	if len(toRemove) == 0 {
+		return combined, nil
+	}
+
+	// Phase 3: Remove components from the initial set
+
+	// We don't use slices.DeleteFunc here because we don't want the members of the original components slice to be
+	// zeroed.
+	results := make(component.Components, 0, len(combined)-len(toRemove))
+	for _, c := range combined {
+		if slices.Contains(toRemove, c) {
+			continue
+		}
+
+		results = append(results, c)
 	}
 
-	return combined, nil
+	return results, nil
 }
 
 // EvaluateOnFiles evaluates the filters on a list of files and returns the filtered result.
@@ -336,12 +361,7 @@ func initialComponents(l log.Logger, positiveFilters []*Filter, components compo
 	seen := make(map[string]component.Component, len(components))
 
 	for _, filter := range positiveFilters {
-		var (
-			result component.Components
-			err    error
-		)
-
-		result, err = filter.Evaluate(l, components)
+		result, err := filter.Evaluate(l, components)
 		if err != nil {
 			return nil, err
 		}
@@ -376,9 +396,12 @@ func (f Filters) String() string {
 
 // startsWithNegation checks if an expression starts with a negation operator.
 func startsWithNegation(expr Expression) bool {
-	if prefixExpr, ok := expr.(*PrefixExpression); ok {
-		return prefixExpr.Operator == "!"
+	switch node := expr.(type) {
+	case *PrefixExpression:
+		return node.Operator == "!"
+	case *InfixExpression:
+		return startsWithNegation(node.Left)
+	default:
+		return false
 	}
-
-	return false
 }

test/fixtures/exclude-by-default/_stacks/terragrunt.stack.hcl

@@ -29,6 +29,7 @@ const (
 	testFixtureFilterSource           = "fixtures/filter-source"
 	testFixtureMinimizeParsing        = "fixtures/filter/minimize-parsing"
 	testFixtureMinimizeParsingDestroy = "fixtures/filter/minimize-parsing-destroy"
+	testFixtureExcludeByDefault       = "fixtures/exclude-by-default"
 )
 
 // createTestUnit creates a unit directory with terragrunt.hcl and main.tf files.
@@ -2387,3 +2388,18 @@ resource "null_resource" "test" {}
 	require.NotContains(t, output, "Too many command line arguments")
 	require.NotContains(t, output, "Expected at most one positional argument")
 }
+
+func TestFilterExcludeByDefault(t *testing.T) {
+	t.Parallel()
+
+	tmpEnvPath := helpers.CopyEnvironment(t, testFixtureExcludeByDefault)
+	rootPath := filepath.Join(tmpEnvPath, testFixtureExcludeByDefault)
+
+	helpers.CleanupTerraformFolder(t, rootPath)
+
+	cmd := "terragrunt run --all --no-color --working-dir " + rootPath + " --filter '!_stacks | type=stack' -- plan"
+	_, stderr, err := helpers.RunTerragruntCommandWithOutput(t, cmd)
+	require.NoError(t, err)
+
+	assert.NotContains(t, stderr, "No units discovered", "Filter should discover units, not result in empty discovery")
+}