Add a dedicated token indexer for continuations and comments (#1886)

The primary motivation is that we can now robustly detect `\` continuations due to the addition of `Tok::NonLogicalNewline`. This PR generalizes the approach we took to comments (track all lines that contain any comments), and applies it to continuations too.
This commit is contained in:
Charlie Marsh 2023-01-15 01:57:31 -05:00 committed by GitHub
parent 2c644619e0
commit 3791ca721a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
40 changed files with 259 additions and 114 deletions

View file

@ -13,7 +13,7 @@ use rustpython_parser::token::StringKind;
use crate::ast::types::{Binding, BindingKind, Range};
use crate::checkers::ast::Checker;
use crate::source_code::{Generator, Locator, Stylist};
use crate::source_code::{Generator, Indexer, Locator, Stylist};
/// Create an `Expr` with default location from an `ExprKind`.
pub fn create_expr(node: ExprKind) -> Expr {
@ -601,27 +601,6 @@ pub fn else_range(stmt: &Stmt, locator: &Locator) -> Option<Range> {
}
}
/// Return `true` if a `Stmt` appears to be part of a multi-statement line, with
/// other statements preceding it.
pub fn preceded_by_continuation(stmt: &Stmt, locator: &Locator) -> bool {
// Does the previous line end in a continuation? This will have a specific
// false-positive, which is that if the previous line ends in a comment, it
// will be treated as a continuation. So we should only use this information to
// make conservative choices.
// TODO(charlie): Come up with a more robust strategy.
if stmt.location.row() > 1 {
let range = Range::new(
Location::new(stmt.location.row() - 1, 0),
Location::new(stmt.location.row(), 0),
);
let line = locator.slice_source_code_range(&range);
if line.trim_end().ends_with('\\') {
return true;
}
}
false
}
/// Return the `Range` of the first `Tok::Colon` token in a `Range`.
pub fn first_colon_range(range: Range, locator: &Locator) -> Option<Range> {
let contents = locator.slice_source_code_range(&range);
@ -637,8 +616,17 @@ pub fn first_colon_range(range: Range, locator: &Locator) -> Option<Range> {
/// Return `true` if a `Stmt` appears to be part of a multi-statement line, with
/// other statements preceding it.
pub fn preceded_by_multi_statement_line(stmt: &Stmt, locator: &Locator) -> bool {
match_leading_content(stmt, locator) || preceded_by_continuation(stmt, locator)
pub fn preceded_by_continuation(stmt: &Stmt, indexer: &Indexer) -> bool {
stmt.location.row() > 1
&& indexer
.continuation_lines()
.contains(&(stmt.location.row() - 1))
}
/// Return `true` if a `Stmt` appears to be part of a multi-statement line, with
/// other statements preceding it.
pub fn preceded_by_multi_statement_line(stmt: &Stmt, locator: &Locator, indexer: &Indexer) -> bool {
match_leading_content(stmt, locator) || preceded_by_continuation(stmt, indexer)
}
/// Return `true` if a `Stmt` appears to be part of a multi-statement line, with

View file

@ -12,7 +12,7 @@ use crate::ast::whitespace::LinesWithTrailingNewline;
use crate::cst::helpers::compose_module_path;
use crate::cst::matchers::match_module;
use crate::fix::Fix;
use crate::source_code::Locator;
use crate::source_code::{Indexer, Locator};
/// Determine if a body contains only a single statement, taking into account
/// deleted.
@ -156,6 +156,7 @@ pub fn delete_stmt(
parent: Option<&Stmt>,
deleted: &[&Stmt],
locator: &Locator,
indexer: &Indexer,
) -> Result<Fix> {
if parent
.map(|parent| is_lone_child(stmt, parent, deleted))
@ -175,7 +176,7 @@ pub fn delete_stmt(
Fix::deletion(stmt.location, next)
} else if helpers::match_leading_content(stmt, locator) {
Fix::deletion(stmt.location, stmt.end_location.unwrap())
} else if helpers::preceded_by_continuation(stmt, locator) {
} else if helpers::preceded_by_continuation(stmt, indexer) {
if is_end_of_file(stmt, locator) && stmt.location.column() == 0 {
// Special-case: a file can't end in a continuation.
Fix::replacement("\n".to_string(), stmt.location, stmt.end_location.unwrap())
@ -198,6 +199,7 @@ pub fn remove_unused_imports<'a>(
parent: Option<&Stmt>,
deleted: &[&Stmt],
locator: &Locator,
indexer: &Indexer,
) -> Result<Fix> {
let module_text = locator.slice_source_code_range(&Range::from_located(stmt));
let mut tree = match_module(&module_text)?;
@ -235,7 +237,7 @@ pub fn remove_unused_imports<'a>(
if !found_star {
bail!("Expected \'*\' for unused import");
}
return delete_stmt(stmt, parent, deleted, locator);
return delete_stmt(stmt, parent, deleted, locator, indexer);
} else {
bail!("Expected: ImportNames::Aliases | ImportNames::Star");
}
@ -296,7 +298,7 @@ pub fn remove_unused_imports<'a>(
}
if aliases.is_empty() {
delete_stmt(stmt, parent, deleted, locator)
delete_stmt(stmt, parent, deleted, locator, indexer)
} else {
let mut state = CodegenState::default();
tree.codegen(&mut state);

View file

@ -39,7 +39,7 @@ use crate::rules::{
};
use crate::settings::types::PythonVersion;
use crate::settings::{flags, Settings};
use crate::source_code::{Locator, Stylist};
use crate::source_code::{Indexer, Locator, Stylist};
use crate::violations::DeferralKeyword;
use crate::visibility::{module_visibility, transition_scope, Modifier, Visibility, VisibleScope};
use crate::{autofix, docstrings, noqa, violations, visibility};
@ -57,7 +57,8 @@ pub struct Checker<'a> {
pub(crate) settings: &'a Settings,
pub(crate) noqa_line_for: &'a IntMap<usize, usize>,
pub(crate) locator: &'a Locator<'a>,
pub(crate) style: &'a Stylist<'a>,
pub(crate) stylist: &'a Stylist<'a>,
pub(crate) indexer: &'a Indexer,
// Computed diagnostics.
pub(crate) diagnostics: Vec<Diagnostic>,
// Function and class definition tracking (e.g., for docstring enforcement).
@ -98,6 +99,7 @@ pub struct Checker<'a> {
}
impl<'a> Checker<'a> {
#[allow(clippy::too_many_arguments)]
pub fn new(
settings: &'a Settings,
noqa_line_for: &'a IntMap<usize, usize>,
@ -106,6 +108,7 @@ impl<'a> Checker<'a> {
path: &'a Path,
locator: &'a Locator,
style: &'a Stylist,
indexer: &'a Indexer,
) -> Checker<'a> {
Checker {
settings,
@ -114,7 +117,8 @@ impl<'a> Checker<'a> {
noqa,
path,
locator,
style,
stylist: style,
indexer,
diagnostics: vec![],
definitions: vec![],
deletions: FxHashSet::default(),
@ -4001,6 +4005,7 @@ impl<'a> Checker<'a> {
parent,
&deleted,
self.locator,
self.indexer,
) {
Ok(fix) => {
if fix.content.is_empty() || fix.content == "pass" {
@ -4296,6 +4301,7 @@ pub fn check_ast(
python_ast: &Suite,
locator: &Locator,
stylist: &Stylist,
indexer: &Indexer,
noqa_line_for: &IntMap<usize, usize>,
settings: &Settings,
autofix: flags::Autofix,
@ -4310,6 +4316,7 @@ pub fn check_ast(
path,
locator,
stylist,
indexer,
);
checker.push_scope(Scope::new(ScopeKind::Module));
checker.bind_builtins();

View file

@ -10,12 +10,13 @@ use crate::registry::{Diagnostic, RuleCode};
use crate::rules::isort;
use crate::rules::isort::track::{Block, ImportTracker};
use crate::settings::{flags, Settings};
use crate::source_code::{Locator, Stylist};
use crate::source_code::{Indexer, Locator, Stylist};
#[allow(clippy::too_many_arguments)]
pub fn check_imports(
python_ast: &Suite,
locator: &Locator,
indexer: &Indexer,
directives: &IsortDirectives,
settings: &Settings,
stylist: &Stylist,
@ -39,7 +40,7 @@ pub fn check_imports(
for block in &blocks {
if !block.imports.is_empty() {
if let Some(diagnostic) = isort::rules::organize_imports(
block, locator, settings, stylist, autofix, package,
block, locator, indexer, settings, stylist, autofix, package,
) {
diagnostics.push(diagnostic);
}

View file

@ -37,14 +37,12 @@ pub struct IsortDirectives {
}
pub struct Directives {
pub commented_lines: Vec<usize>,
pub noqa_line_for: IntMap<usize, usize>,
pub isort: IsortDirectives,
}
pub fn extract_directives(lxr: &[LexResult], flags: Flags) -> Directives {
Directives {
commented_lines: extract_commented_lines(lxr),
noqa_line_for: if flags.contains(Flags::NOQA) {
extract_noqa_line_for(lxr)
} else {
@ -58,16 +56,6 @@ pub fn extract_directives(lxr: &[LexResult], flags: Flags) -> Directives {
}
}
pub fn extract_commented_lines(lxr: &[LexResult]) -> Vec<usize> {
let mut commented_lines = Vec::new();
for (start, tok, ..) in lxr.iter().flatten() {
if matches!(tok, Tok::Comment(_)) {
commented_lines.push(start.row());
}
}
commented_lines
}
/// Extract a mapping from logical line to noqa line.
pub fn extract_noqa_line_for(lxr: &[LexResult]) -> IntMap<usize, usize> {
let mut noqa_line_for: IntMap<usize, usize> = IntMap::default();

View file

@ -10,7 +10,7 @@ use crate::resolver::Relativity;
use crate::rustpython_helpers::tokenize;
use crate::settings::configuration::Configuration;
use crate::settings::{flags, pyproject, Settings};
use crate::source_code::{Locator, Stylist};
use crate::source_code::{Indexer, Locator, Stylist};
use crate::{directives, packaging, resolver};
/// Load the relevant `Settings` for a given `Path`.
@ -44,6 +44,9 @@ pub fn check(path: &Path, contents: &str, autofix: bool) -> Result<Vec<Diagnosti
// Detect the current code style (lazily).
let stylist = Stylist::from_contents(contents, &locator);
// Extra indices from the code.
let indexer: Indexer = tokens.as_slice().into();
// Extract the `# noqa` and `# isort: skip` directives from the source.
let directives =
directives::extract_directives(&tokens, directives::Flags::from_settings(&settings));
@ -56,6 +59,7 @@ pub fn check(path: &Path, contents: &str, autofix: bool) -> Result<Vec<Diagnosti
tokens,
&locator,
&stylist,
&indexer,
&directives,
&settings,
autofix.into(),

View file

@ -18,7 +18,7 @@ use crate::settings::configuration::Configuration;
use crate::settings::options::Options;
use crate::settings::types::PythonVersion;
use crate::settings::{flags, Settings};
use crate::source_code::{Locator, Stylist};
use crate::source_code::{Indexer, Locator, Stylist};
const VERSION: &str = env!("CARGO_PKG_VERSION");
@ -157,6 +157,9 @@ pub fn check(contents: &str, options: JsValue) -> Result<JsValue, JsValue> {
// Detect the current code style (lazily).
let stylist = Stylist::from_contents(contents, &locator);
// Extra indices from the code.
let indexer: Indexer = tokens.as_slice().into();
// Extract the `# noqa` and `# isort: skip` directives from the source.
let directives = directives::extract_directives(&tokens, directives::Flags::empty());
@ -168,6 +171,7 @@ pub fn check(contents: &str, options: JsValue) -> Result<JsValue, JsValue> {
tokens,
&locator,
&stylist,
&indexer,
&directives,
&settings,
flags::Autofix::Enabled,

View file

@ -17,7 +17,7 @@ use crate::message::{Message, Source};
use crate::noqa::add_noqa;
use crate::registry::{Diagnostic, LintSource, RuleCode};
use crate::settings::{flags, Settings};
use crate::source_code::{Locator, Stylist};
use crate::source_code::{Indexer, Locator, Stylist};
use crate::{directives, fs, rustpython_helpers, violations};
const CARGO_PKG_NAME: &str = env!("CARGO_PKG_NAME");
@ -33,6 +33,7 @@ pub fn check_path(
tokens: Vec<LexResult>,
locator: &Locator,
stylist: &Stylist,
indexer: &Indexer,
directives: &Directives,
settings: &Settings,
autofix: flags::Autofix,
@ -79,6 +80,7 @@ pub fn check_path(
&python_ast,
locator,
stylist,
indexer,
&directives.noqa_line_for,
settings,
autofix,
@ -90,6 +92,7 @@ pub fn check_path(
diagnostics.extend(check_imports(
&python_ast,
locator,
indexer,
&directives.isort,
settings,
stylist,
@ -127,7 +130,7 @@ pub fn check_path(
{
diagnostics.extend(check_lines(
contents,
&directives.commented_lines,
indexer.commented_lines(),
&doc_lines,
settings,
autofix,
@ -144,7 +147,7 @@ pub fn check_path(
check_noqa(
&mut diagnostics,
contents,
&directives.commented_lines,
indexer.commented_lines(),
&directives.noqa_line_for,
settings,
autofix,
@ -184,6 +187,9 @@ pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result<usize> {
// Detect the current code style (lazily).
let stylist = Stylist::from_contents(&contents, &locator);
// Extra indices from the code.
let indexer: Indexer = tokens.as_slice().into();
// Extract the `# noqa` and `# isort: skip` directives from the source.
let directives =
directives::extract_directives(&tokens, directives::Flags::from_settings(settings));
@ -196,6 +202,7 @@ pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result<usize> {
tokens,
&locator,
&stylist,
&indexer,
&directives,
settings,
flags::Autofix::Disabled,
@ -230,6 +237,9 @@ pub fn lint_only(
// Detect the current code style (lazily).
let stylist = Stylist::from_contents(contents, &locator);
// Extra indices from the code.
let indexer: Indexer = tokens.as_slice().into();
// Extract the `# noqa` and `# isort: skip` directives from the source.
let directives =
directives::extract_directives(&tokens, directives::Flags::from_settings(settings));
@ -242,6 +252,7 @@ pub fn lint_only(
tokens,
&locator,
&stylist,
&indexer,
&directives,
settings,
autofix,
@ -290,6 +301,9 @@ pub fn lint_fix(
// Detect the current code style (lazily).
let stylist = Stylist::from_contents(&contents, &locator);
// Extra indices from the code.
let indexer: Indexer = tokens.as_slice().into();
// Extract the `# noqa` and `# isort: skip` directives from the source.
let directives =
directives::extract_directives(&tokens, directives::Flags::from_settings(settings));
@ -302,6 +316,7 @@ pub fn lint_fix(
tokens,
&locator,
&stylist,
&indexer,
&directives,
settings,
flags::Autofix::Enabled,
@ -366,6 +381,7 @@ pub fn test_path(path: &Path, settings: &Settings) -> Result<Vec<Diagnostic>> {
let tokens: Vec<LexResult> = rustpython_helpers::tokenize(&contents);
let locator = Locator::new(&contents);
let stylist = Stylist::from_contents(&contents, &locator);
let indexer: Indexer = tokens.as_slice().into();
let directives =
directives::extract_directives(&tokens, directives::Flags::from_settings(settings));
let mut diagnostics = check_path(
@ -375,6 +391,7 @@ pub fn test_path(path: &Path, settings: &Settings) -> Result<Vec<Diagnostic>> {
tokens,
&locator,
&stylist,
&indexer,
&directives,
settings,
flags::Autofix::Enabled,
@ -395,6 +412,7 @@ pub fn test_path(path: &Path, settings: &Settings) -> Result<Vec<Diagnostic>> {
let tokens: Vec<LexResult> = rustpython_helpers::tokenize(&contents);
let locator = Locator::new(&contents);
let stylist = Stylist::from_contents(&contents, &locator);
let indexer: Indexer = tokens.as_slice().into();
let directives =
directives::extract_directives(&tokens, directives::Flags::from_settings(settings));
let diagnostics = check_path(
@ -404,6 +422,7 @@ pub fn test_path(path: &Path, settings: &Settings) -> Result<Vec<Diagnostic>> {
tokens,
&locator,
&stylist,
&indexer,
&directives,
settings,
flags::Autofix::Enabled,

View file

@ -48,7 +48,7 @@ pub fn assert_false(checker: &mut Checker, stmt: &Stmt, test: &Expr, msg: Option
let mut diagnostic = Diagnostic::new(violations::DoNotAssertFalse, Range::from_located(test));
if checker.patch(diagnostic.kind.code()) {
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_stmt(&assertion_error(msg));
diagnostic.amend(Fix::replacement(
generator.generate(),

View file

@ -55,7 +55,7 @@ fn duplicate_handler_exceptions<'a>(
Range::from_located(expr),
);
if checker.patch(diagnostic.kind.code()) {
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
if unique_elts.len() == 1 {
generator.unparse_expr(unique_elts[0], 0);
} else {

View file

@ -48,7 +48,7 @@ pub fn getattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar
let mut diagnostic =
Diagnostic::new(violations::GetAttrWithConstant, Range::from_located(expr));
if checker.patch(diagnostic.kind.code()) {
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_expr(&attribute(obj, value), 0);
diagnostic.amend(Fix::replacement(
generator.generate(),

View file

@ -24,7 +24,7 @@ pub fn redundant_tuple_in_exception_handler(checker: &mut Checker, handlers: &[E
Range::from_located(type_),
);
if checker.patch(diagnostic.kind.code()) {
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_expr(elt, 0);
diagnostic.amend(Fix::replacement(
generator.generate(),

View file

@ -64,7 +64,7 @@ pub fn setattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar
Diagnostic::new(violations::SetAttrWithConstant, Range::from_located(expr));
if checker.patch(diagnostic.kind.code()) {
diagnostic.amend(Fix::replacement(
assignment(obj, name, value, checker.style),
assignment(obj, name, value, checker.stylist),
expr.location,
expr.end_location.unwrap(),
));

View file

@ -32,7 +32,7 @@ pub fn no_unnecessary_pass(checker: &mut Checker, body: &[Stmt]) {
Range::from_located(pass_stmt),
);
if checker.patch(&RuleCode::PIE790) {
match delete_stmt(pass_stmt, None, &[], checker.locator) {
match delete_stmt(pass_stmt, None, &[], checker.locator, checker.indexer) {
Ok(fix) => {
diagnostic.amend(fix);
}
@ -91,7 +91,7 @@ pub fn dupe_class_field_definitions<'a, 'b>(
.map(std::convert::Into::into)
.collect();
let locator = checker.locator;
match delete_stmt(stmt, Some(parent), &deleted, locator) {
match delete_stmt(stmt, Some(parent), &deleted, locator, checker.indexer) {
Ok(fix) => {
checker.deletions.insert(RefEquality(stmt));
diagnostic.amend(fix);

View file

@ -62,6 +62,7 @@ pub fn print_call(checker: &mut Checker, func: &Expr, keywords: &[Keyword]) {
defined_in.map(std::convert::Into::into),
&deleted,
checker.locator,
checker.indexer,
) {
Ok(fix) => {
if fix.content.is_empty() || fix.content == "pass" {

View file

@ -106,7 +106,7 @@ pub fn unittest_assertion(
if checker.patch(diagnostic.kind.code()) {
if let Ok(stmt) = unittest_assert.generate_assert(args, keywords) {
diagnostic.amend(Fix::replacement(
unparse_stmt(&stmt, checker.style),
unparse_stmt(&stmt, checker.stylist),
call.location,
call.end_location.unwrap(),
));

View file

@ -31,7 +31,7 @@ fn elts_to_csv(elts: &[Expr], checker: &Checker) -> Option<String> {
return None;
}
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_expr(
&create_expr(ExprKind::Constant {
value: Constant::Str(elts.iter().fold(String::new(), |mut acc, elt| {
@ -85,7 +85,7 @@ fn check_names(checker: &mut Checker, expr: &Expr) {
Range::from_located(expr),
);
if checker.patch(diagnostic.kind.code()) {
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_expr(
&create_expr(ExprKind::Tuple {
elts: names
@ -115,7 +115,7 @@ fn check_names(checker: &mut Checker, expr: &Expr) {
Range::from_located(expr),
);
if checker.patch(diagnostic.kind.code()) {
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_expr(
&create_expr(ExprKind::List {
elts: names
@ -157,7 +157,7 @@ fn check_names(checker: &mut Checker, expr: &Expr) {
Range::from_located(expr),
);
if checker.patch(diagnostic.kind.code()) {
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_expr(
&create_expr(ExprKind::List {
elts: elts.clone(),
@ -206,7 +206,7 @@ fn check_names(checker: &mut Checker, expr: &Expr) {
Range::from_located(expr),
);
if checker.patch(diagnostic.kind.code()) {
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_expr(
&create_expr(ExprKind::Tuple {
elts: elts.clone(),
@ -284,7 +284,7 @@ fn handle_single_name(checker: &mut Checker, expr: &Expr, value: &Expr) {
);
if checker.patch(diagnostic.kind.code()) {
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_expr(&create_expr(value.node.clone()), 0);
diagnostic.amend(Fix::replacement(
generator.generate(),

View file

@ -126,7 +126,7 @@ pub fn duplicate_isinstance_call(checker: &mut Checker, expr: &Expr) {
// Populate the `Fix`. Replace the _entire_ `BoolOp`. Note that if we have
// multiple duplicates, the fixes will conflict.
diagnostic.amend(Fix::replacement(
unparse_expr(&bool_op, checker.style),
unparse_expr(&bool_op, checker.stylist),
expr.location,
expr.end_location.unwrap(),
));
@ -169,13 +169,13 @@ pub fn compare_with_tuple(checker: &mut Checker, expr: &Expr) {
}
let str_values = values
.iter()
.map(|value| unparse_expr(value, checker.style))
.map(|value| unparse_expr(value, checker.stylist))
.collect();
let mut diagnostic = Diagnostic::new(
violations::CompareWithTuple(
value.to_string(),
str_values,
unparse_expr(expr, checker.style),
unparse_expr(expr, checker.stylist),
),
Range::from_located(expr),
);
@ -193,7 +193,7 @@ pub fn compare_with_tuple(checker: &mut Checker, expr: &Expr) {
})],
});
diagnostic.amend(Fix::replacement(
unparse_expr(&in_expr, checker.style),
unparse_expr(&in_expr, checker.stylist),
expr.location,
expr.end_location.unwrap(),
));

View file

@ -46,7 +46,7 @@ pub fn use_capital_environment_variables(checker: &mut Checker, expr: &Expr) {
kind: kind.clone(),
});
diagnostic.amend(Fix::replacement(
unparse_expr(&new_env_var, checker.style),
unparse_expr(&new_env_var, checker.stylist),
arg.location,
arg.end_location.unwrap(),
));
@ -85,7 +85,7 @@ fn check_os_environ_subscript(checker: &mut Checker, expr: &Expr) {
kind: kind.clone(),
});
diagnostic.amend(Fix::replacement(
unparse_expr(&new_env_var, checker.style),
unparse_expr(&new_env_var, checker.stylist),
slice.location,
slice.end_location.unwrap(),
));

View file

@ -184,7 +184,7 @@ pub fn convert_for_loop_to_any_all(checker: &mut Checker, stmt: &Stmt, sibling:
loop_info.test,
loop_info.target,
loop_info.iter,
checker.style,
checker.stylist,
);
// Don't flag if the resulting expression would exceed the maximum line length.
@ -232,7 +232,7 @@ pub fn convert_for_loop_to_any_all(checker: &mut Checker, stmt: &Stmt, sibling:
&test,
loop_info.target,
loop_info.iter,
checker.style,
checker.stylist,
);
// Don't flag if the resulting expression would exceed the maximum line length.

View file

@ -91,7 +91,7 @@ pub fn return_bool_condition_directly(checker: &mut Checker, stmt: &Stmt) {
if !(is_one_line_return_bool(body) && is_one_line_return_bool(orelse)) {
return;
}
let condition = unparse_expr(test, checker.style);
let condition = unparse_expr(test, checker.stylist);
let mut diagnostic = Diagnostic::new(
violations::ReturnBoolConditionDirectly(condition),
Range::from_located(stmt),
@ -101,7 +101,7 @@ pub fn return_bool_condition_directly(checker: &mut Checker, stmt: &Stmt) {
value: Some(test.clone()),
});
diagnostic.amend(Fix::replacement(
unparse_stmt(&return_stmt, checker.style),
unparse_stmt(&return_stmt, checker.stylist),
stmt.location,
stmt.end_location.unwrap(),
));
@ -191,7 +191,7 @@ pub fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt, parent: Option<&
let target_var = &body_targets[0];
let ternary = ternary(target_var, body_value, test, orelse_value);
let contents = unparse_stmt(&ternary, checker.style);
let contents = unparse_stmt(&ternary, checker.stylist);
// Don't flag if the resulting expression would exceed the maximum line length.
if stmt.location.column() + contents.len() > checker.settings.line_length {
@ -305,7 +305,7 @@ pub fn use_dict_get_with_default(
})),
type_comment: None,
}),
checker.style,
checker.stylist,
);
// Don't flag if the resulting expression would exceed the maximum line length.

View file

@ -29,7 +29,7 @@ pub fn explicit_true_false_in_ifexpr(
}
let mut diagnostic = Diagnostic::new(
violations::IfExprWithTrueFalse(unparse_expr(test, checker.style)),
violations::IfExprWithTrueFalse(unparse_expr(test, checker.stylist)),
Range::from_located(expr),
);
if checker.patch(diagnostic.kind.code()) {
@ -43,7 +43,7 @@ pub fn explicit_true_false_in_ifexpr(
args: vec![create_expr(test.node.clone())],
keywords: vec![],
}),
checker.style,
checker.stylist,
),
expr.location,
expr.end_location.unwrap(),
@ -74,7 +74,7 @@ pub fn explicit_false_true_in_ifexpr(
}
let mut diagnostic = Diagnostic::new(
violations::IfExprWithFalseTrue(unparse_expr(test, checker.style)),
violations::IfExprWithFalseTrue(unparse_expr(test, checker.stylist)),
Range::from_located(expr),
);
if checker.patch(diagnostic.kind.code()) {
@ -84,7 +84,7 @@ pub fn explicit_false_true_in_ifexpr(
op: Unaryop::Not,
operand: Box::new(create_expr(test.node.clone())),
}),
checker.style,
checker.stylist,
),
expr.location,
expr.end_location.unwrap(),
@ -121,8 +121,8 @@ pub fn twisted_arms_in_ifexpr(
let mut diagnostic = Diagnostic::new(
violations::IfExprWithTwistedArms(
unparse_expr(body, checker.style),
unparse_expr(orelse, checker.style),
unparse_expr(body, checker.stylist),
unparse_expr(orelse, checker.stylist),
),
Range::from_located(expr),
);
@ -134,7 +134,7 @@ pub fn twisted_arms_in_ifexpr(
body: Box::new(create_expr(orelse.node.clone())),
orelse: Box::new(create_expr(body.node.clone())),
}),
checker.style,
checker.stylist,
),
expr.location,
expr.end_location.unwrap(),

View file

@ -37,8 +37,8 @@ pub fn negation_with_equal_op(checker: &mut Checker, expr: &Expr, op: &Unaryop,
let mut diagnostic = Diagnostic::new(
violations::NegateEqualOp(
unparse_expr(left, checker.style),
unparse_expr(&comparators[0], checker.style),
unparse_expr(left, checker.stylist),
unparse_expr(&comparators[0], checker.stylist),
),
Range::from_located(expr),
);
@ -50,7 +50,7 @@ pub fn negation_with_equal_op(checker: &mut Checker, expr: &Expr, op: &Unaryop,
ops: vec![Cmpop::NotEq],
comparators: comparators.clone(),
}),
checker.style,
checker.stylist,
),
expr.location,
expr.end_location.unwrap(),
@ -81,8 +81,8 @@ pub fn negation_with_not_equal_op(
let mut diagnostic = Diagnostic::new(
violations::NegateNotEqualOp(
unparse_expr(left, checker.style),
unparse_expr(&comparators[0], checker.style),
unparse_expr(left, checker.stylist),
unparse_expr(&comparators[0], checker.stylist),
),
Range::from_located(expr),
);
@ -94,7 +94,7 @@ pub fn negation_with_not_equal_op(
ops: vec![Cmpop::Eq],
comparators: comparators.clone(),
}),
checker.style,
checker.stylist,
),
expr.location,
expr.end_location.unwrap(),
@ -121,7 +121,7 @@ pub fn double_negation(checker: &mut Checker, expr: &Expr, op: &Unaryop, operand
);
if checker.patch(diagnostic.kind.code()) {
diagnostic.amend(Fix::replacement(
unparse_expr(operand, checker.style),
unparse_expr(operand, checker.stylist),
expr.location,
expr.end_location.unwrap(),
));

View file

@ -13,7 +13,7 @@ use crate::ast::whitespace::leading_space;
use crate::fix::Fix;
use crate::registry::Diagnostic;
use crate::settings::{flags, Settings};
use crate::source_code::{Locator, Stylist};
use crate::source_code::{Indexer, Locator, Stylist};
use crate::violations;
fn extract_range(body: &[&Stmt]) -> Range {
@ -31,6 +31,7 @@ fn extract_indentation_range(body: &[&Stmt]) -> Range {
pub fn organize_imports(
block: &Block,
locator: &Locator,
indexer: &Indexer,
settings: &Settings,
stylist: &Stylist,
autofix: flags::Autofix,
@ -43,7 +44,7 @@ pub fn organize_imports(
// Special-cases: there's leading or trailing content in the import block. These
// are too hard to get right, and relatively rare, so flag but don't fix.
if preceded_by_multi_statement_line(block.imports.first().unwrap(), locator)
if preceded_by_multi_statement_line(block.imports.first().unwrap(), locator, indexer)
|| followed_by_multi_statement_line(block.imports.last().unwrap(), locator)
{
return Some(Diagnostic::new(violations::UnsortedImports, range));

View file

@ -13,7 +13,7 @@ mod tests {
use crate::linter::check_path;
use crate::registry::{RuleCode, RuleCodePrefix};
use crate::settings::flags;
use crate::source_code::{Locator, Stylist};
use crate::source_code::{Indexer, Locator, Stylist};
use crate::{directives, rustpython_helpers, settings};
fn rule_code(contents: &str, expected: &[RuleCode]) -> Result<()> {
@ -22,6 +22,7 @@ mod tests {
let tokens: Vec<LexResult> = rustpython_helpers::tokenize(&contents);
let locator = Locator::new(&contents);
let stylist = Stylist::from_contents(&contents, &locator);
let indexer: Indexer = tokens.as_slice().into();
let directives =
directives::extract_directives(&tokens, directives::Flags::from_settings(&settings));
let diagnostics = check_path(
@ -31,6 +32,7 @@ mod tests {
tokens,
&locator,
&stylist,
&indexer,
&directives,
&settings,
flags::Autofix::Enabled,

View file

@ -284,7 +284,7 @@ pub fn literal_comparisons(
.map(|(idx, op)| bad_ops.get(&idx).unwrap_or(op))
.cloned()
.collect::<Vec<_>>();
let content = compare(left, &ops, comparators, checker.style);
let content = compare(left, &ops, comparators, checker.stylist);
for diagnostic in &mut diagnostics {
diagnostic.amend(Fix::replacement(
content.to_string(),
@ -325,7 +325,7 @@ pub fn not_tests(
);
if checker.patch(diagnostic.kind.code()) && should_fix {
diagnostic.amend(Fix::replacement(
compare(left, &[Cmpop::NotIn], comparators, checker.style),
compare(left, &[Cmpop::NotIn], comparators, checker.stylist),
expr.location,
expr.end_location.unwrap(),
));
@ -341,7 +341,7 @@ pub fn not_tests(
);
if checker.patch(diagnostic.kind.code()) && should_fix {
diagnostic.amend(Fix::replacement(
compare(left, &[Cmpop::IsNot], comparators, checker.style),
compare(left, &[Cmpop::IsNot], comparators, checker.stylist),
expr.location,
expr.end_location.unwrap(),
));
@ -465,7 +465,10 @@ pub fn do_not_assign_lambda(checker: &mut Checker, target: &Expr, value: &Expr,
));
let indentation = &leading_space(&first_line);
let mut indented = String::new();
for (idx, line) in function(id, args, body, checker.style).lines().enumerate() {
for (idx, line) in function(id, args, body, checker.stylist)
.lines()
.enumerate()
{
if idx == 0 {
indented.push_str(line);
} else {

View file

@ -17,7 +17,7 @@ mod tests {
use crate::linter::{check_path, test_path};
use crate::registry::{RuleCode, RuleCodePrefix};
use crate::settings::flags;
use crate::source_code::{Locator, Stylist};
use crate::source_code::{Indexer, Locator, Stylist};
use crate::{directives, rustpython_helpers, settings};
#[test_case(RuleCode::F401, Path::new("F401_0.py"); "F401_0")]
@ -213,6 +213,7 @@ mod tests {
let tokens: Vec<LexResult> = rustpython_helpers::tokenize(&contents);
let locator = Locator::new(&contents);
let stylist = Stylist::from_contents(&contents, &locator);
let indexer: Indexer = tokens.as_slice().into();
let directives =
directives::extract_directives(&tokens, directives::Flags::from_settings(&settings));
let mut diagnostics = check_path(
@ -222,6 +223,7 @@ mod tests {
tokens,
&locator,
&stylist,
&indexer,
&directives,
&settings,
flags::Autofix::Enabled,

View file

@ -42,7 +42,7 @@ pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) {
let is_duplicate_value = seen_values.contains(&comparable_value);
let mut diagnostic = Diagnostic::new(
violations::MultiValueRepeatedKeyLiteral(
unparse_expr(&keys[i], checker.style),
unparse_expr(&keys[i], checker.stylist),
is_duplicate_value,
),
Range::from_located(&keys[i]),

View file

@ -70,7 +70,8 @@ fn remove_unused_variable(
.map(std::convert::Into::into)
.collect();
let locator = checker.locator;
match delete_stmt(stmt, parent, &deleted, locator) {
let indexer = checker.indexer;
match delete_stmt(stmt, parent, &deleted, locator, indexer) {
Ok(fix) => Some((DeletionKind::Whole, fix)),
Err(err) => {
error!("Failed to delete unused variable: {}", err);
@ -108,7 +109,8 @@ fn remove_unused_variable(
.map(std::convert::Into::into)
.collect();
let locator = checker.locator;
match delete_stmt(stmt, parent, &deleted, locator) {
let indexer = checker.indexer;
match delete_stmt(stmt, parent, &deleted, locator, indexer) {
Ok(fix) => Some((DeletionKind::Whole, fix)),
Err(err) => {
error!("Failed to delete unused variable: {}", err);

View file

@ -168,7 +168,7 @@ pub fn convert_named_tuple_functional_to_class(
typename,
properties,
base_class,
checker.style,
checker.stylist,
));
}
Err(err) => debug!("Skipping ineligible `NamedTuple` \"{typename}\": {err}"),

View file

@ -210,7 +210,7 @@ pub fn convert_typed_dict_functional_to_class(
body,
total_keyword,
base_class,
checker.style,
checker.stylist,
));
}
Err(err) => debug!("Skipping ineligible `TypedDict` \"{class_name}\": {err}"),

View file

@ -35,13 +35,13 @@ pub fn native_literals(
if id == "bytes" {
let mut content = String::with_capacity(3);
content.push('b');
content.push(checker.style.quote().into());
content.push(checker.style.quote().into());
content.push(checker.stylist.quote().into());
content.push(checker.stylist.quote().into());
content
} else {
let mut content = String::with_capacity(2);
content.push(checker.style.quote().into());
content.push(checker.style.quote().into());
content.push(checker.stylist.quote().into());
content.push(checker.stylist.quote().into());
content
},
expr.location,

View file

@ -398,7 +398,7 @@ fn handle_next_on_six_dict(expr: &Expr, patch: bool, checker: &Checker) -> Optio
},
arg,
patch,
checker.style,
checker.stylist,
))
}
@ -427,7 +427,7 @@ pub fn remove_six_compat(checker: &mut Checker, expr: &Expr) {
keywords,
expr,
patch,
checker.style,
checker.stylist,
checker.locator,
),
ExprKind::Attribute { attr, .. } => map_name(attr.as_str(), expr, patch),

View file

@ -228,7 +228,7 @@ pub fn rewrite_mock_import(checker: &mut Checker, stmt: &Stmt) {
// Generate the fix, if needed, which is shared between all `mock` imports.
let content = if checker.patch(&RuleCode::UP026) {
let indent = indentation(checker, stmt);
match format_import(stmt, &indent, checker.locator, checker.style) {
match format_import(stmt, &indent, checker.locator, checker.stylist) {
Ok(content) => Some(content),
Err(e) => {
error!("Failed to rewrite `mock` import: {e}");
@ -274,7 +274,7 @@ pub fn rewrite_mock_import(checker: &mut Checker, stmt: &Stmt) {
);
if checker.patch(&RuleCode::UP026) {
let indent = indentation(checker, stmt);
match format_import_from(stmt, &indent, checker.locator, checker.style) {
match format_import_from(stmt, &indent, checker.locator, checker.stylist) {
Ok(content) => {
diagnostic.amend(Fix::replacement(
content,

View file

@ -97,6 +97,7 @@ pub fn unnecessary_builtin_import(
defined_in.map(std::convert::Into::into),
&deleted,
checker.locator,
checker.indexer,
) {
Ok(fix) => {
if fix.content.is_empty() || fix.content == "pass" {

View file

@ -82,6 +82,7 @@ pub fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, names: &[Lo
defined_in.map(std::convert::Into::into),
&deleted,
checker.locator,
checker.indexer,
) {
Ok(fix) => {
if fix.content.is_empty() || fix.content == "pass" {

View file

@ -68,7 +68,7 @@ pub fn use_pep604_annotation(checker: &mut Checker, expr: &Expr, value: &Expr, s
let mut diagnostic =
Diagnostic::new(violations::UsePEP604Annotation, Range::from_located(expr));
if checker.patch(diagnostic.kind.code()) {
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_expr(&optional(slice), 0);
diagnostic.amend(Fix::replacement(
generator.generate(),
@ -88,7 +88,7 @@ pub fn use_pep604_annotation(checker: &mut Checker, expr: &Expr, value: &Expr, s
// Invalid type annotation.
}
ExprKind::Tuple { elts, .. } => {
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_expr(&union(elts), 0);
diagnostic.amend(Fix::replacement(
generator.generate(),
@ -98,7 +98,7 @@ pub fn use_pep604_annotation(checker: &mut Checker, expr: &Expr, value: &Expr, s
}
_ => {
// Single argument.
let mut generator: Generator = checker.style.into();
let mut generator: Generator = checker.stylist.into();
generator.unparse_expr(slice, 0);
diagnostic.amend(Fix::replacement(
generator.generate(),

View file

@ -45,6 +45,7 @@ pub fn useless_metaclass_type(checker: &mut Checker, stmt: &Stmt, value: &Expr,
defined_in.map(std::convert::Into::into),
&deleted,
checker.locator,
checker.indexer,
) {
Ok(fix) => {
if fix.content.is_empty() || fix.content == "pass" {

116
src/source_code/indexer.rs Normal file
View file

@ -0,0 +1,116 @@
//! Struct used to index source code, to enable efficient lookup of tokens that
//! are omitted from the AST (e.g., commented lines).
use rustpython_ast::Location;
use rustpython_parser::lexer::{LexResult, Tok};
pub struct Indexer {
commented_lines: Vec<usize>,
continuation_lines: Vec<usize>,
}
impl Indexer {
pub fn commented_lines(&self) -> &[usize] {
&self.commented_lines
}
pub fn continuation_lines(&self) -> &[usize] {
&self.continuation_lines
}
}
impl From<&[LexResult]> for Indexer {
fn from(lxr: &[LexResult]) -> Self {
let mut commented_lines = Vec::new();
let mut continuation_lines = Vec::new();
let mut prev: Option<(&Location, &Tok, &Location)> = None;
for (start, tok, end) in lxr.iter().flatten() {
if matches!(tok, Tok::Comment(_)) {
commented_lines.push(start.row());
}
if let Some((.., prev_tok, prev_end)) = prev {
if !matches!(
prev_tok,
Tok::Newline | Tok::NonLogicalNewline | Tok::Comment(..)
) {
for line in prev_end.row()..start.row() {
continuation_lines.push(line);
}
}
}
prev = Some((start, tok, end));
}
Self {
commented_lines,
continuation_lines,
}
}
}
#[cfg(test)]
mod tests {
use rustpython_parser::lexer;
use rustpython_parser::lexer::LexResult;
use crate::source_code::Indexer;
#[test]
fn continuation() {
let contents = r#"x = 1"#;
let lxr: Vec<LexResult> = lexer::make_tokenizer(contents).collect();
let indexer: Indexer = lxr.as_slice().into();
assert_eq!(indexer.continuation_lines(), Vec::<usize>::new().as_slice());
let contents = r#"
# Hello, world!
x = 1
y = 2
"#
.trim();
let lxr: Vec<LexResult> = lexer::make_tokenizer(contents).collect();
let indexer: Indexer = lxr.as_slice().into();
assert_eq!(indexer.continuation_lines(), Vec::<usize>::new().as_slice());
let contents = r#"
x = \
1
if True:
z = \
\
2
(
"abc" # Foo
"def" \
"ghi"
)
"#
.trim();
let lxr: Vec<LexResult> = lexer::make_tokenizer(contents).collect();
let indexer: Indexer = lxr.as_slice().into();
assert_eq!(indexer.continuation_lines(), [1, 5, 6, 11]);
let contents = r#"
x = 1; import sys
import os
if True:
x = 1; import sys
import os
if True:
x = 1; \
import os
x = 1; \
import os
"#
.trim();
let lxr: Vec<LexResult> = lexer::make_tokenizer(contents).collect();
let indexer: Indexer = lxr.as_slice().into();
assert_eq!(indexer.continuation_lines(), [9, 12]);
}
}

View file

@ -1,8 +1,10 @@
mod generator;
mod indexer;
mod locator;
mod stylist;
pub(crate) use generator::Generator;
pub(crate) use indexer::Indexer;
pub(crate) use locator::Locator;
use rustpython_parser::error::ParseError;
use rustpython_parser::parser;