Skip to content

Commit

Permalink
Implement Ranged on more structs (#6639)
Browse files Browse the repository at this point in the history
## Summary

I noticed some inconsistencies around uses of `.range.start()`, structs
that have a `TextRange` field but don't implement `Ranged`, etc.

## Test Plan

`cargo test`
  • Loading branch information
charliermarsh authored Aug 17, 2023
1 parent a70807e commit db1c556
Show file tree
Hide file tree
Showing 66 changed files with 219 additions and 174 deletions.
3 changes: 2 additions & 1 deletion crates/ruff/src/checkers/ast/analyze/bindings.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use ruff_diagnostics::{Diagnostic, Fix};
use ruff_python_ast::Ranged;

use crate::checkers::ast::Checker;
use crate::codes::Rule;
Expand Down Expand Up @@ -29,7 +30,7 @@ pub(crate) fn bindings(checker: &mut Checker) {
pyflakes::rules::UnusedVariable {
name: binding.name(checker.locator).to_string(),
},
binding.range,
binding.range(),
);
if checker.patch(Rule::UnusedVariable) {
diagnostic.try_set_fix(|| {
Expand Down
11 changes: 6 additions & 5 deletions crates/ruff/src/checkers/ast/analyze/deferred_scopes.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use ruff_diagnostics::Diagnostic;
use ruff_python_ast::Ranged;
use ruff_python_semantic::analyze::{branch_detection, visibility};
use ruff_python_semantic::{Binding, BindingKind, ScopeKind};

Expand Down Expand Up @@ -81,7 +82,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
pylint::rules::GlobalVariableNotAssigned {
name: (*name).to_string(),
},
binding.range,
binding.range(),
));
}
}
Expand Down Expand Up @@ -122,14 +123,14 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
}

#[allow(deprecated)]
let line = checker.locator.compute_line_index(shadowed.range.start());
let line = checker.locator.compute_line_index(shadowed.start());

checker.diagnostics.push(Diagnostic::new(
pyflakes::rules::ImportShadowedByLoopVar {
name: name.to_string(),
line,
},
binding.range,
binding.range(),
));
}
}
Expand Down Expand Up @@ -218,13 +219,13 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
}

#[allow(deprecated)]
let line = checker.locator.compute_line_index(shadowed.range.start());
let line = checker.locator.compute_line_index(shadowed.start());
let mut diagnostic = Diagnostic::new(
pyflakes::rules::RedefinedWhileUnused {
name: (*name).to_string(),
line,
},
binding.range,
binding.range(),
);
if let Some(range) = binding.parent_range(&checker.semantic) {
diagnostic.set_parent(range.start());
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1855,7 +1855,7 @@ impl<'a> Checker<'a> {
.map(|binding_id| &self.semantic.bindings[binding_id])
.filter_map(|binding| match &binding.kind {
BindingKind::Export(Export { names }) => {
Some(names.iter().map(|name| (*name, binding.range)))
Some(names.iter().map(|name| (*name, binding.range())))
}
_ => None,
})
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff/src/checkers/logical_lines.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use ruff_python_parser::lexer::LexResult;
use ruff_text_size::TextRange;

use ruff_diagnostics::{Diagnostic, DiagnosticKind};
use ruff_python_ast::Ranged;
use ruff_python_codegen::Stylist;
use ruff_python_parser::lexer::LexResult;
use ruff_python_parser::TokenKind;
use ruff_source_file::Locator;
use ruff_text_size::TextRange;

use crate::registry::{AsRule, Rule};
use crate::rules::pycodestyle::rules::logical_lines::{
Expand Down
32 changes: 11 additions & 21 deletions crates/ruff/src/docstrings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ use std::fmt::{Debug, Formatter};
use std::ops::Deref;

use ruff_python_ast::{Expr, Ranged};
use ruff_text_size::{TextRange, TextSize};

use ruff_python_semantic::Definition;
use ruff_text_size::TextRange;

pub(crate) mod extraction;
pub(crate) mod google;
Expand All @@ -28,21 +27,15 @@ impl<'a> Docstring<'a> {
DocstringBody { docstring: self }
}

pub(crate) fn start(&self) -> TextSize {
self.expr.start()
}

pub(crate) fn end(&self) -> TextSize {
self.expr.end()
pub(crate) fn leading_quote(&self) -> &'a str {
&self.contents[TextRange::up_to(self.body_range.start())]
}
}

pub(crate) fn range(&self) -> TextRange {
impl Ranged for Docstring<'_> {
fn range(&self) -> TextRange {
self.expr.range()
}

pub(crate) fn leading_quote(&self) -> &'a str {
&self.contents[TextRange::up_to(self.body_range.start())]
}
}

#[derive(Copy, Clone)]
Expand All @@ -51,18 +44,15 @@ pub(crate) struct DocstringBody<'a> {
}

impl<'a> DocstringBody<'a> {
#[inline]
pub(crate) fn start(self) -> TextSize {
self.range().start()
pub(crate) fn as_str(self) -> &'a str {
&self.docstring.contents[self.docstring.body_range]
}
}

pub(crate) fn range(self) -> TextRange {
impl Ranged for DocstringBody<'_> {
fn range(&self) -> TextRange {
self.docstring.body_range + self.docstring.start()
}

pub(crate) fn as_str(self) -> &'a str {
&self.docstring.contents[self.docstring.body_range]
}
}

impl Deref for DocstringBody<'_> {
Expand Down
7 changes: 7 additions & 0 deletions crates/ruff/src/docstrings/sections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::fmt::{Debug, Formatter};
use std::iter::FusedIterator;

use ruff_python_ast::docstrings::{leading_space, leading_words};
use ruff_python_ast::Ranged;
use ruff_text_size::{TextLen, TextRange, TextSize};
use strum_macros::EnumIter;

Expand Down Expand Up @@ -366,6 +367,12 @@ impl<'a> SectionContext<'a> {
}
}

impl Ranged for SectionContext<'_> {
fn range(&self) -> TextRange {
self.range()
}
}

impl Debug for SectionContext<'_> {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
f.debug_struct("SectionContext")
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/src/importer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ impl<'a> Importer<'a> {
// import and the current location, and thus the symbol would not be available). It's also
// unclear whether should add an import statement at the start of the file, since it could
// be shadowed between the import and the current location.
if imported_name.range().start() > at {
if imported_name.start() > at {
return Some(Err(ResolutionError::ImportAfterUsage));
}

Expand Down
9 changes: 5 additions & 4 deletions crates/ruff/src/renamer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use anyhow::{anyhow, Result};
use itertools::Itertools;

use ruff_diagnostics::Edit;
use ruff_python_ast::Ranged;
use ruff_python_semantic::{Binding, BindingKind, Scope, ScopeId, SemanticModel};

pub(crate) struct Renamer;
Expand Down Expand Up @@ -220,12 +221,12 @@ impl Renamer {
BindingKind::Import(_) | BindingKind::FromImport(_) => {
if binding.is_alias() {
// Ex) Rename `import pandas as alias` to `import pandas as pd`.
Some(Edit::range_replacement(target.to_string(), binding.range))
Some(Edit::range_replacement(target.to_string(), binding.range()))
} else {
// Ex) Rename `import pandas` to `import pandas as pd`.
Some(Edit::range_replacement(
format!("{name} as {target}"),
binding.range,
binding.range(),
))
}
}
Expand All @@ -234,7 +235,7 @@ impl Renamer {
let module_name = import.call_path.first().unwrap();
Some(Edit::range_replacement(
format!("{module_name} as {target}"),
binding.range,
binding.range(),
))
}
// Avoid renaming builtins and other "special" bindings.
Expand All @@ -254,7 +255,7 @@ impl Renamer {
| BindingKind::FunctionDefinition(_)
| BindingKind::Deletion
| BindingKind::UnboundException(_) => {
Some(Edit::range_replacement(target.to_string(), binding.range))
Some(Edit::range_replacement(target.to_string(), binding.range()))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ pub(crate) fn unused_loop_control_variable(checker: &mut Checker, target: &Expr,
if scope
.get_all(name)
.map(|binding_id| checker.semantic().binding(binding_id))
.filter(|binding| binding.range.start() >= expr.range().start())
.filter(|binding| binding.start() >= expr.start())
.all(|binding| !binding.is_used())
{
diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use rustc_hash::FxHashMap;

use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::Ranged;
use ruff_python_semantic::{Binding, Imported};

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -76,7 +77,7 @@ pub(crate) fn unconventional_import_alias(
name: qualified_name,
asname: expected_alias.to_string(),
},
binding.range,
binding.range(),
);
if checker.patch(diagnostic.kind.rule()) {
if checker.semantic().is_available(expected_alias) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::Ranged;
use ruff_python_semantic::Imported;
use ruff_python_semantic::{Binding, BindingKind};

Expand Down Expand Up @@ -63,7 +64,7 @@ pub(crate) fn unaliased_collections_abc_set_import(
return None;
}

let mut diagnostic = Diagnostic::new(UnaliasedCollectionsAbcSetImport, binding.range);
let mut diagnostic = Diagnostic::new(UnaliasedCollectionsAbcSetImport, binding.range());
if checker.patch(diagnostic.kind.rule()) {
if checker.semantic().is_available("AbstractSet") {
diagnostic.try_set_fix(|| {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_python_ast::{self as ast, Expr, Ranged, Stmt};
use ruff_python_semantic::Scope;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -192,7 +192,7 @@ pub(crate) fn unused_private_type_var(
UnusedPrivateTypeVar {
name: id.to_string(),
},
binding.range,
binding.range(),
));
}
}
Expand Down Expand Up @@ -234,7 +234,7 @@ pub(crate) fn unused_private_protocol(
UnusedPrivateProtocol {
name: class_def.name.to_string(),
},
binding.range,
binding.range(),
));
}
}
Expand Down Expand Up @@ -280,7 +280,7 @@ pub(crate) fn unused_private_type_alias(
UnusedPrivateTypeAlias {
name: id.to_string(),
},
binding.range,
binding.range(),
));
}
}
Expand Down Expand Up @@ -321,7 +321,7 @@ pub(crate) fn unused_private_typed_dict(
UnusedPrivateTypedDict {
name: class_def.name.to_string(),
},
binding.range,
binding.range(),
));
}
}
2 changes: 1 addition & 1 deletion crates/ruff/src/rules/flake8_return/rules/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ fn unnecessary_assign(checker: &mut Checker, stack: &Stack) {
// Replace from the start of the assignment statement to the end of the equals
// sign.
TextRange::new(
assign.range().start(),
assign.start(),
assign
.range()
.start()
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ pub(crate) fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt) {
fn body_range(branch: &IfElifBranch, locator: &Locator) -> TextRange {
TextRange::new(
locator.line_end(branch.test.end()),
locator.line_end(branch.range.end()),
locator.line_end(branch.end()),
)
}

Expand Down Expand Up @@ -731,7 +731,7 @@ pub(crate) fn if_with_same_arms(checker: &mut Checker, locator: &Locator, stmt_i

checker.diagnostics.push(Diagnostic::new(
IfWithSameArms,
TextRange::new(current_branch.range.start(), following_branch.range.end()),
TextRange::new(current_branch.start(), following_branch.end()),
));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rustc_hash::FxHashMap;

use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::Ranged;
use ruff_python_semantic::{AnyImport, Imported, ResolvedReferenceId, Scope, StatementId};
use ruff_text_size::TextRange;

Expand Down Expand Up @@ -101,11 +102,11 @@ pub(crate) fn runtime_import_in_type_checking_block(
let import = ImportBinding {
import,
reference_id,
range: binding.range,
range: binding.range(),
parent_range: binding.parent_range(checker.semantic()),
};

if checker.rule_is_ignored(Rule::RuntimeImportInTypeCheckingBlock, import.range.start())
if checker.rule_is_ignored(Rule::RuntimeImportInTypeCheckingBlock, import.start())
|| import.parent_range.is_some_and(|parent_range| {
checker.rule_is_ignored(
Rule::RuntimeImportInTypeCheckingBlock,
Expand Down Expand Up @@ -192,6 +193,12 @@ struct ImportBinding<'a> {
parent_range: Option<TextRange>,
}

impl Ranged for ImportBinding<'_> {
fn range(&self) -> TextRange {
self.range
}
}

/// Generate a [`Fix`] to remove runtime imports from a type-checking block.
fn fix_imports(
checker: &Checker,
Expand All @@ -211,7 +218,7 @@ fn fix_imports(
let at = imports
.iter()
.map(|ImportBinding { reference_id, .. }| {
checker.semantic().reference(*reference_id).range().start()
checker.semantic().reference(*reference_id).start()
})
.min()
.expect("Expected at least one import");
Expand Down
Loading

0 comments on commit db1c556

Please sign in to comment.