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

refactor rewrite_literal & combine_strs_with_missing_comments #6250

Merged
merged 3 commits into from
Aug 3, 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
54 changes: 33 additions & 21 deletions src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::config::IndentStyle;
use crate::expr::rewrite_literal;
use crate::lists::{definitive_tactic, itemize_list, write_list, ListFormatting, Separator};
use crate::overflow;
use crate::rewrite::{Rewrite, RewriteContext, RewriteErrorExt};
use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
use crate::shape::Shape;
use crate::source_map::SpanUtils;
use crate::types::{rewrite_path, PathContext};
Expand Down Expand Up @@ -217,9 +217,9 @@ fn rewrite_initial_doc_comments(
context: &RewriteContext<'_>,
attrs: &[ast::Attribute],
shape: Shape,
) -> Option<(usize, Option<String>)> {
) -> Result<(usize, Option<String>), RewriteError> {
if attrs.is_empty() {
return Some((0, None));
return Ok((0, None));
}
// Rewrite doc comments
let sugared_docs = take_while_with_pred(context, attrs, |a| a.is_doc_comment());
Expand All @@ -229,7 +229,7 @@ fn rewrite_initial_doc_comments(
.map(|a| context.snippet(a.span))
.collect::<Vec<_>>()
.join("\n");
return Some((
return Ok((
sugared_docs.len(),
Some(rewrite_doc_comment(
&snippet,
Expand All @@ -239,13 +239,19 @@ fn rewrite_initial_doc_comments(
));
}

Some((0, None))
Ok((0, None))
}

impl Rewrite for ast::NestedMetaItem {
fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
self.rewrite_result(context, shape).ok()
}

fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
match self {
ast::NestedMetaItem::MetaItem(ref meta_item) => meta_item.rewrite(context, shape),
ast::NestedMetaItem::MetaItem(ref meta_item) => {
meta_item.rewrite_result(context, shape)
}
ast::NestedMetaItem::Lit(ref l) => {
rewrite_literal(context, l.as_token_lit(), l.span, shape)
}
Expand Down Expand Up @@ -277,11 +283,7 @@ impl Rewrite for ast::MetaItem {
self.rewrite_result(context, shape).ok()
}

fn rewrite_result(
&self,
context: &RewriteContext<'_>,
shape: Shape,
) -> crate::rewrite::RewriteResult {
fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
Ok(match self.kind {
ast::MetaItemKind::Word => {
rewrite_path(context, PathContext::Type, &None, &self.path, shape)?
Expand Down Expand Up @@ -317,7 +319,7 @@ impl Rewrite for ast::MetaItem {
// is longer than the max width and continue on formatting.
// See #2479 for example.
let value = rewrite_literal(context, lit.as_token_lit(), lit.span, lit_shape)
.unwrap_or_else(|| context.snippet(lit.span).to_owned());
.unwrap_or_else(|_| context.snippet(lit.span).to_owned());
format!("{path} = {value}")
}
})
Expand All @@ -326,6 +328,10 @@ impl Rewrite for ast::MetaItem {

impl Rewrite for ast::Attribute {
fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
self.rewrite_result(context, shape).ok()
}

fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
let snippet = context.snippet(self.span);
if self.is_doc_comment() {
rewrite_doc_comment(snippet, shape.comment(context.config), context.config)
Expand All @@ -337,7 +343,7 @@ impl Rewrite for ast::Attribute {
let prefix = attr_prefix(self);

if should_skip || contains_comment(snippet) {
return Some(snippet.to_owned());
return Ok(snippet.to_owned());
}

if let Some(ref meta) = self.meta() {
Expand All @@ -362,9 +368,11 @@ impl Rewrite for ast::Attribute {
}

// 1 = `[`
let shape = shape.offset_left(prefix.len() + 1)?;
Some(meta.rewrite(context, shape).map_or_else(
|| snippet.to_owned(),
let shape = shape
.offset_left(prefix.len() + 1)
.max_width_error(shape.width, self.span)?;
Ok(meta.rewrite_result(context, shape).map_or_else(
|_| snippet.to_owned(),
|rw| match &self.kind {
ast::AttrKind::Normal(normal_attr) => match normal_attr.item.unsafety {
// For #![feature(unsafe_attributes)]
Expand All @@ -376,16 +384,20 @@ impl Rewrite for ast::Attribute {
},
))
} else {
Some(snippet.to_owned())
Ok(snippet.to_owned())
}
}
}
}

impl Rewrite for [ast::Attribute] {
fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
self.rewrite_result(context, shape).ok()
}

fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
if self.is_empty() {
return Some(String::new());
return Ok(String::new());
}

// The current remaining attributes.
Expand All @@ -401,7 +413,7 @@ impl Rewrite for [ast::Attribute] {
// merging derives into a single attribute.
loop {
if attrs.is_empty() {
return Some(result);
return Ok(result);
}

// Handle doc comments.
Expand Down Expand Up @@ -440,7 +452,7 @@ impl Rewrite for [ast::Attribute] {
// Handle derives if we will merge them.
if !skip_derives && context.config.merge_derives() && is_derive(&attrs[0]) {
let derives = take_while_with_pred(context, attrs, is_derive);
let derive_str = format_derive(derives, shape, context)?;
let derive_str = format_derive(derives, shape, context).unknown_error()?;
result.push_str(&derive_str);

let missing_span = attrs
Expand Down Expand Up @@ -473,7 +485,7 @@ impl Rewrite for [ast::Attribute] {
// If we get here, then we have a regular attribute, just handle one
// at a time.

let formatted_attr = attrs[0].rewrite(context, shape)?;
let formatted_attr = attrs[0].rewrite_result(context, shape)?;
result.push_str(&formatted_attr);

let missing_span = attrs
Expand Down
17 changes: 11 additions & 6 deletions src/chains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ use crate::config::{IndentStyle, StyleEdition};
use crate::expr::rewrite_call;
use crate::lists::extract_pre_comment;
use crate::macros::convert_try_mac;
use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteResult};
use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
use crate::shape::Shape;
use crate::source_map::SpanUtils;
use crate::utils::{
Expand Down Expand Up @@ -268,7 +268,13 @@ impl ChainItemKind {

impl Rewrite for ChainItem {
fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
let shape = shape.sub_width(self.tries)?;
self.rewrite_result(context, shape).ok()
}

fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
let shape = shape
.sub_width(self.tries)
.max_width_error(shape.width, self.span)?;
let rewrite = match self.kind {
ChainItemKind::Parent {
ref expr,
Expand All @@ -277,10 +283,9 @@ impl Rewrite for ChainItem {
ChainItemKind::Parent {
ref expr,
parens: false,
} => expr.rewrite(context, shape)?,
} => expr.rewrite_result(context, shape)?,
ChainItemKind::MethodCall(ref segment, ref types, ref exprs) => {
Self::rewrite_method_call(segment.ident, types, exprs, self.span, context, shape)
.ok()?
Self::rewrite_method_call(segment.ident, types, exprs, self.span, context, shape)?
}
ChainItemKind::StructField(ident) => format!(".{}", rewrite_ident(context, ident)),
ChainItemKind::TupleField(ident, nested) => format!(
Expand All @@ -297,7 +302,7 @@ impl Rewrite for ChainItem {
rewrite_comment(comment, false, shape, context.config)?
}
};
Some(format!("{rewrite}{}", "?".repeat(self.tries)))
Ok(format!("{rewrite}{}", "?".repeat(self.tries)))
}
}

Expand Down
34 changes: 17 additions & 17 deletions src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use itertools::{multipeek, MultiPeek};
use rustc_span::Span;

use crate::config::Config;
use crate::rewrite::RewriteContext;
use crate::rewrite::{RewriteContext, RewriteErrorExt, RewriteResult};
use crate::shape::{Indent, Shape};
use crate::string::{rewrite_string, StringFormat};
use crate::utils::{
Expand Down Expand Up @@ -157,7 +157,7 @@ pub(crate) fn combine_strs_with_missing_comments(
span: Span,
shape: Shape,
allow_extend: bool,
) -> Option<String> {
) -> RewriteResult {
trace!(
"combine_strs_with_missing_comments `{}` `{}` {:?} {:?}",
prev_str, next_str, span, shape
Expand Down Expand Up @@ -187,7 +187,7 @@ pub(crate) fn combine_strs_with_missing_comments(
result.push_str(&indent.to_string_with_newline(config))
}
result.push_str(next_str);
return Some(result);
return Ok(result);
}

// We have a missing comment between the first expression and the second expression.
Expand Down Expand Up @@ -232,10 +232,10 @@ pub(crate) fn combine_strs_with_missing_comments(
result.push_str(&second_sep);
result.push_str(next_str);

Some(result)
Ok(result)
}

pub(crate) fn rewrite_doc_comment(orig: &str, shape: Shape, config: &Config) -> Option<String> {
pub(crate) fn rewrite_doc_comment(orig: &str, shape: Shape, config: &Config) -> RewriteResult {
identify_comment(orig, false, shape, config, true)
}

Expand All @@ -244,7 +244,7 @@ pub(crate) fn rewrite_comment(
block_style: bool,
shape: Shape,
config: &Config,
) -> Option<String> {
) -> RewriteResult {
identify_comment(orig, block_style, shape, config, false)
}

Expand All @@ -254,7 +254,7 @@ fn identify_comment(
shape: Shape,
config: &Config,
is_doc_comment: bool,
) -> Option<String> {
) -> RewriteResult {
let style = comment_style(orig, false);

// Computes the byte length of line taking into account a newline if the line is part of a
Expand Down Expand Up @@ -346,7 +346,7 @@ fn identify_comment(
let (first_group, rest) = orig.split_at(first_group_ending);
let rewritten_first_group =
if !config.normalize_comments() && has_bare_lines && style.is_block_comment() {
trim_left_preserve_layout(first_group, shape.indent, config)?
trim_left_preserve_layout(first_group, shape.indent, config).unknown_error()?
} else if !config.normalize_comments()
&& !config.wrap_comments()
&& !(
Expand All @@ -367,7 +367,7 @@ fn identify_comment(
)?
};
if rest.is_empty() {
Some(rewritten_first_group)
Ok(rewritten_first_group)
} else {
identify_comment(
rest.trim_start(),
Expand Down Expand Up @@ -899,7 +899,7 @@ fn rewrite_comment_inner(
shape: Shape,
config: &Config,
is_doc_comment: bool,
) -> Option<String> {
) -> RewriteResult {
let mut rewriter = CommentRewrite::new(orig, block_style, shape, config);

let line_breaks = count_newlines(orig.trim_end());
Expand Down Expand Up @@ -933,7 +933,7 @@ fn rewrite_comment_inner(
}
}

Some(rewriter.finish())
Ok(rewriter.finish())
}

const RUSTFMT_CUSTOM_COMMENT_PREFIX: &str = "//#### ";
Expand Down Expand Up @@ -998,15 +998,15 @@ pub(crate) fn rewrite_missing_comment(
span: Span,
shape: Shape,
context: &RewriteContext<'_>,
) -> Option<String> {
) -> RewriteResult {
let missing_snippet = context.snippet(span);
let trimmed_snippet = missing_snippet.trim();
// check the span starts with a comment
let pos = trimmed_snippet.find('/');
if !trimmed_snippet.is_empty() && pos.is_some() {
rewrite_comment(trimmed_snippet, false, shape, context.config)
} else {
Some(String::new())
Ok(String::new())
}
}

Expand All @@ -1018,13 +1018,13 @@ pub(crate) fn recover_missing_comment_in_span(
shape: Shape,
context: &RewriteContext<'_>,
used_width: usize,
) -> Option<String> {
) -> RewriteResult {
let missing_comment = rewrite_missing_comment(span, shape, context)?;
if missing_comment.is_empty() {
Some(String::new())
Ok(String::new())
} else {
let missing_snippet = context.snippet(span);
let pos = missing_snippet.find('/')?;
let pos = missing_snippet.find('/').unknown_error()?;
// 1 = ` `
let total_width = missing_comment.len() + used_width + 1;
let force_new_line_before_comment =
Expand All @@ -1034,7 +1034,7 @@ pub(crate) fn recover_missing_comment_in_span(
} else {
Cow::from(" ")
};
Some(format!("{sep}{missing_comment}"))
Ok(format!("{sep}{missing_comment}"))
}
}

Expand Down
Loading
Loading