Skip to content

Commit

Permalink
Skip LibCST parsing for standard dedent adjustments
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Feb 2, 2024
1 parent ea1c089 commit 01f0444
Show file tree
Hide file tree
Showing 11 changed files with 182 additions and 17 deletions.
10 changes: 10 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP036_0.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,13 @@ def g():

if sys.version_info[:3] > (3,13):
print("py3")

if sys.version_info > (3,0):
f"this is\
allowed too"

f"""the indentation on
this line is significant"""

"this is\
allowed too"
34 changes: 22 additions & 12 deletions crates/ruff_linter/src/fix/edits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Stmt};
use ruff_python_ast::{AnyNodeRef, ArgOrKeyword};
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
use ruff_python_trivia::textwrap::dedent_to;
use ruff_python_trivia::{
has_leading_content, is_python_whitespace, CommentRanges, PythonWhitespace, SimpleTokenKind,
SimpleTokenizer,
Expand Down Expand Up @@ -173,25 +174,34 @@ pub(crate) fn adjust_indentation(
range: TextRange,
indentation: &str,
locator: &Locator,
indexer: &Indexer,
stylist: &Stylist,
) -> Result<String> {
let contents = locator.slice(range);
// If the range includes a multi-line string, use LibCST to ensure that we don't adjust the
// whitespace _within_ the string.
if indexer.multiline_ranges().intersects(range) || indexer.fstring_ranges().intersects(range) {
let contents = locator.slice(range);

let module_text = format!("def f():{}{contents}", stylist.line_ending().as_str());
let module_text = format!("def f():{}{contents}", stylist.line_ending().as_str());

let mut tree = match_statement(&module_text)?;
let mut tree = match_statement(&module_text)?;

let embedding = match_function_def(&mut tree)?;
let embedding = match_function_def(&mut tree)?;

let indented_block = match_indented_block(&mut embedding.body)?;
indented_block.indent = Some(indentation);
let indented_block = match_indented_block(&mut embedding.body)?;
indented_block.indent = Some(indentation);

let module_text = indented_block.codegen_stylist(stylist);
let module_text = module_text
.strip_prefix(stylist.line_ending().as_str())
.unwrap()
.to_string();
Ok(module_text)
let module_text = indented_block.codegen_stylist(stylist);
let module_text = module_text
.strip_prefix(stylist.line_ending().as_str())
.unwrap()
.to_string();
Ok(module_text)
} else {
// Otherwise, we can do a simple adjustment ourselves.
let contents = locator.slice(range);
Ok(dedent_to(contents, indentation))
}
}

/// Determine if a vector contains only one, specific element.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,7 @@ fn remove_else(
TextRange::new(else_colon_end, elif_else.end()),
desired_indentation,
locator,
indexer,
stylist,
)?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub(crate) fn trailing_whitespace(
diagnostic.set_fix(Fix::applicable_edit(
Edit::range_deletion(range),
// Removing trailing whitespace is not safe inside multiline strings.
if indexer.multiline_ranges().intersects(range) {
if indexer.multiline_ranges().contains(range) {
Applicability::Unsafe
} else {
Applicability::Safe
Expand Down
14 changes: 12 additions & 2 deletions crates/ruff_linter/src/rules/pylint/rules/collapsible_else_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, ElifElseClause, Stmt};
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange};

Expand Down Expand Up @@ -84,8 +85,15 @@ pub(crate) fn collapsible_else_if(checker: &mut Checker, stmt: &Stmt) {
CollapsibleElseIf,
TextRange::new(else_clause.start(), first.start()),
);
diagnostic
.try_set_fix(|| convert_to_elif(first, else_clause, checker.locator(), checker.stylist()));
diagnostic.try_set_fix(|| {
convert_to_elif(
first,
else_clause,
checker.locator(),
checker.indexer(),
checker.stylist(),
)
});
checker.diagnostics.push(diagnostic);
}

Expand All @@ -94,6 +102,7 @@ fn convert_to_elif(
first: &Stmt,
else_clause: &ElifElseClause,
locator: &Locator,
indexer: &Indexer,
stylist: &Stylist,
) -> Result<Fix> {
let inner_if_line_start = locator.line_start(first.start());
Expand All @@ -109,6 +118,7 @@ fn convert_to_elif(
TextRange::new(inner_if_line_start, inner_if_line_end),
indentation,
locator,
indexer,
stylist,
)?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::identifier;
use ruff_python_ast::{self as ast, ExceptHandler, MatchCase, Stmt};
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange};

Expand Down Expand Up @@ -81,6 +82,7 @@ pub(crate) fn useless_else_on_loop(
orelse,
else_range,
checker.locator(),
checker.indexer(),
checker.stylist(),
)
});
Expand Down Expand Up @@ -134,6 +136,7 @@ fn remove_else(
orelse: &[Stmt],
else_range: TextRange,
locator: &Locator,
indexer: &Indexer,
stylist: &Stylist,
) -> Result<Fix> {
let Some(start) = orelse.first() else {
Expand Down Expand Up @@ -164,6 +167,7 @@ fn remove_else(
),
desired_indentation,
locator,
indexer,
stylist,
)?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ fn fix_always_false_branch(
),
indentation,
checker.locator(),
checker.indexer(),
checker.stylist(),
)
.ok()
Expand Down Expand Up @@ -376,6 +377,7 @@ fn fix_always_true_branch(
TextRange::new(checker.locator().line_start(start.start()), end.end()),
indentation,
checker.locator(),
checker.indexer(),
checker.stylist(),
)
.ok()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -774,4 +774,32 @@ UP036_0.py:210:4: UP036 [*] Version block is outdated for minimum Python version
213 212 | if sys.version_info[:2] > (3,13):
214 213 | print("py3")

UP036_0.py:219:4: UP036 [*] Version block is outdated for minimum Python version
|
217 | print("py3")
218 |
219 | if sys.version_info > (3,0):
| ^^^^^^^^^^^^^^^^^^^^^^^^ UP036
220 | f"this is\
221 | allowed too"
|
= help: Remove outdated version block

Unsafe fix
216 216 | if sys.version_info[:3] > (3,13):
217 217 | print("py3")
218 218 |
219 |-if sys.version_info > (3,0):
220 |- f"this is\
219 |+f"this is\
221 220 | allowed too"
222 221 |
223 |- f"""the indentation on
222 |+f"""the indentation on
224 223 | this line is significant"""
225 224 |
226 |- "this is\
225 |+"this is\
227 226 | allowed too"


8 changes: 8 additions & 0 deletions crates/ruff_python_index/src/fstring_ranges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ pub struct FStringRanges {
}

impl FStringRanges {
/// Returns `true` if the given range contains an f-string.
pub fn intersects(&self, target: TextRange) -> bool {
self.raw
.values()
.take_while(|range| range.start() < target.end())
.any(|range| target.contains_range(*range))
}

/// Return the [`TextRange`] of the innermost f-string at the given offset.
pub fn innermost(&self, offset: TextSize) -> Option<TextRange> {
self.raw
Expand Down
17 changes: 16 additions & 1 deletion crates/ruff_python_index/src/multiline_ranges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub struct MultilineRanges {

impl MultilineRanges {
/// Returns `true` if the given range is inside a multiline string.
pub fn intersects(&self, target: TextRange) -> bool {
pub fn contains(&self, target: TextRange) -> bool {
self.ranges
.binary_search_by(|range| {
if range.contains_range(target) {
Expand All @@ -22,6 +22,21 @@ impl MultilineRanges {
})
.is_ok()
}

/// Returns `true` if the given range contains a multiline string.
pub fn intersects(&self, target: TextRange) -> bool {
self.ranges
.binary_search_by(|range| {
if target.contains_range(*range) {
std::cmp::Ordering::Equal
} else if range.end() < target.start() {
std::cmp::Ordering::Less
} else {
std::cmp::Ordering::Greater
}
})
.is_ok()
}
}

#[derive(Default)]
Expand Down
79 changes: 78 additions & 1 deletion crates/ruff_python_trivia/src/textwrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ pub fn indent<'a>(text: &'a str, prefix: &str) -> Cow<'a, str> {
/// Removes common leading whitespace from each line.
///
/// This function will look at each non-empty line and determine the
/// maximum amount of whitespace that can be removed from all lines:
/// maximum amount of whitespace that can be removed from all lines.
///
/// Lines that consist solely of whitespace are trimmed to a blank line.
///
/// ```
/// # use ruff_python_trivia::textwrap::dedent;
Expand Down Expand Up @@ -122,6 +124,51 @@ pub fn dedent(text: &str) -> Cow<'_, str> {
Cow::Owned(result)
}

/// Reduce a block's indentation to match the provided indentation.
///
/// This function looks at the first line in the block to determine the
/// current indentation, then removes whitespace from each line to
/// match the provided indentation.
///
/// Lines that are indented by _less_ than the indent of the first line
/// are left unchanged.
///
/// Lines that consist solely of whitespace are trimmed to a blank line.
///
/// # Panics
/// If the first line is indented by less than the provided indent.
pub fn dedent_to(text: &str, indent: &str) -> String {
// Look at the indentation of the first line, to determine the "baseline" indentation.
let existing_indent_len = text
.universal_newlines()
.next()
.map_or(0, |line| line.len() - line.trim_start().len());

// Determine the amount of indentation to remove.
let dedent_len = existing_indent_len - indent.len();

let mut result = String::with_capacity(text.len() + indent.len());
for line in text.universal_newlines() {
let trimmed = line.trim_whitespace_start();
if trimmed.is_empty() {
if let Some(line_ending) = line.line_ending() {
result.push_str(&line_ending);
}
} else {
// Determine the current indentation level.
let current_indent_len = line.len() - trimmed.len();
if current_indent_len < existing_indent_len {
// If the current indentation level is less than the baseline, keep it as is.
result.push_str(line.as_full_str());
} else {
// Otherwise, reduce the indentation level.
result.push_str(&line.as_full_str()[dedent_len..]);
}
}
}
result
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -344,4 +391,34 @@ mod tests {
]";
assert_eq!(dedent(text), text);
}

#[test]
#[rustfmt::skip]
fn adjust_indent() {
let x = [
" foo",
" bar",
" ",
" baz"
].join("\n");
let y = [
" foo",
" bar",
"",
" baz"
].join("\n");
assert_eq!(dedent_to(&x, " "), y);

let x = [
" foo",
" bar",
" baz",
].join("\n");
let y = [
"foo",
" bar",
"baz"
].join("\n");
assert_eq!(dedent_to(&x, ""), y);
}
}

0 comments on commit 01f0444

Please sign in to comment.