Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable-cadence] Fix field assignment via references #2868

Merged
merged 1 commit into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions runtime/sema/check_assignment.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (checker *Checker) enforceViewAssignment(assignment ast.Statement, target a
accessChain = append(accessChain, elementType)
case *ast.MemberExpression:
target = targetExp.Expression
memberType, _, _, _ := checker.visitMember(targetExp)
memberType, _, _, _ := checker.visitMember(targetExp, true)
accessChain = append(accessChain, memberType)
default:
inAccessChain = false
Expand Down Expand Up @@ -352,7 +352,7 @@ func (checker *Checker) visitMemberExpressionAssignment(
target *ast.MemberExpression,
) (memberType Type) {

_, memberType, member, isOptional := checker.visitMember(target)
_, memberType, member, isOptional := checker.visitMember(target, true)

if member == nil {
return InvalidType
Expand Down
2 changes: 1 addition & 1 deletion runtime/sema/check_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ func (checker *Checker) visitIndexExpression(
// 2) is container-typed,
// then the element type should also be a reference.
returnReference := false
if !isAssignment && shouldReturnReference(valueIndexedType, elementType) {
if shouldReturnReference(valueIndexedType, elementType, isAssignment) {
// For index expressions, element are un-authorized.
elementType = checker.getReferenceType(elementType, false, UnauthorizedAccess)

Expand Down
2 changes: 1 addition & 1 deletion runtime/sema/check_invocation_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ func (checker *Checker) checkMemberInvocationArgumentLabels(
invocationExpression *ast.InvocationExpression,
memberExpression *ast.MemberExpression,
) {
_, _, member, _ := checker.visitMember(memberExpression)
_, _, member, _ := checker.visitMember(memberExpression, false)

if member == nil || len(member.ArgumentLabels) == 0 {
return
Expand Down
17 changes: 13 additions & 4 deletions runtime/sema/check_member_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

// NOTE: only called if the member expression is *not* an assignment
func (checker *Checker) VisitMemberExpression(expression *ast.MemberExpression) Type {
accessedType, memberType, member, isOptional := checker.visitMember(expression)
accessedType, memberType, member, isOptional := checker.visitMember(expression, false)

if !accessedType.IsInvalidType() {
memberAccessType := accessedType
Expand Down Expand Up @@ -111,7 +111,11 @@ func (checker *Checker) getReferenceType(typ Type, substituteAuthorization bool,
return NewReferenceType(checker.memoryGauge, auth, typ)
}

func shouldReturnReference(parentType, memberType Type) bool {
func shouldReturnReference(parentType, memberType Type, isAssignment bool) bool {
if isAssignment {
return false
}

if _, isReference := referenceType(parentType); !isReference {
return false
}
Expand All @@ -125,7 +129,12 @@ func referenceType(typ Type) (*ReferenceType, bool) {
return refType, isReference
}

func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedType Type, resultingType Type, member *Member, isOptional bool) {
func (checker *Checker) visitMember(expression *ast.MemberExpression, isAssignment bool) (
accessedType Type,
resultingType Type,
member *Member,
isOptional bool,
) {
memberInfo, ok := checker.Elaboration.MemberExpressionMemberAccessInfo(expression)
if ok {
return memberInfo.AccessedType, memberInfo.ResultingType, memberInfo.Member, memberInfo.IsOptional
Expand Down Expand Up @@ -367,7 +376,7 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedT
// i.e: `accessedSelfMember == nil`

if accessedSelfMember == nil &&
shouldReturnReference(accessedType, resultingType) &&
shouldReturnReference(accessedType, resultingType, isAssignment) &&
member.DeclarationKind == common.DeclarationKindField {

// Get a reference to the type
Expand Down
3 changes: 2 additions & 1 deletion runtime/tests/checker/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -980,10 +980,11 @@ func TestCheckAccountContractsNames(t *testing.T) {
}
`)

errors := RequireCheckerErrors(t, err, 2)
errors := RequireCheckerErrors(t, err, 3)

assert.IsType(t, &sema.InvalidAssignmentAccessError{}, errors[0])
assert.IsType(t, &sema.AssignmentToConstantMemberError{}, errors[1])
assert.IsType(t, &sema.NonReferenceTypeReferenceError{}, errors[2])
})
}

Expand Down
79 changes: 79 additions & 0 deletions runtime/tests/checker/reference_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2998,3 +2998,82 @@ func TestCheckReferenceCreationWithInvalidType(t *testing.T) {
require.ErrorAs(t, errs[0], &nonReferenceTypeReferenceError)
})
}

func TestCheckResourceReferenceFieldNilAssignment(t *testing.T) {
t.Parallel()

_, err := ParseAndCheck(t, `
access(all) resource Outer {
access(all) var inner : @Inner?

init(_ v: @Inner){
self.inner <- v
var outerRef = &self as &Outer
outerRef.inner = nil
}

destroy(){
destroy self.inner
}
}

access(all) resource Inner {}

fun main() {
let inner <- create Inner()
let outer <- create Outer(<- inner)
destroy outer
}
`)

errors := RequireCheckerErrors(t, err, 2)
require.IsType(t, &sema.IncorrectTransferOperationError{}, errors[0])
require.IsType(t, &sema.InvalidResourceAssignmentError{}, errors[1])
}

func TestCheckResourceReferenceIndexNilAssignment(t *testing.T) {
t.Parallel()

t.Run("one level", func(t *testing.T) {
t.Parallel()

_, err := ParseAndCheck(t, `
access(all) resource Foo {}

fun main() {
let array: @[Foo?] <- [<- create Foo()]
let arrayRef = &array as auth(Mutate) &[Foo?]

arrayRef[0] = nil

destroy array
}
`)

errors := RequireCheckerErrors(t, err, 2)
require.IsType(t, &sema.IncorrectTransferOperationError{}, errors[0])
require.IsType(t, &sema.InvalidResourceAssignmentError{}, errors[1])
})

t.Run("nested", func(t *testing.T) {
t.Parallel()

_, err := ParseAndCheck(t, `
access(all) resource Foo {}

fun main() {
let array: @[[Foo?]] <- [<- [<- create Foo()]]
let arrayRef = &array as auth(Mutate) &[[Foo?]]

arrayRef[0][0] = nil

destroy array
}
`)

errors := RequireCheckerErrors(t, err, 3)
require.IsType(t, &sema.UnauthorizedReferenceAssignmentError{}, errors[0])
require.IsType(t, &sema.IncorrectTransferOperationError{}, errors[1])
require.IsType(t, &sema.InvalidResourceAssignmentError{}, errors[2])
})
}
Loading