fix duplicate names in pyrefly report #3097#3130
fix duplicate names in pyrefly report #3097#3130jorenham wants to merge 1 commit intofacebook:mainfrom
pyrefly report #3097#3130Conversation
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
@rchen152 has imported this pull request. If you are a Meta employee, you can view this in D100901014. |
stroxler
left a comment
stroxler
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
connernilsen
left a comment
connernilsen
left a comment
There was a problem hiding this comment.
Thanks for doing this, some very minor requests
| } | ||
|
|
||
| // Overloads and property accessors produce duplicate names. | ||
| let mut seen = HashSet::new(); |
There was a problem hiding this comment.
Can we use a SmallSet here?
| let mut seen = HashSet::new(); | ||
| names.retain(|n| seen.insert(n.clone())); |
There was a problem hiding this comment.
Minor question, but why not use a SmallSet for names and convert to a vec here?
There was a problem hiding this comment.
I didn't know that was a thing tbh. You want me to open a PR for that?
There was a problem hiding this comment.
It's a minor thing, feel free to do it if you want, but since this PR is already closed, you don't have to
Summary: As suggested by connernilsen in #3130 (comment) Pull Request resolved: #3196 Test Plan: Non-functional change; existing tests still pass Reviewed By: connernilsen Differential Revision: D101882136 Pulled By: migeed-z fbshipit-source-id: ec77b85b83d56acd0f3780ad96d7bb81bfddb798
3 participants
Summary
This dedupes names of overloaded functions and properties with multiple accessors.
Fixes #3097
Test Plan
Deduped names in tests