Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use CommentRanges from the parsed output #11591

Merged
merged 2 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/ruff_linter/src/checkers/filesystem.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::path::Path;

use ruff_diagnostics::Diagnostic;
use ruff_python_index::Indexer;
use ruff_python_trivia::CommentRanges;
use ruff_source_file::Locator;

use crate::registry::Rule;
Expand All @@ -13,7 +13,7 @@ pub(crate) fn check_file_path(
path: &Path,
package: Option<&Path>,
locator: &Locator,
indexer: &Indexer,
comment_ranges: &CommentRanges,
settings: &LinterSettings,
) -> Vec<Diagnostic> {
let mut diagnostics: Vec<Diagnostic> = vec![];
Expand All @@ -24,7 +24,7 @@ pub(crate) fn check_file_path(
path,
package,
locator,
indexer,
comment_ranges,
&settings.project_root,
&settings.src,
) {
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(crate) fn check_imports(
settings,
package,
source_type,
program.tokens(),
program,
) {
diagnostics.push(diagnostic);
}
Expand Down
6 changes: 4 additions & 2 deletions crates/ruff_linter/src/checkers/physical_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use ruff_diagnostics::Diagnostic;
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
use ruff_python_trivia::CommentRanges;
use ruff_source_file::{Locator, UniversalNewlines};
use ruff_text_size::TextSize;

Expand All @@ -19,6 +20,7 @@ pub(crate) fn check_physical_lines(
locator: &Locator,
stylist: &Stylist,
indexer: &Indexer,
comment_ranges: &CommentRanges,
doc_lines: &[TextSize],
settings: &LinterSettings,
) -> Vec<Diagnostic> {
Expand All @@ -42,7 +44,7 @@ pub(crate) fn check_physical_lines(
.is_some()
{
if enforce_doc_line_too_long {
if let Some(diagnostic) = doc_line_too_long(&line, indexer, settings) {
if let Some(diagnostic) = doc_line_too_long(&line, comment_ranges, settings) {
diagnostics.push(diagnostic);
}
}
Expand All @@ -55,7 +57,7 @@ pub(crate) fn check_physical_lines(
}

if enforce_line_too_long {
if let Some(diagnostic) = line_too_long(&line, indexer, settings) {
if let Some(diagnostic) = line_too_long(&line, comment_ranges, settings) {
diagnostics.push(diagnostic);
}
}
Expand Down
34 changes: 20 additions & 14 deletions crates/ruff_linter/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,16 @@
use std::path::Path;

use ruff_notebook::CellOffsets;
use ruff_python_ast::PySourceType;
use ruff_python_ast::{ModModule, PySourceType};
use ruff_python_codegen::Stylist;

use ruff_diagnostics::Diagnostic;
use ruff_python_index::Indexer;
use ruff_python_parser::Tokens;
use ruff_python_parser::{Program, Tokens};
use ruff_source_file::Locator;
use ruff_text_size::Ranged;

use crate::directives::TodoComment;
use crate::linter::TokenSource;
use crate::registry::{AsRule, Rule};
use crate::rules::pycodestyle::rules::BlankLinesChecker;
use crate::rules::{
Expand All @@ -24,7 +23,7 @@ use crate::settings::LinterSettings;

#[allow(clippy::too_many_arguments)]
pub(crate) fn check_tokens(
tokens: &Tokens,
program: &Program<ModModule>,
path: &Path,
locator: &Locator,
indexer: &Indexer,
Expand All @@ -35,6 +34,9 @@ pub(crate) fn check_tokens(
) -> Vec<Diagnostic> {
let mut diagnostics: Vec<Diagnostic> = vec![];

let tokens = program.tokens();
let comment_ranges = program.comment_ranges();

if settings.rules.any_enabled(&[
Rule::BlankLineBetweenMethods,
Rule::BlankLinesTopLevel,
Expand All @@ -48,18 +50,18 @@ pub(crate) fn check_tokens(
}

if settings.rules.enabled(Rule::BlanketTypeIgnore) {
pygrep_hooks::rules::blanket_type_ignore(&mut diagnostics, indexer, locator);
pygrep_hooks::rules::blanket_type_ignore(&mut diagnostics, comment_ranges, locator);
}

if settings.rules.enabled(Rule::EmptyComment) {
pylint::rules::empty_comments(&mut diagnostics, indexer, locator);
pylint::rules::empty_comments(&mut diagnostics, comment_ranges, locator);
}

if settings
.rules
.enabled(Rule::AmbiguousUnicodeCharacterComment)
{
for range in indexer.comment_ranges() {
for range in comment_ranges {
ruff::rules::ambiguous_unicode_character_comment(
&mut diagnostics,
locator,
Expand All @@ -70,11 +72,16 @@ pub(crate) fn check_tokens(
}

if settings.rules.enabled(Rule::CommentedOutCode) {
eradicate::rules::commented_out_code(&mut diagnostics, locator, indexer, settings);
eradicate::rules::commented_out_code(&mut diagnostics, locator, comment_ranges, settings);
}

if settings.rules.enabled(Rule::UTF8EncodingDeclaration) {
pyupgrade::rules::unnecessary_coding_comment(&mut diagnostics, locator, indexer);
pyupgrade::rules::unnecessary_coding_comment(
&mut diagnostics,
locator,
indexer,
comment_ranges,
);
}

if settings.rules.enabled(Rule::TabIndentation) {
Expand Down Expand Up @@ -139,7 +146,7 @@ pub(crate) fn check_tokens(
}

if source_type.is_stub() && settings.rules.enabled(Rule::TypeCommentInStub) {
flake8_pyi::rules::type_comment_in_stub(&mut diagnostics, locator, indexer);
flake8_pyi::rules::type_comment_in_stub(&mut diagnostics, locator, comment_ranges);
}

if settings.rules.any_enabled(&[
Expand All @@ -149,7 +156,7 @@ pub(crate) fn check_tokens(
Rule::ShebangNotFirstLine,
Rule::ShebangMissingPython,
]) {
flake8_executable::rules::from_tokens(&mut diagnostics, path, locator, indexer);
flake8_executable::rules::from_tokens(&mut diagnostics, path, locator, comment_ranges);
}

if settings.rules.any_enabled(&[
Expand All @@ -165,16 +172,15 @@ pub(crate) fn check_tokens(
Rule::LineContainsTodo,
Rule::LineContainsHack,
]) {
let todo_comments: Vec<TodoComment> = indexer
.comment_ranges()
let todo_comments: Vec<TodoComment> = comment_ranges
.iter()
.enumerate()
.filter_map(|(i, comment_range)| {
let comment = locator.slice(*comment_range);
TodoComment::from_comment(comment, *comment_range, i)
})
.collect();
flake8_todos::rules::todos(&mut diagnostics, &todo_comments, locator, indexer);
flake8_todos::rules::todos(&mut diagnostics, &todo_comments, locator, comment_ranges);
flake8_fixme::rules::todos(&mut diagnostics, &todo_comments);
}

Expand Down
13 changes: 7 additions & 6 deletions crates/ruff_linter/src/directives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ use std::iter::Peekable;
use std::str::FromStr;

use bitflags::bitflags;
use ruff_python_ast::StringFlags;
use ruff_python_ast::{ModModule, StringFlags};
use ruff_python_parser::lexer::LexResult;
use ruff_python_parser::Tok;
use ruff_python_parser::{Program, Tok};
use ruff_python_trivia::CommentRanges;
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};

use ruff_python_index::Indexer;
Expand Down Expand Up @@ -68,7 +69,7 @@ pub struct Directives {
}

pub fn extract_directives(
lxr: &[LexResult],
program: &Program<ModModule>,
flags: Flags,
locator: &Locator,
indexer: &Indexer,
Expand All @@ -80,7 +81,7 @@ pub fn extract_directives(
NoqaMapping::default()
},
isort: if flags.intersects(Flags::ISORT) {
extract_isort_directives(locator, indexer)
extract_isort_directives(locator, program.comment_ranges())
} else {
IsortDirectives::default()
},
Expand Down Expand Up @@ -213,12 +214,12 @@ fn extract_noqa_line_for(lxr: &[LexResult], locator: &Locator, indexer: &Indexer
}

/// Extract a set of ranges over which to disable isort.
fn extract_isort_directives(locator: &Locator, indexer: &Indexer) -> IsortDirectives {
fn extract_isort_directives(locator: &Locator, comment_ranges: &CommentRanges) -> IsortDirectives {
let mut exclusions: Vec<TextRange> = Vec::default();
let mut splits: Vec<TextSize> = Vec::default();
let mut off: Option<TextSize> = None;

for range in indexer.comment_ranges() {
for range in comment_ranges {
let comment_text = locator.slice(range);

// `isort` allows for `# isort: skip` and `# isort: skip_file` to include or
Expand Down
57 changes: 39 additions & 18 deletions crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub fn check_path(
.any(|rule_code| rule_code.lint_source().is_tokens())
{
diagnostics.extend(check_tokens(
program.tokens(),
&program,
path,
locator,
indexer,
Expand All @@ -120,7 +120,13 @@ pub fn check_path(
.iter_enabled()
.any(|rule_code| rule_code.lint_source().is_filesystem())
{
diagnostics.extend(check_file_path(path, package, locator, indexer, settings));
diagnostics.extend(check_file_path(
path,
package,
locator,
program.comment_ranges(),
settings,
));
}

// Run the logical line-based rules.
Expand Down Expand Up @@ -209,48 +215,63 @@ pub fn check_path(
.any(|rule_code| rule_code.lint_source().is_physical_lines())
{
diagnostics.extend(check_physical_lines(
locator, stylist, indexer, &doc_lines, settings,
locator,
stylist,
indexer,
program.comment_ranges(),
&doc_lines,
settings,
));
}

// Raise violations for internal test rules
#[cfg(any(feature = "test-rules", test))]
{
let comment_ranges = program.comment_ranges();

for test_rule in TEST_RULES {
if !settings.rules.enabled(*test_rule) {
continue;
}
let diagnostic = match test_rule {
Rule::StableTestRule => test_rules::StableTestRule::diagnostic(locator, indexer),
Rule::StableTestRule => {
test_rules::StableTestRule::diagnostic(locator, comment_ranges)
}
Rule::StableTestRuleSafeFix => {
dhruvmanila marked this conversation as resolved.
Show resolved Hide resolved
test_rules::StableTestRuleSafeFix::diagnostic(locator, indexer)
test_rules::StableTestRuleSafeFix::diagnostic(locator, comment_ranges)
}
Rule::StableTestRuleUnsafeFix => {
test_rules::StableTestRuleUnsafeFix::diagnostic(locator, indexer)
test_rules::StableTestRuleUnsafeFix::diagnostic(locator, comment_ranges)
}
Rule::StableTestRuleDisplayOnlyFix => {
test_rules::StableTestRuleDisplayOnlyFix::diagnostic(locator, indexer)
test_rules::StableTestRuleDisplayOnlyFix::diagnostic(locator, comment_ranges)
}
Rule::NurseryTestRule => {
test_rules::NurseryTestRule::diagnostic(locator, comment_ranges)
}
Rule::PreviewTestRule => {
test_rules::PreviewTestRule::diagnostic(locator, comment_ranges)
}
Rule::NurseryTestRule => test_rules::NurseryTestRule::diagnostic(locator, indexer),
Rule::PreviewTestRule => test_rules::PreviewTestRule::diagnostic(locator, indexer),
Rule::DeprecatedTestRule => {
test_rules::DeprecatedTestRule::diagnostic(locator, indexer)
test_rules::DeprecatedTestRule::diagnostic(locator, comment_ranges)
}
Rule::AnotherDeprecatedTestRule => {
test_rules::AnotherDeprecatedTestRule::diagnostic(locator, indexer)
test_rules::AnotherDeprecatedTestRule::diagnostic(locator, comment_ranges)
}
Rule::RemovedTestRule => {
test_rules::RemovedTestRule::diagnostic(locator, comment_ranges)
}
Rule::RemovedTestRule => test_rules::RemovedTestRule::diagnostic(locator, indexer),
Rule::AnotherRemovedTestRule => {
test_rules::AnotherRemovedTestRule::diagnostic(locator, indexer)
test_rules::AnotherRemovedTestRule::diagnostic(locator, comment_ranges)
}
Rule::RedirectedToTestRule => {
test_rules::RedirectedToTestRule::diagnostic(locator, indexer)
test_rules::RedirectedToTestRule::diagnostic(locator, comment_ranges)
}
Rule::RedirectedFromTestRule => {
test_rules::RedirectedFromTestRule::diagnostic(locator, indexer)
test_rules::RedirectedFromTestRule::diagnostic(locator, comment_ranges)
}
Rule::RedirectedFromPrefixTestRule => {
test_rules::RedirectedFromPrefixTestRule::diagnostic(locator, indexer)
test_rules::RedirectedFromPrefixTestRule::diagnostic(locator, comment_ranges)
}
_ => unreachable!("All test rules must have an implementation"),
};
Expand Down Expand Up @@ -287,7 +308,7 @@ pub fn check_path(
&mut diagnostics,
path,
locator,
indexer.comment_ranges(),
program.comment_ranges(),
&directives.noqa_line_for,
error.is_none(),
&per_file_ignores,
Expand Down Expand Up @@ -407,7 +428,7 @@ pub fn add_noqa_to_path(
path,
&diagnostics,
&locator,
indexer.comment_ranges(),
program.comment_ranges(),
&settings.external,
&directives.noqa_line_for,
stylist.line_ending(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_index::Indexer;
use ruff_python_trivia::CommentRanges;
use ruff_source_file::Locator;

use crate::settings::LinterSettings;
Expand Down Expand Up @@ -47,14 +47,14 @@ impl Violation for CommentedOutCode {
pub(crate) fn commented_out_code(
diagnostics: &mut Vec<Diagnostic>,
locator: &Locator,
indexer: &Indexer,
comment_ranges: &CommentRanges,
settings: &LinterSettings,
) {
// Skip comments within `/// script` tags.
let mut in_script_tag = false;

// Iterate over all comments in the document.
for range in indexer.comment_ranges() {
for range in comment_ranges {
let line = locator.lines(*range);

// Detect `/// script` tags.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub(crate) fn zip_without_explicit_strict(checker: &mut Checker, call: &ast::Exp
add_argument(
"strict=False",
&call.arguments,
checker.indexer().comment_ranges(),
checker.program().comment_ranges(),
checker.locator().contents(),
),
// If the function call contains `**kwargs`, mark the fix as unsafe.
Expand Down
Loading
Loading