Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
feat(rome_json_formatter): Fallback to Verbatim for nodes with syntax…
Browse files Browse the repository at this point in the history
… errors

Some prettier tests for JSON5 are failing after implementing array formatting in #4064

The tests fail because the formatting returns a `FormatError::SyntaxError` if a mandatory child node is missing but the error is never handled by the JSON formatting.

This PR fixes this by

* Gracefully handle missing values on the root value by calling into verbatim formatting if that's the case
* Use `format_or_verbatim` when formatting values

I had to pull out the `format_or_verbatim` from the `rome_js_formatter` and move it to `rome_formatter` so that it can be re-used between js and json formatting.
  • Loading branch information
MichaReiser committed Dec 17, 2022
1 parent fab5440 commit 4b19191
Show file tree
Hide file tree
Showing 15 changed files with 163 additions and 63 deletions.
4 changes: 3 additions & 1 deletion crates/rome_formatter/src/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ pub use crate::trivia::{
pub use crate::diagnostics::FormatError;
pub use crate::format_element::document::Document;
pub use crate::format_element::tag::{LabelId, Tag, TagKind};
pub use crate::verbatim::{format_bogus_node, format_suppressed_node, format_verbatim_node};
pub use crate::verbatim::{
format_bogus_node, format_or_verbatim, format_suppressed_node, format_verbatim_node,
};

pub use crate::{
best_fitting, dbg_write, format, format_args, write, Buffer as _, BufferExtensions, Format,
Expand Down
40 changes: 38 additions & 2 deletions crates/rome_formatter/src/verbatim.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::format_element::tag::VerbatimKind;
use crate::prelude::*;
use crate::trivia::{FormatLeadingComments, FormatTrailingComments};
use crate::{write, CstFormatContext};
use rome_rowan::{Direction, Language, SyntaxElement, SyntaxNode, TextRange};
use crate::{write, CstFormatContext, FormatWithRule};
use rome_rowan::{AstNode, Direction, Language, SyntaxElement, SyntaxNode, TextRange};

/// "Formats" a node according to its original formatting in the source text. Being able to format
/// a node "as is" is useful if a node contains syntax errors. Formatting a node with syntax errors
Expand Down Expand Up @@ -167,3 +167,39 @@ pub fn format_suppressed_node<L: Language>(node: &SyntaxNode<L>) -> FormatVerbat
format_comments: true,
}
}

/// Formats an object using its [`Format`] implementation but falls back to printing the object as
/// it is in the source document if formatting it returns an [`FormatError::SyntaxError`].
pub const fn format_or_verbatim<F>(inner: F) -> FormatNodeOrVerbatim<F> {
FormatNodeOrVerbatim { inner }
}

/// Formats a node or falls back to verbatim printing if formating this node fails.
#[derive(Copy, Clone)]
pub struct FormatNodeOrVerbatim<F> {
inner: F,
}

impl<F, Context, Item> Format<Context> for FormatNodeOrVerbatim<F>
where
F: FormatWithRule<Context, Item = Item>,
Item: AstNode,
Context: CstFormatContext<Language = Item::Language>,
{
fn fmt(&self, f: &mut Formatter<Context>) -> FormatResult<()> {
let snapshot = Formatter::state_snapshot(f);

match self.inner.fmt(f) {
Ok(result) => Ok(result),

Err(FormatError::SyntaxError) => {
f.restore_state_snapshot(snapshot);

// Lists that yield errors are formatted as they were suppressed nodes.
// Doing so, the formatter formats the nodes/tokens as is.
format_suppressed_node(self.inner.item().syntax()).fmt(f)
}
Err(err) => Err(err),
}
}
}
41 changes: 0 additions & 41 deletions crates/rome_js_formatter/src/builders.rs

This file was deleted.

2 changes: 1 addition & 1 deletion crates/rome_js_formatter/src/js/lists/class_member_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ impl FormatRule<JsClassMemberList> for FormatJsClassMemberList {
let mut join = f.join_nodes_with_hardline();

for member in node {
join.entry(member.syntax(), &format_or_verbatim(&member));
join.entry(member.syntax(), &format_or_verbatim(member.format()));
}

join.finish()
Expand Down
5 changes: 4 additions & 1 deletion crates/rome_js_formatter/src/js/lists/module_item_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ impl FormatRule<JsModuleItemList> for FormatJsModuleItemList {
join.entry_no_separator(&empty.format());
}
_ => {
join.entry(module_item.syntax(), &format_or_verbatim(&module_item));
join.entry(
module_item.syntax(),
&format_or_verbatim(module_item.format()),
);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_js_formatter/src/js/lists/statement_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ impl FormatRule<JsStatementList> for FormatJsStatementList {
join.entry_no_separator(&empty.format());
}
_ => {
join.entry(statement.syntax(), &format_or_verbatim(&statement));
join.entry(statement.syntax(), &format_or_verbatim(statement.format()));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_js_formatter/src/js/lists/switch_case_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ impl FormatRule<JsSwitchCaseList> for FormatJsSwitchCaseList {
let mut join = f.join_nodes_with_hardline();

for case in node {
join.entry(case.syntax(), &format_or_verbatim(&case));
join.entry(case.syntax(), &format_or_verbatim(case.format()));
}

join.finish()
Expand Down
1 change: 0 additions & 1 deletion crates/rome_js_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ pub mod utils;

#[rustfmt::skip]
mod generated;
mod builders;
pub mod comments;
pub mod context;
mod parentheses;
Expand Down
4 changes: 2 additions & 2 deletions crates/rome_js_formatter/src/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
//! when implementing a syntax formatter.

pub(crate) use crate::{
builders::format_or_verbatim, comments::JsComments, AsFormat as _, FormatNodeRule,
FormattedIterExt, JsFormatContext, JsFormatter,
comments::JsComments, AsFormat as _, FormatNodeRule, FormattedIterExt, JsFormatContext,
JsFormatter,
};
pub use rome_formatter::prelude::*;
pub use rome_formatter::separated::TrailingSeparator;
Expand Down
14 changes: 10 additions & 4 deletions crates/rome_json_formatter/src/json/auxiliary/member.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
use crate::prelude::*;
use rome_formatter::{format_args, write};
use rome_json_syntax::JsonMember;
use rome_json_syntax::{JsonMember, JsonMemberFields};

#[derive(Debug, Clone, Default)]
pub(crate) struct FormatJsonMember;

impl FormatNodeRule<JsonMember> for FormatJsonMember {
fn fmt_fields(&self, node: &JsonMember, f: &mut JsonFormatter) -> FormatResult<()> {
let JsonMemberFields {
name,
colon_token,
value,
} = node.as_fields();

write!(
f,
[group(&format_args![
&node.name().format(),
node.colon_token().format(),
&name.format(),
colon_token.format(),
space(),
node.value().format()
format_or_verbatim(value?.format())
])]
)
}
Expand Down
24 changes: 16 additions & 8 deletions crates/rome_json_formatter/src/json/auxiliary/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,21 @@ impl FormatNodeRule<JsonRoot> for FormatJsonRoot {
fn fmt_fields(&self, node: &JsonRoot, f: &mut JsonFormatter) -> FormatResult<()> {
let JsonRootFields { value, eof_token } = node.as_fields();

write!(
f,
[
value.format(),
format_removed(&eof_token?),
hard_line_break()
]
)
match &value {
Ok(value) => {
write!(
f,
[
format_or_verbatim(value.format()),
format_removed(&eof_token?),
hard_line_break()
]
)
}
// Don't fail formatting if the root contains no root value
Err(_) => {
write!(f, [format_verbatim_node(node.syntax())])
}
}
}
}
Empty file.
32 changes: 32 additions & 0 deletions crates/rome_json_formatter/tests/specs/json/empty.json.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
source: crates/rome_formatter_test/src/snapshot_builder.rs
info: "json\\empty.json"
---

# Input

```json
```


=============================

# Outputs

## Output 1

-----
Indent style: Tab
Line width: 80
-----

```json
```



## Unimplemented nodes/tokens

"" => 0..0

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"missing_value": {
"a":,
"b": 1
},

"b": 1,
"c": "2", "d": 3
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
---
source: crates/rome_formatter_test/src/snapshot_builder.rs
info: "json\\object\\missing_value.json"
---

# Input

```json
{
"missing_value": {
"a":,
"b": 1
},
"b": 1,
"c": "2", "d": 3
}
```


=============================

# Outputs

## Output 1

-----
Indent style: Tab
Line width: 80
-----

```json
{
"missing_value": {
"a":,
"b": 1
},
"b": 1,
"c": "2",
"d": 3
}
```


0 comments on commit 4b19191

Please sign in to comment.