Skip to content

Commit

Permalink
respond to review
Browse files Browse the repository at this point in the history
  • Loading branch information
dsainati1 committed May 28, 2024
1 parent 21aa553 commit 76f8841
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) checkNestedDeclarationR
nestedDeclaration ast.Declaration,
oldContainingDeclaration ast.Declaration,
newContainingDeclaration ast.Declaration,
removedTypes []ast.Identifier,
removedTypes map[string]struct{},
) {

// enums can be removed from contract interfaces, as they have no interface equivalent and are not
Expand Down
35 changes: 14 additions & 21 deletions runtime/stdlib/contract_update_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (
"fmt"
"sort"

"golang.org/x/exp/slices"

"github.com/onflow/cadence/runtime/ast"
"github.com/onflow/cadence/runtime/common"
"github.com/onflow/cadence/runtime/errors"
Expand All @@ -43,7 +41,7 @@ type UpdateValidator interface {
nestedDeclaration ast.Declaration,
oldContainingDeclaration ast.Declaration,
newContainingDeclaration ast.Declaration,
removedTypes []ast.Identifier,
removedTypes map[string]struct{},
)
getAccountContractNames(address common.Address) ([]string, error)

Expand Down Expand Up @@ -218,10 +216,12 @@ func (validator *ContractUpdateValidator) hasErrors() bool {
return len(validator.errors) > 0
}

func collectRemovedTypePragmas(validator UpdateValidator, pragmas []*ast.PragmaDeclaration) (removedTypes []ast.Identifier) {
func collectRemovedTypePragmas(validator UpdateValidator, pragmas []*ast.PragmaDeclaration) map[string]struct{} {
removedTypes := map[string]struct{}{}

for _, pragma := range pragmas {
invocationExpression, isInvocation := pragma.Expression.(*ast.InvocationExpression)
if !isInvocation {
if !isInvocation || len(invocationExpression.Arguments) != 1 {
continue
}
invokedIdentifier, isIdentifier := invocationExpression.InvokedExpression.(*ast.IdentifierExpression)
Expand All @@ -236,7 +236,7 @@ func collectRemovedTypePragmas(validator UpdateValidator, pragmas []*ast.PragmaD
})
continue
}
removedTypes = append(removedTypes, removedTypeName.Identifier)
removedTypes[removedTypeName.Identifier.Identifier] = struct{}{}
}

return removedTypes
Expand Down Expand Up @@ -337,18 +337,15 @@ func (validator *ContractUpdateValidator) checkNestedDeclarationRemoval(
nestedDeclaration ast.Declaration,
_ ast.Declaration,
newContainingDeclaration ast.Declaration,
removedTypes []ast.Identifier,
removedTypes map[string]struct{},
) {
// OK to remove events - they are not stored
if nestedDeclaration.DeclarationKind() == common.DeclarationKindEvent {
return
}

// OK to remove a type if it is included in a #removedType pragma
if nestedDeclaration.DeclarationIdentifier() != nil &&
slices.ContainsFunc(removedTypes, func(removedType ast.Identifier) bool {
return removedType.Identifier == nestedDeclaration.DeclarationIdentifier().Identifier
}) {
if _, present := removedTypes[nestedDeclaration.DeclarationIdentifier().Identifier]; present {
return
}

Expand All @@ -373,11 +370,9 @@ func (validator *ContractUpdateValidator) oldTypeID(oldType *ast.NominalType) co
func checkTypeNotRemoved(
validator UpdateValidator,
newDeclaration ast.Declaration,
removedTypes []ast.Identifier,
removedTypes map[string]struct{},
) {
if slices.ContainsFunc(removedTypes, func(removedType ast.Identifier) bool {
return removedType.Identifier == newDeclaration.DeclarationIdentifier().Identifier
}) {
if _, present := removedTypes[newDeclaration.DeclarationIdentifier().Identifier]; present {
validator.report(&UseOfRemovedTypeError{
Declaration: newDeclaration,
Range: ast.NewUnmeteredRangeFromPositioned(newDeclaration),
Expand All @@ -398,10 +393,8 @@ func checkNestedDeclarations(

// #typeRemoval pragmas cannot be removed, so any that appear in the old program must appear in the new program
// they can however, be added, so use the new program's type removals for the purposes of checking the upgrade
for _, oldRemovedType := range oldRemovedTypes {
if !slices.ContainsFunc(removedTypes, func(newRemovedType ast.Identifier) bool {
return newRemovedType.Identifier == oldRemovedType.Identifier
}) {
for oldRemovedType, _ := range oldRemovedTypes { //nolint:maprange

Check failure on line 396 in runtime/stdlib/contract_update_validation.go

View workflow job for this annotation

GitHub Actions / Lint

File is not `gofmt`-ed with `-s` (gofmt)
if _, present := removedTypes[oldRemovedType]; !present {
validator.report(&TypeRemovalPragmaRemovalError{
RemovedType: oldRemovedType,
})
Expand Down Expand Up @@ -884,7 +877,7 @@ func (e *UseOfRemovedTypeError) Error() string {
// TypeRemovalPragmaRemovalError is reported during a contract update
// if a #removedType pragma is removed
type TypeRemovalPragmaRemovalError struct {
RemovedType ast.Identifier
RemovedType string
}

var _ errors.UserError = &TypeRemovalPragmaRemovalError{}
Expand All @@ -894,6 +887,6 @@ func (*TypeRemovalPragmaRemovalError) IsUserError() {}
func (e *TypeRemovalPragmaRemovalError) Error() string {
return fmt.Sprintf(
"missing #removedType pragma for %s",
e.RemovedType.String(),
e.RemovedType,
)
}

0 comments on commit 76f8841

Please sign in to comment.