Skip to content

Commit

Permalink
fix(core): raise an error when invalid config extension is passed (#4949
Browse files Browse the repository at this point in the history
)
  • Loading branch information
ematipico authored Jan 25, 2025
1 parent e750523 commit 7b91d19
Show file tree
Hide file tree
Showing 12 changed files with 303 additions and 136 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
cli: patch
---

# Biome logs a warning in case a folder contains `biome.json` and `biome.jsonc`, and it will use `biome.json` by default.
5 changes: 5 additions & 0 deletions crates/biome_cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,11 @@ pub(crate) trait CommandRunner: Sized {
cli_options.verbose,
)?;
}
if loaded_configuration.double_configuration_found {
console.log(markup! {
<Warn>"Both biome.json and biome.jsonc files were found in the same folder. Biome will use the biome.json file."</Warn>
})
}
info!(
"Configuration file loaded: {:?}, diagnostics detected {}",
loaded_configuration.file_path,
Expand Down
28 changes: 28 additions & 0 deletions crates/biome_cli/tests/cases/config_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,31 @@ fn raises_an_error_when_the_config_file_is_not_json() {
result,
));
}

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

let file = Utf8Path::new("file.js");
fs.insert(
file.into(),
r#"function name() { return "lorem" }"#.as_bytes(),
);

let (fs, result) = run_cli(
fs,
&mut console,
Args::from(["check", "--config-path=config", file.as_str()].as_slice()),
);

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

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"raises_an_error_for_no_configuration_file_found",
fs,
console,
result,
));
}
4 changes: 1 addition & 3 deletions crates/biome_cli/tests/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,8 +421,6 @@ fn custom_config_file_path() {

// Should throw an error when an invalid configuration path is specified
#[test]
// FIXME: redact snapshot for custom paths in configuration
#[cfg(not(windows))]
fn invalid_config_file_path() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();
Expand All @@ -438,7 +436,7 @@ fn invalid_config_file_path() {
[
"format",
"--config-path",
(config_path.to_string().as_str()),
config_path.as_str(),
"--write",
file_path.as_str(),
]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: redactor(content)
snapshot_kind: text
---
## `biome.jsonc`

```json
{}
```

## `file.js`

```js
function name() { return "lorem" }
```

# Termination Message

```block
check ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Some errors were emitted while running checks.
```

# Emitted Messages

```block
Both biome.json and biome.jsonc files were found in the same folder. Biome will use the biome.json file.
```

```block
file.js format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Formatter would have printed the following content:
1 │ - function·name()·{·return·"lorem"·}
1 │ + function·name()·{
2 │ + ········return·"lorem";
3 │ + }
4 │ +
```

```block
Checked 1 file in <TIME>. No fixes applied.
Found 1 error.
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: redactor(content)
snapshot_kind: text
---
## `file.js`

```js
function name() { return "lorem" }
```

# Termination Message

```block
config configuration ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Biome couldn't find a configuration in the directory config.
```
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
expression: redactor(content)
snapshot_kind: text
---
## `file.js`

Expand All @@ -11,11 +12,9 @@ content
# Termination Message

```block
test/biome.json internalError/fs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
test configuration ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Cannot read file
× Biome can't read the following file, maybe for permissions reasons or it doesn't exist: test/biome.json
× Biome couldn't find a configuration in the directory test.
Expand Down
22 changes: 22 additions & 0 deletions crates/biome_configuration/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ pub enum BiomeDiagnostic {
/// Thrown when the program can't serialize the configuration, while saving it
SerializationError(SerializationError),

NoConfigurationFileFound(NoConfigurationFileFound),

/// Thrown when trying to **create** a new configuration file, but it exists already
ConfigAlreadyExists(ConfigAlreadyExists),

Expand Down Expand Up @@ -121,6 +123,12 @@ impl BiomeDiagnostic {
})
}

pub fn no_configuration_file_found(path: &Utf8Path) -> Self {
Self::NoConfigurationFileFound(NoConfigurationFileFound {
path: path.to_string(),
})
}

pub fn cant_resolve(path: impl Display, source: oxc_resolver::ResolveError) -> Self {
Self::CantResolve(CantResolve {
message: MessageAndDescription::from(
Expand Down Expand Up @@ -177,6 +185,20 @@ pub struct SerializationError;
)]
pub struct ConfigAlreadyExists {}

#[derive(Debug, Diagnostic, Serialize, Deserialize)]
#[diagnostic(
category = "configuration",
severity = Error,
message(
message("Biome couldn't find a configuration in the directory "<Emphasis>{self.path}</Emphasis>"."),
description = "Biome couldn't find a configuration in the directory {path}."
)
)]
pub struct NoConfigurationFileFound {
#[location(resource)]
path: String,
}

#[derive(Debug, Serialize, Deserialize, Diagnostic)]
#[diagnostic(
category = "configuration",
Expand Down
3 changes: 3 additions & 0 deletions crates/biome_configuration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,13 +358,16 @@ pub struct FilesConfiguration {
pub include: Option<Vec<Box<str>>>,
}

#[derive(Debug)]
pub struct ConfigurationPayload {
/// The result of the deserialization
pub deserialized: Deserialized<Configuration>,
/// The path of where the `biome.json` or `biome.jsonc` file was found. This contains the file name.
pub configuration_file_path: Utf8PathBuf,
/// The base path where the external configuration in a package should be resolved from
pub external_resolution_base_path: Utf8PathBuf,
/// Whether `biome.json` and `biome.jsonc` were found in the same folder
pub double_configuration_found: bool,
}

#[derive(Debug, Default, PartialEq, Clone)]
Expand Down
87 changes: 30 additions & 57 deletions crates/biome_fs/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ impl ConfigName {
}
}

type AutoSearchResultAlias = Result<Option<AutoSearchResult>, FileSystemDiagnostic>;

/// Represents the kind of filesystem entry a path points at.
#[derive(Clone, Copy, Debug)]
pub enum PathKind {
Expand Down Expand Up @@ -117,74 +115,47 @@ pub trait FileSystem: Send + Sync + RefUnwindSafe {
/// Returns metadata about the path.
fn path_kind(&self, path: &Utf8Path) -> Result<PathKind, FileSystemDiagnostic>;

/// This method accepts a directory path (`search_dir`) and a list of filenames (`file_names`),
/// It looks for the files in the specified directory in the order they appear in the list.
/// If a file is not found in the initial directory, the search may continue into the parent
/// directories based on the `should_error_if_file_not_found` flag.
///
/// Behavior if files are not found in `search_dir`:
/// This method accepts a directory path (`search_dir`) and a file name `search_file`,
///
/// - If `should_error_if_file_not_found` is set to `true`, the method will return an error.
/// - If `should_error_if_file_not_found` is set to `false`, the method will search for the files in the parent
/// directories of `search_dir` recursively until:
/// - It finds a file, reads it, and returns its contents along with its path.
/// - It confirms that the file doesn't exist in any of the checked directories.
///
/// ## Errors
/// It looks for `search_file` starting from the given `search_dir`, and then it starts
/// navigating upwards the parent directories until it finds a file that matches `search_file` or
/// there aren't any more parent folders.
///
/// The method returns an error if `should_error_if_file_not_found` is `true`,
/// and the file is not found or cannot be opened or read.
///
fn auto_search(
/// If no file is found, the method returns `None`
fn auto_search_file(
&self,
search_dir: &Utf8Path,
file_names: &[&str],
should_error_if_file_not_found: bool,
) -> AutoSearchResultAlias {
let mut curret_search_dir = search_dir.to_path_buf();
search_file: &str,
) -> Option<AutoSearchResult> {
let mut current_search_dir = search_dir.to_path_buf();
let mut is_searching_in_parent_dir = false;

loop {
let mut errors: Vec<FileSystemDiagnostic> = vec![];

// Iterate all possible file names
for file_name in file_names {
let file_path = curret_search_dir.join(file_name);
match self.read_file_from_path(&file_path) {
Ok(content) => {
if is_searching_in_parent_dir {
info!(
let file_path = current_search_dir.join(search_file);
match self.read_file_from_path(&file_path) {
Ok(content) => {
if is_searching_in_parent_dir {
info!(
"Biome auto discovered the file at the following path that isn't in the working directory:\n{:?}",
curret_search_dir
current_search_dir
);
}
return Ok(Some(AutoSearchResult { content, file_path }));
}
Err(error) => {
// We don't return the error immediately because
// there're multiple valid file names to search for
if !is_searching_in_parent_dir && should_error_if_file_not_found {
errors.push(error);
}
}
return Some(AutoSearchResult {
content,
file_path,
directory_path: current_search_dir,
});
}
}

if !is_searching_in_parent_dir && should_error_if_file_not_found {
if let Some(diagnostic) = errors.into_iter().next() {
// We can only return one Err, so we return the first diagnostic.
return Err(diagnostic);
_ => {
if let Some(parent_search_dir) = current_search_dir.parent() {
current_search_dir = Utf8PathBuf::from(parent_search_dir);
is_searching_in_parent_dir = true;
} else {
return None;
}
}
}

if let Some(parent_search_dir) = curret_search_dir.parent() {
curret_search_dir = Utf8PathBuf::from(parent_search_dir);
is_searching_in_parent_dir = true;
} else {
break;
}
}

Ok(None)
}

/// Reads the content of a file specified by `file_path`.
Expand Down Expand Up @@ -251,6 +222,8 @@ pub struct AutoSearchResult {
pub content: String,
/// The path of the file found
pub file_path: Utf8PathBuf,
/// The directory where the file was found
pub directory_path: Utf8PathBuf,
}

pub trait File {
Expand Down
7 changes: 6 additions & 1 deletion crates/biome_lsp/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use tower_lsp::lsp_types;
use tower_lsp::lsp_types::{Diagnostic, Url};
use tower_lsp::lsp_types::{MessageType, Registration};
use tower_lsp::lsp_types::{Unregistration, WorkspaceFolder};
use tracing::{error, info};
use tracing::{error, info, warn};

pub(crate) struct ClientInformation {
/// The name of the client
Expand Down Expand Up @@ -558,6 +558,11 @@ impl Session {
return ConfigurationStatus::Error;
}

if loaded_configuration.double_configuration_found {
warn!("Both biome.json and biome.jsonc files were found in the same folder. Biome will use the biome.json file.");
self.client.log_message(MessageType::WARNING, "Both biome.json and biome.jsonc files were found in the same folder. Biome will use the biome.json file.").await;
}

info!("Configuration loaded successfully from disk.");
info!("Update workspace settings.");

Expand Down
Loading

0 comments on commit 7b91d19

Please sign in to comment.