Skip to content

Commit

Permalink
Deref &String arguments to &str where appropriate (nushell#10321)
Browse files Browse the repository at this point in the history
# Description
This generally makes for nicer APIs, as you are not forced to use an
existing allocation covering the full `String`.

Some exceptions remain where the underlying type requirements favor it.

# User-Facing Changes
None
  • Loading branch information
sholderbach authored and hardfau1t committed Dec 14, 2023
1 parent 9174547 commit 27fd058
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 71 deletions.
2 changes: 1 addition & 1 deletion crates/nu-cli/src/completions/completer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ impl NuCompleter {
for (flat_idx, flat) in flattened.iter().enumerate() {
let is_passthrough_command = spans
.first()
.filter(|content| *content == &String::from("sudo"))
.filter(|content| content.as_str() == "sudo")
.is_some();
// Read the current spam to string
let current_span = working_set.get_span_contents(flat.0).to_vec();
Expand Down
5 changes: 2 additions & 3 deletions crates/nu-cli/src/eval_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,8 @@ pub fn evaluate_file(
if block.signature.name == "main" {
block.signature.name = source_filename.to_string_lossy().to_string();
} else if block.signature.name.starts_with("main ") {
block.signature.name = source_filename.to_string_lossy().to_string()
+ " "
+ &String::from_utf8_lossy(&block.signature.name.as_bytes()[5..]);
block.signature.name =
source_filename.to_string_lossy().to_string() + " " + &block.signature.name[5..];
}
}

Expand Down
6 changes: 3 additions & 3 deletions crates/nu-command/tests/commands/rm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use nu_test_support::fs::{files_exist_at, Stub::EmptyFile};
use nu_test_support::nu;
use nu_test_support::playground::Playground;
use std::fs;
use std::path::PathBuf;
use std::path::Path;

#[test]
fn removes_a_file() {
Expand Down Expand Up @@ -376,10 +376,10 @@ fn removes_symlink() {
}

struct Cleanup<'a> {
dir_to_clean: &'a PathBuf,
dir_to_clean: &'a Path,
}

fn set_dir_read_only(directory: &PathBuf, read_only: bool) {
fn set_dir_read_only(directory: &Path, read_only: bool) {
let mut permissions = fs::metadata(directory).unwrap().permissions();
permissions.set_readonly(read_only);
fs::set_permissions(directory, permissions).expect("failed to set directory permissions");
Expand Down
6 changes: 3 additions & 3 deletions crates/nu-protocol/src/engine/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,16 +376,16 @@ impl Stack {
engine_state.env_vars.contains_key(name)
}

pub fn is_overlay_active(&self, name: &String) -> bool {
self.active_overlays.contains(name)
pub fn is_overlay_active(&self, name: &str) -> bool {
self.active_overlays.iter().any(|n| n == name)
}

pub fn add_overlay(&mut self, name: String) {
self.active_overlays.retain(|o| o != &name);
self.active_overlays.push(name);
}

pub fn remove_overlay(&mut self, name: &String) {
pub fn remove_overlay(&mut self, name: &str) {
self.active_overlays.retain(|o| o != name);
}
}
Expand Down
8 changes: 4 additions & 4 deletions crates/nu-protocol/src/eval_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@ use crate::{
record, HistoryFileFormat, PipelineData, Range, Record, ShellError, Span, Value,
};
use nu_system::os_info::{get_kernel_version, get_os_arch, get_os_family, get_os_name};
use std::path::PathBuf;
use std::path::{Path, PathBuf};

pub fn create_nu_constant(engine_state: &EngineState, span: Span) -> Result<Value, ShellError> {
fn canonicalize_path(engine_state: &EngineState, path: &PathBuf) -> PathBuf {
fn canonicalize_path(engine_state: &EngineState, path: &Path) -> PathBuf {
let cwd = engine_state.current_work_dir();

if path.exists() {
match nu_path::canonicalize_with(path, cwd) {
Ok(canon_path) => canon_path,
Err(_) => path.clone(),
Err(_) => path.to_owned(),
}
} else {
path.clone()
path.to_owned()
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/nu-test-support/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,9 +469,9 @@ pub fn read_std(std: &[u8]) -> String {
out.replace('\n', "")
}

use std::{path::PathBuf, process::Command};
use std::{path::Path, process::Command};

pub fn run_command(executable_path: &PathBuf, target_cwd: &str) -> Command {
pub fn run_command(executable_path: &Path, target_cwd: &str) -> Command {
let mut command = Command::new(executable_path);

command
Expand Down
8 changes: 4 additions & 4 deletions crates/nu-test-support/src/playground/matchers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub fn says() -> Play {

trait CheckerMatchers {
fn output(&self, actual: &Outcome) -> MatchResult;
fn std(&self, actual: &[u8], expected: Option<&String>, description: &str) -> MatchResult;
fn std(&self, actual: &[u8], expected: Option<&str>, description: &str) -> MatchResult;
fn stdout(&self, actual: &Outcome) -> MatchResult;
}

Expand All @@ -40,18 +40,18 @@ impl CheckerMatchers for Play {
}

fn stdout(&self, actual: &Outcome) -> MatchResult {
self.std(&actual.out, self.stdout_expectation.as_ref(), "stdout")
self.std(&actual.out, self.stdout_expectation.as_deref(), "stdout")
}

fn std(&self, actual: &[u8], expected: Option<&String>, description: &str) -> MatchResult {
fn std(&self, actual: &[u8], expected: Option<&str>, description: &str) -> MatchResult {
let out = match expected {
Some(out) => out,
None => return Ok(()),
};
let actual =
str::from_utf8(actual).map_err(|_| format!("{description} was not utf8 encoded"))?;

if actual != *out {
if actual != out {
return Err(format!(
"not equal:\n actual: {actual}\n expected: {out}\n\n"
));
Expand Down
8 changes: 4 additions & 4 deletions crates/nu-test-support/src/playground/play.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ impl Dirs {
self.fixtures.join("playground/config")
}

pub fn root(&self) -> &PathBuf {
&self.root
pub fn root(&self) -> &Path {
self.root.as_path()
}

pub fn test(&self) -> &PathBuf {
&self.test
pub fn test(&self) -> &Path {
self.test.as_path()
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/ide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ fn find_id(

fn read_in_file<'a>(
engine_state: &'a mut EngineState,
file_path: &String,
file_path: &str,
) -> (Vec<u8>, StateWorkingSet<'a>) {
let file = std::fs::read(file_path)
.into_diagnostic()
Expand All @@ -74,7 +74,7 @@ fn read_in_file<'a>(
(file, working_set)
}

pub fn check(engine_state: &mut EngineState, file_path: &String, max_errors: &Value) {
pub fn check(engine_state: &mut EngineState, file_path: &str, max_errors: &Value) {
let cwd = std::env::current_dir().expect("Could not get current working directory.");
engine_state.add_env_var("PWD".into(), Value::test_string(cwd.to_string_lossy()));

Expand Down Expand Up @@ -137,7 +137,7 @@ pub fn check(engine_state: &mut EngineState, file_path: &String, max_errors: &Va
}
}

pub fn goto_def(engine_state: &mut EngineState, file_path: &String, location: &Value) {
pub fn goto_def(engine_state: &mut EngineState, file_path: &str, location: &Value) {
let cwd = std::env::current_dir().expect("Could not get current working directory.");
engine_state.add_env_var("PWD".into(), Value::test_string(cwd.to_string_lossy()));

Expand Down Expand Up @@ -191,7 +191,7 @@ pub fn goto_def(engine_state: &mut EngineState, file_path: &String, location: &V
println!("{{}}");
}

pub fn hover(engine_state: &mut EngineState, file_path: &String, location: &Value) {
pub fn hover(engine_state: &mut EngineState, file_path: &str, location: &Value) {
let cwd = std::env::current_dir().expect("Could not get current working directory.");
engine_state.add_env_var("PWD".into(), Value::test_string(cwd.to_string_lossy()));

Expand Down Expand Up @@ -577,7 +577,7 @@ pub fn hover(engine_state: &mut EngineState, file_path: &String, location: &Valu
}
}

pub fn complete(engine_reference: Arc<EngineState>, file_path: &String, location: &Value) {
pub fn complete(engine_reference: Arc<EngineState>, file_path: &str, location: &Value) {
let stack = Stack::new();
let mut completer = NuCompleter::new(engine_reference, stack);

Expand All @@ -603,7 +603,7 @@ pub fn complete(engine_reference: Arc<EngineState>, file_path: &String, location
}
}

pub fn ast(engine_state: &mut EngineState, file_path: &String) {
pub fn ast(engine_state: &mut EngineState, file_path: &str) {
let cwd = std::env::current_dir().expect("Could not get current working directory.");
engine_state.add_env_var("PWD".into(), Value::test_string(cwd.to_string_lossy()));

Expand Down
46 changes: 23 additions & 23 deletions tests/path/canonicalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ fn canonicalize_path() {
Playground::setup("nu_path_test_1", |dirs, sandbox| {
sandbox.with_files(vec![EmptyFile("spam.txt")]);

let mut spam = dirs.test().clone();
let mut spam = dirs.test().to_owned();
spam.push("spam.txt");

let cwd = std::env::current_dir().expect("Could not get current directory");
Expand All @@ -24,7 +24,7 @@ fn canonicalize_unicode_path() {
Playground::setup("nu_path_test_1", |dirs, sandbox| {
sandbox.with_files(vec![EmptyFile("🚒.txt")]);

let mut spam = dirs.test().clone();
let mut spam = dirs.test().to_owned();
spam.push("🚒.txt");

let cwd = std::env::current_dir().expect("Could not get current directory");
Expand All @@ -47,7 +47,7 @@ fn canonicalize_path_relative_to() {
sandbox.with_files(vec![EmptyFile("spam.txt")]);

let actual = canonicalize_with("spam.txt", dirs.test()).expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand All @@ -60,11 +60,11 @@ fn canonicalize_unicode_path_relative_to_unicode_path_with_spaces() {
sandbox.mkdir("e-$ èрт🚒♞中片-j");
sandbox.with_files(vec![EmptyFile("e-$ èрт🚒♞中片-j/🚒.txt")]);

let mut relative_to = dirs.test().clone();
let mut relative_to = dirs.test().to_owned();
relative_to.push("e-$ èрт🚒♞中片-j");

let actual = canonicalize_with("🚒.txt", relative_to).expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("e-$ èрт🚒♞中片-j/🚒.txt");

assert_eq!(actual, expected);
Expand All @@ -82,7 +82,7 @@ fn canonicalize_absolute_path_relative_to() {
Playground::setup("nu_path_test_1", |dirs, sandbox| {
sandbox.with_files(vec![EmptyFile("spam.txt")]);

let mut absolute_path = dirs.test().clone();
let mut absolute_path = dirs.test().to_owned();
absolute_path.push("spam.txt");

let actual = canonicalize_with(&absolute_path, "non/existent/directory")
Expand Down Expand Up @@ -118,7 +118,7 @@ fn canonicalize_path_with_dot_relative_to() {
sandbox.with_files(vec![EmptyFile("spam.txt")]);

let actual = canonicalize_with("./spam.txt", dirs.test()).expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand All @@ -132,7 +132,7 @@ fn canonicalize_path_with_many_dots_relative_to() {

let actual = canonicalize_with("././/.//////./././//.////spam.txt", dirs.test())
.expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand All @@ -158,7 +158,7 @@ fn canonicalize_path_with_double_dot_relative_to() {

let actual =
canonicalize_with("foo/../spam.txt", dirs.test()).expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand All @@ -173,7 +173,7 @@ fn canonicalize_path_with_many_double_dots_relative_to() {

let actual = canonicalize_with("foo/bar/baz/../../../spam.txt", dirs.test())
.expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand Down Expand Up @@ -201,7 +201,7 @@ fn canonicalize_path_with_3_ndots_relative_to() {

let actual =
canonicalize_with("foo/bar/.../spam.txt", dirs.test()).expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand All @@ -219,7 +219,7 @@ fn canonicalize_path_with_many_3_ndots_relative_to() {
dirs.test(),
)
.expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand All @@ -234,7 +234,7 @@ fn canonicalize_path_with_4_ndots_relative_to() {

let actual = canonicalize_with("foo/bar/baz/..../spam.txt", dirs.test())
.expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand All @@ -252,7 +252,7 @@ fn canonicalize_path_with_many_4_ndots_relative_to() {
dirs.test(),
)
.expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand All @@ -265,12 +265,12 @@ fn canonicalize_path_with_way_too_many_dots_relative_to() {
sandbox.mkdir("foo/bar/baz/eggs/sausage/bacon/vikings");
sandbox.with_files(vec![EmptyFile("spam.txt")]);

let mut relative_to = dirs.test().clone();
let mut relative_to = dirs.test().to_owned();
relative_to.push("foo/bar/baz/eggs/sausage/bacon/vikings");

let actual = canonicalize_with("././..////././...///././.....///spam.txt", relative_to)
.expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand All @@ -283,12 +283,12 @@ fn canonicalize_unicode_path_with_way_too_many_dots_relative_to_unicode_path_wit
sandbox.mkdir("foo/áčěéí +šř=é/baz/eggs/e-$ èрт🚒♞中片-j/bacon/öäöä öäöä");
sandbox.with_files(vec![EmptyFile("🚒.txt")]);

let mut relative_to = dirs.test().clone();
let mut relative_to = dirs.test().to_owned();
relative_to.push("foo/áčěéí +šř=é/baz/eggs/e-$ èрт🚒♞中片-j/bacon/öäöä öäöä");

let actual = canonicalize_with("././..////././...///././.....///🚒.txt", relative_to)
.expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("🚒.txt");

assert_eq!(actual, expected);
Expand Down Expand Up @@ -324,12 +324,12 @@ fn canonicalize_symlink() {
sandbox.with_files(vec![EmptyFile("spam.txt")]);
sandbox.symlink("spam.txt", "link_to_spam.txt");

let mut symlink_path = dirs.test().clone();
let mut symlink_path = dirs.test().to_owned();
symlink_path.push("link_to_spam.txt");

let cwd = std::env::current_dir().expect("Could not get current directory");
let actual = canonicalize_with(symlink_path, cwd).expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand All @@ -345,7 +345,7 @@ fn canonicalize_symlink_relative_to() {

let actual =
canonicalize_with("link_to_spam.txt", dirs.test()).expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand Down Expand Up @@ -377,7 +377,7 @@ fn canonicalize_nested_symlink_relative_to() {

let actual = canonicalize_with("link_to_link_to_spam.txt", dirs.test())
.expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("spam.txt");

assert_eq!(actual, expected);
Expand All @@ -396,7 +396,7 @@ fn canonicalize_nested_symlink_within_symlink_dir_relative_to() {

let actual = canonicalize_with("link_to_foo/link_to_link_to_spam.txt", dirs.test())
.expect("Failed to canonicalize");
let mut expected = dirs.test().clone();
let mut expected = dirs.test().to_owned();
expected.push("foo/bar/baz/spam.txt");

assert_eq!(actual, expected);
Expand Down

0 comments on commit 27fd058

Please sign in to comment.