Merge pull request #3380 from onflow/sainati/type-removal-interface

Forbid interface removals
onflow · May 29, 2024 · 415d30d · 415d30d
2 parents fe30f83 + 7c77436
commit 415d30d
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 6 deletions.
diff --git a/runtime/contract_update_validation_test.go b/runtime/contract_update_validation_test.go
@@ -3246,21 +3246,59 @@ func TestPragmaUpdates(t *testing.T) {
 		const oldCode = `
 				access(all) contract Test {
 					access(all) resource R {}
-					access(all) struct interface I {}
+					access(all) struct S {}
 				}
 			`
 
 		const newCode = `
 				access(all) contract Test {
 					#removedType(R)
-					#removedType(I)
+					#removedType(S)
 				}
 			`
 
 		err := testDeployAndUpdate(t, "Test", oldCode, newCode, withC1Upgrade)
 		require.NoError(t, err)
 	})
 
+	testWithValidators(t, "#removedType does not allow resource interface type removal", func(t *testing.T, withC1Upgrade bool) {
+
+		const oldCode = `
+				access(all) contract Test {
+					access(all) resource interface R {}
+				}
+			`
+
+		const newCode = `
+				access(all) contract Test {
+					#removedType(R)
+				}
+			`
+
+		err := testDeployAndUpdate(t, "Test", oldCode, newCode, withC1Upgrade)
+		var expectedErr *stdlib.MissingDeclarationError
+		require.ErrorAs(t, err, &expectedErr)
+	})
+
+	testWithValidators(t, "#removedType does not allow struct interface type removal", func(t *testing.T, withC1Upgrade bool) {
+
+		const oldCode = `
+				access(all) contract Test {
+					access(all) struct interface S {}
+				}
+			`
+
+		const newCode = `
+				access(all) contract Test {
+					#removedType(S)
+				}
+			`
+
+		err := testDeployAndUpdate(t, "Test", oldCode, newCode, withC1Upgrade)
+		var expectedErr *stdlib.MissingDeclarationError
+		require.ErrorAs(t, err, &expectedErr)
+	})
+
 	testWithValidators(t, "#removedType can be added", func(t *testing.T, withC1Upgrade bool) {
 
 		const oldCode = `

diff --git a/runtime/stdlib/contract_update_validation.go b/runtime/stdlib/contract_update_validation.go
@@ -347,19 +347,22 @@ func (validator *ContractUpdateValidator) checkNestedDeclarationRemoval(
 	newContainingDeclaration ast.Declaration,
 	removedTypes *orderedmap.OrderedMap[string, struct{}],
 ) {
+	declarationKind := nestedDeclaration.DeclarationKind()
+
 	// OK to remove events - they are not stored
-	if nestedDeclaration.DeclarationKind() == common.DeclarationKindEvent {
+	if declarationKind == common.DeclarationKindEvent {
 		return
 	}
 
-	// OK to remove a type if it is included in a #removedType pragma
-	if removedTypes.Contains(nestedDeclaration.DeclarationIdentifier().Identifier) {
+	// OK to remove a type if it is included in a #removedType pragma, and it is not an interface
+	if removedTypes.Contains(nestedDeclaration.DeclarationIdentifier().Identifier) &&
+		!declarationKind.IsInterfaceDeclaration() {
 		return
 	}
 
 	validator.report(&MissingDeclarationError{
 		Name: nestedDeclaration.DeclarationIdentifier().Identifier,
-		Kind: nestedDeclaration.DeclarationKind(),
+		Kind: declarationKind,
 		Range: ast.NewUnmeteredRangeFromPositioned(
 			newContainingDeclaration.DeclarationIdentifier(),
 		),