language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-10-01 22:31:47 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Adjust line numbers when reporting rules in f-strings (#244)

Browse source
This commit is contained in:
Charlie Marsh 2022-09-21 13:42:58 -04:00 • committed by GitHub
parent 1e171ce0e8
commit 65d29d9734
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 151 additions and 87 deletions
Download patch file Download diff file Expand all files Collapse all files

7
resources/test/fixtures/F821.py vendored
View file

@ -82,3 +82,10 @@ class Ticket:
def update_tomato():
print(TOMATO)
TOMATO = "cherry tomato"
A = f'{B}'
A = (
f'B'
f'{B}'
)

53
src/ast/checks.rs
View file

@ -7,7 +7,7 @@ use rustpython_parser::ast::{
};
use crate::ast::operations::SourceCodeLocator;
use crate::ast::types::{Binding, BindingKind, FunctionScope, Scope, ScopeKind};
use crate::ast::types::{Binding, BindingKind, CheckLocator, FunctionScope, Scope, ScopeKind};
use crate::autofix::{fixer, fixes};
use crate::checks::{Check, CheckKind, Fix, RejectedCmpop};
@ -37,6 +37,7 @@ pub fn check_not_tests(
operand: &Expr,
check_not_in: bool,
check_not_is: bool,
locator: &dyn CheckLocator,
) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
@ -46,12 +47,18 @@ pub fn check_not_tests(
match op {
Cmpop::In => {
if check_not_in {
checks.push(Check::new(CheckKind::NotInTest, operand.location));
checks.push(Check::new(
CheckKind::NotInTest,
locator.locate_check(operand.location),
));
}
}
Cmpop::Is => {
if check_not_is {
checks.push(Check::new(CheckKind::NotIsTest, operand.location));
checks.push(Check::new(
CheckKind::NotIsTest,
locator.locate_check(operand.location),
));
}
}
_ => {}
@ -64,7 +71,7 @@ pub fn check_not_tests(
}
/// Check UnusedVariable compliance.
pub fn check_unused_variables(scope: &Scope) -> Vec<Check> {
pub fn check_unused_variables(scope: &Scope, locator: &dyn CheckLocator) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
if matches!(
@ -85,7 +92,7 @@ pub fn check_unused_variables(scope: &Scope) -> Vec<Check> {
{
checks.push(Check::new(
CheckKind::UnusedVariable(name.to_string()),
binding.location,
locator.locate_check(binding.location),
));
}
}
@ -299,6 +306,7 @@ pub fn check_repeated_keys(
keys: &Vec<Expr>,
check_repeated_literals: bool,
check_repeated_variables: bool,
locator: &dyn CheckLocator,
) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
@ -313,7 +321,7 @@ pub fn check_repeated_keys(
if check_repeated_literals && v1 == v2 {
checks.push(Check::new(
CheckKind::MultiValueRepeatedKeyLiteral,
k2.location,
locator.locate_check(k2.location),
))
}
}
@ -321,7 +329,7 @@ pub fn check_repeated_keys(
if check_repeated_variables && v1 == v2 {
checks.push(Check::new(
CheckKind::MultiValueRepeatedKeyVariable((*v2).to_string()),
k2.location,
locator.locate_check(k2.location),
))
}
}
@ -340,6 +348,7 @@ pub fn check_literal_comparisons(
comparators: &Vec<Expr>,
check_none_comparisons: bool,
check_true_false_comparisons: bool,
locator: &dyn CheckLocator,
) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
@ -359,13 +368,13 @@ pub fn check_literal_comparisons(
if matches!(op, Cmpop::Eq) {
checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::Eq),
comparator.location,
locator.locate_check(comparator.location),
));
}
if matches!(op, Cmpop::NotEq) {
checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::NotEq),
comparator.location,
locator.locate_check(comparator.location),
));
}
}
@ -379,13 +388,13 @@ pub fn check_literal_comparisons(
if matches!(op, Cmpop::Eq) {
checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq),
comparator.location,
locator.locate_check(comparator.location),
));
}
if matches!(op, Cmpop::NotEq) {
checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq),
comparator.location,
locator.locate_check(comparator.location),
));
}
}
@ -405,13 +414,13 @@ pub fn check_literal_comparisons(
if matches!(op, Cmpop::Eq) {
checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::Eq),
comparator.location,
locator.locate_check(comparator.location),
));
}
if matches!(op, Cmpop::NotEq) {
checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::NotEq),
comparator.location,
locator.locate_check(comparator.location),
));
}
}
@ -425,13 +434,13 @@ pub fn check_literal_comparisons(
if matches!(op, Cmpop::Eq) {
checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq),
comparator.location,
locator.locate_check(comparator.location),
));
}
if matches!(op, Cmpop::NotEq) {
checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq),
comparator.location,
locator.locate_check(comparator.location),
));
}
}
@ -528,9 +537,9 @@ pub fn check_type_comparison(
/// Check TwoStarredExpressions and TooManyExpressionsInStarredAssignment compliance.
pub fn check_starred_expressions(
elts: &[Expr],
location: Location,
check_too_many_expressions: bool,
check_two_starred_expressions: bool,
location: Location,
) -> Option<Check> {
let mut has_starred: bool = false;
let mut starred_index: Option<usize> = None;
@ -563,6 +572,7 @@ pub fn check_break_outside_loop(
stmt: &Stmt,
parents: &[&Stmt],
parent_stack: &[usize],
locator: &dyn CheckLocator,
) -> Option<Check> {
let mut allowed: bool = false;
let mut parent = stmt;
@ -589,7 +599,10 @@ pub fn check_break_outside_loop(
}
if !allowed {
Some(Check::new(CheckKind::BreakOutsideLoop, stmt.location))
Some(Check::new(
CheckKind::BreakOutsideLoop,
locator.locate_check(stmt.location),
))
} else {
None
}
@ -600,6 +613,7 @@ pub fn check_continue_outside_loop(
stmt: &Stmt,
parents: &[&Stmt],
parent_stack: &[usize],
locator: &dyn CheckLocator,
) -> Option<Check> {
let mut allowed: bool = false;
let mut parent = stmt;
@ -626,7 +640,10 @@ pub fn check_continue_outside_loop(
}
if !allowed {
Some(Check::new(CheckKind::ContinueOutsideLoop, stmt.location))
Some(Check::new(
CheckKind::ContinueOutsideLoop,
locator.locate_check(stmt.location),
))
} else {
None
}

4
src/ast/types.rs
View file

@ -65,3 +65,7 @@ pub struct Binding {
/// last used.
pub used: Option<(usize, Location)>,
}
pub trait CheckLocator {
fn locate_check(&self, default: Location) -> Location;
}

162
src/check_ast.rs
View file

@ -9,7 +9,7 @@ use rustpython_parser::parser;
use crate::ast::operations::{extract_all_names, SourceCodeLocator};
use crate::ast::relocate::relocate_expr;
use crate::ast::types::{Binding, BindingKind, FunctionScope, Scope, ScopeKind};
use crate::ast::types::{Binding, BindingKind, CheckLocator, FunctionScope, Scope, ScopeKind};
use crate::ast::visitor::{walk_excepthandler, Visitor};
use crate::ast::{checks, operations, visitor};
use crate::autofix::fixer;
@ -42,7 +42,7 @@ struct Checker<'a> {
deferred_lambdas: Vec<(&'a Expr, Vec<usize>, Vec<usize>)>,
deferred_assignments: Vec<usize>,
// Derivative state.
in_f_string: bool,
in_f_string: Option<Location>,
in_annotation: bool,
in_literal: bool,
seen_non_import: bool,
@ -74,7 +74,7 @@ impl<'a> Checker<'a> {
deferred_functions: vec![],
deferred_lambdas: vec![],
deferred_assignments: vec![],
in_f_string: false,
in_f_string: None,
in_annotation: false,
in_literal: false,
seen_non_import: false,
@ -171,7 +171,6 @@ where
// Pre-visit.
match &stmt.node {
StmtKind::Global { names } | StmtKind::Nonlocal { names } => {
// TODO(charlie): Handle doctests.
let global_scope_id = self.scopes[GLOBAL_SCOPE_INDEX].id;
let current_scope =
@ -193,25 +192,34 @@ where
}
if self.settings.select.contains(&CheckCode::E741) {
self.checks.extend(names.iter().filter_map(|name| {
checks::check_ambiguous_variable_name(name, stmt.location)
}));
let location = self.locate_check(stmt.location);
self.checks.extend(
names.iter().filter_map(|name| {
checks::check_ambiguous_variable_name(name, location)
}),
);
}
}
StmtKind::Break => {
if self.settings.select.contains(&CheckCode::F701) {
if let Some(check) =
checks::check_break_outside_loop(stmt, &self.parents, &self.parent_stack)
{
if let Some(check) = checks::check_break_outside_loop(
stmt,
&self.parents,
&self.parent_stack,
self,
) {
self.checks.push(check);
}
}
}
StmtKind::Continue => {
if self.settings.select.contains(&CheckCode::F702) {
if let Some(check) =
checks::check_continue_outside_loop(stmt, &self.parents, &self.parent_stack)
{
if let Some(check) = checks::check_continue_outside_loop(
stmt,
&self.parents,
&self.parent_stack,
self,
) {
self.checks.push(check);
}
}
@ -231,8 +239,10 @@ where
..
} => {
if self.settings.select.contains(&CheckCode::E743) {
if let Some(check) = checks::check_ambiguous_function_name(name, stmt.location)
{
if let Some(check) = checks::check_ambiguous_function_name(
name,
self.locate_check(stmt.location),
) {
self.checks.push(check);
}
}
@ -293,7 +303,7 @@ where
ScopeKind::Class | ScopeKind::Module => {
self.checks.push(Check::new(
CheckKind::ReturnOutsideFunction,
stmt.location,
self.locate_check(stmt.location),
));
}
_ => {}
@ -325,7 +335,9 @@ where
}
if self.settings.select.contains(&CheckCode::E742) {
if let Some(check) = checks::check_ambiguous_class_name(name, stmt.location) {
if let Some(check) =
checks::check_ambiguous_class_name(name, self.locate_check(stmt.location))
{
self.checks.push(check);
}
}
@ -351,7 +363,7 @@ where
{
self.checks.push(Check::new(
CheckKind::ModuleImportNotAtTopOfFile,
stmt.location,
self.locate_check(stmt.location),
));
}
@ -405,7 +417,7 @@ where
{
self.checks.push(Check::new(
CheckKind::ModuleImportNotAtTopOfFile,
stmt.location,
self.locate_check(stmt.location),
));
}
@ -441,14 +453,16 @@ where
{
self.checks.push(Check::new(
CheckKind::FutureFeatureNotDefined(alias.node.name.to_string()),
stmt.location,
self.locate_check(stmt.location),
));
}
if self.settings.select.contains(&CheckCode::F404) && !self.futures_allowed
{
self.checks
.push(Check::new(CheckKind::LateFutureImport, stmt.location));
self.checks.push(Check::new(
CheckKind::LateFutureImport,
self.locate_check(stmt.location),
));
}
} else if alias.node.name == "*" {
let module_name = format!(
@ -472,7 +486,7 @@ where
if !matches!(scope.kind, ScopeKind::Module) {
self.checks.push(Check::new(
CheckKind::ImportStarNotPermitted(module_name.to_string()),
stmt.location,
self.locate_check(stmt.location),
));
}
}
@ -480,7 +494,7 @@ where
if self.settings.select.contains(&CheckCode::F403) {
self.checks.push(Check::new(
CheckKind::ImportStarUsed(module_name.to_string()),
stmt.location,
self.locate_check(stmt.location),
));
}
@ -516,14 +530,18 @@ where
}
StmtKind::If { test, .. } => {
if self.settings.select.contains(&CheckCode::F634) {
if let Some(check) = checks::check_if_tuple(test, stmt.location) {
if let Some(check) =
checks::check_if_tuple(test, self.locate_check(stmt.location))
{
self.checks.push(check);
}
}
}
StmtKind::Assert { test, .. } => {
if self.settings.select.contains(CheckKind::AssertTuple.code()) {
if let Some(check) = checks::check_assert_tuple(test, stmt.location) {
if let Some(check) =
checks::check_assert_tuple(test, self.locate_check(stmt.location))
{
self.checks.push(check);
}
}
@ -537,7 +555,9 @@ where
}
StmtKind::Assign { value, .. } => {
if self.settings.select.contains(&CheckCode::E731) {
if let Some(check) = checks::check_do_not_assign_lambda(value, stmt.location) {
if let Some(check) =
checks::check_do_not_assign_lambda(value, self.locate_check(stmt.location))
{
self.checks.push(check);
}
}
@ -545,9 +565,10 @@ where
StmtKind::AnnAssign { value, .. } => {
if self.settings.select.contains(&CheckCode::E731) {
if let Some(value) = value {
if let Some(check) =
checks::check_do_not_assign_lambda(value, stmt.location)
{
if let Some(check) = checks::check_do_not_assign_lambda(
value,
self.locate_check(stmt.location),
) {
self.checks.push(check);
}
}
@ -628,9 +649,9 @@ where
self.settings.select.contains(&CheckCode::F622);
if let Some(check) = checks::check_starred_expressions(
elts,
expr.location,
check_too_many_expressions,
check_two_starred_expressions,
self.locate_check(expr.location),
) {
self.checks.push(check);
}
@ -640,9 +661,10 @@ where
ExprContext::Load => self.handle_node_load(expr),
ExprContext::Store => {
if self.settings.select.contains(&CheckCode::E741) {
if let Some(check) =
checks::check_ambiguous_variable_name(id, expr.location)
{
if let Some(check) = checks::check_ambiguous_variable_name(
id,
self.locate_check(expr.location),
) {
self.checks.push(check);
}
}
@ -673,18 +695,6 @@ where
}
}
}
//
// if id == "locals" {
// let scope = &self.scopes
// [*(self.scope_stack.last().expect("No current scope found."))];
// if matches!(scope.kind, ScopeKind::Function(_)) {
// let parent =
// self.parents[*(self.parent_stack.last().expect("No parent found."))];
// if matches!(parent.node, StmtKind::Call)
// }
//
// }
}
ExprKind::Dict { keys, .. } => {
let check_repeated_literals = self.settings.select.contains(&CheckCode::F601);
@ -694,6 +704,7 @@ where
keys,
check_repeated_literals,
check_repeated_variables,
self,
));
}
}
@ -706,12 +717,14 @@ where
.contains(CheckKind::YieldOutsideFunction.code())
&& matches!(scope.kind, ScopeKind::Class | ScopeKind::Module)
{
self.checks
.push(Check::new(CheckKind::YieldOutsideFunction, expr.location));
self.checks.push(Check::new(
CheckKind::YieldOutsideFunction,
self.locate_check(expr.location),
));
}
}
ExprKind::JoinedStr { values } => {
if !self.in_f_string
if self.in_f_string.is_none()
&& self
.settings
.select
@ -722,10 +735,10 @@ where
{
self.checks.push(Check::new(
CheckKind::FStringMissingPlaceholders,
expr.location,
self.locate_check(expr.location),
));
}
self.in_f_string = true;
self.in_f_string = Some(expr.location);
}
ExprKind::BinOp {
left,
@ -758,6 +771,7 @@ where
operand,
check_not_in,
check_not_is,
self,
));
}
}
@ -775,6 +789,7 @@ where
comparators,
check_none_comparisons,
check_true_false_comparisons,
self,
));
}
@ -783,7 +798,7 @@ where
left,
ops,
comparators,
expr.location,
self.locate_check(expr.location),
));
}
@ -791,7 +806,7 @@ where
self.checks.extend(checks::check_type_comparison(
ops,
comparators,
expr.location,
self.locate_check(expr.location),
));
}
}
@ -956,9 +971,10 @@ where
match name {
Some(name) => {
if self.settings.select.contains(&CheckCode::E741) {
if let Some(check) =
checks::check_ambiguous_variable_name(name, excepthandler.location)
{
if let Some(check) = checks::check_ambiguous_variable_name(
name,
self.locate_check(excepthandler.location),
) {
self.checks.push(check);
}
}
@ -1058,14 +1074,22 @@ where
);
if self.settings.select.contains(&CheckCode::E741) {
if let Some(check) = checks::check_ambiguous_variable_name(&arg.node.arg, arg.location)
{
if let Some(check) = checks::check_ambiguous_variable_name(
&arg.node.arg,
self.locate_check(arg.location),
) {
self.checks.push(check);
}
}
}
}
impl CheckLocator for Checker<'_> {
fn locate_check(&self, default: Location) -> Location {
self.in_f_string.unwrap_or(default)
}
}
impl<'a> Checker<'a> {
fn push_parent(&mut self, parent: &'a Stmt) {
self.parent_stack.push(self.parents.len());
@ -1185,7 +1209,7 @@ impl<'a> Checker<'a> {
self.checks.push(Check::new(
CheckKind::ImportStarUsage(id.clone(), from_list.join(", ")),
expr.location,
self.locate_check(expr.location),
));
}
return;
@ -1198,7 +1222,7 @@ impl<'a> Checker<'a> {
}
self.checks.push(Check::new(
CheckKind::UndefinedName(id.clone()),
expr.location,
self.locate_check(expr.location),
))
}
}
@ -1220,7 +1244,7 @@ impl<'a> Checker<'a> {
if scope_id == current.id {
self.checks.push(Check::new(
CheckKind::UndefinedLocal(id.clone()),
location,
self.locate_check(location),
));
}
}
@ -1313,7 +1337,7 @@ impl<'a> Checker<'a> {
{
self.checks.push(Check::new(
CheckKind::UndefinedName(id.clone()),
expr.location,
self.locate_check(expr.location),
))
}
}
@ -1338,7 +1362,7 @@ impl<'a> Checker<'a> {
} else if self.settings.select.contains(&CheckCode::F722) {
self.checks.push(Check::new(
CheckKind::ForwardAnnotationSyntaxError(expression.to_string()),
location,
self.locate_check(location),
));
}
}
@ -1390,10 +1414,10 @@ impl<'a> Checker<'a> {
}
fn check_deferred_assignments(&mut self) {
while let Some(index) = self.deferred_assignments.pop() {
if self.settings.select.contains(&CheckCode::F841) {
if self.settings.select.contains(&CheckCode::F841) {
while let Some(index) = self.deferred_assignments.pop() {
self.checks
.extend(checks::check_unused_variables(&self.scopes[index]));
.extend(checks::check_unused_variables(&self.scopes[index], self));
}
}
}
@ -1425,7 +1449,7 @@ impl<'a> Checker<'a> {
if !scope.values.contains_key(name) {
self.checks.push(Check::new(
CheckKind::UndefinedExport(name.to_string()),
all_binding.location,
self.locate_check(all_binding.location),
));
}
}
@ -1448,7 +1472,7 @@ impl<'a> Checker<'a> {
if !scope.values.contains_key(name) {
self.checks.push(Check::new(
CheckKind::ImportStarUsage(name.clone(), from_list.join(", ")),
all_binding.location,
self.locate_check(all_binding.location),
));
}
}
@ -1469,7 +1493,7 @@ impl<'a> Checker<'a> {
| BindingKind::SubmoduleImportation(full_name) => {
self.checks.push(Check::new(
CheckKind::UnusedImport(full_name.to_string()),
binding.location,
self.locate_check(binding.location),
));
}
_ => {}

12
src/snapshots/ruff__linter__tests__f821.snap
View file

@ -38,4 +38,16 @@ expression: checks
row: 83
column: 11
fix: ~
- kind:
UndefinedName: B
location:
row: 87
column: 7
fix: ~
- kind:
UndefinedName: B
location:
row: 89
column: 7
fix: ~