Where did rewrite go?
Rustfmt visits each top level items, call functions to format each item,
and then pushes the returned String into buffer. If it returns Some(String)
then it is directly pushed into buffer, but if it returns None, original
code snippet (which may left unformatted) is pushed instead. Preserving
user written code on internal formatting error is natural, otherwise user has to suffer losing their code.
fn push_rewrite_inner(&mut self, span: Span, rewrite: Option<String>) {
if let Some(ref s) = rewrite {
self.push_str(s);
} else {
let snippet = self.snippet(span);
self.push_str(snippet.trim());
}
self.last_pos = source!(self, span).hi();
}
Where do we report internal formatting error?
Basically, internal formatting errors are recorded in FormatReport.
But the point where rustfmt appends the err to report differs.
Some errors are manually checked while we join the Strings in visitor's buffer.
Rustfmt iterates through all the chars and when it encounters new line character (\n),
it checks for TrailingWhiteSpace, and LineOverflow, and decides whether we should report these
based on rustfmt configurations. These errors are checked after all rewrite.
There are other errors that are reported during rewrites. fn recover_comment_removed checks
for any lost comment, and directly generates FormattingError and append it to FormatReport.
How do we emit error msg?
fn format_and_emit_report<T: Write>(session: &mut Session<'_, T>, input: Input) {
match session.format(input) {
Ok(report) => {
if report.has_warnings() {
eprintln!(
"{}",
FormatReportFormatterBuilder::new(&report)
.enable_colors(should_print_with_colors(session))
.build()
);
}
}
Err(msg) => {
eprintln!("Error writing files: {msg}");
session.add_operational_error();
}
}
}
Rustfmt has FormatReportFormatter for emitting error msg. One thing notable here is
not only this formatter adds some prefix message, but also it generates annotation snippets
based on given FormattingError. When we propagate our new RewriteError, since ExceedsMaxWidth has more information than ErrorKind::LineOverflow, we will be able to display where formatting failed based on span, not just marking last n chars.
Propagating errors
1. Return RewriteResult in format_***
These are the list of functions that I have to update. Compared to rewrite implementation
for each AST node, these functions are for formatting higher level item, while calling rewrite.
- [] rewrite_reorderable_or_regroupable_items
- [] functions in visit_item
-- Use format_import
-- Impl format_impl
-- Trait format_trait
-- TraitAlias format_trait_alias
-- ExternCrate rewrite_extern_crate
-- Struct visit_struct
-- Mod format_mod
-- MacCall visit_mac
-- ForeignMod format_foreign_mod
-- Static / Const visit_static
-- Fn (body) visit_fn
(only sig) rewrite_required_fn
-- TyAlias visit_ty_alias_kind
-- GlobalAsm (nothing to do)
-- MacroDef rewrite_macro_def
-- Delegation (currently pushing None since it does not support rewrite delegation)
2. Append FormattingError while Converting RewriteError to ErrorKind
We can modify push_rewrite_inner to take RewriteResult as input,
and report RewriteError here. Still, original code snippet will be pushed into the buffer
when we encounter RewriteError, but we can convert RewriteError into ErrorKind
and then append a FormattingError.
fn push_rewrite_inner(&mut self, span: Span, rewrite: RewriteResult) {
match rewrite {
Ok(ref s) => {
self.push_str(s);
},
Err(err) => {
// push original str on err
let snippet = self.snippet(span);
self.push_str(snippet.trim());
// to track err
self.report.append(
file_name,
vec![FormattingError::from_span(
span,
self.psess,
ErrorKind::TODO,
)],
);
},
}
self.last_pos = source!(self, span).hi();
}
We also need to visit functions to create snippet annotations and try to highlight corresponding span.
// (space, target)
pub(crate) fn format_len(&self) -> (usize, usize) {
match self.kind {
ErrorKind::LineOverflow(found, max) => (max, found - max),
ErrorKind::TrailingWhitespace
| ErrorKind::DeprecatedAttr
| ErrorKind::BadAttr
| ErrorKind::LostComment => {
let trailing_ws_start = self
.line_buffer
.rfind(|c: char| !c.is_whitespace())
.map(|pos| pos + 1)
.unwrap_or(0);
(
trailing_ws_start,
self.line_buffer.len() - trailing_ws_start,
)
}
_ => unreachable!(),
}
}
Issue 1. Dealing with unknown error variant
If we encounter unknown_error, what should we do? Should we tell users that there is some sort of suspicious internal error? Or should we just ignore this on push_rewrite_inner? I guess we will try to give at least generic error message to user since this unknown_error may indicate a rustfmt bug.
Issue 2. Max Width Error in each node level vs line level
The err variant ExceedsMaxWidth in RewriteError and LineOverflow in ErrorKind
overlaps. The former is the one we introduced by this project, containing more detailed
information and the latter is generated when joining the result of rewrites. It may be possible to have duplicated error for single overflow error, one propagated up from RewriteError and the other manually checked in the fn new_line.
Probably we want to emit only one error, so I'll have to filter out these and keep only one. Hopefully we have line number and span information, and the only thing I need to do is
making sure that this works.
But we may have to keep line overflow checking logic in the fn new_line since
there may be unknown_errors that is actually max width issue. Besides, I need to check
whether it is possible to overflow the line max width even if we did not encounter
any RewriteError on formatting.
Also, we need to investigate whether it is possible to move manual error checking
logic from the function new_line to push_rewrite.
Issue 3. Reorganizing ErrorKind
#[derive(Error, Debug)]
pub enum ErrorKind {
/// Line has exceeded character limit (found, maximum).
#[error(
"line formatted, but exceeded maximum width \
(maximum: {1} (see `max_width` option), found: {0})"
)]
LineOverflow(usize, usize),
/// Line ends in whitespace.
#[error("left behind trailing whitespace")]
TrailingWhitespace,
/// Used deprecated skip attribute.
#[error("`rustfmt_skip` is deprecated; use `rustfmt::skip`")]
DeprecatedAttr,
/// Used a rustfmt:: attribute other than skip or skip::macros.
#[error("invalid attribute")]
BadAttr,
/// An io error during reading or writing.
#[error("io error: {0}")]
IoError(io::Error),
/// Error during module resolution.
#[error("{0}")]
ModuleResolutionError(#[from] ModuleResolutionError),
/// Parse error occurred when parsing the input.
#[error("parse error")]
ParseError,
/// The user mandated a version and the current version of Rustfmt does not
/// satisfy that requirement.
#[error("version mismatch")]
VersionMismatch,
/// If we had formatted the given node, then we would have lost a comment.
#[error("not formatted because a comment would be lost")]
LostComment,
/// Invalid glob pattern in `ignore` configuration option.
#[error("Invalid glob pattern found in ignore list: {0}")]
InvalidGlobPattern(ignore::Error),
// TODO ! We'll add this variant
#[error("Rewrite Error: {0}")]
RewriteError(#[from] RewriteError)
}
When converting RewriteError to ErrorKind, we may consider reorganizing the enum varaints
of ErrorKind. Currently ErrorKind does not group the errors that can pop up only in
formatting stage. Considering that rustfmt creates FormattingError from this subset of
ErrorKind, it will worth refactoring this part. Anyway, since our primary goal is to
propagate the RewriteError and display improved err message, we'll leave this issue as further improvement.