Skip to content

Commit

Permalink
Remove nested allocations
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Mar 26, 2023
1 parent cb408a3 commit 2c8fbb7
Show file tree
Hide file tree
Showing 7 changed files with 382 additions and 202 deletions.
7 changes: 0 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion crates/ruff/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ ruff_python_stdlib = { path = "../ruff_python_stdlib" }
ruff_rustpython = { path = "../ruff_rustpython" }

anyhow = { workspace = true }
bisection = { version = "0.1.0" }
bitflags = { workspace = true }
chrono = { workspace = true }
clap = { workspace = true, features = ["derive", "string"], optional = true }
Expand Down
106 changes: 46 additions & 60 deletions crates/ruff/src/checkers/logical_lines.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![allow(dead_code, unused_imports, unused_variables)]

use bisection::bisect_left;
use itertools::Itertools;
use rustpython_parser::ast::Location;
use rustpython_parser::lexer::LexResult;
Expand All @@ -10,7 +9,7 @@ use ruff_python_ast::source_code::{Locator, Stylist};
use ruff_python_ast::types::Range;

use crate::registry::{AsRule, Rule};
use crate::rules::pycodestyle::logical_lines::{iter_logical_lines, TokenFlags};
use crate::rules::pycodestyle::logical_lines::{LogicalLines, TokenFlags};
use crate::rules::pycodestyle::rules::{
extraneous_whitespace, indentation, missing_whitespace, missing_whitespace_after_keyword,
missing_whitespace_around_operator, space_around_operator, whitespace_around_keywords,
Expand All @@ -20,23 +19,18 @@ use crate::rules::pycodestyle::rules::{
use crate::settings::{flags, Settings};

/// Return the amount of indentation, expanding tabs to the next multiple of 8.
fn expand_indent(mut line: &str) -> usize {
while line.ends_with("\n\r") {
line = &line[..line.len() - 2];
}
if !line.contains('\t') {
return line.len() - line.trim_start().len();
}
fn expand_indent(line: &str) -> usize {
let line = line.trim_end_matches(['\n', '\r']);

let mut indent = 0;
for c in line.chars() {
if c == '\t' {
indent = (indent / 8) * 8 + 8;
} else if c == ' ' {
indent += 1;
} else {
break;
for c in line.bytes() {
match c {
b'\t' => indent = (indent / 8) * 8 + 8,
b' ' => indent += 1,
_ => break,
}
}

indent
}

Expand All @@ -52,25 +46,18 @@ pub fn check_logical_lines(
let indent_char = stylist.indentation().as_char();
let mut prev_line = None;
let mut prev_indent_level = None;
for line in iter_logical_lines(tokens, locator) {
if line.mapping.is_empty() {
continue;
}

for line in &LogicalLines::from_tokens(tokens, locator) {
// Extract the indentation level.
let start_loc = line.mapping[0].1;
let start_line = locator.slice(Range::new(Location::new(start_loc.row(), 0), start_loc));
let Some(start_loc) = line.first_token_location() else { continue; };
let start_line = locator.slice(Range::new(Location::new(start_loc.row(), 0), *start_loc));
let indent_level = expand_indent(start_line);
let indent_size = 4;

// Generate mapping from logical to physical offsets.
let mapping_offsets = line.mapping.iter().map(|(offset, _)| *offset).collect_vec();

if line.flags.contains(TokenFlags::OPERATOR) {
for (index, kind) in space_around_operator(&line.text) {
let (token_offset, pos) = line.mapping[bisect_left(&mapping_offsets, &index)];
let location = Location::new(pos.row(), pos.column() + index - token_offset);
if line.flags().contains(TokenFlags::OPERATOR) {
for (index, kind) in space_around_operator(line.text()) {
if settings.rules.enabled(kind.rule()) {
let (token_offset, pos) = line.mapping(index);
let location = Location::new(pos.row(), pos.column() + index - token_offset);
diagnostics.push(Diagnostic {
kind,
location,
Expand All @@ -82,13 +69,13 @@ pub fn check_logical_lines(
}
}
if line
.flags
.flags()
.contains(TokenFlags::OPERATOR | TokenFlags::PUNCTUATION)
{
for (index, kind) in extraneous_whitespace(&line.text) {
let (token_offset, pos) = line.mapping[bisect_left(&mapping_offsets, &index)];
let location = Location::new(pos.row(), pos.column() + index - token_offset);
for (index, kind) in extraneous_whitespace(line.text()) {
if settings.rules.enabled(kind.rule()) {
let (token_offset, pos) = line.mapping(index);
let location = Location::new(pos.row(), pos.column() + index - token_offset);
diagnostics.push(Diagnostic {
kind,
location,
Expand All @@ -99,11 +86,11 @@ pub fn check_logical_lines(
}
}
}
if line.flags.contains(TokenFlags::KEYWORD) {
for (index, kind) in whitespace_around_keywords(&line.text) {
let (token_offset, pos) = line.mapping[bisect_left(&mapping_offsets, &index)];
let location = Location::new(pos.row(), pos.column() + index - token_offset);
if line.flags().contains(TokenFlags::KEYWORD) {
for (index, kind) in whitespace_around_keywords(line.text()) {
if settings.rules.enabled(kind.rule()) {
let (token_offset, pos) = line.mapping(index);
let location = Location::new(pos.row(), pos.column() + index - token_offset);
diagnostics.push(Diagnostic {
kind,
location,
Expand All @@ -114,7 +101,7 @@ pub fn check_logical_lines(
}
}

for (location, kind) in missing_whitespace_after_keyword(&line.tokens) {
for (location, kind) in missing_whitespace_after_keyword(line.tokens()) {
if settings.rules.enabled(kind.rule()) {
diagnostics.push(Diagnostic {
kind,
Expand All @@ -126,8 +113,8 @@ pub fn check_logical_lines(
}
}
}
if line.flags.contains(TokenFlags::COMMENT) {
for (range, kind) in whitespace_before_comment(&line.tokens, locator) {
if line.flags().contains(TokenFlags::COMMENT) {
for (range, kind) in whitespace_before_comment(line.tokens(), locator) {
if settings.rules.enabled(kind.rule()) {
diagnostics.push(Diagnostic {
kind,
Expand All @@ -139,9 +126,9 @@ pub fn check_logical_lines(
}
}
}
if line.flags.contains(TokenFlags::OPERATOR) {
if line.flags().contains(TokenFlags::OPERATOR) {
for (location, kind) in
whitespace_around_named_parameter_equals(&line.tokens, &line.text)
whitespace_around_named_parameter_equals(line.tokens(), line.text())
{
if settings.rules.enabled(kind.rule()) {
diagnostics.push(Diagnostic {
Expand All @@ -153,7 +140,7 @@ pub fn check_logical_lines(
});
}
}
for (location, kind) in missing_whitespace_around_operator(&line.tokens) {
for (location, kind) in missing_whitespace_around_operator(line.tokens()) {
if settings.rules.enabled(kind.rule()) {
diagnostics.push(Diagnostic {
kind,
Expand All @@ -172,23 +159,23 @@ pub fn check_logical_lines(
let should_fix = false;

for diagnostic in
missing_whitespace(&line.text, start_loc.row(), should_fix, indent_level)
missing_whitespace(line.text(), start_loc.row(), should_fix, indent_level)
{
if settings.rules.enabled(diagnostic.kind.rule()) {
diagnostics.push(diagnostic);
}
}
}

if line.flags.contains(TokenFlags::BRACKET) {
if line.flags().contains(TokenFlags::BRACKET) {
#[cfg(feature = "logical_lines")]
let should_fix =
autofix.into() && settings.rules.should_fix(Rule::WhitespaceBeforeParameters);

#[cfg(not(feature = "logical_lines"))]
let should_fix = false;

for diagnostic in whitespace_before_parameters(&line.tokens, should_fix) {
for diagnostic in whitespace_before_parameters(line.tokens(), should_fix) {
if settings.rules.enabled(diagnostic.kind.rule()) {
diagnostics.push(diagnostic);
}
Expand All @@ -203,7 +190,7 @@ pub fn check_logical_lines(
prev_indent_level,
indent_size,
) {
let (token_offset, pos) = line.mapping[bisect_left(&mapping_offsets, &index)];
let (token_offset, pos) = line.mapping(index);
let location = Location::new(pos.row(), pos.column() + index - token_offset);
if settings.rules.enabled(kind.rule()) {
diagnostics.push(Diagnostic {
Expand All @@ -229,10 +216,9 @@ mod tests {
use rustpython_parser::lexer::LexResult;
use rustpython_parser::{lexer, Mode};

use crate::rules::pycodestyle::logical_lines::LogicalLines;
use ruff_python_ast::source_code::Locator;

use crate::checkers::logical_lines::iter_logical_lines;

#[test]
fn split_logical_lines() {
let contents = r#"
Expand All @@ -241,9 +227,9 @@ y = 2
z = x + 1"#;
let lxr: Vec<LexResult> = lexer::lex(contents, Mode::Module).collect();
let locator = Locator::new(contents);
let actual: Vec<String> = iter_logical_lines(&lxr, &locator)
let actual: Vec<String> = LogicalLines::from_tokens(&lxr, &locator)
.into_iter()
.map(|line| line.text)
.map(|line| line.text().to_string())
.collect();
let expected = vec![
"x = 1".to_string(),
Expand All @@ -262,9 +248,9 @@ y = 2
z = x + 1"#;
let lxr: Vec<LexResult> = lexer::lex(contents, Mode::Module).collect();
let locator = Locator::new(contents);
let actual: Vec<String> = iter_logical_lines(&lxr, &locator)
let actual: Vec<String> = LogicalLines::from_tokens(&lxr, &locator)
.into_iter()
.map(|line| line.text)
.map(|line| line.text().to_string())
.collect();
let expected = vec![
"x = [1, 2, 3, ]".to_string(),
Expand All @@ -276,9 +262,9 @@ z = x + 1"#;
let contents = "x = 'abc'";
let lxr: Vec<LexResult> = lexer::lex(contents, Mode::Module).collect();
let locator = Locator::new(contents);
let actual: Vec<String> = iter_logical_lines(&lxr, &locator)
let actual: Vec<String> = LogicalLines::from_tokens(&lxr, &locator)
.into_iter()
.map(|line| line.text)
.map(|line| line.text().to_string())
.collect();
let expected = vec!["x = \"xxx\"".to_string()];
assert_eq!(actual, expected);
Expand All @@ -289,9 +275,9 @@ def f():
f()"#;
let lxr: Vec<LexResult> = lexer::lex(contents, Mode::Module).collect();
let locator = Locator::new(contents);
let actual: Vec<String> = iter_logical_lines(&lxr, &locator)
let actual: Vec<String> = LogicalLines::from_tokens(&lxr, &locator)
.into_iter()
.map(|line| line.text)
.map(|line| line.text().to_string())
.collect();
let expected = vec!["def f():", "x = 1", "f()"];
assert_eq!(actual, expected);
Expand All @@ -304,9 +290,9 @@ def f():
f()"#;
let lxr: Vec<LexResult> = lexer::lex(contents, Mode::Module).collect();
let locator = Locator::new(contents);
let actual: Vec<String> = iter_logical_lines(&lxr, &locator)
let actual: Vec<String> = LogicalLines::from_tokens(&lxr, &locator)
.into_iter()
.map(|line| line.text)
.map(|line| line.text().to_string())
.collect();
let expected = vec!["def f():", "\"xxxxxxxxxxxxxxxxxxxx\"", "", "x = 1", "f()"];
assert_eq!(actual, expected);
Expand Down
Loading

0 comments on commit 2c8fbb7

Please sign in to comment.