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

fix(deserialize): unescape JSON strings #3414

Merged
merged 2 commits into from
Jul 11, 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
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)
Sec-ant marked this conversation as resolved.
Show resolved Hide resolved
}

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"]
}
]
}
}
}
}
}
}