From 1943071d127fdeda14dc2cfbb9846e85706d13bd Mon Sep 17 00:00:00 2001
From: Bruce Mitchener <bruce.mitchener@gmail.com>
Date: Wed, 7 Jul 2021 19:53:07 +0700
Subject: [PATCH] Simplify `is_executable` in `nu-completion`. (#3742)

On Windows, we used the `is-exeuctable` crate but on Unix, we
duplicated the check that it did, with one difference: We also
looked at whether or not it was a symlink.

The `is-executable` crate uses `std::fs::metadata` which follows
symlinks, so this scenario should never occur here, as it will
return the metadata for the target file.

Using the `is-executable` crate on both Unix and Windows lets us
make it non-optional. This lets us remove the `executable-support`
feature. (It is worth noting that this code didn't compile on
Windows when the `executable-support` feature was not specified.)

Right now, there is an alternate code path for `target_arch` being
`wasm32`. This isn't exactly correct as it should probably handle
something different for when the `target_os` is `wasi`.
---
 Cargo.toml                          |  2 --
 crates/nu-completion/Cargo.toml     |  4 +++-
 crates/nu-completion/src/command.rs | 22 +++-------------------
 3 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index be7d59c75..d053f26f6 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -71,7 +71,6 @@ rustyline-support = ["nu-cli/rustyline-support", "nu-command/rustyline-support"]
 term-support = ["nu-command/term"]
 uuid-support = ["nu-command/uuid_crate"]
 which-support = ["nu-command/which", "nu-engine/which"]
-executable-support = ["nu-completion/is_executable"]
 
 default = [
     "nu-cli/shadow-rs",
@@ -85,7 +84,6 @@ default = [
     "post",
     "fetch",
     "zip-support",
-    "executable-support",
     "dataframe",
 ]
 
diff --git a/crates/nu-completion/Cargo.toml b/crates/nu-completion/Cargo.toml
index 8a947d77a..cb23b27ee 100644
--- a/crates/nu-completion/Cargo.toml
+++ b/crates/nu-completion/Cargo.toml
@@ -21,7 +21,9 @@ nu-test-support = { version="0.33.1", path="../nu-test-support" }
 
 dirs-next = "2.0.0"
 indexmap = { version="1.6.1", features=["serde-1"] }
-is_executable = { version="1.0.1", optional=true }
+
+[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
+is_executable = "1.0.1"
 
 [dev-dependencies]
 parking_lot = "0.11.1"
diff --git a/crates/nu-completion/src/command.rs b/crates/nu-completion/src/command.rs
index bf04f6450..9d7eb45fc 100644
--- a/crates/nu-completion/src/command.rs
+++ b/crates/nu-completion/src/command.rs
@@ -1,8 +1,7 @@
 use super::matchers::Matcher;
 use crate::{Completer, CompletionContext, Suggestion};
 use indexmap::set::IndexSet;
-#[cfg(feature = "is_executable")]
-#[allow(unused)]
+#[cfg(not(target_arch = "wasm32"))]
 use is_executable::IsExecutable;
 use nu_test_support::NATIVE_PATH_ENV_VAR;
 use std::iter::FromIterator;
@@ -56,11 +55,12 @@ where
     }
 }
 
-#[cfg(windows)]
+#[cfg(not(target_arch = "wasm32"))]
 fn is_executable(path: &Path) -> bool {
     // This call to a crate essentially checks the PATHEXT on Windows and does some
     // low level WinAPI calls to determine if the file is executable. It seems quite
     // a bit faster than calling path.metadata().
+    // On Unix, this checks the file metadata. The underlying code traverses symlinks.
     path.is_executable()
 }
 
@@ -69,22 +69,6 @@ fn is_executable(_path: &Path) -> bool {
     false
 }
 
-#[cfg(unix)]
-fn is_executable(path: &Path) -> bool {
-    use std::os::unix::fs::PermissionsExt;
-
-    if let Ok(metadata) = path.metadata() {
-        let filetype = metadata.file_type();
-        let permissions = metadata.permissions();
-
-        // The file is executable if it is a directory or a symlink and the permissions are set for
-        // owner, group, or other
-        (filetype.is_file() || filetype.is_symlink()) && (permissions.mode() & 0o111 != 0)
-    } else {
-        false
-    }
-}
-
 // TODO cache these, but watch for changes to PATH
 fn find_path_executables() -> Option<IndexSet<String>> {
     let path_var = std::env::var_os(NATIVE_PATH_ENV_VAR)?;