Allow default functions to co-exist with empty function declarations

runtime/error_test.go

@@ -485,133 +485,6 @@ func TestRuntimeError(t *testing.T) {
 	})
 }
 
-func TestRuntimeDefaultFunctionConflictPrintingError(t *testing.T) {
-	t.Parallel()
-
-	runtime := NewTestInterpreterRuntime()
-
-	makeDeployTransaction := func(name, code string) []byte {
-		return []byte(fmt.Sprintf(
-			`
-              transaction {
-                prepare(signer: auth(BorrowValue) &Account) {
-                  let acct = Account(payer: signer)
-                  acct.contracts.add(name: "%s", code: "%s".decodeHex())
-                }
-              }
-            `,
-			name,
-			hex.EncodeToString([]byte(code)),
-		))
-	}
-
-	contractInterfaceCode := `
-      access(all) contract TestInterfaces {
-
-          access(all) resource interface A {
-              access(all) fun foo() {
-                  let x = 3
-              }
-          }
-
-		  access(all) resource interface B {
-			access(all) fun foo()
-		}
-      }
-    `
-
-	contractCode := `
-      import TestInterfaces from 0x2
-      access(all) contract TestContract {
-          access(all) resource R: TestInterfaces.A, TestInterfaces.B {}
-		  // fill space
-		  // fill space
-		  // fill space
-		  // fill space
-		  // fill space
-		  // fill space
-		  // filling lots of space
-		  // filling lots of space
-		  // filling lots of space
-      }
-    `
-
-	accountCodes := map[Location][]byte{}
-	var events []cadence.Event
-
-	var nextAccount byte = 0x2
-
-	runtimeInterface := &TestRuntimeInterface{
-		OnGetCode: func(location Location) (bytes []byte, err error) {
-			return accountCodes[location], nil
-		},
-		Storage: NewTestLedger(nil, nil),
-		OnCreateAccount: func(payer Address) (address Address, err error) {
-			result := interpreter.NewUnmeteredAddressValueFromBytes([]byte{nextAccount})
-			nextAccount++
-			return result.ToAddress(), nil
-		},
-		OnGetSigningAccounts: func() ([]Address, error) {
-			return []Address{{0x1}}, nil
-		},
-		OnResolveLocation: NewSingleIdentifierLocationResolver(t),
-		OnGetAccountContractCode: func(location common.AddressLocation) (code []byte, err error) {
-			return accountCodes[location], nil
-		},
-		OnUpdateAccountContractCode: func(location common.AddressLocation, code []byte) error {
-			accountCodes[location] = code
-			return nil
-		},
-		OnEmitEvent: func(event cadence.Event) error {
-			events = append(events, event)
-			return nil
-		},
-	}
-
-	nextTransactionLocation := NewTransactionLocationGenerator()
-
-	deployTransaction := makeDeployTransaction("TestInterfaces", contractInterfaceCode)
-	err := runtime.ExecuteTransaction(
-		Script{
-			Source: deployTransaction,
-		},
-		Context{
-			Interface: runtimeInterface,
-			Location:  nextTransactionLocation(),
-		},
-	)
-	require.NoError(t, err)
-
-	deployTransaction = makeDeployTransaction("TestContract", contractCode)
-	err = runtime.ExecuteTransaction(
-		Script{
-			Source: deployTransaction,
-		},
-		Context{
-			Interface: runtimeInterface,
-			Location:  nextTransactionLocation(),
-		},
-	)
-	require.Error(t, err)
-	require.Contains(t, err.Error(), "access(all) resource R: TestInterfaces.A, TestInterfaces.B {}")
-
-	var errType *sema.CheckerError
-	require.ErrorAs(t, err, &errType)
-
-	checkerErr := err.(Error).
-		Err.(interpreter.Error).
-		Err.(*stdlib.InvalidContractDeploymentError).
-		Err.(*ParsingCheckingError).
-		Err.(*sema.CheckerError)
-
-	var specificErrType *sema.DefaultFunctionConflictError
-	require.ErrorAs(t, checkerErr.Errors[0], &specificErrType)
-
-	errorRange := checkerErr.Errors[0].(*sema.DefaultFunctionConflictError).Range
-
-	require.Equal(t, errorRange.StartPos.Line, 4)
-}
-
 func TestRuntimeMultipleInterfaceDefaultImplementationsError(t *testing.T) {
 	t.Parallel()
 

runtime/sema/check_composite_declaration.go

@@ -1436,33 +1436,6 @@ func (checker *Checker) checkMemberConflicts(
 
 			return true
 		}
-
-		// At most one of them have could default impls.
-		// If one has a default impl, then the other MUST have a condition.
-		// FLIP: https://github.com/onflow/flips/pull/83
-
-		if newMember.HasImplementation {
-			if existingMember.HasConditions {
-				continue
-			}
-		} else if existingMember.HasImplementation {
-			if newMember.HasConditions {
-				continue
-			}
-		} else {
-			// None of them have default impls
-			continue
-		}
-
-		checker.report(
-			&DefaultFunctionConflictError{
-				CompositeKindedType: compositeType,
-				Member:              newMember,
-				Range:               errorRange,
-			},
-		)
-
-		return true
 	}
 
 	return false

runtime/sema/check_interface_declaration.go

@@ -598,7 +598,7 @@ func (checker *Checker) checkInterfaceConformance(
 
 		var isDuplicate bool
 
-		// Check if the members coming from other conformances have conflicts.
+		// Check if the members coming from other conformances (siblings) have conflicts.
 		inheritedMembers, ok := inheritedMembersByName[name]
 		if ok {
 			for _, conflictingMember := range inheritedMembers {
@@ -609,6 +609,7 @@ func (checker *Checker) checkInterfaceConformance(
 					conflictingInterface,
 					conflictingMember,
 					interfaceDeclaration.Identifier,
+					false, // conflicting member is a sibling
 				)
 			}
 		}
@@ -622,6 +623,7 @@ func (checker *Checker) checkInterfaceConformance(
 				conformance,
 				conformanceMember,
 				declarationMember.Identifier,
+				true, // conflicting member is an inherited member
 			)
 		}
 
@@ -639,6 +641,7 @@ func (checker *Checker) checkDuplicateInterfaceMember(
 	conflictingInterfaceType *InterfaceType,
 	conflictingMember *Member,
 	hasPosition ast.HasPosition,
+	isConflictingMemberInherited bool,
 ) (isDuplicate bool) {
 
 	reportMemberConflictError := func() {
@@ -672,24 +675,31 @@ func (checker *Checker) checkDuplicateInterfaceMember(
 	// Having conflicting identical functions with:
 	//  - (1) and (1) - OK
 	//  - (1) and (2) - OK
-	//  - (1) and (3) - Not OK
+	//  - (1) and (3) - OK
+	//  - (2) and (1) - OK
 	//  - (2) and (2) - OK
 	//  - (2) and (3) - OK
+	//  - (3) and (1) - Not OK (order matters)
+	//  - (3) and (2) - OK
 	//  - (3) and (3) - Not OK
 
-	if interfaceMember.HasImplementation {
-		if conflictingMember.HasImplementation || !conflictingMember.HasConditions {
-			reportMemberConflictError()
-			return
-		}
+	if interfaceMember.HasImplementation && conflictingMember.HasImplementation {
+		reportMemberConflictError()
+		return
 	}
 
-	if conflictingMember.HasImplementation {
-		if !interfaceMember.HasConditions {
-			reportMemberConflictError()
-			return
-		}
+	// If the conflicting member is an inherited member, it is OK to override
+	// the inherited declaration, by a default implementation.
+	// However, a default implementation cannot be overridden by an empty declaration.
+
+	if isConflictingMemberInherited &&
+		conflictingMember.HasImplementation && !interfaceMember.HasConditions {
+		reportMemberConflictError()
+		return
 	}
 
+	// If the conflicting member is not an inherited one, (i.e.a member from a sibling conformance)
+	// then default implementation takes the precedence.
+
 	return
 }

runtime/sema/errors.go

@@ -1645,29 +1645,6 @@ func (e *SpecialFunctionDefaultImplementationError) EndPosition(memoryGauge comm
 	return e.Identifier.EndPosition(memoryGauge)
 }
 
-// DefaultFunctionConflictError
-type DefaultFunctionConflictError struct {
-	CompositeKindedType CompositeKindedType
-	Member              *Member
-	ast.Range
-}
-
-var _ SemanticError = &DefaultFunctionConflictError{}
-var _ errors.UserError = &DefaultFunctionConflictError{}
-
-func (*DefaultFunctionConflictError) isSemanticError() {}
-
-func (*DefaultFunctionConflictError) IsUserError() {}
-
-func (e *DefaultFunctionConflictError) Error() string {
-	return fmt.Sprintf(
-		"%s `%s` has conflicting requirements for function `%s`",
-		e.CompositeKindedType.GetCompositeKind().Name(),
-		e.CompositeKindedType.QualifiedString(),
-		e.Member.Identifier.Identifier,
-	)
-}
-
 // InterfaceMemberConflictError
 type InterfaceMemberConflictError struct {
 	InterfaceType            *InterfaceType

runtime/tests/checker/interface_test.go

@@ -1894,9 +1894,7 @@ func TestCheckMultipleInterfaceSingleInterfaceDefaultImplementation(t *testing.T
           }
         `)
 
-		errs := RequireCheckerErrors(t, err, 1)
-
-		require.IsType(t, &sema.DefaultFunctionConflictError{}, errs[0])
+		require.NoError(t, err)
 	})
 }
 
@@ -3017,12 +3015,28 @@ func TestCheckInterfaceDefaultMethodsInheritance(t *testing.T) {
             }
         `)
 
-		errs := RequireCheckerErrors(t, err, 1)
+		require.NoError(t, err)
+	})
 
-		memberConflictError := &sema.InterfaceMemberConflictError{}
-		require.ErrorAs(t, errs[0], &memberConflictError)
-		assert.Equal(t, "hello", memberConflictError.MemberName)
-		assert.Equal(t, "A", memberConflictError.ConflictingInterfaceType.QualifiedIdentifier())
+	t.Run("default impl in child, declaration in parent, concrete type", func(t *testing.T) {
+
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+            struct interface A {
+                access(all) fun hello()
+            }
+
+            struct interface B: A {
+                access(all) fun hello() {
+                    var a = 1
+                }
+            }
+
+            struct C: B {}
+        `)
+
+		require.NoError(t, err)
 	})
 
 	t.Run("default impl in both", func(t *testing.T) {
@@ -3073,7 +3087,7 @@ func TestCheckInterfaceDefaultMethodsInheritance(t *testing.T) {
             }
         `)
 
-		errs := RequireCheckerErrors(t, err, 3)
+		errs := RequireCheckerErrors(t, err, 2)
 
 		memberConflictError := &sema.InterfaceMemberConflictError{}
 		require.ErrorAs(t, errs[0], &memberConflictError)
@@ -3084,11 +3098,6 @@ func TestCheckInterfaceDefaultMethodsInheritance(t *testing.T) {
 		require.ErrorAs(t, errs[1], &memberConflictError)
 		assert.Equal(t, "C", memberConflictError.InterfaceType.QualifiedIdentifier())
 		assert.Equal(t, "hello", memberConflictError.MemberName)
-		assert.Equal(t, "B", memberConflictError.ConflictingInterfaceType.QualifiedIdentifier())
-
-		require.ErrorAs(t, errs[2], &memberConflictError)
-		assert.Equal(t, "C", memberConflictError.InterfaceType.QualifiedIdentifier())
-		assert.Equal(t, "hello", memberConflictError.MemberName)
 		assert.Equal(t, "A", memberConflictError.ConflictingInterfaceType.QualifiedIdentifier())
 	})
 
@@ -3360,13 +3369,7 @@ func TestCheckInterfaceDefaultMethodsInheritance(t *testing.T) {
             struct interface D: A, B, C {}
         `)
 
-		errs := RequireCheckerErrors(t, err, 1)
-
-		memberConflictError := &sema.InterfaceMemberConflictError{}
-		require.ErrorAs(t, errs[0], &memberConflictError)
-		assert.Equal(t, "hello", memberConflictError.MemberName)
-		assert.Equal(t, "A", memberConflictError.ConflictingInterfaceType.QualifiedIdentifier())
-		assert.Equal(t, "C", memberConflictError.InterfaceType.QualifiedIdentifier())
+		require.NoError(t, err)
 	})
 
 	t.Run("all three formats of function, concrete type", func(t *testing.T) {
@@ -3393,10 +3396,7 @@ func TestCheckInterfaceDefaultMethodsInheritance(t *testing.T) {
             struct D: A, B, C {}
         `)
 
-		errs := RequireCheckerErrors(t, err, 1)
-
-		defaultFunctionConflictError := &sema.DefaultFunctionConflictError{}
-		require.ErrorAs(t, errs[0], &defaultFunctionConflictError)
+		require.NoError(t, err)
 	})
 }