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

Unify line size settings between ruff and the formatter #6873

Merged
merged 1 commit into from
Aug 28, 2023
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
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
Loading