perf: minor tweaks

brush-core/src/builtins/dot.rs

@@ -21,14 +21,12 @@ impl builtins::Command for DotCommand {
         context: commands::ExecutionContext<'_>,
     ) -> Result<crate::builtins::ExitCode, crate::error::Error> {
         // TODO: Handle trap inheritance.
-        let script_args: Vec<_> = self.script_args.iter().map(|a| a.as_str()).collect();
-
         let params = context.params.clone();
         let result = context
             .shell
             .source_script(
                 Path::new(&self.script_path),
-                script_args.as_slice(),
+                self.script_args.iter(),
                 &params,
             )
             .await?;

brush-core/src/env.rs

@@ -1,4 +1,5 @@
 use std::borrow::Cow;
+use std::collections::hash_map;
 use std::collections::HashMap;
 
 use crate::error;
@@ -36,6 +37,8 @@ pub struct ShellEnvironment {
     scopes: Vec<(EnvironmentScope, ShellVariableMap)>,
     /// Whether or not to auto-export variables on creation or modification.
     export_variables_on_modification: bool,
+    /// Count of total entries (may include duplicates with shadowed variables).
+    entry_count: usize,
 }
 
 impl Default for ShellEnvironment {
@@ -50,6 +53,7 @@ impl ShellEnvironment {
         Self {
             scopes: vec![(EnvironmentScope::Global, ShellVariableMap::new())],
             export_variables_on_modification: false,
+            entry_count: 0,
         }
     }
 
@@ -94,7 +98,10 @@ impl ShellEnvironment {
         &self,
         lookup_policy: EnvironmentLookup,
     ) -> impl Iterator<Item = (&String, &ShellVariable)> {
-        let mut visible_vars: HashMap<&String, &ShellVariable> = HashMap::new();
+        // We won't actually need to store all entries, but we expect it should be
+        // within the same order.
+        let mut visible_vars: HashMap<&String, &ShellVariable> =
+            HashMap::with_capacity(self.entry_count);
 
         let mut local_count = 0;
         for (scope_type, var_map) in self.scopes.iter().rev() {
@@ -122,8 +129,9 @@ impl ShellEnvironment {
             }
 
             for (name, var) in var_map.iter() {
-                if !visible_vars.contains_key(name) {
-                    visible_vars.insert(name, var);
+                // Only insert the variable if it hasn't been seen yet.
+                if let hash_map::Entry::Vacant(entry) = visible_vars.entry(name) {
+                    entry.insert(var);
                 }
             }
 
@@ -226,6 +234,9 @@ impl ShellEnvironment {
                         name,
                         ShellVariable::new(ShellValue::Unset(ShellValueUnsetType::Untyped)),
                     );
+                } else if self.entry_count > 0 {
+                    // Entry count should never be 0 here, but we're being defensive.
+                    self.entry_count -= 1;
                 }
 
                 return Ok(unset_result);
@@ -459,7 +470,11 @@ impl ShellEnvironment {
 
         for (scope_type, map) in self.scopes.iter_mut().rev() {
             if *scope_type == target_scope {
-                map.set(name, var);
+                let prev_var = map.set(name, var);
+                if prev_var.is_none() {
+                    self.entry_count += 1;
+                }
+
                 return Ok(());
             }
         }
@@ -543,7 +558,7 @@ impl ShellVariableMap {
     ///
     /// * `name` - The name of the variable to set.
     /// * `var` - The variable to set.
-    pub fn set<N: Into<String>>(&mut self, name: N, var: ShellVariable) {
-        self.variables.insert(name.into(), var);
+    pub fn set<N: Into<String>>(&mut self, name: N, var: ShellVariable) -> Option<ShellVariable> {
+        self.variables.insert(name.into(), var)
     }
 }

brush-core/src/shell.rs

@@ -639,7 +639,7 @@ impl Shell {
         // we inherited an out-of-sync version of the variable. Future updates
         // will be handled by set_working_dir().
         //
-        let pwd = std::env::current_dir()?.to_string_lossy().to_string();
+        let pwd = self.working_dir.to_string_lossy().to_string();
         let mut pwd_var = ShellVariable::new(pwd.into());
         pwd_var.export();
         self.env.set_global("PWD", pwd_var)?;
@@ -739,8 +739,8 @@ impl Shell {
         params: &ExecutionParameters,
     ) -> Result<bool, error::Error> {
         if path.exists() {
-            let args: Vec<String> = vec![];
-            self.source_script(path, &args, params).await?;
+            self.source_script(path, std::iter::empty::<String>(), params)
+                .await?;
             Ok(true)
         } else {
             tracing::debug!("skipping non-existent file: {}", path.display());
@@ -755,10 +755,10 @@ impl Shell {
     /// * `path` - The path to the file to source.
     /// * `args` - The arguments to pass to the script as positional parameters.
     /// * `params` - Execution parameters.
-    pub async fn source_script<S: AsRef<str>>(
+    pub async fn source_script<S: AsRef<str>, I: Iterator<Item = S>>(
         &mut self,
         path: &Path,
-        args: &[S],
+        args: I,
         params: &ExecutionParameters,
     ) -> Result<ExecutionResult, error::Error> {
         self.parse_and_execute_script_file(path, args, params, ScriptCallType::Sourced)
@@ -773,10 +773,10 @@ impl Shell {
     /// * `args` - The arguments to pass to the script as positional parameters.
     /// * `params` - Execution parameters.
     /// * `call_type` - The type of script call being made.
-    async fn parse_and_execute_script_file<S: AsRef<str>>(
+    async fn parse_and_execute_script_file<S: AsRef<str>, I: Iterator<Item = S>>(
         &mut self,
         path: &Path,
-        args: &[S],
+        args: I,
         params: &ExecutionParameters,
         call_type: ScriptCallType,
     ) -> Result<ExecutionResult, error::Error> {
@@ -809,11 +809,11 @@ impl Shell {
     /// * `args` - The arguments to pass to the script as positional parameters.
     /// * `params` - Execution parameters.
     /// * `call_type` - The type of script call being made.
-    async fn source_file<F: Read, S: AsRef<str>>(
+    async fn source_file<F: Read, S: AsRef<str>, I: Iterator<Item = S>>(
         &mut self,
         file: F,
         source_info: &brush_parser::SourceInfo,
-        args: &[S],
+        args: I,
         params: &ExecutionParameters,
         call_type: ScriptCallType,
     ) -> Result<ExecutionResult, error::Error> {
@@ -824,7 +824,7 @@ impl Shell {
         tracing::debug!(target: trace_categories::PARSE, "Parsing sourced file: {}", source_info.source);
         let parse_result = parser.parse();
 
-        let mut other_positional_parameters = args.iter().map(|s| s.as_ref().to_owned()).collect();
+        let mut other_positional_parameters = args.map(|s| s.as_ref().to_owned()).collect();
         let mut other_shell_name = Some(source_info.source.clone());
 
         // TODO: Find a cleaner way to change args.
@@ -975,10 +975,10 @@ impl Shell {
     ///
     /// * `script_path` - The path to the script file to execute.
     /// * `args` - The arguments to pass to the script as positional parameters.
-    pub async fn run_script<S: AsRef<str>>(
+    pub async fn run_script<S: AsRef<str>, I: Iterator<Item = S>>(
         &mut self,
         script_path: &Path,
-        args: &[S],
+        args: I,
     ) -> Result<ExecutionResult, error::Error> {
         self.parse_and_execute_script_file(
             script_path,

brush-shell/src/main.rs

@@ -164,7 +164,7 @@ async fn run_impl(
         shell
             .shell_mut()
             .as_mut()
-            .run_script(Path::new(&script_path), args.script_args.as_slice())
+            .run_script(Path::new(&script_path), args.script_args.iter())
             .await?;
     } else {
         // In all other cases, run interactively, taking input from stdin.

brush-shell/tests/cases/well_known_vars.yaml

@@ -39,7 +39,7 @@ cases:
       echo "Final BASH_ALIASES: " ${BASH_ALIASES[@]}
 
   - name: "BASHOPTS"
-    known_failure: true # Need to normalize which options are enabled in oracle
+    skip: true # Need to normalize which options are enabled in oracle
     stdin: |
       # Workaround for brush forcing on extglob
       shopt -u extglob

brush-shell/tests/completion_tests.rs

@@ -30,7 +30,11 @@ impl TestShellWithBashCompletion {
         let mut shell = brush_core::Shell::new(&create_options).await?;
         let exec_params = shell.default_exec_params();
         let source_result = shell
-            .source_script::<String>(bash_completion_script_path.as_path(), &[], &exec_params)
+            .source_script(
+                bash_completion_script_path.as_path(),
+                std::iter::empty::<String>(),
+                &exec_params,
+            )
             .await?;
 
         if source_result.exit_code != 0 {