Merge pull request #3539 from onflow/supun/fix-view-checking

Handle invalid index-expressions in view assignment
onflow · Aug 20, 2024 · 7ae2648 · 7ae2648
2 parents 4b6f36c + 5966b1b
commit 7ae2648
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 11 deletions.
diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go
@@ -148,7 +148,11 @@ func (interpreter *Interpreter) valueIndexExpressionGetterSetter(
 
 	elaboration := interpreter.Program.Elaboration
 
-	indexExpressionTypes := elaboration.IndexExpressionTypes(indexExpression)
+	indexExpressionTypes, ok := elaboration.IndexExpressionTypes(indexExpression)
+	if !ok {
+		panic(errors.NewUnreachableError())
+	}
+
 	indexedType := indexExpressionTypes.IndexedType
 	indexingType := indexExpressionTypes.IndexingType
 
@@ -1096,7 +1100,7 @@ func (interpreter *Interpreter) maybeGetReference(
 	expression *ast.IndexExpression,
 	memberValue Value,
 ) Value {
-	indexExpressionTypes := interpreter.Program.Elaboration.IndexExpressionTypes(expression)
+	indexExpressionTypes, _ := interpreter.Program.Elaboration.IndexExpressionTypes(expression)
 
 	if indexExpressionTypes.ReturnReference {
 		expectedType := indexExpressionTypes.ResultType

diff --git a/runtime/sema/check_assignment.go b/runtime/sema/check_assignment.go
@@ -148,7 +148,13 @@ func (checker *Checker) rootOfAccessChain(target ast.Expression) (baseVariable *
 			inAccessChain = false
 		case *ast.IndexExpression:
 			target = targetExp.TargetExpression
-			elementType := checker.Elaboration.IndexExpressionTypes(targetExp).IndexedType.ElementType(true)
+			indexExprTypes, ok := checker.Elaboration.IndexExpressionTypes(targetExp)
+			var elementType Type
+			if !ok {
+				elementType = InvalidType
+			} else {
+				elementType = indexExprTypes.IndexedType.ElementType(true)
+			}
 			accessChain = append(accessChain, elementType)
 		case *ast.MemberExpression:
 			target = targetExp.Expression
@@ -361,7 +367,11 @@ func (checker *Checker) visitIndexExpressionAssignment(
 		return checker.visitIndexExpression(indexExpression, true)
 	})
 
-	indexExprTypes := checker.Elaboration.IndexExpressionTypes(indexExpression)
+	indexExprTypes, ok := checker.Elaboration.IndexExpressionTypes(indexExpression)
+	if !ok {
+		return InvalidType
+	}
+
 	indexedRefType, isReference := MaybeReferenceType(indexExprTypes.IndexedType)
 
 	if isReference &&

diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go
@@ -2049,7 +2049,10 @@ func (checker *Checker) checkDefaultDestroyParamExpressionKind(
 		checker.checkDefaultDestroyParamExpressionKind(arg.TargetExpression, containerDeclarationKind)
 		checker.checkDefaultDestroyParamExpressionKind(arg.IndexingExpression, containerDeclarationKind)
 
-		indexExprType := checker.Elaboration.IndexExpressionTypes(arg)
+		indexExprType, ok := checker.Elaboration.IndexExpressionTypes(arg)
+		if !ok {
+			return
+		}
 
 		// indexing expressions on arrays can fail, and must be disallowed, but
 		// indexing expressions on dicts, or composites (for attachments) will return `nil` and thus never fail

diff --git a/runtime/sema/check_swap.go b/runtime/sema/check_swap.go
@@ -62,16 +62,14 @@ func (checker *Checker) VisitSwapStatement(swap *ast.SwapStatement) (_ struct{})
 
 	for _, side := range []ast.Expression{swap.Left, swap.Right} {
 		if indexExpression, ok := side.(*ast.IndexExpression); ok {
-			indexExpressionTypes := checker.Elaboration.IndexExpressionTypes(indexExpression)
+			indexExpressionTypes, ok := checker.Elaboration.IndexExpressionTypes(indexExpression)
 
 			// If the indexed type is a resource type,
 			// then the target expression must be considered as a nested resource move expression.
 			//
 			// The index expression might have been invalid,
 			// so the indexed type might be unavailable.
-
-			indexedType := indexExpressionTypes.IndexedType
-			if indexedType != nil && indexedType.IsResourceType() {
+			if ok && indexExpressionTypes.IndexedType.IsResourceType() {
 				targetExpression := indexExpression.TargetExpression
 				checker.elaborateNestedResourceMoveExpression(targetExpression)
 			}

diff --git a/runtime/sema/elaboration.go b/runtime/sema/elaboration.go
@@ -884,11 +884,13 @@ func (e *Elaboration) SetReferenceExpressionBorrowType(expression *ast.Reference
 	e.referenceExpressionBorrowTypes[expression] = ty
 }
 
-func (e *Elaboration) IndexExpressionTypes(expression *ast.IndexExpression) (types IndexExpressionTypes) {
+func (e *Elaboration) IndexExpressionTypes(expression *ast.IndexExpression) (types IndexExpressionTypes, contains bool) {
 	if e.indexExpressionTypes == nil {
 		return
 	}
-	return e.indexExpressionTypes[expression]
+
+	types, contains = e.indexExpressionTypes[expression]
+	return
 }
 
 func (e *Elaboration) SetIndexExpressionTypes(expression *ast.IndexExpression, types IndexExpressionTypes) {

diff --git a/runtime/tests/checker/function_test.go b/runtime/tests/checker/function_test.go
@@ -598,3 +598,36 @@ func TestCheckResultVariable(t *testing.T) {
 		require.NoError(t, err)
 	})
 }
+
+func TestCheckViewFunctionWithErrors(t *testing.T) {
+
+	t.Parallel()
+
+	t.Run("index assignment", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+            view fun foo() {
+                a[b] = 1
+            }`,
+		)
+
+		errs := RequireCheckerErrors(t, err, 2)
+		assert.IsType(t, &sema.NotDeclaredError{}, errs[0])
+		assert.IsType(t, &sema.PurityError{}, errs[1])
+	})
+
+	t.Run("member assignment", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+            view fun foo() {
+                a.b = 1
+            }`,
+		)
+
+		errs := RequireCheckerErrors(t, err, 2)
+		assert.IsType(t, &sema.NotDeclaredError{}, errs[0])
+		assert.IsType(t, &sema.PurityError{}, errs[1])
+	})
+}