refactor(ext/node): add more methods to 'NodeFs' trait (#18604)

Added more methods to `ext/node/clippy.toml` that are not allowed
to be used in the crate.

Prerequisite for https://github.com/denoland/deno/pull/18544
This commit is contained in:
Bartek Iwańczuk 2023-04-06 15:08:14 +02:00 committed by GitHub
parent 9626c48a01
commit 339165bd95
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 52 additions and 21 deletions

View file

@ -1,6 +1,25 @@
disallowed-methods = [
{ path = "std::env::current_dir", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::Path::exists", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::Path::canonicalize", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::Path::is_dir", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::Path::is_file", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::Path::is_symlink", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::Path::metadata", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::Path::read_dir", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::Path::read_link", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::Path::symlink_metadata", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::Path::try_exists", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::PathBuf::exists", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::PathBuf::canonicalize", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::PathBuf::is_dir", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::PathBuf::is_file", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::PathBuf::is_symlink", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::PathBuf::metadata", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::PathBuf::read_dir", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::PathBuf::read_link", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::PathBuf::symlink_metadata", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::path::PathBuf::try_exists", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::fs::canonicalize", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::fs::copy", reason = "File system operations should be done using NodeFs trait" },
{ path = "std::fs::create_dir", reason = "File system operations should be done using NodeFs trait" },

View file

@ -52,8 +52,11 @@ pub trait NodePermissions {
pub trait NodeFs {
fn current_dir() -> io::Result<PathBuf>;
fn metadata<P: AsRef<Path>>(path: P) -> io::Result<std::fs::Metadata>;
fn is_file<P: AsRef<Path>>(path: P) -> bool;
fn is_dir<P: AsRef<Path>>(path: P) -> bool;
fn exists<P: AsRef<Path>>(path: P) -> bool;
fn read_to_string<P: AsRef<Path>>(path: P) -> io::Result<String>;
fn canonicalize<P: AsRef<Path>>(path: P) -> io::Result<PathBuf>;
}
pub struct RealFs;
@ -63,15 +66,32 @@ impl NodeFs for RealFs {
std::env::current_dir()
}
fn metadata<P: AsRef<Path>>(path: P) -> io::Result<std::fs::Metadata> {
fn exists<P: AsRef<Path>>(path: P) -> bool {
#[allow(clippy::disallowed_methods)]
std::fs::metadata(path).is_ok()
}
fn is_file<P: AsRef<Path>>(path: P) -> bool {
#[allow(clippy::disallowed_methods)]
std::fs::metadata(path)
.map(|m| m.is_file())
.unwrap_or(false)
}
fn is_dir<P: AsRef<Path>>(path: P) -> bool {
#[allow(clippy::disallowed_methods)]
std::fs::metadata(path).map(|m| m.is_dir()).unwrap_or(false)
}
fn read_to_string<P: AsRef<Path>>(path: P) -> io::Result<String> {
#[allow(clippy::disallowed_methods)]
std::fs::read_to_string(path)
}
fn canonicalize<P: AsRef<Path>>(path: P) -> io::Result<PathBuf> {
#[allow(clippy::disallowed_methods)]
std::path::Path::canonicalize(path.as_ref())
}
}
pub trait RequireNpmResolver {

View file

@ -264,8 +264,8 @@ where
{
let path = PathBuf::from(path);
ensure_read_permission::<Env::P>(state, &path)?;
if let Ok(metadata) = Env::Fs::metadata(&path) {
if metadata.is_file() {
if Env::Fs::exists(&path) {
if Env::Fs::is_file(&path) {
return Ok(0);
} else {
return Ok(1);
@ -285,7 +285,7 @@ where
{
let path = PathBuf::from(request);
ensure_read_permission::<Env::P>(state, &path)?;
let mut canonicalized_path = path.canonicalize()?;
let mut canonicalized_path = Env::Fs::canonicalize(&path)?;
if cfg!(windows) {
canonicalized_path = PathBuf::from(
canonicalized_path

View file

@ -53,11 +53,11 @@ pub fn path_to_declaration_path<Fs: NodeFs>(
NodeModuleKind::Cjs => with_known_extension(path, "d.cts"),
NodeModuleKind::Esm => with_known_extension(path, "d.mts"),
};
if Fs::metadata(&specific_dts_path).is_ok() {
if Fs::exists(&specific_dts_path) {
return Some(specific_dts_path);
}
let dts_path = with_known_extension(path, "d.ts");
if Fs::metadata(&dts_path).is_ok() {
if Fs::exists(&dts_path) {
Some(dts_path)
} else {
None
@ -74,7 +74,7 @@ pub fn path_to_declaration_path<Fs: NodeFs>(
if let Some(path) = probe_extensions::<Fs>(&path, referrer_kind) {
return Some(path);
}
if path.is_dir() {
if Fs::is_dir(&path) {
if let Some(path) =
probe_extensions::<Fs>(&path.join("index"), referrer_kind)
{
@ -842,7 +842,7 @@ fn get_closest_package_json_path<Fs: NodeFs>(
let file_path = url.to_file_path().unwrap();
let mut current_dir = file_path.parent().unwrap();
let package_json_path = current_dir.join("package.json");
if Fs::metadata(&package_json_path).is_ok() {
if Fs::exists(&package_json_path) {
return Ok(package_json_path);
}
let root_pkg_folder = npm_resolver
@ -850,7 +850,7 @@ fn get_closest_package_json_path<Fs: NodeFs>(
while current_dir.starts_with(&root_pkg_folder) {
current_dir = current_dir.parent().unwrap();
let package_json_path = current_dir.join("package.json");
if Fs::metadata(&package_json_path).is_ok() {
if Fs::exists(&package_json_path) {
return Ok(package_json_path);
}
}
@ -858,14 +858,6 @@ fn get_closest_package_json_path<Fs: NodeFs>(
bail!("did not find package.json in {}", root_pkg_folder.display())
}
fn file_exists<Fs: NodeFs>(path: &Path) -> bool {
if let Ok(stats) = Fs::metadata(path) {
stats.is_file()
} else {
false
}
}
pub fn legacy_main_resolve<Fs: NodeFs>(
package_json: &PackageJson,
referrer_kind: NodeModuleKind,
@ -894,7 +886,7 @@ pub fn legacy_main_resolve<Fs: NodeFs>(
if let Some(main) = maybe_main {
let guess = package_json.path.parent().unwrap().join(main).clean();
if file_exists::<Fs>(&guess) {
if Fs::is_file(&guess) {
return Ok(Some(guess));
}
@ -923,7 +915,7 @@ pub fn legacy_main_resolve<Fs: NodeFs>(
.unwrap()
.join(format!("{main}{ending}"))
.clean();
if file_exists::<Fs>(&guess) {
if Fs::is_file(&guess) {
// TODO(bartlomieju): emitLegacyIndexDeprecation()
return Ok(Some(guess));
}
@ -946,7 +938,7 @@ pub fn legacy_main_resolve<Fs: NodeFs>(
.unwrap()
.join(index_file_name)
.clean();
if file_exists::<Fs>(&guess) {
if Fs::is_file(&guess) {
// TODO(bartlomieju): emitLegacyIndexDeprecation()
return Ok(Some(guess));
}