Skip to content

Commit

Permalink
deduplicate entries when creating typed dicts from dict literals
Browse files Browse the repository at this point in the history
Summary:
Overview:

This fixes a Pyre crash reported from the Pyre + Pysa chat.

Background:

The root cause is that dictionary literals with duplicate entries were not being deduplicated when generating anonymous typed dictionary types in weakenMutableLiterals.

The change in D63430844 crashes when typed dict types have duplicate fields, which seems OK since duplicate fields makes no sense and shouldn't be allowed here.

Solution:

This diff fixes the issue by deduplicating the fields before constructing the typed dictionary type.

Reviewed By: samwgoldman, rchen152

Differential Revision: D64914727

fbshipit-source-id: d06cf1b4c55583d9d5e2fab26e88c299284f7ad2
  • Loading branch information
yangdanny97 authored and facebook-github-bot committed Oct 24, 2024
1 parent 1e7c05e commit 887fe84
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 1 deletion.
10 changes: 10 additions & 0 deletions source/analysis/test/integration/typedDictionaryTest.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,16 @@ let test_check_typed_dictionaries =
return {'name' : "Blade Runner", 'year' : 1982}
|}
[];
(* Dictionary literals might have duplicate keys *)
labeled_test_case __FUNCTION__ __LINE__
@@ assert_test_typed_dictionary
{|
import mypy_extensions
Movie = mypy_extensions.TypedDict('Movie', {'name': str, 'year': 'int'})
def f() -> Movie:
return {'name' : "Blade Runner", 'year' : 1981, 'year': 1982}
|}
[];
labeled_test_case __FUNCTION__ __LINE__
@@ assert_test_typed_dictionary
{|
Expand Down
1 change: 1 addition & 0 deletions source/analysis/type.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3965,6 +3965,7 @@ module TypedDictionary = struct

open T

(* Precondition: `fields` cannot have duplicate entries *)
let anonymous fields = { name = "$anonymous"; fields }

let create_field ~annotation ~has_non_total_typed_dictionary_base_class name =
Expand Down
19 changes: 18 additions & 1 deletion source/analysis/weakenMutableLiterals.ml
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,26 @@ let rec weaken_mutable_literals
| Type.Primitive name when String.equal name fresh_class_name -> Some typed_dictionary
| _ -> None
in
(* Dictionary literals may have duplicate entries, but their typed dict representation
should not. When deduplicating, the last entry takes precedence. *)
let deduplicate_fields fields =
List.rev fields
|> List.fold ~init:[] ~f:(fun sofar (({ name; _ } : typed_dictionary_field) as field) ->
if
List.exists
~f:(fun ({ name = existing_name; _ } : typed_dictionary_field) ->
String.equal name existing_name)
sofar
then
sofar
else
field :: sofar)
in
let weaken_valid_fields fields =
let ({ fields = actual_fields; _ } as resolved_typed_dictionary) =
add_missing_fields_if_all_non_required fields |> Type.TypedDictionary.anonymous
add_missing_fields_if_all_non_required fields
|> deduplicate_fields
|> Type.TypedDictionary.anonymous
in
let less_than_expected =
comparator_without_override
Expand Down

0 comments on commit 887fe84

Please sign in to comment.