Skip to content

Commit

Permalink
Unify line size settings between ruff and the formatter (#6873)
Browse files Browse the repository at this point in the history
  • Loading branch information
konstin committed Aug 28, 2023
1 parent a6aa166 commit e615870
Show file tree
Hide file tree
Showing 28 changed files with 217 additions and 180 deletions.
3 changes: 2 additions & 1 deletion crates/flake8_to_ruff/src/black.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
//! Extract Black configuration settings from a pyproject.toml.

use ruff::line_width::LineLength;
use ruff::settings::types::PythonVersion;
use serde::{Deserialize, Serialize};

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

use itertools::Itertools;

Expand Down Expand Up @@ -119,10 +120,8 @@ pub(crate) 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 @@ -405,7 +404,7 @@ pub(crate) 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 @@ -512,7 +511,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 @@ -529,7 +528,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
10 changes: 4 additions & 6 deletions crates/ruff/src/checkers/physical_lines.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
//! Lint rules based on checking physical lines.
use ruff_text_size::TextSize;

use ruff_diagnostics::Diagnostic;
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
use ruff_source_file::{Locator, UniversalNewlines};
use ruff_text_size::TextSize;

use crate::registry::Rule;
use crate::rules::flake8_copyright::rules::missing_copyright_notice;
Expand Down Expand Up @@ -99,11 +98,10 @@ pub(crate) fn check_physical_lines(

#[cfg(test)]
mod tests {
use ruff_python_parser::lexer::lex;
use ruff_python_parser::Mode;

use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
use ruff_python_parser::lexer::lex;
use ruff_python_parser::Mode;
use ruff_source_file::Locator;

use crate::line_width::LineLength;
Expand Down Expand Up @@ -132,7 +130,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
129 changes: 105 additions & 24 deletions crates/ruff/src/line_width.rs
Original file line number Diff line number Diff line change
@@ -1,30 +1,111 @@
use ruff_cache::{CacheKey, CacheKeyHasher};
use serde::{Deserialize, Serialize};
use std::num::NonZeroU8;
use std::error::Error;
use std::hash::Hasher;
use std::num::{NonZeroU16, NonZeroU8, ParseIntError};
use std::str::FromStr;
use unicode_width::UnicodeWidthChar;

use ruff_macros::CacheKey;

/// The length of a line of text that is considered too long.
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, CacheKey)]
///
/// The allowed range of values is 1..=320
#[derive(Clone, Copy, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct LineLength(usize);
pub struct LineLength(NonZeroU16);

impl LineLength {
/// Maximum allowed value for a valid [`LineLength`]
pub const MAX: u16 = 320;

/// Return the numeric value for this [`LineLength`]
pub fn value(&self) -> u16 {
self.0.get()
}
}

impl Default for LineLength {
/// The default line length.
fn default() -> Self {
Self(88)
Self(NonZeroU16::new(88).unwrap())
}
}

impl LineLength {
pub const fn get(&self) -> usize {
self.0
impl CacheKey for LineLength {
fn cache_key(&self, state: &mut CacheKeyHasher) {
state.write_u16(self.0.get());
}
}

/// Error type returned when parsing a [`LineLength`] from a string fails
pub enum ParseLineWidthError {
/// The string could not be parsed as a valid [u16]
ParseError(ParseIntError),
/// The [u16] value of the string is not a valid [LineLength]
TryFromIntError(LineLengthFromIntError),
}

impl std::fmt::Debug for ParseLineWidthError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Display::fmt(self, f)
}
}

impl std::fmt::Display for ParseLineWidthError {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ParseLineWidthError::ParseError(err) => std::fmt::Display::fmt(err, fmt),
ParseLineWidthError::TryFromIntError(err) => std::fmt::Display::fmt(err, fmt),
}
}
}

impl Error for ParseLineWidthError {}

impl FromStr for LineLength {
type Err = ParseLineWidthError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let value = u16::from_str(s).map_err(ParseLineWidthError::ParseError)?;
let value = Self::try_from(value).map_err(ParseLineWidthError::TryFromIntError)?;
Ok(value)
}
}

/// Error type returned when converting a u16 to a [`LineLength`] fails
#[derive(Clone, Copy, Debug)]
pub struct LineLengthFromIntError(pub u16);

impl TryFrom<u16> for LineLength {
type Error = LineLengthFromIntError;

fn try_from(value: u16) -> Result<Self, Self::Error> {
match NonZeroU16::try_from(value) {
Ok(value) if value.get() <= Self::MAX => Ok(LineLength(value)),
Ok(_) | Err(_) => Err(LineLengthFromIntError(value)),
}
}
}

impl std::fmt::Display for LineLengthFromIntError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(
f,
"The line width must be a value between 1 and {}.",
LineLength::MAX
)
}
}

impl From<LineLength> for u16 {
fn from(value: LineLength) -> Self {
value.0.get()
}
}

impl From<usize> for LineLength {
fn from(value: usize) -> Self {
Self(value)
impl From<LineLength> for NonZeroU16 {
fn from(value: LineLength) -> Self {
value.0
}
}

Expand All @@ -33,7 +114,7 @@ impl From<usize> for LineLength {
/// 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 +124,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 +200,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 +211,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::{Ranged, 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 @@ -15,7 +15,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 @@ -414,7 +414,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 @@ -660,7 +660,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 @@ -964,7 +964,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::{Ranged, 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
Loading

0 comments on commit e615870

Please sign in to comment.