language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-30 13:51:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Account for selector specificity when merging extend_unsafe_fixes and override extend_safe_fixes (#8444)

Browse source 
## Summary

Prior to this change `extend_unsafe_fixes` took precedence over
`extend_safe_fixes` selectors, so any conflicts were resolved in favour
of `extend_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. The
`check_extend_unsafe_fixes_conflict_with_extend_safe_fixes_by_specificity`
integration test was added to test conflicting rules of different
specificity.

Fixes #8404

---------

Co-authored-by: Zanie <contact@zanie.dev>
This commit is contained in:
Lukasz Piatkowski 2023-11-07 17:33:40 +01:00 • committed by GitHub
parent 7873ca38e5
commit 03303a9edd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 251 additions and 41 deletions
Download patch file Download diff file Expand all files Collapse all files

23
crates/ruff_linter/src/linter.rs
View file

@ -8,7 +8,7 @@ use itertools::Itertools;
use log::error;
use rustc_hash::FxHashMap;
use ruff_diagnostics::{Applicability, Diagnostic};
use ruff_diagnostics::Diagnostic;
use ruff_python_ast::imports::ImportMap;
use ruff_python_ast::PySourceType;
use ruff_python_codegen::Stylist;
@ -268,24 +268,13 @@ pub fn check_path(
}
// Update fix applicability to account for overrides
if !settings.extend_safe_fixes.is_empty() || !settings.extend_unsafe_fixes.is_empty() {
if !settings.fix_safety.is_empty() {
for diagnostic in &mut diagnostics {
if let Some(fix) = diagnostic.fix.take() {
// Enforce demotions over promotions so if someone puts a rule in both we are conservative
if fix.applicability().is_safe()
&& settings
.extend_unsafe_fixes
.contains(diagnostic.kind.rule())
{
diagnostic.set_fix(fix.with_applicability(Applicability::Unsafe));
} else if fix.applicability().is_unsafe()
&& settings.extend_safe_fixes.contains(diagnostic.kind.rule())
{
diagnostic.set_fix(fix.with_applicability(Applicability::Safe));
} else {
// Retain the existing fix (will be dropped from `.take()` otherwise)
diagnostic.set_fix(fix);
}
let fixed_applicability = settings
.fix_safety
.resolve_applicability(diagnostic.kind.rule(), fix.applicability());
diagnostic.set_fix(fix.with_applicability(fixed_applicability));
}
}
}