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(config): handle unset and off values in editorconfig files #3907

Merged
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
45 changes: 45 additions & 0 deletions crates/biome_cli/tests/cases/editorconfig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,3 +452,48 @@ max_line_length = 300
result,
));
}

#[test]
fn should_emit_diagnostics() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let editorconfig = Path::new(".editorconfig");
fs.insert(
editorconfig.into(),
r#"
[*]
insert_final_newline = false
"#,
);

let test_file = Path::new("test.js");
let contents = r#"console.log("foo");
"#;
fs.insert(test_file.into(), contents);

let result = run_cli(
DynRef::Borrowed(&mut fs),
&mut console,
Args::from(
[
("format"),
("--write"),
("--use-editorconfig=true"),
test_file.as_os_str().to_str().unwrap(),
]
.as_slice(),
),
);

assert!(result.is_ok(), "run_cli returned {result:?}");

assert_file_contents(&fs, test_file, contents);
assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"should_emit_diagnostics",
fs,
console,
result,
));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
---
## `.editorconfig`

```editorconfig

[*]
insert_final_newline = false

```

## `test.js`

```js
console.log("foo");

```

# Emitted Messages

```block
configuration ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× Key 'insert_final_newline' is incompatible with biome: Biome always inserts a final newline. Set this option to true.


```

```block
Formatted 1 file in <TIME>. No fixes applied.
```
6 changes: 3 additions & 3 deletions crates/biome_configuration/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ pub enum EditorConfigDiagnostic {
/// Failed to parse the .editorconfig file.
ParseFailed(ParseFailedDiagnostic),
/// An option is completely incompatible with biome.
Incompatible(InconpatibleDiagnostic),
Incompatible(IncompatibleDiagnostic),
/// A glob pattern that biome doesn't support.
UnknownGlobPattern(UnknownGlobPatternDiagnostic),
/// A glob pattern that contains invalid syntax.
Expand All @@ -255,7 +255,7 @@ pub enum EditorConfigDiagnostic {

impl EditorConfigDiagnostic {
pub fn incompatible(key: impl Into<String>, message: impl Into<String>) -> Self {
Self::Incompatible(InconpatibleDiagnostic {
Self::Incompatible(IncompatibleDiagnostic {
message: MessageAndDescription::from(
markup! { "Key '"{key.into()}"' is incompatible with biome: "{message.into()}}
.to_owned(),
Expand Down Expand Up @@ -299,7 +299,7 @@ pub struct ParseFailedDiagnostic {
category = "configuration",
severity = Error,
)]
pub struct InconpatibleDiagnostic {
pub struct IncompatibleDiagnostic {
#[message]
#[description]
pub message: MessageAndDescription,
Expand Down
131 changes: 99 additions & 32 deletions crates/biome_configuration/src/editorconfig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,14 @@ impl EditorConfig {
#[derive(Debug, Clone, Deserialize, Default)]
#[serde(default)]
pub struct EditorConfigOptions {
indent_style: Option<IndentStyle>,
Conaclos marked this conversation as resolved.
Show resolved Hide resolved
#[serde(deserialize_with = "deserialize_optional_indent_width_from_string")]
indent_size: Option<IndentWidth>,
end_of_line: Option<LineEnding>,
#[serde(deserialize_with = "deserialize_optional_line_width_from_string")]
max_line_length: Option<LineWidth>,
#[serde(deserialize_with = "deserialize_optional_value_from_string")]
indent_style: EditorconfigValue<IndentStyle>,
#[serde(deserialize_with = "deserialize_optional_value_from_string")]
indent_size: EditorconfigValue<IndentWidth>,
#[serde(deserialize_with = "deserialize_optional_value_from_string")]
end_of_line: EditorconfigValue<LineEnding>,
#[serde(deserialize_with = "deserialize_optional_value_from_string")]
max_line_length: EditorconfigValue<LineWidth>,
// Not a biome option, but we need it to emit a diagnostic when this is set to false.
#[serde(deserialize_with = "deserialize_optional_bool_from_string")]
insert_final_newline: Option<bool>,
Expand All @@ -97,20 +99,20 @@ pub struct EditorConfigOptions {
impl EditorConfigOptions {
pub fn to_biome(self) -> PartialFormatterConfiguration {
PartialFormatterConfiguration {
indent_style: self.indent_style,
indent_width: self.indent_size,
line_ending: self.end_of_line,
line_width: self.max_line_length,
indent_style: self.indent_style.into(),
indent_width: self.indent_size.into(),
line_ending: self.end_of_line.into(),
line_width: self.max_line_length.into(),
..Default::default()
}
}

pub fn to_biome_override(self) -> OverrideFormatterConfiguration {
OverrideFormatterConfiguration {
indent_style: self.indent_style,
indent_width: self.indent_size,
line_ending: self.end_of_line,
line_width: self.max_line_length,
indent_style: self.indent_style.into(),
indent_width: self.indent_size.into(),
line_ending: self.end_of_line.into(),
line_width: self.max_line_length.into(),
..Default::default()
}
}
Expand All @@ -121,13 +123,38 @@ impl EditorConfigOptions {
if let Some(false) = self.insert_final_newline {
errors.push(EditorConfigDiagnostic::incompatible(
"insert_final_newline",
"Biome always inserts a final newline.",
"Biome always inserts a final newline. Set this option to true.",
));
}
errors
}
}

/// Represents a value in an .editorconfig file.
#[derive(Debug, Clone, Deserialize, Default)]
#[serde(untagged)]
pub enum EditorconfigValue<T> {
/// The value was explicitly specified.
Explicit(T),
/// Use the default value for this option. This occurs when the value is `unset`.
Default,
/// The value was not specified.
#[default]
None,
}

// This is an `Into` because implementing `From` is not possible because you can't implement traits for a type you don't own.
#[allow(clippy::from_over_into)]
impl<T: Default> Into<Option<T>> for EditorconfigValue<T> {
fn into(self) -> Option<T> {
match self {
EditorconfigValue::Explicit(v) => Some(v),
EditorconfigValue::Default => Some(T::default()),
EditorconfigValue::None => None,
}
}
}

fn deserialize_bool_from_string<'de, D>(deserializer: D) -> Result<bool, D::Error>
where
D: Deserializer<'de>,
Expand All @@ -147,28 +174,21 @@ where
deserialize_bool_from_string(deserializer).map(Some)
}

fn deserialize_optional_indent_width_from_string<'de, D>(
deserializer: D,
) -> Result<Option<IndentWidth>, D::Error>
where
D: Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
IndentWidth::from_str(s.as_str())
.map_err(serde::de::Error::custom)
.map(Some)
}

fn deserialize_optional_line_width_from_string<'de, D>(
fn deserialize_optional_value_from_string<'de, D, T>(
deserializer: D,
) -> Result<Option<LineWidth>, D::Error>
) -> Result<EditorconfigValue<T>, D::Error>
where
D: Deserializer<'de>,
T: FromStr,
T::Err: std::fmt::Display,
{
let s = String::deserialize(deserializer)?;
LineWidth::from_str(s.as_str())
.map_err(serde::de::Error::custom)
.map(Some)
match s.as_str() {
"unset" | "off" => Ok(EditorconfigValue::Default),
_ => T::from_str(s.as_str())
.map_err(serde::de::Error::custom)
.map(EditorconfigValue::Explicit),
}
}

/// Turn an unknown glob pattern into a list of known glob patterns. This is part of a hack to support all editorconfig patterns.
Expand Down Expand Up @@ -410,6 +430,53 @@ insert_final_newline = false
assert!(matches!(errors[0], EditorConfigDiagnostic::Incompatible(_)));
}

#[test]
fn should_parse_editorconfig_with_unset_values() {
let input = r#"
root = true

[*]
indent_style = unset
indent_size = unset
end_of_line = unset
max_line_length = unset
"#;

let conf = parse_str(input).expect("Failed to parse editorconfig");
assert!(matches!(
conf.options["*"].indent_style,
EditorconfigValue::Default
));
assert!(matches!(
conf.options["*"].indent_size,
EditorconfigValue::Default
));
assert!(matches!(
conf.options["*"].end_of_line,
EditorconfigValue::Default
));
assert!(matches!(
conf.options["*"].max_line_length,
EditorconfigValue::Default
));
}

#[test]
fn should_parse_editorconfig_with_max_line_length_off() {
let input = r#"
root = true

[*]
max_line_length = off
"#;

let conf = parse_str(input).expect("Failed to parse editorconfig");
assert!(matches!(
conf.options["*"].max_line_length,
EditorconfigValue::Default,
));
}

#[test]
fn should_expand_glob_pattern_list() {
let pattern = "package.json";
Expand Down