Fix crash with duplicate function parameters

nicklockwood · nicklockwood · commit 97e88e45680e · 2025-10-12T17:02:02.000+01:00
diff --git a/ShapeScript/Interpreter.swift b/ShapeScript/Interpreter.swift
@@ -692,9 +692,7 @@ extension Definition {
         case let .function(names, block):
             let declarationContext = context
             let returnType: ValueType
-            var params = Dictionary(uniqueKeysWithValues: names.map {
-                ($0.name, ValueType.any)
-            })
+            var params = Dictionary(names.map { ($0.name, ValueType.any) }) { $1 }
             do {
                 let context = context.push(.init(.all, [:], .any, .any))
                 block.inferTypes(for: &params, in: context)
diff --git a/ShapeScript/Parser.swift b/ShapeScript/Parser.swift
@@ -96,6 +96,7 @@ public struct Identifier: Equatable {
 
 public enum ParserErrorType: Equatable {
     case unexpectedToken(Token, expected: String?)
+    case duplicateParameter(String, at: SourceRange)
     case custom(String, hint: String?, at: SourceRange?)
 }
 
@@ -112,6 +113,8 @@ public extension ParserError {
         switch type {
         case let .unexpectedToken(token, _):
             return token.range
+        case let .duplicateParameter(_, at: range):
+            return range
         case let .custom(_, _, at: range):
             return range
         }
@@ -121,6 +124,8 @@ public extension ParserError {
         switch type {
         case let .unexpectedToken(token, _):
             return "Unexpected \(token.type.errorDescription)"
+        case let .duplicateParameter(name, _):
+            return "Duplicate function parameter '\(name)'"
         case let .custom(message, _, _):
             return message
         }
@@ -146,7 +151,7 @@ public extension ParserError {
             default:
                 return nil
             }
-        case .custom:
+        case .duplicateParameter, .custom:
             return nil
         }
     }
@@ -158,6 +163,8 @@ public extension ParserError {
                 return "Did you mean '\(suggestion)'?"
             }
             return expected.map { "Expected \($0)." }
+        case .duplicateParameter:
+            return "Function parameter names must be unique."
         case let .custom(_, hint: hint, _):
             return hint
         }
@@ -277,6 +284,9 @@ private extension ArraySlice where Element == Token {
                 guard case let .identifier(name) = expression.type else {
                     fallthrough
                 }
+                if names.contains(where: { $0.name == name }) {
+                    throw ParserError(.duplicateParameter(name, at: expression.range))
+                }
                 names.append(Identifier(name: name, range: expression.range))
             }
             return names
diff --git a/ShapeScriptTests/InterpreterTests.swift b/ShapeScriptTests/InterpreterTests.swift
@@ -1928,6 +1928,17 @@ final class InterpreterTests: XCTestCase {
         XCTAssertEqual(delegate.log, [3])
     }
 
+    func testFunctionWithDuplicateArgumentName() throws {
+        let program = """
+        define foo(a a) { a + a }
+        print foo(1 2)
+        """
+        XCTAssertThrowsError(try evaluate(parse(program), delegate: nil)) { error in
+            let error = try? XCTUnwrap(error as? ParserError)
+            XCTAssertEqual(error?.message, "Duplicate function parameter 'a'")
+        }
+    }
+
     // MARK: Block invocation
 
     func testInvokePrimitive() {
diff --git a/ShapeScriptTests/ParserTests.swift b/ShapeScriptTests/ParserTests.swift
@@ -1189,6 +1189,14 @@ final class ParserTests: XCTestCase {
         ]))
     }
 
+    func testFunctionWithDuplicateArgumentName() throws {
+        let input = "define foo(a a) { a + a }"
+        XCTAssertThrowsError(try parse(input)) { error in
+            let error = try? XCTUnwrap(error as? ParserError)
+            XCTAssertEqual(error?.message, "Duplicate function parameter 'a'")
+        }
+    }
+
     // MARK: Comments
 
     func testBlockCommentDoesntMessUpLineCalculation() {

Original file line number	Diff line number	Diff line change
`@@ -96,6 +96,7 @@ public struct Identifier: Equatable {`
`96`	`96`
`97`	`97`	`public enum ParserErrorType: Equatable {`
`98`	`98`	`case unexpectedToken(Token, expected: String?)`
	`99`	`+ case duplicateParameter(String, at: SourceRange)`
`99`	`100`	`case custom(String, hint: String?, at: SourceRange?)`
`100`	`101`	`}`
`101`	`102`
`@@ -112,6 +113,8 @@ public extension ParserError {`
`112`	`113`	`switch type {`
`113`	`114`	`case let .unexpectedToken(token, _):`
`114`	`115`	`return token.range`
	`116`	`+ case let .duplicateParameter(_, at: range):`
	`117`	`+ return range`
`115`	`118`	`case let .custom(_, _, at: range):`
`116`	`119`	`return range`
`117`	`120`	`}`
`@@ -121,6 +124,8 @@ public extension ParserError {`
`121`	`124`	`switch type {`
`122`	`125`	`case let .unexpectedToken(token, _):`
`123`	`126`	`return "Unexpected \(token.type.errorDescription)"`
	`127`	`+ case let .duplicateParameter(name, _):`
	`128`	`+ return "Duplicate function parameter '\(name)'"`
`124`	`129`	`case let .custom(message, _, _):`
`125`	`130`	`return message`
`126`	`131`	`}`
`@@ -146,7 +151,7 @@ public extension ParserError {`
`146`	`151`	`default:`
`147`	`152`	`return nil`
`148`	`153`	`}`
`149`		`- case .custom:`
	`154`	`+ case .duplicateParameter, .custom:`
`150`	`155`	`return nil`
`151`	`156`	`}`
`152`	`157`	`}`
`@@ -158,6 +163,8 @@ public extension ParserError {`
`158`	`163`	`return "Did you mean '\(suggestion)'?"`
`159`	`164`	`}`
`160`	`165`	`return expected.map { "Expected \($0)." }`
	`166`	`+ case .duplicateParameter:`
	`167`	`+ return "Function parameter names must be unique."`
`161`	`168`	`case let .custom(_, hint: hint, _):`
`162`	`169`	`return hint`
`163`	`170`	`}`
`@@ -277,6 +284,9 @@ private extension ArraySlice where Element == Token {`
`277`	`284`	`guard case let .identifier(name) = expression.type else {`
`278`	`285`	`fallthrough`
`279`	`286`	`}`
	`287`	`+ if names.contains(where: { $0.name == name }) {`
	`288`	`+ throw ParserError(.duplicateParameter(name, at: expression.range))`
	`289`	`+ }`
`280`	`290`	`names.append(Identifier(name: name, range: expression.range))`
`281`	`291`	`}`
`282`	`292`	`return names`