-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Account for selector specificity when merging extend_unsafe_fixes
and override extend_safe_fixes
#8444
Account for selector specificity when merging extend_unsafe_fixes
and override extend_safe_fixes
#8444
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for contributing! I have some questions about simplifying the implementation but otherwise this looks great.
extend_unsafe_fixes
and override extend_safe_fixes
|
I was going to ask if there is a chance this will make it to the next release, but I see the release happened yesterday :) is this good to go as is @zanieb? |
Sorry for the delay this one got lost! |
LGTM |
Summary
Prior to this change
extend_unsafe_fixes
took precedence overextend_safe_fixes
selectors, so any conflicts were resolved in favour ofextend_unsafe_fixes
. Thanks to that ruff were conservatively assuming that if configs conlict the fix corresponding to selected rule will be treated as unsafe.After this change we take into account Specificity of the selectors. For conflicts between selectors of the same Specificity we will treat the corresponding fixes as unsafe. But if the conflicting selectors are of different specificity the more specific one will win.
Test Plan
Tests were added for the
FixSafetyTable
struct. Thecheck_extend_unsafe_fixes_conflict_with_extend_safe_fixes_by_specificity
integration test was added to test conflicting rules of different specificity.Fixes #8404