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(js_formatter): normalize keys then check if quotes are needed #3558

Merged
merged 1 commit into from
Jul 31, 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
114 changes: 2 additions & 112 deletions crates/biome_js_formatter/report.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
$$average = \frac\{\sum_{file}^\{files}compatibility_\{file}}\{files}$$
</details>

**Compatible lines**: 98.09
**Compatible lines**: 98.11

<details>
<summary>Definition</summary>
Expand Down Expand Up @@ -4301,118 +4301,8 @@


### js/quote-props/objects.js
```diff
a = {
a: "a",
};

a = {
b: "b",
};

a = {
// Escapes should stay as escapes and not be unquoted.
"\u0062": "b",
"\u0031": "1",
};

a = {
c1: "c1",
c2: "c2",
};

a = {
d1: "d1",
"d-2": "d2",
};

// None of these should become quoted, regardless of the quoteProps value.
a = {
NaN: null,
1: null,
1.5: null,
0.1: null,
1: null,
1.0: null,
999999999999999999999: null,
0.99999999999999999: null,
1e2: null,
1e3: null,
1e100: null,
0b10: null,
0o10: null,
0xf: null,
2n: null,
};

a = {
// These should be unquoted for quoteProps=as-needed.
NaN: null,
1: null,
1.5: null,
// These should never be unquoted. `1e+100` technically could (it’s the only
// one where `String(Number(key)) === key`), but we came to the conclusion
// that it is unexpected.
".1": null,
"1.": null,
"1.0": null,
"999999999999999999999": null,
"0.99999999999999999": null,
"1E2": null,
"1e+3": null,
"1e+100": null,
"0b10": null,
"0o10": null,
"0xf": null,
"2n": null,
};

Object.entries({
// To force quotes for quoteProps=consistent.
"a-": "a-",
// These can be quoted:
NaN: "NaN",
1: "1",
1.5: "1.5",
// Prettier will normalize these to `0.1` and `1` – then they can be quoted.
0.1: ".1",
1: "1.",
// These should never be quoted. The _actual_ keys are shown as comments.
// Copy-paste this into the console to verify. If we were to convert these
// numbers into decimal (which completely valid), “information/intent” is
// lost. Either way, writing code like this is super confusing.
1.0: "1.0", // 1
999999999999999999999: "999999999999999999999", // 1e+21
0.99999999999999999: "0.99999999999999999", // 1
1e2: "1E2", // 100
1e3: "1e+3", // 1000
1e100: "1e+100", // 1e+100 – this one is identical, but would be inconsistent to quote.
0b10: "0b10", // 2
0o10: "0o10", // 8
0xf: "0xf", // 15
2n: "2n", // 2
0xan: "0xan", // 10
});

// Negative numbers cannot be unquoted.
!{
"-1": null,
"-1.5": null,
};

-a = {
- "a": 1,
- b: 2,
-};
+// FIXME: reformat issue
+//a = {
+// "\a": 1,
+// "b": 2
+//}

```

**Prettier Similarity**: 95.15%
**Prettier Similarity**: 100.00%


### js/quote-props/with_member_expressions.js
Expand Down
117 changes: 1 addition & 116 deletions crates/biome_js_formatter/report_incompatible.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
$$average = \frac\{\sum_{file}^\{files}compatibility_\{file}}\{files}$$
</details>

**Compatible lines**: 98.09
**Compatible lines**: 98.11

<details>
<summary>Definition</summary>
Expand Down Expand Up @@ -1610,121 +1610,6 @@
**Prettier Similarity**: 91.04%


### js/quote-props/objects.js
```diff
a = {
a: "a",
};

a = {
b: "b",
};

a = {
// Escapes should stay as escapes and not be unquoted.
"\u0062": "b",
"\u0031": "1",
};

a = {
c1: "c1",
c2: "c2",
};

a = {
d1: "d1",
"d-2": "d2",
};

// None of these should become quoted, regardless of the quoteProps value.
a = {
NaN: null,
1: null,
1.5: null,
0.1: null,
1: null,
1.0: null,
999999999999999999999: null,
0.99999999999999999: null,
1e2: null,
1e3: null,
1e100: null,
0b10: null,
0o10: null,
0xf: null,
2n: null,
};

a = {
// These should be unquoted for quoteProps=as-needed.
NaN: null,
1: null,
1.5: null,
// These should never be unquoted. `1e+100` technically could (it’s the only
// one where `String(Number(key)) === key`), but we came to the conclusion
// that it is unexpected.
".1": null,
"1.": null,
"1.0": null,
"999999999999999999999": null,
"0.99999999999999999": null,
"1E2": null,
"1e+3": null,
"1e+100": null,
"0b10": null,
"0o10": null,
"0xf": null,
"2n": null,
};

Object.entries({
// To force quotes for quoteProps=consistent.
"a-": "a-",
// These can be quoted:
NaN: "NaN",
1: "1",
1.5: "1.5",
// Prettier will normalize these to `0.1` and `1` – then they can be quoted.
0.1: ".1",
1: "1.",
// These should never be quoted. The _actual_ keys are shown as comments.
// Copy-paste this into the console to verify. If we were to convert these
// numbers into decimal (which completely valid), “information/intent” is
// lost. Either way, writing code like this is super confusing.
1.0: "1.0", // 1
999999999999999999999: "999999999999999999999", // 1e+21
0.99999999999999999: "0.99999999999999999", // 1
1e2: "1E2", // 100
1e3: "1e+3", // 1000
1e100: "1e+100", // 1e+100 – this one is identical, but would be inconsistent to quote.
0b10: "0b10", // 2
0o10: "0o10", // 8
0xf: "0xf", // 15
2n: "2n", // 2
0xan: "0xan", // 10
});

// Negative numbers cannot be unquoted.
!{
"-1": null,
"-1.5": null,
};

-a = {
- "a": 1,
- b: 2,
-};
+// FIXME: reformat issue
+//a = {
+// "\a": 1,
+// "b": 2
+//}

```

**Prettier Similarity**: 95.15%


### js/quotes/objects.js
```diff
const obj = {
Expand Down
31 changes: 10 additions & 21 deletions crates/biome_js_formatter/src/utils/string_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,40 +292,28 @@ impl<'token> LiteralStringNormaliser<'token> {
is_js_ident(text_to_check)
}

/// We can change the text only if there are alphanumeric or alphabetic characters, depending on the file source
fn can_remove_quotes(&self, file_source: SourceFileKind) -> bool {
if self.is_preserve_quote_properties() {
return false;
}
if self.can_remove_number_quotes_by_file_type(file_source) {
return true;
}
self.is_js_ident()
}

fn normalise_type_member(
&mut self,
string_information: StringInformation,
file_source: SourceFileKind,
) -> Cow<'token, str> {
if self.can_remove_quotes(file_source) {
return Cow::Owned(self.raw_content().to_string());
let normalised = self.normalise_string_literal(string_information);
let quoteless = &normalised[1..normalised.len() - 1];
let can_remove_quotes = !self.is_preserve_quote_properties()
&& (self.can_remove_number_quotes_by_file_type(file_source) || is_js_ident(quoteless));
if can_remove_quotes {
Cow::Owned(quoteless.to_string())
} else {
normalised
}
self.normalise_string_literal(string_information)
}

fn normalise_string_literal(&self, string_information: StringInformation) -> Cow<'token, str> {
let preferred_quote = string_information.preferred_quote;
let polished_raw_content = self.normalize_string(&string_information);

match polished_raw_content {
Cow::Borrowed(raw_content) => {
let final_content = self.swap_quotes(raw_content, &string_information);
match final_content {
Cow::Borrowed(final_content) => Cow::Borrowed(final_content),
Cow::Owned(final_content) => Cow::Owned(final_content),
}
}
Cow::Borrowed(raw_content) => self.swap_quotes(raw_content, &string_information),
Cow::Owned(s) => {
// content is owned, meaning we allocated a new string,
// so we force replacing quotes, regardless
Expand Down Expand Up @@ -355,6 +343,7 @@ impl<'token> LiteralStringNormaliser<'token> {
)
}

/// Returns the string without its quotes.
fn raw_content(&self) -> &'token str {
let content = self.get_token().text_trimmed();
&content[1..content.len() - 1]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ Object.entries({
"-1.5": null,
}

// FIXME: reformat issue
//a = {
// "\a": 1,
// "b": 2
//}
a = {
"\a": 1,
"b": 2
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,6 @@ Object.entries({
};

a = {
"a": 1,
a: 1,
b: 2,
};
Loading
Loading