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

Commit

Permalink
fix(rome_fs): improve the messaging of the "unhandled file type" diag…
Browse files Browse the repository at this point in the history
…nostic (#3332)

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

* implement "error entries" for the memory file system
  • Loading branch information
leops committed Oct 5, 2022
1 parent b32b4b1 commit 5042251
Show file tree
Hide file tree
Showing 7 changed files with 198 additions and 19 deletions.
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;

0 comments on commit 5042251

Please sign in to comment.