Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_fs): improve the messaging of the "unhandled file type" diagnostic #3332

Merged
merged 2 commits into from
Oct 5, 2022
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
52 changes: 51 additions & 1 deletion crates/rome_cli/tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::{ffi::OsString, path::Path};
use pico_args::Arguments;
use rome_cli::{CliSession, Termination};
use rome_console::{BufferConsole, Console};
use rome_fs::{FileSystem, MemoryFileSystem};
use rome_fs::{ErrorEntry, FileSystem, MemoryFileSystem};
use rome_service::{App, DynRef};

const UNFORMATTED: &str = " statement( ) ";
Expand Down Expand Up @@ -104,6 +104,8 @@ const CUSTOM_CONFIGURATION_AFTER: &str = "function f() {
";

mod check {
use std::path::PathBuf;

use super::*;
use crate::configs::{
CONFIG_LINTER_DISABLED, CONFIG_LINTER_DOWNGRADE_DIAGNOSTIC, CONFIG_LINTER_IGNORED_FILES,
Expand Down Expand Up @@ -674,6 +676,54 @@ mod check {
result,
));
}

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

fs.insert_error(PathBuf::from("prefix/ci.js"), ErrorEntry::SymbolicLink);

let result = run_cli(
DynRef::Borrowed(&mut fs),
DynRef::Borrowed(&mut console),
Arguments::from_vec(vec![OsString::from("check"), OsString::from("prefix")]),
);

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

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"fs_error_symlink",
fs,
console,
result,
));
}

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

fs.insert_error(PathBuf::from("prefix/ci.js"), ErrorEntry::Unknown);

let result = run_cli(
DynRef::Borrowed(&mut fs),
DynRef::Borrowed(&mut console),
Arguments::from_vec(vec![OsString::from("check"), OsString::from("prefix")]),
);

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

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"fs_error_unknown",
fs,
console,
result,
));
}
}

mod ci {
Expand Down
17 changes: 17 additions & 0 deletions crates/rome_cli/tests/snapshots/main_check/fs_error_symlink.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: crates/rome_cli/tests/snap_test.rs
expression: content
---
# Emitted Messages

```block
prefix/ci.js internalError/fs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Symbolic links are not supported

i Rome does not currently support visiting the content of symbolic links since it could lead to an infinite traversal loop


```


17 changes: 17 additions & 0 deletions crates/rome_cli/tests/snapshots/main_check/fs_error_unknown.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: crates/rome_cli/tests/snap_test.rs
expression: content
---
# Emitted Messages

```block
prefix/ci.js internalError/fs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Encountered an unknown file type

i Rome encountered a file system entry that's neither a file, directory or symbolic link


```


57 changes: 55 additions & 2 deletions crates/rome_fs/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ use std::{
};

use crate::{PathInterner, RomePath};
use rome_diagnostics::file::FileId;
use rome_diagnostics::{
file::FileId,
v2::{console, Advices, Diagnostic, LogCategory, Visit},
};

mod memory;
mod os;

pub use memory::MemoryFileSystem;
pub use memory::{ErrorEntry, MemoryFileSystem};
pub use os::OsFileSystem;
use rome_diagnostics::v2::Error;
pub const CONFIG_NAME: &str = "rome.json";
Expand Down Expand Up @@ -152,3 +155,53 @@ where
T::traversal(self, func)
}
}

#[derive(Debug, Diagnostic)]
#[diagnostic(severity = Warning, category = "internalError/fs")]
struct UnhandledDiagnostic {
#[location(resource)]
file_id: FileId,
#[message]
#[description]
#[advice]
file_kind: UnhandledKind,
}

#[derive(Clone, Copy, Debug)]
enum UnhandledKind {
Symlink,
Other,
}

impl console::fmt::Display for UnhandledKind {
fn fmt(&self, fmt: &mut console::fmt::Formatter) -> io::Result<()> {
match self {
UnhandledKind::Symlink => fmt.write_str("Symbolic links are not supported"),
UnhandledKind::Other => fmt.write_str("Encountered an unknown file type"),
}
}
}

impl std::fmt::Display for UnhandledKind {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
UnhandledKind::Symlink => write!(fmt, "Symbolic links are not supported"),
UnhandledKind::Other => write!(fmt, "Encountered an unknown file type"),
}
}
}

impl Advices for UnhandledKind {
fn record(&self, visitor: &mut dyn Visit) -> io::Result<()> {
match self {
UnhandledKind::Symlink => visitor.record_log(
LogCategory::Info,
&"Rome does not currently support visiting the content of symbolic links since it could lead to an infinite traversal loop",
),
UnhandledKind::Other => visitor.record_log(
LogCategory::Info,
&"Rome encountered a file system entry that's neither a file, directory or symbolic link",
),
}
}
}
43 changes: 38 additions & 5 deletions crates/rome_fs/src/fs/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,18 @@ use std::{
};

use parking_lot::{lock_api::ArcMutexGuard, Mutex, RawMutex, RwLock};
use rome_diagnostics::v2::Error;

use crate::fs::{FileSystemExt, OpenOptions};
use crate::{FileSystem, TraversalContext, TraversalScope};

use super::{BoxedTraversal, File};
use super::{BoxedTraversal, File, UnhandledDiagnostic, UnhandledKind};

/// Fully in-memory file system, stores the content of all known files in a hashmap
#[derive(Default)]
pub struct MemoryFileSystem {
files: AssertUnwindSafe<RwLock<HashMap<PathBuf, FileEntry>>>,
errors: HashMap<PathBuf, ErrorEntry>,
}

/// This is what's actually being stored for each file in the filesystem
Expand All @@ -36,13 +38,28 @@ pub struct MemoryFileSystem {
/// or write either happens or not, but will never panic halfway through)
type FileEntry = Arc<Mutex<Vec<u8>>>;

/// Error entries are special file system entries that cause an error to be
/// emitted when they are reached through a filesystem traversal. This is
/// mainly useful as a mechanism to test the handling of filesystem error in
/// client code.
#[derive(Clone, Copy, Debug)]
pub enum ErrorEntry {
SymbolicLink,
Unknown,
}

impl MemoryFileSystem {
/// Create or update a file in the filesystem
pub fn insert(&mut self, path: PathBuf, content: impl Into<Vec<u8>>) {
let files = &mut self.files.0.write();
let files = self.files.0.get_mut();
files.insert(path, Arc::new(Mutex::new(content.into())));
}

/// Create or update an error in the filesystem
pub fn insert_error(&mut self, path: PathBuf, kind: ErrorEntry) {
self.errors.insert(path, kind);
}

pub fn files(self) -> IntoIter<PathBuf, FileEntry> {
let files = self.files.0.into_inner();
files.into_iter()
Expand Down Expand Up @@ -136,11 +153,27 @@ impl<'scope> TraversalScope<'scope> for MemoryTraversalScope<'scope> {
fn spawn(&self, ctx: &'scope dyn TraversalContext, base: PathBuf) {
// Traversal is implemented by iterating on all keys, and matching on
// those that are prefixed with the provided `base` path
let files = &self.fs.files.0.read();
for path in files.keys() {
{
let files = &self.fs.files.0.read();
for path in files.keys() {
if path.strip_prefix(&base).is_ok() {
let file_id = ctx.interner().intern_path(path.into());
ctx.handle_file(path, file_id);
}
}
}

for (path, entry) in &self.fs.errors {
if path.strip_prefix(&base).is_ok() {
let file_id = ctx.interner().intern_path(path.into());
ctx.handle_file(path, file_id);

ctx.push_diagnostic(Error::from(UnhandledDiagnostic {
file_id,
file_kind: match entry {
ErrorEntry::SymbolicLink => UnhandledKind::Symlink,
ErrorEntry::Unknown => UnhandledKind::Other,
},
}));
}
}
}
Expand Down
27 changes: 18 additions & 9 deletions crates/rome_fs/src/fs/os.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
//! Implementation of the [FileSystem] and related traits for the underlying OS filesystem
use super::{BoxedTraversal, File};
use super::{BoxedTraversal, File, UnhandledDiagnostic, UnhandledKind};
use crate::fs::{FileSystemExt, OpenOptions};
use crate::{
fs::{TraversalContext, TraversalScope},
FileSystem, RomePath,
};
use rayon::{scope, Scope};
use rome_diagnostics::v2::{adapters::IoError, Diagnostic, DiagnosticExt, Error, FileId};
use rome_diagnostics::v2::{adapters::IoError, DiagnosticExt, Error, FileId};
use std::{
ffi::OsStr,
fs,
Expand Down Expand Up @@ -126,7 +126,10 @@ impl<'scope> TraversalScope<'scope> for OsTraversalScope<'scope> {
return;
}

ctx.push_diagnostic(Error::from(UnhandledDiagnostic { file_id }));
ctx.push_diagnostic(Error::from(UnhandledDiagnostic {
file_id,
file_kind: UnhandledKind::from(file_type),
}));
}
}

Expand Down Expand Up @@ -200,13 +203,19 @@ fn handle_dir<'scope>(
continue;
}

ctx.push_diagnostic(Error::from(UnhandledDiagnostic { file_id }));
ctx.push_diagnostic(Error::from(UnhandledDiagnostic {
file_id,
file_kind: UnhandledKind::from(file_type),
}));
}
}

#[derive(Debug, Diagnostic)]
#[diagnostic(category = "internalError/fs", message = "unhandled file type")]
struct UnhandledDiagnostic {
#[location(resource)]
file_id: FileId,
impl From<fs::FileType> for UnhandledKind {
fn from(file_type: fs::FileType) -> Self {
if file_type.is_symlink() {
Self::Symlink
} else {
Self::Other
}
}
}
4 changes: 2 additions & 2 deletions crates/rome_fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ mod interner;
mod path;

pub use fs::{
FileSystem, FileSystemExt, MemoryFileSystem, OpenOptions, OsFileSystem, TraversalContext,
TraversalScope,
ErrorEntry, FileSystem, FileSystemExt, MemoryFileSystem, OpenOptions, OsFileSystem,
TraversalContext, TraversalScope,
};
pub use interner::{AtomicInterner, IndexSetInterner, PathInterner};
pub use path::RomePath;