Skip to content

Commit

Permalink
Ignore overlong pragma comments when enforcing linter line length
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Sep 28, 2023
1 parent 695dbbc commit be29c74
Show file tree
Hide file tree
Showing 10 changed files with 155 additions and 56 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# TODO: comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep`
# TODO(charlie): comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep`
# TODO comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep`
# TODO comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep`
# FIXME: comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep`
# FIXME comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep`
# FIXME comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep`
# FIXME(charlie): comments starting with one of the configured task-tags sometimes are longer than line-length so that you can easily find them with `git grep`
11 changes: 11 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E501_3.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# OK (88 characters)
"shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:aaa" # type: ignore

# OK (88 characters)
"shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:aaa"# type: ignore

# OK (88 characters)
"shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:aaa" # type: ignore

# Error (89 characters)
"shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:aaaa" # type: ignore
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/checkers/physical_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub(crate) fn check_physical_lines(
.is_some()
{
if enforce_doc_line_too_long {
if let Some(diagnostic) = doc_line_too_long(&line, settings) {
if let Some(diagnostic) = doc_line_too_long(&line, indexer, settings) {
diagnostics.push(diagnostic);
}
}
Expand All @@ -54,7 +54,7 @@ pub(crate) fn check_physical_lines(
}

if enforce_line_too_long {
if let Some(diagnostic) = line_too_long(&line, settings) {
if let Some(diagnostic) = line_too_long(&line, indexer, settings) {
diagnostics.push(diagnostic);
}
}
Expand Down
54 changes: 41 additions & 13 deletions crates/ruff_linter/src/rules/pycodestyle/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use std::borrow::Cow;
use unicode_width::UnicodeWidthStr;

use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::parenthesize::parenthesized_range;
use ruff_python_ast::{CmpOp, Expr};
use ruff_python_trivia::CommentRanges;
use ruff_python_index::Indexer;
use ruff_python_trivia::{is_pragma_comment, CommentRanges};
use ruff_source_file::{Line, Locator};
use ruff_text_size::{Ranged, TextLen, TextRange};

Expand Down Expand Up @@ -68,6 +70,7 @@ pub(super) fn generate_comparison(
pub(super) fn is_overlong(
line: &Line,
limit: LineLength,
indexer: &Indexer,
ignore_overlong_task_comments: bool,
task_tags: &[String],
tab_size: TabSize,
Expand All @@ -80,27 +83,51 @@ pub(super) fn is_overlong(
return None;
}

let mut width = LineWidthBuilder::new(tab_size);
width = width.add_str(line.as_str());
// If the line ends in a pragma, remove it from the considered width.
let mut line = Cow::Borrowed(line);
if let [comment_range] = indexer.comment_ranges().comments_in_range(line.range()) {
// Convert from absolute to relative range.
let comment_range = comment_range - line.start();
let comment = &line.as_str()[comment_range];

// Ex) `# type: ignore`
if is_pragma_comment(comment) {
// Remove the pragma from the line.
let prefix = &line.as_str()[..usize::from(comment_range.start())].trim_end();
line = Cow::Owned(Line::new(prefix, line.start()));
}

// Ex) `# TODO(charlie): ...`
if ignore_overlong_task_comments {
let trimmed = comment.strip_prefix('#').unwrap_or(comment);
let trimmed = trimmed.trim_start();
if task_tags
.iter()
.any(|task_tag| trimmed.starts_with(task_tag))
{
// Remove the task tag from the line.
let prefix = &line.as_str()[..usize::from(comment_range.start())].trim_end();
line = Cow::Owned(Line::new(prefix, line.start()));
}
}
}

let width = {
let mut width = LineWidthBuilder::new(tab_size);
width = width.add_str(line.as_str());
width
};

if width <= limit {
return None;
}

let mut chunks = line.split_whitespace();
let (Some(first_chunk), Some(second_chunk)) = (chunks.next(), chunks.next()) else {
let (Some(_), Some(second_chunk)) = (chunks.next(), chunks.next()) else {
// Single word / no printable chars - no way to make the line shorter
return None;
};

if first_chunk == "#" {
if ignore_overlong_task_comments {
let second = second_chunk.trim_end_matches(':');
if task_tags.iter().any(|task_tag| task_tag == second) {
return None;
}
}
}

// Do not enforce the line length for lines that end with a URL, as long as the URL
// begins before the limit.
let last_chunk = chunks.last().unwrap_or(second_chunk);
Expand All @@ -121,6 +148,7 @@ pub(super) fn is_overlong(
break;
}
}

Some(Overlong {
range: TextRange::new(start_offset, line.end()),
width: width.get(),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pycodestyle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ mod tests {
#[test_case(Rule::InvalidEscapeSequence, Path::new("W605_0.py"))]
#[test_case(Rule::InvalidEscapeSequence, Path::new("W605_1.py"))]
#[test_case(Rule::LineTooLong, Path::new("E501.py"))]
#[test_case(Rule::LineTooLong, Path::new("E501_3.py"))]
#[test_case(Rule::MixedSpacesAndTabs, Path::new("E101.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E40.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402.py"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_index::Indexer;
use ruff_source_file::Line;

use crate::rules::pycodestyle::helpers::is_overlong;
Expand All @@ -19,10 +20,17 @@ use crate::settings::LinterSettings;
/// of either a standalone comment or a standalone string, like a docstring.
///
/// In the interest of pragmatism, this rule makes a few exceptions when
/// determining whether a line is overlong. Namely, it ignores lines that
/// consist of a single "word" (i.e., without any whitespace between its
/// characters), and lines that end with a URL (as long as the URL starts
/// before the line-length threshold).
/// determining whether a line is overlong. Namely, it:
///
/// 1. Ignores lines that consist of a single "word" (i.e., without any
/// whitespace between its characters).
/// 2. Ignores lines that end with a URL, as long as the URL starts before
/// the line-length threshold.
/// 3. Ignores line that end with a pragma comment (e.g., `# type: ignore`
/// or `# noqa`), as long as the pragma comment starts before the
/// line-length threshold. That is, a line will not be flagged as
/// overlong if a pragma comment _causes_ it to exceed the line length.
/// (This behavior aligns with that of the Ruff formatter.)
///
/// If [`pycodestyle.ignore-overlong-task-comments`] is `true`, this rule will
/// also ignore comments that start with any of the specified [`task-tags`]
Expand Down Expand Up @@ -50,7 +58,7 @@ use crate::settings::LinterSettings;
///
/// [PEP 8]: https://peps.python.org/pep-0008/#maximum-line-length
#[violation]
pub struct DocLineTooLong(pub usize, pub usize);
pub struct DocLineTooLong(usize, usize);

impl Violation for DocLineTooLong {
#[derive_message_formats]
Expand All @@ -61,14 +69,19 @@ impl Violation for DocLineTooLong {
}

/// W505
pub(crate) fn doc_line_too_long(line: &Line, settings: &LinterSettings) -> Option<Diagnostic> {
pub(crate) fn doc_line_too_long(
line: &Line,
indexer: &Indexer,
settings: &LinterSettings,
) -> Option<Diagnostic> {
let Some(limit) = settings.pycodestyle.max_doc_length else {
return None;
};

is_overlong(
line,
limit,
indexer,
settings.pycodestyle.ignore_overlong_task_comments,
&settings.task_tags,
settings.tab_size,
Expand Down
25 changes: 19 additions & 6 deletions crates/ruff_linter/src/rules/pycodestyle/rules/line_too_long.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_index::Indexer;
use ruff_source_file::Line;

use crate::rules::pycodestyle::helpers::is_overlong;
Expand All @@ -15,10 +16,17 @@ use crate::settings::LinterSettings;
/// configurable via the [`line-length`] setting.
///
/// In the interest of pragmatism, this rule makes a few exceptions when
/// determining whether a line is overlong. Namely, it ignores lines that
/// consist of a single "word" (i.e., without any whitespace between its
/// characters), and lines that end with a URL (as long as the URL starts
/// before the line-length threshold).
/// determining whether a line is overlong. Namely, it:
///
/// 1. Ignores lines that consist of a single "word" (i.e., without any
/// whitespace between its characters).
/// 2. Ignores lines that end with a URL, as long as the URL starts before
/// the line-length threshold.
/// 3. Ignores line that end with a pragma comment (e.g., `# type: ignore`
/// or `# noqa`), as long as the pragma comment starts before the
/// line-length threshold. That is, a line will not be flagged as
/// overlong if a pragma comment _causes_ it to exceed the line length.
/// (This behavior aligns with that of the Ruff formatter.)
///
/// If [`pycodestyle.ignore-overlong-task-comments`] is `true`, this rule will
/// also ignore comments that start with any of the specified [`task-tags`]
Expand All @@ -44,7 +52,7 @@ use crate::settings::LinterSettings;
///
/// [PEP 8]: https://peps.python.org/pep-0008/#maximum-line-length
#[violation]
pub struct LineTooLong(pub usize, pub usize);
pub struct LineTooLong(usize, usize);

impl Violation for LineTooLong {
#[derive_message_formats]
Expand All @@ -55,12 +63,17 @@ impl Violation for LineTooLong {
}

/// E501
pub(crate) fn line_too_long(line: &Line, settings: &LinterSettings) -> Option<Diagnostic> {
pub(crate) fn line_too_long(
line: &Line,
indexer: &Indexer,
settings: &LinterSettings,
) -> Option<Diagnostic> {
let limit = settings.line_length;

is_overlong(
line,
limit,
indexer,
settings.pycodestyle.ignore_overlong_task_comments,
&settings.task_tags,
settings.tab_size,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E501_3.py:11:89: E501 Line too long (89 > 88 characters)
|
10 | # Error (89 characters)
11 | "shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:" + "shape:aaaa" # type: ignore
| ^ E501
|


0 comments on commit be29c74

Please sign in to comment.