Intro
The mid-term evaluation period of the GSoC program has already passed, and there's just a little over a month left for the coding period of the project. I want to reflect on where my project, "Rewriting the Rewrite Trait" stands. This project involves refactoring the rewrite trait and its implementation used for internal formatting mechanism, in order to track error context.
pub(crate) trait Rewrite {
/// Rewrite self into shape.
fn rewrite(&self, context: RewriteContext<'_>, shape: Shape) -> Option<String>;
}
From an implementation perspective, the primary goal is to update this rewrite method to return some sort of Result type so that we can track the error context on rewrite failure.
The original plan in my initial GSoC proposal
was to reuse exisiting enum ErrorKind
to represent rewrite errors. However, some of the
enum variants for ErrorKind
represents errors that could not occur during
these rewrites. When my mentor informed me that I could freely design a new enum or add a struct, I realized I had been narrowly focused on sticking to the existing codebase.
This led me to rethink the project plan, and I spent time discussing the new plan with my mentor during the community bonding weeks.
Updated plan & progress
Our updated plan consists of 3 steps:
designing a new struct to represent our new error type, updating the codebase to return the Result type, and actually propagating these errors.
(1) design a new dedicated struct
First, we need to add a RewriteError
enum for the error type of Result that we'll return.
After invesitigating all the rewrite
implementation for ast nodes, I marked the points
where we originally return None
. The majority of failures were due to exceeding the max width, where rustfmt tries to calculate the shape for a given node but finds it does not fit within the specified width.
For exceeds max width error, we hold width
and span
for detailed information.
Instead of the configuration's max_width, which is shown to the user on max width error currently, we'll try to save a narrower, exact span and width that might be useful in the future
Other variants include SkipFormatting and macro failures.
We could add more variants, but we decided to keep it minimal at first and then add a new one when needed.
(2) refactor & impl rewrite_result
add a new method: rewrite_result
Now that we have a Result type with our new RewriteError, called RewriteResult
, the core concern at this stage was, "how do we make a gradual change from returning Option to RewriteResult?" We can't directly update the rewrite method to return RewriteResult because this method has numerous implementations and call sites.
Therefore, we decided to add a new method rewrite_result
that returns RewriteResult.
Except for the error handling part, the formatting logic should remain the same for rewrite and rewrite_result.
To avoid writing duplicated code for rewrite_result, I first implemented rewrite_result based on the existing rewrite implementation,
and then replaced the rewrite implementation with calling rewrite_result.ok()
. Once this project proceeds, rewrite method will
be replaced with rewrite_result eventually. Considering that the rewrite method will be removed some day,
rewrite_result should calls rewrite_result for its sub node/expr formatting instead of rewrite. For this,
I added a default implementation for rewrite_result.
extension trait
While implementing rewrite_result methods and updating other related helper functions, I often had to
convert Option to RewriteResult. To be specific, I need to add logic to return corresponding RewriteError
if given value was None, or convert Some(value) into Ok(value) if it's not the case.
Since rust Option type is built in type, I could not add custom methods directly for that. Instead of
defining new helper functions, we added a extension trait RewriteErrExt
. Thereby, we could easily
call methods like max_width_error
to convert Option into RewriteResult especially when None indicates
we failed to fit in given shape.
This project is currently at the end of (2) second stage, and need to move to propagation part soon. Above about 40 ast nodes, about 60-70% has rewrite_result implementation. You can find more detailed checklist as tracking issue.
(3) propagation
The final step is to replace the rewrite method with the rewrite_result method and propagate the returned RewriteResult to the upper-level call sites. This process involves modifying the functions that visit and format top-level items (functions, traits, etc.), rather than just the implementations of the rewrite method. Additionally, we need to consider how to modify the existing error reporting logic and how to transform RewriteError into ErrorKind.
Ongoing concerns & thoughts
Reflecting on the considerations I've had while progressing through the project, the following come to mind: What information should be included in RewriteError so that the user can understand the reason behind the failure of code formatting from the error message and, if possible, make corrections? Specifically, for max width errors, I currently include the width and span, but sometimes, due to the logic, I have to return a wider range instead of the specific span. Additionally, when converting Option to the Result type, it can sometimes be challenging to determine how to represent what kind of error it indicates.