[issue #9] moving restriction validation to VariableNode fixes issue (#11)

* [issue #9] moving restriction validation to VariableNode fixes issue; removed empty comment

Author: xelalexv

parse/node.go

@@ -39,28 +39,49 @@ func (t *TextNode) String() (string, error) {
 
 type VariableNode struct {
 	NodeType
-	Ident string
-	Env   Env
+	Ident    string
+	Env      Env
+	Restrict *Restrictions
 }
 
-func NewVariable(ident string, env Env) *VariableNode {
-	return &VariableNode{NodeVariable, ident, env}
+func NewVariable(ident string, env Env, restrict *Restrictions) *VariableNode {
+	return &VariableNode{NodeVariable, ident, env, restrict}
 }
 
 func (t *VariableNode) String() (string, error) {
-	return t.Env.Get(t.Ident), nil
+	if err := t.validateNoUnset(); err != nil {
+		return "", err
+	}
+	value := t.Env.Get(t.Ident)
+	if err := t.validateNoEmpty(value); err != nil {
+		return "", err
+	}
+	return value, nil
 }
 
 func (t *VariableNode) isSet() bool {
 	return t.Env.Has(t.Ident)
 }
 
+func (t *VariableNode) validateNoUnset() error {
+	if t.Restrict.NoUnset && !t.isSet() {
+		return fmt.Errorf("variable ${%s} not set", t.Ident)
+	}
+	return nil
+}
+
+func (t *VariableNode) validateNoEmpty(value string) error {
+	if t.Restrict.NoEmpty && value == "" && t.isSet() {
+		return fmt.Errorf("variable ${%s} set but empty", t.Ident)
+	}
+	return nil
+}
+
 type SubstitutionNode struct {
 	NodeType
 	ExpType  itemType
 	Variable *VariableNode
 	Default  Node // Default could be variable or text
-	Restrict *Restrictions
 }
 
 func (t *SubstitutionNode) String() (string, error) {
@@ -70,39 +91,17 @@ func (t *SubstitutionNode) String() (string, error) {
 			if s, _ := t.Variable.String(); s != "" {
 				return s, nil
 			}
-			return t.validate(t.Default)
+			return t.Default.String()
 		case itemPlus, itemColonPlus:
 			if t.Variable.isSet() {
-				return t.validate(t.Default)
+				return t.Default.String()
 			}
 			return "", nil
 		default:
 			if !t.Variable.isSet() {
-				return t.validate(t.Default)
+				return t.Default.String()
 			}
 		}
 	}
-	return t.validate(t.Variable)
-}
-
-func (t *SubstitutionNode) validate(node Node) (string, error) {
-	if err := t.validateNoUnset(node); err != nil {
-		return "", err
-	}
-	return t.validateNoEmpty(node)
-}
-
-func (t *SubstitutionNode) validateNoUnset(node Node) error {
-	if t.Restrict.NoUnset && node.Type() == NodeVariable && !node.(*VariableNode).isSet() {
-		return fmt.Errorf("variable ${%s} not set", t.Variable.Ident)
-	}
-	return nil
-}
-
-func (t *SubstitutionNode) validateNoEmpty(node Node) (string, error) {
-	value, _ := node.String()
-	if t.Restrict.NoEmpty && value == "" && (node.Type() != NodeVariable || node.(*VariableNode).isSet()) {
-		return "", fmt.Errorf("variable ${%s} set but empty", t.Variable.Ident)
-	}
-	return value, nil
+	return t.Variable.String()
 }

parse/parse.go

@@ -67,7 +67,7 @@ Loop:
 		case itemError:
 			return p.errorf(t.val)
 		case itemVariable:
-			varNode := NewVariable(strings.TrimPrefix(t.val, "$"), p.Env)
+			varNode := NewVariable(strings.TrimPrefix(t.val, "$"), p.Env, p.Restrict)
 			p.nodes = append(p.nodes, varNode)
 		case itemLeftDelim:
 			if p.peek().typ == itemVariable {
@@ -91,7 +91,7 @@ Loop:
 func (p *Parser) action() (Node, error) {
 	var expType itemType
 	var defaultNode Node
-	varNode := NewVariable(p.next().val, p.Env)
+	varNode := NewVariable(p.next().val, p.Env, p.Restrict)
 Loop:
 	for {
 		switch t := p.next(); t.typ {
@@ -100,7 +100,7 @@ Loop:
 		case itemError:
 			return nil, p.errorf(t.val)
 		case itemVariable:
-			defaultNode = NewVariable(strings.TrimPrefix(t.val, "$"), p.Env)
+			defaultNode = NewVariable(strings.TrimPrefix(t.val, "$"), p.Env, p.Restrict)
 		case itemText:
 			n := NewText(t.val)
 		Text:
@@ -118,7 +118,7 @@ Loop:
 			expType = t.typ
 		}
 	}
-	return &SubstitutionNode{NodeSubstitution, expType, varNode, defaultNode, p.Restrict}, nil
+	return &SubstitutionNode{NodeSubstitution, expType, varNode, defaultNode}, nil
 }
 
 func (p *Parser) errorf(s string) error {

parse/parse_test.go

@@ -68,6 +68,9 @@ var parseTests = []parseTest{
 	// test specifically for failure modes
 	{"$var not set", "${NOTSET}", "", errUnset},
 	{"$var set to empty", "${EMPTY}", "", errEmpty},
+	// restrictions for plain variables without braces
+	{"gh-issue-9", "$NOTSET", "", errUnset},
+	{"gh-issue-9", "$EMPTY", "", errEmpty},
 
 	{"$var and $DEFAULT not set -", "${NOTSET-$ALSO_NOTSET}", "", errUnset},
 	{"$var and $DEFAULT not set :-", "${NOTSET:-$ALSO_NOTSET}", "", errUnset},