Forward to 1.7.0 (#17)

* Prepare for a potential future 1.6.1 release

* build: remove go 1.11 and 1.12 from travis config

* cty: RawEquals correct behavior for sets with partially-unknown values

Sets with the same type and same unknown values are equal enough for
RawEqual's purposes. This commit takes advantage of the setRules's
defined ordering to compare each element of the sets as lists.

* v1.6.1

* Prepare for an assumed future v1.6.2 release

* cty: Utilities to unmark with marks at specific paths

This allows a caller to call a method like Unmark, but instead of getting a superset of marks, get an array of marks and their associated paths. This allows for a Value to be unmarked and then remarked later without losing so much detail about the marks.

* Update CHANGELOG.md

* cty: Fix deep marks functions with new transformer

The UnmarkDeep, UnmarkDeepWithPaths, and MarkWithPaths functions could
previously panic with nested complex values. This was due to the
Transform callback function only being called as part of a postorder
traversal, when it is not permitted to traverse a marked value.

This commit introduces a new TransformWithTransformer function,
requiring an implementation of a new Transformer interface. Using this
interface, callers can implement either preorder or postorder
traversals.

In turn, this allows us to write deep transformation for marks which
successfully cope with all nested structures.

The commit includes significantly more tests for these functions, which
should now cover all complex value types.

* cty: Remove already-fixed FIXME for SetVal

The immediately preceding code already unmarks the values and applies
the marking to the set, so this comment and panic are out of date.

* functions/stdlib: formatdate to correctly handle literals at the end of the format string

* Update CHANGELOG.md

* cty: Fix path array reuse with UnmarkDeepWithPaths

When unmarking a complex value and retaining the marked paths, a
sufficiently deep structure would result in incorrect duplicate path
output. For example, this structure (in HCL syntax):

{
  environment = [
    {
      variables = {
        "x" = 1
        "y" = 2
      }
    }
  ]
}

If the 1 and 2 values are marked, the resulting path value marks from
UnmarkDeepWithPaths would have two entries, as expected. However, both
would have the same Path attribute, like so:

[
  { environment[0].variables["x"], "mark" },
  { environment[0].variables["x"], "mark" },
]

This is caused by calling `append` in the walk transform function with
the same path object repeatedly, which eventually does not result in
array reallocation, and therefore the path object is modified in-place.

* stdlib: Add tests for join function

* function: Deeply unmark function arguments

For functions which do not explicitly support marks, we now deeply
unmark arguments before calling, and reapply those marks to the return
value. This ensures that these functions do not panic with arguments
which are collection values with marked descendants.

This does result in overly conservatively applying marks in some cases,
but that can be fixed with later work to add explicit support for marks
to those functions.

* Update CHANGELOG.md

* v1.7.0

* Update CHANGELOG.md

---------

Co-authored-by: Martin Atkins <[email protected]>
Co-authored-by: Kristin Laemmert <[email protected]>
Co-authored-by: Pam Selle <[email protected]>
Co-authored-by: Alisdair McDiarmid <[email protected]>
Co-authored-by: OwenTuz <[email protected]>

Author: 6 people

CHANGELOG.md

@@ -1,6 +1,12 @@
+# 1.7.0 (Unreleased)
+
+* `cty`: `Value.UnmarkDeepWithPaths` and `Value.MarkWithPaths` are like `Value.UnmarkDeep` and `Value.Mark` but they retain path information for each marked value, so that marks can be re-applied later without all the loss of detail that results from `Value.UnmarkDeep` aggregating together all of the nested marks.
+* `function`: Unless a parameter has `AllowMarks: true` explicitly set, the functions infrastructure will now guarantee that it never sees a marked value even if the mark is deep inside a data structure. Previously that guarantee was only shallow for the top-level value, similar to `AllowUnknown`, but because marks are a relatively new addition to `cty` and numerous existing functions are not written to deal with them this is the more conservative and robust default. ([#72](https://github.com/zclconf/go-cty/pull/72))
+* `function/stdlib`: The `formatdate` function was not correctly handling literal sequences at the end of the format string. It will now handle those as intended. ([#69](https://github.com/zclconf/go-cty/pull/69))
+
 # 1.6.1 (Unreleased)
 
-* Fix a regression from 1.6.0 where `Value.RawEqual` no longer returned the correct result given a pair of sets containing partially-unknown values. ([#64](https://github.com/zclconf/go-cty/pull/64))
+* `cty`:: Fix a regression from 1.6.0 where `Value.RawEqual` no longer returned the correct result given a pair of sets containing partially-unknown values. ([#64](https://github.com/zclconf/go-cty/pull/64))
 
 # 1.6.0 (Unreleased)
 

cty/function/function.go

@@ -244,19 +244,21 @@ func (f Function) Call(args []cty.Value) (val cty.Value, err error) {
 			return cty.UnknownVal(expectedType), nil
 		}
 
-		if val.IsMarked() && !spec.AllowMarked {
-			unwrappedVal, marks := val.Unmark()
-			// In order to avoid additional overhead on applications that
-			// are not using marked values, we copy the given args only
-			// if we encounter a marked value we need to unmark. However,
-			// as a consequence we end up doing redundant copying if multiple
-			// marked values need to be unwrapped. That seems okay because
-			// argument lists are generally small.
-			newArgs := make([]cty.Value, len(args))
-			copy(newArgs, args)
-			newArgs[i] = unwrappedVal
-			resultMarks = append(resultMarks, marks)
-			args = newArgs
+		if !spec.AllowMarked {
+			unwrappedVal, marks := val.UnmarkDeep()
+			if len(marks) > 0 {
+				// In order to avoid additional overhead on applications that
+				// are not using marked values, we copy the given args only
+				// if we encounter a marked value we need to unmark. However,
+				// as a consequence we end up doing redundant copying if multiple
+				// marked values need to be unwrapped. That seems okay because
+				// argument lists are generally small.
+				newArgs := make([]cty.Value, len(args))
+				copy(newArgs, args)
+				newArgs[i] = unwrappedVal
+				resultMarks = append(resultMarks, marks)
+				args = newArgs
+			}
 		}
 	}
 
@@ -266,13 +268,15 @@ func (f Function) Call(args []cty.Value) (val cty.Value, err error) {
 			if !val.IsKnown() && !spec.AllowUnknown {
 				return cty.UnknownVal(expectedType), nil
 			}
-			if val.IsMarked() && !spec.AllowMarked {
-				unwrappedVal, marks := val.Unmark()
-				newArgs := make([]cty.Value, len(args))
-				copy(newArgs, args)
-				newArgs[len(posArgs)+i] = unwrappedVal
-				resultMarks = append(resultMarks, marks)
-				args = newArgs
+			if !spec.AllowMarked {
+				unwrappedVal, marks := val.UnmarkDeep()
+				if len(marks) > 0 {
+					newArgs := make([]cty.Value, len(args))
+					copy(newArgs, args)
+					newArgs[len(posArgs)+i] = unwrappedVal
+					resultMarks = append(resultMarks, marks)
+					args = newArgs
+				}
 			}
 		}
 	}

cty/function/stdlib/datetime.go

@@ -362,9 +362,14 @@ func splitDateFormat(data []byte, atEOF bool) (advance int, token []byte, err er
 		for i := 1; i < len(data); i++ {
 			if data[i] == esc {
 				if (i + 1) == len(data) {
-					// We need at least one more byte to decide if this is an
-					// escape or a terminator.
-					return 0, nil, nil
+					if atEOF {
+						// We have a closing quote and are at the end of our input
+						return len(data), data, nil
+					} else {
+						// We need at least one more byte to decide if this is an
+						// escape or a terminator.
+						return 0, nil, nil
+					}
 				}
 				if data[i+1] == esc {
 					i++ // doubled-up quotes are an escape sequence

cty/function/stdlib/datetime_test.go

@@ -39,6 +39,11 @@ func TestFormatDate(t *testing.T) {
 			cty.StringVal("3 o'clock PM"),
 			``,
 		},
+		{
+			cty.StringVal("H 'o''clock'"),
+			cty.StringVal("3 o'clock"),
+			``,
+		},
 		{
 			cty.StringVal("hh:mm:ssZZZZ"),
 			cty.StringVal("15:04:05+0000"),

cty/function/stdlib/string_test.go

@@ -393,3 +393,83 @@ func TestSubstr(t *testing.T) {
 		})
 	}
 }
+
+func TestJoin(t *testing.T) {
+	tests := map[string]struct {
+		Separator cty.Value
+		Lists     []cty.Value
+		Want      cty.Value
+	}{
+		"single two-element list": {
+			cty.StringVal("-"),
+			[]cty.Value{
+				cty.ListVal([]cty.Value{cty.StringVal("hello"), cty.StringVal("world")}),
+			},
+			cty.StringVal("hello-world"),
+		},
+		"multiple single-element lists": {
+			cty.StringVal("-"),
+			[]cty.Value{
+				cty.ListVal([]cty.Value{cty.StringVal("chicken")}),
+				cty.ListVal([]cty.Value{cty.StringVal("egg")}),
+			},
+			cty.StringVal("chicken-egg"),
+		},
+		"single single-element list": {
+			cty.StringVal("-"),
+			[]cty.Value{
+				cty.ListVal([]cty.Value{cty.StringVal("chicken")}),
+			},
+			cty.StringVal("chicken"),
+		},
+		"blank separator": {
+			cty.StringVal(""),
+			[]cty.Value{
+				cty.ListVal([]cty.Value{cty.StringVal("horse"), cty.StringVal("face")}),
+			},
+			cty.StringVal("horseface"),
+		},
+		"marked list": {
+			cty.StringVal("-"),
+			[]cty.Value{
+				cty.ListVal([]cty.Value{cty.StringVal("hello"), cty.StringVal("world")}).Mark("sensitive"),
+			},
+			cty.StringVal("hello-world").Mark("sensitive"),
+		},
+		"marked separator": {
+			cty.StringVal("-").Mark("sensitive"),
+			[]cty.Value{
+				cty.ListVal([]cty.Value{cty.StringVal("hello"), cty.StringVal("world")}),
+			},
+			cty.StringVal("hello-world").Mark("sensitive"),
+		},
+		"list with some marked elements": {
+			cty.StringVal("-"),
+			[]cty.Value{
+				cty.ListVal([]cty.Value{cty.StringVal("hello").Mark("sensitive"), cty.StringVal("world")}),
+			},
+			cty.StringVal("hello-world").Mark("sensitive"),
+		},
+		"multiple marks": {
+			cty.StringVal("-").Mark("a"),
+			[]cty.Value{
+				cty.ListVal([]cty.Value{cty.StringVal("hello").Mark("b"), cty.StringVal("world").Mark("c")}),
+			},
+			cty.StringVal("hello-world").WithMarks(cty.NewValueMarks("a", "b", "c")),
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			got, err := Join(test.Separator, test.Lists...)
+
+			if err != nil {
+				t.Fatalf("unexpected error: %s", err)
+			}
+
+			if !got.RawEquals(test.Want) {
+				t.Errorf("wrong result\ngot:  %#v\nwant: %#v", got, test.Want)
+			}
+		})
+	}
+}

cty/json/marshal.go

@@ -10,7 +10,7 @@ import (
 
 func marshal(val cty.Value, t cty.Type, path cty.Path, b *bytes.Buffer) error {
 	if val.IsMarked() {
-		return path.NewErrorf("value has marks, so it cannot be seralized")
+		return path.NewErrorf("value has marks, so it cannot be serialized as JSON")
 	}
 
 	// If we're going to decode as DynamicPseudoType then we need to save

cty/marks.go

@@ -67,6 +67,23 @@ func (m ValueMarks) GoString() string {
 	return s.String()
 }
 
+// PathValueMarks is a structure that enables tracking marks
+// and the paths where they are located in one type
+type PathValueMarks struct {
+	Path  Path
+	Marks ValueMarks
+}
+
+func (p PathValueMarks) Equal(o PathValueMarks) bool {
+	if !p.Path.Equals(o.Path) {
+		return false
+	}
+	if !p.Marks.Equal(o.Marks) {
+		return false
+	}
+	return true
+}
+
 // IsMarked returns true if and only if the receiving value carries at least
 // one mark. A marked value cannot be used directly with integration methods
 // without explicitly unmarking it (and retrieving the markings) first.
@@ -174,6 +191,31 @@ func (val Value) Mark(mark interface{}) Value {
 	}
 }
 
+type applyPathValueMarksTransformer struct {
+	pvm []PathValueMarks
+}
+
+func (t *applyPathValueMarksTransformer) Enter(p Path, v Value) (Value, error) {
+	return v, nil
+}
+
+func (t *applyPathValueMarksTransformer) Exit(p Path, v Value) (Value, error) {
+	for _, path := range t.pvm {
+		if p.Equals(path.Path) {
+			return v.WithMarks(path.Marks), nil
+		}
+	}
+	return v, nil
+}
+
+// MarkWithPaths accepts a slice of PathValueMarks to apply
+// markers to particular paths and returns the marked
+// Value.
+func (val Value) MarkWithPaths(pvm []PathValueMarks) Value {
+	ret, _ := TransformWithTransformer(val, &applyPathValueMarksTransformer{pvm})
+	return ret
+}
+
 // Unmark separates the marks of the receiving value from the value itself,
 // removing a new unmarked value and a map (representing a set) of the marks.
 //
@@ -191,24 +233,54 @@ func (val Value) Unmark() (Value, ValueMarks) {
 	}, marks
 }
 
+type unmarkTransformer struct {
+	pvm []PathValueMarks
+}
+
+func (t *unmarkTransformer) Enter(p Path, v Value) (Value, error) {
+	unmarkedVal, marks := v.Unmark()
+	if len(marks) > 0 {
+		path := make(Path, len(p), len(p)+1)
+		copy(path, p)
+		t.pvm = append(t.pvm, PathValueMarks{path, marks})
+	}
+	return unmarkedVal, nil
+}
+
+func (t *unmarkTransformer) Exit(p Path, v Value) (Value, error) {
+	return v, nil
+}
+
 // UnmarkDeep is similar to Unmark, but it works with an entire nested structure
 // rather than just the given value directly.
 //
 // The result is guaranteed to contain no nested values that are marked, and
 // the returned marks set includes the superset of all of the marks encountered
 // during the operation.
 func (val Value) UnmarkDeep() (Value, ValueMarks) {
+	t := unmarkTransformer{}
+	ret, _ := TransformWithTransformer(val, &t)
+
 	marks := make(ValueMarks)
-	ret, _ := Transform(val, func(_ Path, v Value) (Value, error) {
-		unmarkedV, valueMarks := v.Unmark()
-		for m, s := range valueMarks {
+	for _, pvm := range t.pvm {
+		for m, s := range pvm.Marks {
 			marks[m] = s
 		}
-		return unmarkedV, nil
-	})
+	}
+
 	return ret, marks
 }
 
+// UnmarkDeepWithPaths is like UnmarkDeep, except it returns a slice
+// of PathValueMarks rather than a superset of all marks. This allows
+// a caller to know which marks are associated with which paths
+// in the Value.
+func (val Value) UnmarkDeepWithPaths() (Value, []PathValueMarks) {
+	t := unmarkTransformer{}
+	ret, _ := TransformWithTransformer(val, &t)
+	return ret, t.pvm
+}
+
 func (val Value) unmarkForce() Value {
 	unw, _ := val.Unmark()
 	return unw