handle variable evaluation in one location

internal/terraform/context_plan.go

@@ -110,6 +110,11 @@ func (c *Context) Plan(config *configs.Config, prevRunState *states.State, opts
 	// includes language asking the user to report a bug.
 	varDiags := checkInputVariables(config.Module.Variables, variables)
 	diags = diags.Append(varDiags)
+	if diags.HasErrors() {
+		// We can't plan anything with values missing entirely, so let the user
+		// know which variables must be set as early as possible.
+		return nil, diags
+	}
 
 	if len(opts.Targets) > 0 {
 		diags = diags.Append(tfdiags.Sourceless(

internal/terraform/context_validate_test.go

@@ -314,6 +314,8 @@ func TestContext2Validate_countVariableNoDefault(t *testing.T) {
 	})
 	assertNoDiagnostics(t, diags)
 
+	// This is a type of validation which happens during plan, since there are
+	// no variables values during actual validation.
 	_, diags = c.Plan(m, nil, &PlanOpts{})
 	if !diags.HasErrors() {
 		// Error should be: The input variable "foo" has not been assigned a value.
@@ -2088,3 +2090,36 @@ output "out" {
 		t.Fatal(diags.ErrWithWarnings())
 	}
 }
+
+func TestContext2Validate_nonNullableVariableDefaultValidation(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+module "first" {
+  source = "./mod"
+  input = null
+}
+`,
+
+		"mod/main.tf": `
+variable "input" {
+  type        = string
+  default     = "default"
+  nullable    = false
+
+  // Validation expressions should receive the default with nullable=false and
+  // a null input.
+  validation {
+    condition     = var.input != null
+    error_message = "Input cannot be null!"
+  }
+}
+`,
+	})
+
+	ctx := testContext2(t, &ContextOpts{})
+
+	diags := ctx.Validate(m)
+	if diags.HasErrors() {
+		t.Fatal(diags.ErrWithWarnings())
+	}
+}

internal/terraform/evaluate.go

@@ -267,16 +267,18 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
 		return cty.UnknownVal(config.Type), diags
 	}
 
-	val, isSet := vals[addr.Name]
-	switch {
-	case !isSet:
-		// The config loader will ensure there is a default if the value is not
-		// set at all.
-		val = config.Default
-
-	case val.IsNull() && !config.Nullable && config.Default != cty.NilVal:
-		// If nullable=false a null value will use the configured default.
-		val = config.Default
+	val, ok := vals[addr.Name]
+	if !ok {
+		log.Printf("[ERROR] GetInputVariable: missing value for %s", addr)
+		val = cty.UnknownVal(config.Type)
+		// All variables should have been stored in the context already,
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  `Missing variable value`,
+			Detail:   fmt.Sprintf(`No value found for %s. This is a bug in Terraform; please report it!`, addr),
+			Subject:  &config.DeclRange,
+		})
+		val = cty.UnknownVal(config.Type)
 	}
 
 	var err error

internal/terraform/node_module_variable.go

@@ -2,7 +2,6 @@ package terraform
 
 import (
 	"fmt"
-	"log"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/terraform/internal/addrs"
@@ -143,25 +142,20 @@ func (n *nodeModuleVariable) ModulePath() addrs.Module {
 
 // GraphNodeExecutable
 func (n *nodeModuleVariable) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) {
-	// If we have no value, do nothing
-	if n.Expr == nil {
-		return nil
-	}
-
 	// Otherwise, interpolate the value of this variable and set it
 	// within the variables mapping.
 	var vals map[string]cty.Value
 	var err error
 
 	switch op {
 	case walkValidate:
-		vals, err = n.evalModuleCallArgument(ctx, true)
+		vals, err = n.evalModuleVariable(ctx, true)
 		diags = diags.Append(err)
 		if diags.HasErrors() {
 			return diags
 		}
 	default:
-		vals, err = n.evalModuleCallArgument(ctx, false)
+		vals, err = n.evalModuleVariable(ctx, false)
 		diags = diags.Append(err)
 		if diags.HasErrors() {
 			return diags
@@ -187,7 +181,7 @@ func (n *nodeModuleVariable) DotNode(name string, opts *dag.DotOpts) *dag.DotNod
 	}
 }
 
-// evalModuleCallArgument produces the value for a particular variable as will
+// evalModuleVariable produces the value for a particular variable as will
 // be used by a child module instance.
 //
 // The result is written into a map, with its key set to the local name of the
@@ -199,17 +193,17 @@ func (n *nodeModuleVariable) DotNode(name string, opts *dag.DotOpts) *dag.DotNod
 // validateOnly indicates that this evaluation is only for config
 // validation, and we will not have any expansion module instance
 // repetition data.
-func (n *nodeModuleVariable) evalModuleCallArgument(ctx EvalContext, validateOnly bool) (map[string]cty.Value, error) {
+func (n *nodeModuleVariable) evalModuleVariable(ctx EvalContext, validateOnly bool) (map[string]cty.Value, error) {
 	name := n.Addr.Variable.Name
 	expr := n.Expr
+	vals := make(map[string]cty.Value)
 
 	if expr == nil {
-		// Should never happen, but we'll bail out early here rather than
-		// crash in case it does. We set no value at all in this case,
-		// making a subsequent call to EvalContext.SetModuleCallArguments
-		// a no-op.
-		log.Printf("[ERROR] attempt to evaluate %s with nil expression", n.Addr.String())
-		return nil, nil
+		// If there is no module input expression, we must use the default. The
+		// config loader ensures there is a default if the input value is not
+		// set at all.
+		vals[name] = n.Config.Default
+		return vals, nil
 	}
 
 	var moduleInstanceRepetitionData instances.RepetitionData
@@ -268,8 +262,13 @@ func (n *nodeModuleVariable) evalModuleCallArgument(ctx EvalContext, validateOnl
 		val = cty.UnknownVal(n.Config.Type)
 	}
 
-	vals := make(map[string]cty.Value)
-	vals[name] = val
+	// If nullable=false, a null input must be replaced with the default. The
+	// config loader has already verified the default exists and is valid in
+	// this case.
+	if val.IsNull() && !n.Config.Nullable {
+		val = n.Config.Default
+	}
 
+	vals[name] = val
 	return vals, diags.ErrWithWarnings()
 }

internal/terraform/transform_module_variable.go

@@ -3,7 +3,6 @@ package terraform
 import (
 	"fmt"
 
-	"github.com/hashicorp/hcl/v2/hclsyntax"
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 	"github.com/zclconf/go-cty/cty"
@@ -94,13 +93,6 @@ func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, c *configs
 		var expr hcl.Expression
 		if attr := content.Attributes[v.Name]; attr != nil {
 			expr = attr.Expr
-		} else {
-			// No expression provided for this variable, so we'll make a
-			// synthetic one using the variable's default value.
-			expr = &hclsyntax.LiteralValueExpr{
-				Val:      v.Default,
-				SrcRange: v.DeclRange, // This is not exact, but close enough
-			}
 		}
 
 		// Add a plannable node, as the variable may expand