ban duplicate parameter names

Summary: same as the below diffs but for parameters in function definitions. the ordering is already enforced at the parser level, so I just need to ban duplicates for this.

Reviewed By: stroxler

Differential Revision: D57404953

fbshipit-source-id: e4c078f69c4ccf10844e96dc135cbd424d26bdb3

source/analysis/analysisError.ml

@@ -863,6 +863,7 @@ and kind =
       variable: Type.Variable.t;
       base: polymorphism_base_class;
     }
+  | DuplicateParameter of string
   | TupleConcatenationError of tuple_concatenation_problem
   (* Additional errors. *)
   (* TODO(T38384376): split this into a separate module. *)
@@ -943,6 +944,7 @@ let code_of_kind = function
   | NonLiteralString _ -> 62
   | SuppressionCommentWithoutErrorCode _ -> 63
   | InconsistentMethodResolutionOrder _ -> 64
+  | DuplicateParameter _ -> 65
   | ParserFailure _ -> 404
   (* Additional errors. *)
   | UnawaitedAwaitable _ -> 1001
@@ -959,6 +961,7 @@ let name_of_kind = function
   | AnalysisFailure _ -> "Analysis failure"
   | BroadcastError _ -> "Broadcast error"
   | DuplicateTypeVariables _ -> "Duplicate type variables"
+  | DuplicateParameter _ -> "Duplicate parameter"
   | ParserFailure _ -> "Parsing failure"
   | DeadStore _ -> "Dead store"
   | Deobfuscation _ -> "Deobfuscation"
@@ -2726,6 +2729,8 @@ let rec messages ~concise ~signature location kind =
           (show_sanitized_expression expression)
           start_line;
       ]
+  | DuplicateParameter name ->
+      [Format.asprintf "Duplicate parameter name `%a`." Identifier.pp_sanitized name]
   | DuplicateTypeVariables { variable; base } -> (
       let format : ('b, Format.formatter, unit, string) format4 =
         match base with
@@ -3159,6 +3164,7 @@ let due_to_analysis_limitations { kind; _ } =
   | DeadStore _
   | Deobfuscation _
   | DuplicateTypeVariables _
+  | DuplicateParameter _
   | IllegalAnnotationTarget _
   | IncompatibleAsyncGeneratorReturnType _
   | IncompatibleConstructorAnnotation _
@@ -3257,6 +3263,8 @@ let join ~resolution left right =
     | ParserFailure left_message, ParserFailure right_message
       when String.equal left_message right_message ->
         ParserFailure left_message
+    | DuplicateParameter left, DuplicateParameter right when String.equal left right ->
+        DuplicateParameter left
     | DeadStore left, DeadStore right when Identifier.equal left right -> DeadStore left
     | Deobfuscation left, Deobfuscation right when [%compare.equal: Source.t] left right ->
         Deobfuscation left
@@ -3721,6 +3729,7 @@ let join ~resolution left right =
     | UnawaitedAwaitable _, _
     | UnboundName _, _
     | UninitializedLocal _, _
+    | DuplicateParameter _, _
     | DuplicateTypeVariables _, _
     | UndefinedAttribute _, _
     | UndefinedImport _, _
@@ -4341,6 +4350,7 @@ let dequalify
     | UnsafeCast kind -> UnsafeCast kind
     | UnawaitedAwaitable { references; expression } ->
         UnawaitedAwaitable { references = List.map references ~f:dequalify_reference; expression }
+    | DuplicateParameter name -> DuplicateParameter (dequalify_identifier name)
     | DuplicateTypeVariables { variable; base } ->
         DuplicateTypeVariables { variable = Type.Variable.dequalify dequalify_map variable; base }
     | UnboundName name -> UnboundName (dequalify_identifier name)

source/analysis/analysisError.mli

@@ -517,6 +517,7 @@ and kind =
       variable: Type.Variable.t;
       base: polymorphism_base_class;
     }
+  | DuplicateParameter of string
   | TupleConcatenationError of tuple_concatenation_problem
   (* Additional errors. *)
   | DeadStore of Identifier.t

source/analysis/test/integration/callableTest.ml

@@ -101,6 +101,23 @@ let test_callable_parameters =
               foo(a=42, a=42)
             |}
            ["Unexpected keyword [28]: Unexpected keyword argument `a` to call `foo`."];
+      labeled_test_case __FUNCTION__ __LINE__
+      @@ assert_type_errors
+           {|
+              def foo(a:int, a:int) -> None:
+                pass
+            |}
+           ["Duplicate parameter [65]: Duplicate parameter name `a`."];
+      labeled_test_case __FUNCTION__ __LINE__
+      @@ assert_type_errors
+           {|
+              def foo(a:int, a:int, b: int, b: int) -> None:
+                pass
+            |}
+           [
+             "Duplicate parameter [65]: Duplicate parameter name `a`.";
+             "Duplicate parameter [65]: Duplicate parameter name `b`.";
+           ];
     ]
 
 

source/analysis/typeCheck.ml

@@ -5472,7 +5472,8 @@ module State (Context : Context) = struct
             return_type
         in
         Value resolution, validate_return expression ~resolution ~errors ~actual ~is_implicit
-    | Define { signature = { Define.Signature.name; nesting_define; _ } as signature; _ } ->
+    | Define
+        { signature = { Define.Signature.name; nesting_define; parameters; _ } as signature; _ } ->
         let resolution =
           match nesting_define with
           | Some _ ->
@@ -5483,7 +5484,20 @@ module State (Context : Context) = struct
               |> fun annotation -> Resolution.new_local resolution ~reference:name ~annotation
           | None -> resolution
         in
-        Value resolution, []
+        let duplicate_parameters, _ =
+          List.fold
+            parameters
+            ~init:([], Identifier.Set.empty)
+            ~f:(fun (duplicates, seen) { Node.value = { Parameter.name; _ }; location } ->
+              match Set.mem seen name with
+              | true -> (name, location) :: duplicates, seen
+              | false -> duplicates, Set.add seen name)
+        in
+        let errors =
+          List.fold duplicate_parameters ~init:[] ~f:(fun errors (name, location) ->
+              emit_error ~errors ~location ~kind:(Error.DuplicateParameter name))
+        in
+        Value resolution, errors
     | Import { Import.from; imports } ->
         let get_export_kind = function
           | ResolvedReference.Exported export -> Some export