Skip to content

Commit

Permalink
Unify line size settings between ruff and the formatter
Browse files Browse the repository at this point in the history
**Summary** Previuosly, there had been `LineLength` in ruff and `LineWidth` in the formatter. `LineLength` was a lenient usize wrapper, while `LineWidth` was a strict u16. With the formatter moving into the ruff cli, we need to unify the two types. This PR makes both crates share a new `LineLength` type based on the previous ruff formatter type. It currently lives in `ruff_python_trivia` but i'm happy to move it wherever.

This is technically a breaking change because the line limit afterwards has a limit of 320 that it didn't have before.

**Test Plan** No functional changes.
  • Loading branch information
konstin committed Aug 25, 2023
1 parent 55c38e5 commit 499e311
Show file tree
Hide file tree
Showing 42 changed files with 297 additions and 236 deletions.
5 changes: 5 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/ruff/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ ruff_python_index = { path = "../ruff_python_index" }
ruff_python_literal = { path = "../ruff_python_literal" }
ruff_python_semantic = { path = "../ruff_python_semantic" }
ruff_python_stdlib = { path = "../ruff_python_stdlib" }
ruff_python_trivia = { path = "../ruff_python_trivia" }
ruff_python_trivia = { path = "../ruff_python_trivia", features = ["schemars"] }
ruff_python_parser = { path = "../ruff_python_parser" }
ruff_source_file = { path = "../ruff_source_file", features = ["serde"] }
ruff_text_size = { path = "../ruff_text_size" }
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/checkers/physical_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ mod tests {

use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
use ruff_python_trivia::LineLength;
use ruff_source_file::Locator;

use crate::line_width::LineLength;
use crate::registry::Rule;
use crate::settings::Settings;

Expand All @@ -132,7 +132,7 @@ mod tests {
},
)
};
let line_length = LineLength::from(8);
let line_length = LineLength::try_from(8).unwrap();
assert_eq!(check_with_max_line_length(line_length), vec![]);
assert_eq!(check_with_max_line_length(line_length), vec![]);
}
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff/src/flake8_to_ruff/black.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
//! Extract Black configuration settings from a pyproject.toml.

use ruff_python_trivia::LineLength;
use serde::{Deserialize, Serialize};

use crate::settings::types::PythonVersion;

#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Default)]
pub struct Black {
#[serde(alias = "line-length", alias = "line_length")]
pub line_length: Option<usize>,
pub line_length: Option<LineLength>,
#[serde(alias = "target-version", alias = "target_version")]
pub target_version: Option<Vec<PythonVersion>>,
}
17 changes: 8 additions & 9 deletions crates/ruff/src/flake8_to_ruff/converter.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use std::collections::{HashMap, HashSet};
use std::str::FromStr;

use anyhow::Result;
use itertools::Itertools;
use ruff_python_trivia::LineLength;

use crate::line_width::LineLength;
use crate::registry::Linter;
use crate::rule_selector::RuleSelector;
use crate::rules::flake8_pytest_style::types::{
Expand Down Expand Up @@ -120,10 +121,8 @@ pub fn convert(
"builtins" => {
options.builtins = Some(parser::parse_strings(value.as_ref()));
}
"max-line-length" | "max_line_length" => match value.parse::<usize>() {
Ok(line_length) => {
options.line_length = Some(LineLength::from(line_length));
}
"max-line-length" | "max_line_length" => match LineLength::from_str(value) {
Ok(line_length) => options.line_length = Some(line_length),
Err(e) => {
warn_user!("Unable to parse '{key}' property: {e}");
}
Expand Down Expand Up @@ -406,7 +405,7 @@ pub fn convert(
// Extract any settings from the existing `pyproject.toml`.
if let Some(black) = &external_config.black {
if let Some(line_length) = &black.line_length {
options.line_length = Some(LineLength::from(*line_length));
options.line_length = Some(*line_length);
}

if let Some(target_version) = &black.target_version {
Expand Down Expand Up @@ -459,11 +458,11 @@ mod tests {
use itertools::Itertools;
use pep440_rs::VersionSpecifiers;
use pretty_assertions::assert_eq;
use ruff_python_trivia::LineLength;

use crate::flake8_to_ruff::converter::DEFAULT_SELECTORS;
use crate::flake8_to_ruff::pep621::Project;
use crate::flake8_to_ruff::ExternalConfig;
use crate::line_width::LineLength;
use crate::registry::Linter;
use crate::rule_selector::RuleSelector;
use crate::rules::pydocstyle::settings::Convention;
Expand Down Expand Up @@ -514,7 +513,7 @@ mod tests {
Some(vec![]),
)?;
let expected = Pyproject::new(Options {
line_length: Some(LineLength::from(100)),
line_length: Some(LineLength::try_from(100).unwrap()),
..default_options([])
});
assert_eq!(actual, expected);
Expand All @@ -533,7 +532,7 @@ mod tests {
Some(vec![]),
)?;
let expected = Pyproject::new(Options {
line_length: Some(LineLength::from(100)),
line_length: Some(LineLength::try_from(100).unwrap()),
..default_options([])
});
assert_eq!(actual, expected);
Expand Down
29 changes: 16 additions & 13 deletions crates/ruff/src/line_width.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use std::num::NonZeroU8;
use unicode_width::UnicodeWidthChar;

use ruff_macros::CacheKey;
use ruff_python_trivia::LineLength;

/*
/// The length of a line of text that is considered too long.
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, CacheKey)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
Expand All @@ -27,13 +29,14 @@ impl From<usize> for LineLength {
Self(value)
}
}
*/

/// A measure of the width of a line of text.
///
/// This is used to determine if a line is too long.
/// It should be compared to a [`LineLength`].
#[derive(Clone, Copy, Debug)]
pub struct LineWidth {
pub struct LineWidthBuilder {
/// The width of the line.
width: usize,
/// The column of the line.
Expand All @@ -43,40 +46,40 @@ pub struct LineWidth {
tab_size: TabSize,
}

impl Default for LineWidth {
impl Default for LineWidthBuilder {
fn default() -> Self {
Self::new(TabSize::default())
}
}

impl PartialEq for LineWidth {
impl PartialEq for LineWidthBuilder {
fn eq(&self, other: &Self) -> bool {
self.width == other.width
}
}

impl Eq for LineWidth {}
impl Eq for LineWidthBuilder {}

impl PartialOrd for LineWidth {
impl PartialOrd for LineWidthBuilder {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
Some(self.cmp(other))
}
}

impl Ord for LineWidth {
impl Ord for LineWidthBuilder {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.width.cmp(&other.width)
}
}

impl LineWidth {
impl LineWidthBuilder {
pub fn get(&self) -> usize {
self.width
}

/// Creates a new `LineWidth` with the given tab size.
pub fn new(tab_size: TabSize) -> Self {
LineWidth {
LineWidthBuilder {
width: 0,
column: 0,
tab_size,
Expand Down Expand Up @@ -119,7 +122,7 @@ impl LineWidth {

/// Adds the given width to the line width.
/// Also adds the given width to the column.
/// It is generally better to use [`LineWidth::add_str`] or [`LineWidth::add_char`].
/// It is generally better to use [`LineWidthBuilder::add_str`] or [`LineWidthBuilder::add_char`].
/// The width and column should be the same for the corresponding text.
/// Currently, this is only used to add spaces.
#[must_use]
Expand All @@ -130,15 +133,15 @@ impl LineWidth {
}
}

impl PartialEq<LineLength> for LineWidth {
impl PartialEq<LineLength> for LineWidthBuilder {
fn eq(&self, other: &LineLength) -> bool {
self.width == other.0
self.width == (other.value() as usize)
}
}

impl PartialOrd<LineLength> for LineWidth {
impl PartialOrd<LineLength> for LineWidthBuilder {
fn partial_cmp(&self, other: &LineLength) -> Option<std::cmp::Ordering> {
self.width.partial_cmp(&other.0)
self.width.partial_cmp(&(other.value() as usize))
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/message/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use ruff_text_size::{TextRange, TextSize};

use crate::fs::relativize_path;
use crate::jupyter::{Notebook, NotebookIndex};
use crate::line_width::{LineWidth, TabSize};
use crate::line_width::{LineWidthBuilder, TabSize};
use crate::message::diff::Diff;
use crate::message::{Emitter, EmitterContext, Message};
use crate::registry::AsRule;
Expand Down Expand Up @@ -292,7 +292,7 @@ fn replace_whitespace(source: &str, annotation_range: TextRange) -> SourceCode {
let mut result = String::new();
let mut last_end = 0;
let mut range = annotation_range;
let mut line_width = LineWidth::new(TabSize::default());
let mut line_width = LineWidthBuilder::new(TabSize::default());

for (index, c) in source.char_indices() {
let old_width = line_width.get();
Expand Down
8 changes: 4 additions & 4 deletions crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_source_file::{Locator, UniversalNewlines};

use crate::checkers::ast::Checker;
use crate::line_width::LineWidth;
use crate::line_width::LineWidthBuilder;
use crate::registry::AsRule;
use crate::rules::flake8_simplify::rules::fix_if;

Expand Down Expand Up @@ -415,7 +415,7 @@ pub(crate) fn nested_if_statements(
.unwrap_or_default()
.universal_newlines()
.all(|line| {
LineWidth::new(checker.settings.tab_size).add_str(&line)
LineWidthBuilder::new(checker.settings.tab_size).add_str(&line)
<= checker.settings.line_length
})
{
Expand Down Expand Up @@ -661,7 +661,7 @@ pub(crate) fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt) {

// Don't flag if the resulting expression would exceed the maximum line length.
let line_start = checker.locator().line_start(stmt.start());
if LineWidth::new(checker.settings.tab_size)
if LineWidthBuilder::new(checker.settings.tab_size)
.add_str(&checker.locator().contents()[TextRange::new(line_start, stmt.start())])
.add_str(&contents)
> checker.settings.line_length
Expand Down Expand Up @@ -965,7 +965,7 @@ pub(crate) fn use_dict_get_with_default(checker: &mut Checker, stmt_if: &ast::St

// Don't flag if the resulting expression would exceed the maximum line length.
let line_start = checker.locator().line_start(stmt_if.start());
if LineWidth::new(checker.settings.tab_size)
if LineWidthBuilder::new(checker.settings.tab_size)
.add_str(&checker.locator().contents()[TextRange::new(line_start, stmt_if.start())])
.add_str(&contents)
> checker.settings.line_length
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/rules/flake8_simplify/rules/ast_with.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use ruff_source_file::UniversalNewlines;
use ruff_text_size::TextRange;

use crate::checkers::ast::Checker;
use crate::line_width::LineWidth;
use crate::line_width::LineWidthBuilder;
use crate::registry::AsRule;

use super::fix_with;
Expand Down Expand Up @@ -142,7 +142,7 @@ pub(crate) fn multiple_with_statements(
.unwrap_or_default()
.universal_newlines()
.all(|line| {
LineWidth::new(checker.settings.tab_size).add_str(&line)
LineWidthBuilder::new(checker.settings.tab_size).add_str(&line)
<= checker.settings.line_length
})
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use ruff_python_ast::traversal;
use ruff_python_codegen::Generator;

use crate::checkers::ast::Checker;
use crate::line_width::LineWidth;
use crate::line_width::LineWidthBuilder;
use crate::registry::AsRule;

/// ## What it does
Expand Down Expand Up @@ -99,7 +99,7 @@ pub(crate) fn convert_for_loop_to_any_all(checker: &mut Checker, stmt: &Stmt) {

// Don't flag if the resulting expression would exceed the maximum line length.
let line_start = checker.locator().line_start(stmt.start());
if LineWidth::new(checker.settings.tab_size)
if LineWidthBuilder::new(checker.settings.tab_size)
.add_str(&checker.locator().contents()[TextRange::new(line_start, stmt.start())])
.add_str(&contents)
> checker.settings.line_length
Expand Down Expand Up @@ -181,7 +181,7 @@ pub(crate) fn convert_for_loop_to_any_all(checker: &mut Checker, stmt: &Stmt) {

// Don't flag if the resulting expression would exceed the maximum line length.
let line_start = checker.locator().line_start(stmt.start());
if LineWidth::new(checker.settings.tab_size)
if LineWidthBuilder::new(checker.settings.tab_size)
.add_str(&checker.locator().contents()[TextRange::new(line_start, stmt.start())])
.add_str(&contents)
> checker.settings.line_length
Expand Down
9 changes: 5 additions & 4 deletions crates/ruff/src/rules/isort/format.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use ruff_python_codegen::Stylist;
use ruff_python_trivia::LineLength;

use crate::line_width::{LineLength, LineWidth};
use crate::line_width::LineWidthBuilder;

use super::types::{AliasData, CommentSet, ImportFromData, Importable};

Expand Down Expand Up @@ -46,7 +47,7 @@ pub(crate) fn format_import_from(
comments: &CommentSet,
aliases: &[(AliasData, CommentSet)],
line_length: LineLength,
indentation_width: LineWidth,
indentation_width: LineWidthBuilder,
stylist: &Stylist,
force_wrap_aliases: bool,
is_first: bool,
Expand Down Expand Up @@ -103,8 +104,8 @@ fn format_single_line(
aliases: &[(AliasData, CommentSet)],
is_first: bool,
stylist: &Stylist,
indentation_width: LineWidth,
) -> (String, LineWidth) {
indentation_width: LineWidthBuilder,
) -> (String, LineWidthBuilder) {
let mut output = String::with_capacity(CAPACITY);
let mut line_width = indentation_width;

Expand Down
Loading

0 comments on commit 499e311

Please sign in to comment.