Reject duplicate cases in switch-statement

runtime/sema/check_casting_expression.go

@@ -124,7 +124,7 @@ func (checker *Checker) VisitCastingExpression(expression *ast.CastingExpression
 						Range:        ast.NewRangeFromPositioned(leftHandExpression),
 					},
 				)
-			} else if checker.lintEnabled && IsSubType(leftHandType, rightHandType) {
+			} else if checker.devModeEnabled && IsSubType(leftHandType, rightHandType) {
 
 				switch expression.Operation {
 				case ast.OperationFailableCast:
@@ -162,7 +162,7 @@ func (checker *Checker) VisitCastingExpression(expression *ast.CastingExpression
 		// the inferred-type of the expression. i.e: exprActualType == rightHandType
 		// Then, it is not possible to determine whether the target type is redundant.
 		// Therefore, don't check for redundant casts, if there are errors.
-		if checker.lintEnabled &&
+		if checker.devModeEnabled &&
 			!hasErrors &&
 			isRedundantCast(leftHandExpression, exprActualType, rightHandType, checker.expectedType) {
 			checker.hint(

runtime/sema/check_force_expression.go

@@ -44,7 +44,7 @@ func (checker *Checker) VisitForceExpression(expression *ast.ForceExpression) as
 	if !ok {
 		// A non-optional type is forced. Suggest removing it
 
-		if checker.lintEnabled {
+		if checker.devModeEnabled {
 			checker.hint(
 				&RemovalHint{
 					Description: "unnecessary force operator",

runtime/sema/check_switch.go

@@ -49,6 +49,8 @@ func (checker *Checker) VisitSwitchStatement(statement *ast.SwitchStatement) ast
 		checker.visitSwitchCase(switchCase, defaultAllowed, testType, testTypeIsValid)
 	}
 
+	checker.checkDuplicateCases(statement.Cases)
+
 	checker.functionActivations.WithSwitch(func() {
 		checker.checkSwitchCasesStatements(statement.Cases)
 	})
@@ -182,3 +184,277 @@ func (checker *Checker) checkSwitchCaseStatements(switchCase *ast.SwitchCase) {
 	}
 	block.Accept(checker)
 }
+
+func (checker *Checker) checkDuplicateCases(cases []*ast.SwitchCase) {
+	// This check is expensive, and catching duplicates is a 'nice to have'
+	// feature. Therefore, only enabled it during development time.
+	if !checker.devModeEnabled {
+		return
+	}
+
+	duplicates := make(map[*ast.SwitchCase]bool)
+
+	duplicateChecker := newDuplicateCaseChecker(checker)
+
+	for i, switchCase := range cases {
+
+		// If the current case is already identified as a duplicate,
+		// then no need to check it again. Can simply skip.
+		if _, isDuplicate := duplicates[switchCase]; isDuplicate {
+			continue
+		}
+
+		for j := i + 1; j < len(cases); j++ {
+			otherCase := cases[j]
+			if !duplicateChecker.isDuplicate(switchCase.Expression, otherCase.Expression) {
+				continue
+			}
+
+			duplicates[otherCase] = true
+
+			checker.report(
+				&DuplicateSwitchCaseError{
+					Range: ast.NewRangeFromPositioned(otherCase.Expression),
+				},
+			)
+		}
+	}
+}
+
+var _ ast.ExpressionVisitor = &duplicateCaseChecker{}
+
+type duplicateCaseChecker struct {
+	expr    ast.Expression
+	checker *Checker
+}
+
+func newDuplicateCaseChecker(checker *Checker) *duplicateCaseChecker {
+	return &duplicateCaseChecker{
+		checker: checker,
+	}
+}
+
+func (d *duplicateCaseChecker) isDuplicate(this ast.Expression, other ast.Expression) bool {
+	if this == nil || other == nil {
+		return false
+	}
+
+	tempExpr := d.expr
+	d.expr = this
+	defer func() {
+		d.expr = tempExpr
+	}()
+
+	return other.AcceptExp(d).(bool)
+}
+
+func (d *duplicateCaseChecker) VisitBoolExpression(otherExpr *ast.BoolExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.BoolExpression)
+	if !ok {
+		return false
+	}
+
+	return otherExpr.Value == expr.Value
+}
+
+func (d *duplicateCaseChecker) VisitNilExpression(_ *ast.NilExpression) ast.Repr {
+	_, ok := d.expr.(*ast.NilExpression)
+	return ok
+}
+
+func (d *duplicateCaseChecker) VisitIntegerExpression(otherExpr *ast.IntegerExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.IntegerExpression)
+	if !ok {
+		return false
+	}
+
+	return expr.Value.Cmp(otherExpr.Value) == 0
+}
+
+func (d *duplicateCaseChecker) VisitFixedPointExpression(otherExpr *ast.FixedPointExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.FixedPointExpression)
+	if !ok {
+		return false
+	}
+
+	return expr.Negative == otherExpr.Negative &&
+		expr.Fractional.Cmp(otherExpr.Fractional) == 0 &&
+		expr.UnsignedInteger.Cmp(otherExpr.UnsignedInteger) == 0 &&
+		expr.Scale == otherExpr.Scale
+}
+
+func (d *duplicateCaseChecker) VisitArrayExpression(otherExpr *ast.ArrayExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.ArrayExpression)
+	if !ok || len(expr.Values) != len(otherExpr.Values) {
+		return false
+	}
+
+	for index, value := range expr.Values {
+		if !d.isDuplicate(value, otherExpr.Values[index]) {
+			return false
+		}
+	}
+
+	return true
+}
+
+func (d *duplicateCaseChecker) VisitDictionaryExpression(otherExpr *ast.DictionaryExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.DictionaryExpression)
+	if !ok || len(expr.Entries) != len(otherExpr.Entries) {
+		return false
+	}
+
+	for index, entry := range expr.Entries {
+		otherEntry := otherExpr.Entries[index]
+
+		if !d.isDuplicate(entry.Key, otherEntry.Key) ||
+			!d.isDuplicate(entry.Value, otherEntry.Value) {
+			return false
+		}
+	}
+
+	return true
+}
+
+func (d *duplicateCaseChecker) VisitIdentifierExpression(otherExpr *ast.IdentifierExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.IdentifierExpression)
+	if !ok {
+		return false
+	}
+
+	return expr.Identifier.Identifier == otherExpr.Identifier.Identifier
+}
+
+func (d *duplicateCaseChecker) VisitInvocationExpression(_ *ast.InvocationExpression) ast.Repr {
+	// Invocations can be stateful. Thus, it's not possible to determine if
+	// invoking the same function in two cases would produce the same results.
+	return false
+}
+
+func (d *duplicateCaseChecker) VisitMemberExpression(otherExpr *ast.MemberExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.MemberExpression)
+	if !ok {
+		return false
+	}
+
+	return d.isDuplicate(expr.Expression, otherExpr.Expression) &&
+		expr.Optional == otherExpr.Optional &&
+		expr.Identifier.Identifier == otherExpr.Identifier.Identifier
+}
+
+func (d *duplicateCaseChecker) VisitIndexExpression(otherExpr *ast.IndexExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.IndexExpression)
+	if !ok {
+		return false
+	}
+
+	return d.isDuplicate(expr.TargetExpression, otherExpr.TargetExpression) &&
+		d.isDuplicate(expr.IndexingExpression, otherExpr.IndexingExpression)
+}
+
+func (d *duplicateCaseChecker) VisitConditionalExpression(otherExpr *ast.ConditionalExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.ConditionalExpression)
+	if !ok {
+		return false
+	}
+
+	return d.isDuplicate(expr.Test, otherExpr.Test) &&
+		d.isDuplicate(expr.Then, otherExpr.Then) &&
+		d.isDuplicate(expr.Else, otherExpr.Else)
+}
+
+func (d *duplicateCaseChecker) VisitUnaryExpression(otherExpr *ast.UnaryExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.UnaryExpression)
+	if !ok {
+		return false
+	}
+
+	return d.isDuplicate(expr.Expression, otherExpr.Expression) &&
+		expr.Operation == otherExpr.Operation
+}
+
+func (d *duplicateCaseChecker) VisitBinaryExpression(otherExpr *ast.BinaryExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.BinaryExpression)
+	if !ok {
+		return false
+	}
+
+	return d.isDuplicate(expr.Left, otherExpr.Left) &&
+		d.isDuplicate(expr.Right, otherExpr.Right) &&
+		expr.Operation == otherExpr.Operation
+}
+
+func (d *duplicateCaseChecker) VisitFunctionExpression(_ *ast.FunctionExpression) ast.Repr {
+	// Not a valid expression for switch-case. Hence, skip.
+	return false
+}
+
+func (d *duplicateCaseChecker) VisitStringExpression(otherExpr *ast.StringExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.StringExpression)
+	if !ok {
+		return false
+	}
+
+	return expr.Value == otherExpr.Value
+}
+
+func (d *duplicateCaseChecker) VisitCastingExpression(otherExpr *ast.CastingExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.CastingExpression)
+	if !ok {
+		return false
+	}
+
+	if !d.isDuplicate(expr.Expression, otherExpr.Expression) ||
+		expr.Operation != otherExpr.Operation {
+		return false
+	}
+
+	typeAnnot := d.checker.ConvertTypeAnnotation(expr.TypeAnnotation)
+	otherTypeAnnot := d.checker.ConvertTypeAnnotation(expr.TypeAnnotation)
+	return typeAnnot.Equal(otherTypeAnnot)
+}
+
+func (d *duplicateCaseChecker) VisitCreateExpression(_ *ast.CreateExpression) ast.Repr {
+	// Not a valid expression for switch-case. Hence, skip.
+	return false
+}
+
+func (d *duplicateCaseChecker) VisitDestroyExpression(_ *ast.DestroyExpression) ast.Repr {
+	// Not a valid expression for switch-case. Hence, skip.
+	return false
+}
+
+func (d *duplicateCaseChecker) VisitReferenceExpression(otherExpr *ast.ReferenceExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.ReferenceExpression)
+	if !ok {
+		return false
+	}
+
+	if !d.isDuplicate(expr.Expression, otherExpr.Expression) {
+		return false
+	}
+
+	targetType := d.checker.ConvertType(expr.Type)
+	otherTargetType := d.checker.ConvertType(otherExpr.Type)
+
+	return targetType.Equal(otherTargetType)
+}
+
+func (d *duplicateCaseChecker) VisitForceExpression(otherExpr *ast.ForceExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.ForceExpression)
+	if !ok {
+		return false
+	}
+
+	return d.isDuplicate(expr.Expression, otherExpr.Expression)
+}
+
+func (d *duplicateCaseChecker) VisitPathExpression(otherExpr *ast.PathExpression) ast.Repr {
+	expr, ok := d.expr.(*ast.PathExpression)
+	if !ok {
+		return false
+	}
+
+	return expr.Domain == otherExpr.Domain &&
+		expr.Identifier.Identifier == otherExpr.Identifier.Identifier
+}

runtime/sema/checker.go

@@ -115,7 +115,7 @@ type Checker struct {
 	checkHandler                       CheckHandlerFunc
 	expectedType                       Type
 	memberAccountAccessHandler         MemberAccountAccessHandlerFunc
-	lintEnabled                        bool
+	devModeEnabled                     bool
 }
 
 type Option func(*Checker) error
@@ -236,12 +236,13 @@ func WithPositionInfoEnabled(enabled bool) Option {
 	}
 }
 
-// WithLintingEnabled returns a checker option which enables/disables
-// advanced linting.
+// WithDevModeEnabled returns a checker option which enables/disables
+// advanced semantic checks that are not mandatory.
+// e.g: advanced linting, duplicate switch-cases
 //
-func WithLintingEnabled(enabled bool) Option {
+func WithDevModeEnabled(enabled bool) Option {
 	return func(checker *Checker) error {
-		checker.lintEnabled = enabled
+		checker.devModeEnabled = enabled
 		return nil
 	}
 }

runtime/sema/errors.go

@@ -2966,3 +2966,15 @@ func (e *InvalidEntryPointTypeError) Error() string {
 		e.Type.QualifiedString(),
 	)
 }
+
+// DuplicateSwitchCaseError
+
+type DuplicateSwitchCaseError struct {
+	ast.Range
+}
+
+func (e *DuplicateSwitchCaseError) Error() string {
+	return "duplicate switch case"
+}
+
+func (*DuplicateSwitchCaseError) isSemanticError() {}

runtime/sema/type.go

@@ -3086,14 +3086,14 @@ func numberFunctionArgumentExpressionsChecker(targetType Type) ArgumentExpressio
 		switch argument := argument.(type) {
 		case *ast.IntegerExpression:
 			if CheckIntegerLiteral(argument, targetType, checker.report) {
-				if checker.lintEnabled {
+				if checker.devModeEnabled {
 					suggestIntegerLiteralConversionReplacement(checker, argument, targetType, invocationRange)
 				}
 			}
 
 		case *ast.FixedPointExpression:
 			if CheckFixedPointLiteral(argument, targetType, checker.report) {
-				if checker.lintEnabled {
+				if checker.devModeEnabled {
 					suggestFixedPointLiteralConversionReplacement(checker, targetType, argument, invocationRange)
 				}
 			}