Merge pull request #3338 from onflow/bastian/improve-path-constructor

runtime/ast/position.go

@@ -146,6 +146,10 @@ func (e Range) EndPosition(common.MemoryGauge) Position {
 // NewRangeFromPositioned
 
 func NewRangeFromPositioned(memoryGauge common.MemoryGauge, hasPosition HasPosition) Range {
+	if hasPosition == nil {
+		return EmptyRange
+	}
+
 	return NewRange(
 		memoryGauge,
 		hasPosition.StartPosition(),

runtime/interpreter/interpreter.go

@@ -3320,21 +3320,21 @@ var ConverterDeclarations = []ValueConverterDeclaration{
 		name:         sema.PublicPathType.Name,
 		functionType: sema.PublicPathConversionFunctionType,
 		convert: func(interpreter *Interpreter, value Value, _ LocationRange) Value {
-			return ConvertPublicPath(interpreter, value)
+			return newPathFromStringValue(interpreter, common.PathDomainPublic, value)
 		},
 	},
 	{
 		name:         sema.PrivatePathType.Name,
 		functionType: sema.PrivatePathConversionFunctionType,
 		convert: func(interpreter *Interpreter, value Value, _ LocationRange) Value {
-			return ConvertPrivatePath(interpreter, value)
+			return newPathFromStringValue(interpreter, common.PathDomainPrivate, value)
 		},
 	},
 	{
 		name:         sema.StoragePathType.Name,
 		functionType: sema.StoragePathConversionFunctionType,
 		convert: func(interpreter *Interpreter, value Value, _ LocationRange) Value {
-			return ConvertStoragePath(interpreter, value)
+			return newPathFromStringValue(interpreter, common.PathDomainStorage, value)
 		},
 	},
 }

runtime/interpreter/value.go

@@ -21646,21 +21646,13 @@ func (PathValue) IsStorable() bool {
 	return true
 }
 
-func convertPath(interpreter *Interpreter, domain common.PathDomain, value Value) Value {
+func newPathFromStringValue(interpreter *Interpreter, domain common.PathDomain, value Value) Value {
 	stringValue, ok := value.(*StringValue)
 	if !ok {
 		return Nil
 	}
 
-	_, err := sema.CheckPathLiteral(
-		domain.Identifier(),
-		stringValue.Str,
-		ReturnEmptyRange,
-		ReturnEmptyRange,
-	)
-	if err != nil {
-		return Nil
-	}
+	// NOTE: any identifier is allowed, it does not have to match the syntax for path literals
 
 	return NewSomeValueNonCopying(
 		interpreter,
@@ -21672,18 +21664,6 @@ func convertPath(interpreter *Interpreter, domain common.PathDomain, value Value
 	)
 }
 
-func ConvertPublicPath(interpreter *Interpreter, value Value) Value {
-	return convertPath(interpreter, common.PathDomainPublic, value)
-}
-
-func ConvertPrivatePath(interpreter *Interpreter, value Value) Value {
-	return convertPath(interpreter, common.PathDomainPrivate, value)
-}
-
-func ConvertStoragePath(interpreter *Interpreter, value Value) Value {
-	return convertPath(interpreter, common.PathDomainStorage, value)
-}
-
 func (v PathValue) Storable(
 	storage atree.SlabStorage,
 	address atree.Address,

runtime/literal.go

@@ -156,14 +156,11 @@ func pathLiteralValue(
 	pathIdentifier := pathExpression.Identifier.Identifier
 
 	pathType, err := sema.CheckPathLiteral(
+		memoryGauge,
 		pathDomain,
 		pathIdentifier,
-		func() ast.Range {
-			return ast.NewRangeFromPositioned(memoryGauge, pathExpression.Domain)
-		},
-		func() ast.Range {
-			return ast.NewRangeFromPositioned(memoryGauge, pathExpression.Identifier)
-		},
+		pathExpression.Domain,
+		pathExpression.Identifier,
 	)
 	if err != nil {
 		return nil, InvalidLiteralError

runtime/sema/check_path_expression.go

@@ -23,19 +23,17 @@ import (
 
 	"github.com/onflow/cadence/runtime/ast"
 	"github.com/onflow/cadence/runtime/common"
+	"github.com/onflow/cadence/runtime/errors"
 )
 
 func (checker *Checker) VisitPathExpression(expression *ast.PathExpression) Type {
 
 	ty, err := CheckPathLiteral(
+		checker.memoryGauge,
 		expression.Domain.Identifier,
 		expression.Identifier.Identifier,
-		func() ast.Range {
-			return ast.NewRangeFromPositioned(checker.memoryGauge, expression.Domain)
-		},
-		func() ast.Range {
-			return ast.NewRangeFromPositioned(checker.memoryGauge, expression.Identifier)
-		},
+		expression.Domain,
+		expression.Identifier,
 	)
 
 	checker.report(err)
@@ -45,33 +43,39 @@ func (checker *Checker) VisitPathExpression(expression *ast.PathExpression) Type
 
 var isValidIdentifier = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`).MatchString
 
-func CheckPathLiteral(domainString, identifier string, domainRangeThunk, idRangeThunk func() ast.Range) (Type, error) {
+func CheckPathLiteral(
+	gauge common.MemoryGauge,
+	domain string,
+	identifier string,
+	domainRange ast.HasPosition,
+	identifierRange ast.HasPosition,
+) (Type, error) {
 
 	// Check that the domain is valid
-	domain := common.PathDomainFromIdentifier(domainString)
-	if domain == common.PathDomainUnknown {
+	pathDomain := common.PathDomainFromIdentifier(domain)
+	if pathDomain == common.PathDomainUnknown {
 		return PathType, &InvalidPathDomainError{
-			ActualDomain: domainString,
-			Range:        domainRangeThunk(),
+			ActualDomain: domain,
+			Range:        ast.NewRangeFromPositioned(gauge, domainRange),
 		}
 	}
 
 	// Check that the identifier is valid
 	if !isValidIdentifier(identifier) {
 		return PathType, &InvalidPathIdentifierError{
 			ActualIdentifier: identifier,
-			Range:            idRangeThunk(),
+			Range:            ast.NewRangeFromPositioned(gauge, identifierRange),
 		}
 	}
 
-	switch domain {
+	switch pathDomain {
 	case common.PathDomainStorage:
 		return StoragePathType, nil
 	case common.PathDomainPublic:
 		return PublicPathType, nil
 	case common.PathDomainPrivate:
 		return PrivatePathType, nil
 	default:
-		return PathType, nil
+		panic(errors.NewUnreachableError())
 	}
 }

runtime/sema/check_path_expression_test.go

@@ -23,62 +23,72 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
-	"github.com/onflow/cadence/runtime/ast"
 )
 
 func TestCheckPathLiteral(t *testing.T) {
 
 	t.Parallel()
 
-	rangeThunk := func() ast.Range {
-		return ast.EmptyRange
-	}
-
 	t.Run("valid domain (storage), valid identifier", func(t *testing.T) {
-		ty, err := CheckPathLiteral("storage", "test", rangeThunk, rangeThunk)
+		t.Parallel()
+
+		ty, err := CheckPathLiteral(nil, "storage", "test", nil, nil)
 		require.NoError(t, err)
 		assert.Equal(t, StoragePathType, ty)
 	})
 
 	t.Run("valid domain (private), valid identifier", func(t *testing.T) {
-		ty, err := CheckPathLiteral("private", "test", rangeThunk, rangeThunk)
+		t.Parallel()
+
+		ty, err := CheckPathLiteral(nil, "private", "test", nil, nil)
 		require.NoError(t, err)
 		assert.Equal(t, PrivatePathType, ty)
 	})
 
 	t.Run("valid domain (public), valid identifier", func(t *testing.T) {
-		ty, err := CheckPathLiteral("public", "test", rangeThunk, rangeThunk)
+		t.Parallel()
+
+		ty, err := CheckPathLiteral(nil, "public", "test", nil, nil)
 		require.NoError(t, err)
 		assert.Equal(t, PublicPathType, ty)
 	})
 
 	t.Run("invalid domain (empty), valid identifier", func(t *testing.T) {
-		_, err := CheckPathLiteral("", "test", rangeThunk, rangeThunk)
+		t.Parallel()
+
+		_, err := CheckPathLiteral(nil, "", "test", nil, nil)
 		var invalidPathDomainError *InvalidPathDomainError
 		require.ErrorAs(t, err, &invalidPathDomainError)
 	})
 
 	t.Run("invalid domain (foo), valid identifier", func(t *testing.T) {
-		_, err := CheckPathLiteral("foo", "test", rangeThunk, rangeThunk)
+		t.Parallel()
+
+		_, err := CheckPathLiteral(nil, "foo", "test", nil, nil)
 		var invalidPathDomainError *InvalidPathDomainError
 		require.ErrorAs(t, err, &invalidPathDomainError)
 	})
 
 	t.Run("valid domain (public), invalid identifier (empty)", func(t *testing.T) {
-		_, err := CheckPathLiteral("public", "", rangeThunk, rangeThunk)
+		t.Parallel()
+
+		_, err := CheckPathLiteral(nil, "public", "", nil, nil)
 		var invalidPathIdentifierError *InvalidPathIdentifierError
 		require.ErrorAs(t, err, &invalidPathIdentifierError)
 	})
 
 	t.Run("valid domain (public), invalid identifier ($)", func(t *testing.T) {
-		_, err := CheckPathLiteral("public", "$", rangeThunk, rangeThunk)
+		t.Parallel()
+
+		_, err := CheckPathLiteral(nil, "public", "$", nil, nil)
 		var invalidPathIdentifierError *InvalidPathIdentifierError
 		require.ErrorAs(t, err, &invalidPathIdentifierError)
 	})
 
 	t.Run("valid domain (public), invalid identifier (0)", func(t *testing.T) {
-		_, err := CheckPathLiteral("public", "0", rangeThunk, rangeThunk)
+		t.Parallel()
+
+		_, err := CheckPathLiteral(nil, "public", "0", nil, nil)
 		var invalidPathIdentifierError *InvalidPathIdentifierError
 		require.ErrorAs(t, err, &invalidPathIdentifierError)
 	})

runtime/stdlib/account_test.go

@@ -23,18 +23,13 @@ import (
 
 	"github.com/stretchr/testify/require"
 
-	"github.com/onflow/cadence/runtime/ast"
 	"github.com/onflow/cadence/runtime/sema"
 )
 
 func TestSemaCheckPathLiteralForInternalStorageDomains(t *testing.T) {
 
 	t.Parallel()
 
-	rangeThunk := func() ast.Range {
-		return ast.EmptyRange
-	}
-
 	internalStorageDomains := []string{
 		InboxStorageDomain,
 		AccountCapabilityStorageDomain,
@@ -44,8 +39,11 @@ func TestSemaCheckPathLiteralForInternalStorageDomains(t *testing.T) {
 	}
 
 	test := func(domain string) {
+
 		t.Run(domain, func(t *testing.T) {
-			_, err := sema.CheckPathLiteral(domain, "test", rangeThunk, rangeThunk)
+			t.Parallel()
+
+			_, err := sema.CheckPathLiteral(nil, domain, "test", nil, nil)
 			var invalidPathDomainError *sema.InvalidPathDomainError
 			require.ErrorAs(t, err, &invalidPathDomainError)
 		})

runtime/tests/interpreter/path_test.go

@@ -95,7 +95,7 @@ func TestInterpretConvertStringToPath(t *testing.T) {
 			)
 		})
 
-		t.Run(fmt.Sprintf("invalid identifier 2: %s", domain.Identifier()), func(t *testing.T) {
+		t.Run(fmt.Sprintf("syntactically invalid identifier 2: %s", domain.Identifier()), func(t *testing.T) {
 
 			t.Parallel()
 
@@ -104,19 +104,22 @@ func TestInterpretConvertStringToPath(t *testing.T) {
 			inter := parseCheckAndInterpret(t,
 				fmt.Sprintf(
 					`
-                      let x = %[1]s(identifier: "2")
+                      let x = %[1]s(identifier: "2")!
                     `,
 					domainType.String(),
 				),
 			)
 
 			assert.Equal(t,
-				interpreter.Nil,
+				interpreter.PathValue{
+					Domain:     domain,
+					Identifier: "2",
+				},
 				inter.Globals.Get("x").GetValue(inter),
 			)
 		})
 
-		t.Run(fmt.Sprintf("invalid identifier -: %s", domain.Identifier()), func(t *testing.T) {
+		t.Run(fmt.Sprintf("syntactically invalid identifier -: %s", domain.Identifier()), func(t *testing.T) {
 
 			t.Parallel()
 
@@ -125,14 +128,17 @@ func TestInterpretConvertStringToPath(t *testing.T) {
 			inter := parseCheckAndInterpret(t,
 				fmt.Sprintf(
 					`
-                      let x = %[1]s(identifier: "fo-o")
+                      let x = %[1]s(identifier: "fo-o")!
                     `,
 					domainType.String(),
 				),
 			)
 
 			assert.Equal(t,
-				interpreter.Nil,
+				interpreter.PathValue{
+					Domain:     domain,
+					Identifier: "fo-o",
+				},
 				inter.Globals.Get("x").GetValue(inter),
 			)
 		})