Skip to content

Commit

Permalink
fix(deserialize): unescape JSON strings (#3414)
Browse files Browse the repository at this point in the history
Co-authored-by: Emanuele Stoppa <[email protected]>
  • Loading branch information
Conaclos and ematipico authored Jul 11, 2024
1 parent a24d361 commit 6163e58
Show file tree
Hide file tree
Showing 8 changed files with 162 additions and 12 deletions.
26 changes: 19 additions & 7 deletions crates/biome_deserialize/src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,39 @@ use std::{
path::PathBuf,
};

/// Type that allows deserializing a string without heap-allocation.
/// Type that allows deserializing a string without heap-allocation when possible.
/// This is analog to [std::borrow::Cow]:
#[derive(Debug, Eq, PartialEq, Hash, Clone)]
pub struct Text(pub(crate) TokenText);
pub enum Text {
Borrowed(TokenText),
Owned(String),
}
impl Text {
pub fn text(&self) -> &str {
self.0.text()
match self {
Text::Borrowed(token_text) => token_text.text(),
Text::Owned(string) => string,
}
}
}
impl From<Text> for String {
fn from(value: Text) -> Self {
match value {
Text::Borrowed(token_text) => token_text.text().to_string(),
Text::Owned(string) => string,
}
}
}

impl PartialOrd for Text {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
Some(self.cmp(other))
}
}

impl Ord for Text {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.text().cmp(other.text())
}
}

impl Deref for Text {
type Target = str;
fn deref(&self) -> &Self::Target {
Expand Down Expand Up @@ -482,7 +494,7 @@ impl Deserializable for String {
name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self> {
Text::deserialize(value, name, diagnostics).map(|value| value.text().to_string())
Text::deserialize(value, name, diagnostics).map(|value| value.into())
}
}

Expand Down
23 changes: 18 additions & 5 deletions crates/biome_deserialize/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
use biome_diagnostics::{DiagnosticExt, Error};
use biome_json_parser::{parse_json, JsonParserOptions};
use biome_json_syntax::{AnyJsonValue, JsonMemberName, JsonRoot, T};
use biome_rowan::{AstNode, AstSeparatedList};
use biome_rowan::{AstNode, AstSeparatedList, TokenText};

/// It attempts to parse and deserialize a source file in JSON. Diagnostics from the parse phase
/// are consumed and joined with the diagnostics emitted during the deserialization.
Expand Down Expand Up @@ -119,8 +119,8 @@ impl DeserializableValue for AnyJsonValue {
visitor.visit_map(members, range, name, diagnostics)
}
AnyJsonValue::JsonStringValue(value) => {
let value = value.inner_string_text().ok()?;
visitor.visit_str(Text(value), range, name, diagnostics)
let value = unescape_json(value.inner_string_text().ok()?);
visitor.visit_str(value, range, name, diagnostics)
}
}
}
Expand Down Expand Up @@ -245,15 +245,28 @@ impl DeserializableValue for JsonMemberName {
name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<V::Output> {
let value = self.inner_string_text().ok()?;
visitor.visit_str(Text(value), AstNode::range(self), name, diagnostics)
let value = unescape_json(self.inner_string_text().ok()?);
visitor.visit_str(value, AstNode::range(self), name, diagnostics)
}

fn visitable_type(&self) -> Option<VisitableType> {
Some(VisitableType::STR)
}
}

/// Returns an unescaped version of `s`.
/// If nothing is escaped, then `s` is returned without any allocation.
/// If at least one character is escaped, then a string is allocated and holds the unescaped string.
fn unescape_json(s: TokenText) -> Text {
if s.text().bytes().any(|c| c == b'\\') {
// Searching and replacing at the same time should be more optimal.
// However, strings are expected to be small and escapees are expected to be rare.
Text::Owned(s.text().replace(r"\\", r"\"))
} else {
Text::Borrowed(s)
}
}

#[cfg(test)]
mod tests {
use std::{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
"linter": {
"rules": {
"style": {
"useNamingConvention": {
"level": "error",
"options": {
"conventions": [
{
"selector": {
"kind": "const"
},
"match": "(.*?)[$]?",
"formats": ["camelCase"]
}
]
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: invalidCustomRegexAnchor.js
---
# Input
```jsx
{
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
"linter": {
"rules": {
"style": {
"useNamingConvention": {
"level": "error",
"options": {
"conventions": [
{
"selector": {
"kind": "const"
},
"match": "(.*?)[$]?",
"formats": ["camelCase"]
}
]
}
}
}
}
}
}

```

# Diagnostics
```
invalidCustomRegexAnchor.options:14:18 deserialize ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Anchors `^` and `$` are not supported. They are implciitly present.
12 │ "kind": "const"
13 │ },
> 14 │ "match": "(.*?)$",
│ ^^^^^^^^
15 │ "formats": ["camelCase"]
16 │ }
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
"linter": {
"rules": {
"style": {
"useNamingConvention": {
"level": "error",
"options": {
"conventions": [
{
"selector": {
"kind": "const"
},
"match": "(.*?)$",
"formats": ["camelCase"]
}
]
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
const x$ = 0;
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: validCustomStyleDollarSuffix.js
---
# Input
```jsx
const x$ = 0;
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
"linter": {
"rules": {
"style": {
"useNamingConvention": {
"level": "error",
"options": {
"conventions": [
{
"selector": {
"kind": "const"
},
"match": "(.*?)\\$",
"formats": ["camelCase"]
}
]
}
}
}
}
}
}

0 comments on commit 6163e58

Please sign in to comment.