From e015118478c2471b557a9973d832bf538e703d50 Mon Sep 17 00:00:00 2001 From: x10an14 Date: Sat, 17 May 2025 16:44:27 +0200 Subject: [PATCH 1/9] flake: Update inputs --- flake.lock | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/flake.lock b/flake.lock index 93d06ac..794123c 100644 --- a/flake.lock +++ b/flake.lock @@ -5,11 +5,11 @@ "systems": "systems" }, "locked": { - "lastModified": 1726560853, - "narHash": "sha256-X6rJYSESBVr3hBoH0WbKE5KvhPU5bloyZ2L4K60/fPQ=", + "lastModified": 1731533236, + "narHash": "sha256-l0KFg5HjrsfsO/JpG+r7fRrqm12kzFHyUHqHCVpMMbI=", "owner": "numtide", "repo": "flake-utils", - "rev": "c1dfcf08411b08f6b8615f7d8971a2bfa81d5e8a", + "rev": "11707dc2f618dd54ca8739b309ec4fc024de578b", "type": "github" }, "original": { @@ -20,11 +20,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1728538411, - "narHash": "sha256-f0SBJz1eZ2yOuKUr5CA9BHULGXVSn6miBuUWdTyhUhU=", + "lastModified": 1747426788, + "narHash": "sha256-N4cp0asTsJCnRMFZ/k19V9akkxb7J/opG+K+jU57JGc=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "b69de56fac8c2b6f8fd27f2eca01dcda8e0a4221", + "rev": "12a55407652e04dcf2309436eb06fef0d3713ef3", "type": "github" }, "original": { @@ -36,11 +36,11 @@ }, "nixpkgs_2": { "locked": { - "lastModified": 1718428119, - "narHash": "sha256-WdWDpNaq6u1IPtxtYHHWpl5BmabtpmLnMAx0RdJ/vo8=", + "lastModified": 1744536153, + "narHash": "sha256-awS2zRgF4uTwrOKwwiJcByDzDOdo3Q1rPZbiHQg/N38=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "e6cea36f83499eb4e9cd184c8a8e823296b50ad5", + "rev": "18dd725c29603f582cf1900e0d25f9f1063dbf11", "type": "github" }, "original": { @@ -62,11 +62,11 @@ "nixpkgs": "nixpkgs_2" }, "locked": { - "lastModified": 1728959392, - "narHash": "sha256-fp4he1QQjE+vasDMspZYeXrwTm9otwEqLwEN6FKZ5v0=", + "lastModified": 1747449297, + "narHash": "sha256-veyXchTz6eWwvuW5X49UluHkheHkFcqHJSwGuKBhrmQ=", "owner": "oxalica", "repo": "rust-overlay", - "rev": "4c6e317300f05b8871f585b826b6f583e7dc4a9b", + "rev": "f44db7d7cea4528288780c6347756173a8248225", "type": "github" }, "original": { From acdd2f8a96e1c6bf6c51cc2bb171617b2ff674fb Mon Sep 17 00:00:00 2001 From: x10an14 Date: Sat, 17 May 2025 16:48:47 +0200 Subject: [PATCH 2/9] fix(direnv): Specify shell w/shebang + use direnv stdlib + use correct syntax --- .envrc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.envrc b/.envrc index 664e752..304e777 100644 --- a/.envrc +++ b/.envrc @@ -1,11 +1,11 @@ -# If nix is installed hook into it. -if [ $(which nix) ] -then +#!/usr/bin/env bash + +if has nix; then + # If nix is installed hook into it. use flake fi # If nu is the current shell use toolkit.nu -if [ $(echo $SHELL) == $(which nu) ] -then +if [ $(echo $SHELL) == $(which nu) ]; then nu -e "use toolkit.nu" fi From 7a132dee8daa6184d028b1535d554092f63aa40a Mon Sep 17 00:00:00 2001 From: x10an14 Date: Sat, 17 May 2025 16:56:04 +0200 Subject: [PATCH 3/9] flake: Simplify input dependencies --- flake.lock | 20 +++----------------- flake.nix | 5 ++++- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/flake.lock b/flake.lock index 794123c..fd219ce 100644 --- a/flake.lock +++ b/flake.lock @@ -34,22 +34,6 @@ "type": "github" } }, - "nixpkgs_2": { - "locked": { - "lastModified": 1744536153, - "narHash": "sha256-awS2zRgF4uTwrOKwwiJcByDzDOdo3Q1rPZbiHQg/N38=", - "owner": "NixOS", - "repo": "nixpkgs", - "rev": "18dd725c29603f582cf1900e0d25f9f1063dbf11", - "type": "github" - }, - "original": { - "owner": "NixOS", - "ref": "nixpkgs-unstable", - "repo": "nixpkgs", - "type": "github" - } - }, "root": { "inputs": { "flake-utils": "flake-utils", @@ -59,7 +43,9 @@ }, "rust-overlay": { "inputs": { - "nixpkgs": "nixpkgs_2" + "nixpkgs": [ + "nixpkgs" + ] }, "locked": { "lastModified": 1747449297, diff --git a/flake.nix b/flake.nix index 032683c..f3b5aca 100644 --- a/flake.nix +++ b/flake.nix @@ -5,7 +5,10 @@ nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable"; flake-utils.url = "github:numtide/flake-utils"; - rust-overlay.url = "github:oxalica/rust-overlay"; + rust-overlay = { + url = "github:oxalica/rust-overlay"; + inputs.nixpkgs.follows = "nixpkgs"; + }; }; outputs = { self, nixpkgs, rust-overlay, flake-utils, ... }: From 157e7c82eec9b505b4e3807ee06e1e2a9ad5a78f Mon Sep 17 00:00:00 2001 From: x10an14 Date: Sat, 17 May 2025 17:09:08 +0200 Subject: [PATCH 4/9] refactor(flake): Add formatter for nix + tidy up flake --- flake.lock | 23 ++++++++++++++++++++++- flake.nix | 37 ++++++++++++++++++++++--------------- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/flake.lock b/flake.lock index fd219ce..2b07bb3 100644 --- a/flake.lock +++ b/flake.lock @@ -38,7 +38,8 @@ "inputs": { "flake-utils": "flake-utils", "nixpkgs": "nixpkgs", - "rust-overlay": "rust-overlay" + "rust-overlay": "rust-overlay", + "treefmt-nix": "treefmt-nix" } }, "rust-overlay": { @@ -75,6 +76,26 @@ "repo": "default", "type": "github" } + }, + "treefmt-nix": { + "inputs": { + "nixpkgs": [ + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1747469671, + "narHash": "sha256-bo1ptiFoNqm6m1B2iAhJmWCBmqveLVvxom6xKmtuzjg=", + "owner": "numtide", + "repo": "treefmt-nix", + "rev": "ab0378b61b0d85e73a8ab05d5c6029b5bd58c9fb", + "type": "github" + }, + "original": { + "owner": "numtide", + "repo": "treefmt-nix", + "type": "github" + } } }, "root": "root", diff --git a/flake.nix b/flake.nix index f3b5aca..6f5ac7c 100644 --- a/flake.nix +++ b/flake.nix @@ -9,36 +9,43 @@ url = "github:oxalica/rust-overlay"; inputs.nixpkgs.follows = "nixpkgs"; }; + + treefmt-nix = { + url = "github:numtide/treefmt-nix"; + inputs.nixpkgs.follows = "nixpkgs"; + }; }; - outputs = { self, nixpkgs, rust-overlay, flake-utils, ... }: - flake-utils.lib.eachDefaultSystem (system: + outputs = + { self, ... }@inputs: + inputs.flake-utils.lib.eachDefaultSystem ( + system: let - overlays = [ (import rust-overlay) ]; - pkgs = import nixpkgs { + overlays = [ (import inputs.rust-overlay) ]; + pkgs = import inputs.nixpkgs { inherit system overlays; }; in { devShells.default = pkgs.mkShell { - nativeBuildInputs = with pkgs; [ - rust-bin.stable.latest.default - rust-analyzer - + inputsFrom = [ self.packages.${system}.default ]; + packages = with pkgs; [ nushell - ]; - buildInputs = with pkgs; [ ]; + # Not included in the package dependencies, but used for development + rust-analyzer + ]; }; - packages.default = pkgs.rustPlatform.buildRustPackage rec { + packages.default = self.packages.${system}.nufmt; + packages.nufmt = pkgs.rustPlatform.buildRustPackage { name = "nufmt"; - src = ./.; + cargoLock.lockFile = ./Cargo.lock; + }; - cargoLock = { - lockFile = ./Cargo.lock; - }; + formatter = inputs.treefmt-nix.lib.mkWrapper pkgs { + programs.nixfmt.enable = true; }; } ); From a77e2e66e2306b5de654eb89be1eebcf0ef390f7 Mon Sep 17 00:00:00 2001 From: x10an14 Date: Sat, 17 May 2025 17:31:22 +0200 Subject: [PATCH 5/9] fix(flake): Nix builds release + always specifies target triple Patch is necessary because Nix doesn't believe in testing what's not being distributed (the release version). See (unofficial) https://ryantm.github.io/nixpkgs/languages-frameworks/rust/#tests-relying-on-the-structure-of-the-target-directory. And since Nix supports distributing to multiple architectures, target architecture is always specified. --- flake.nix | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/flake.nix b/flake.nix index 6f5ac7c..c949aa8 100644 --- a/flake.nix +++ b/flake.nix @@ -25,6 +25,80 @@ pkgs = import inputs.nixpkgs { inherit system overlays; }; + integrationTestPatch = pkgs.writeTextFile { + name = "integration-tests-nix-fix.patch"; + text = + # patch + '' + diff --git a/tests/main.rs b/tests/main.rs + index c19811d..33c3829 100644 + --- a/tests/main.rs + +++ b/tests/main.rs + @@ -15,7 +15,7 @@ fn failure_with_invalid_config() { + let config_file = dir.path().join("nufmt.nuon"); + fs::write(&config_file, r#"{unknown: 1}"#).unwrap(); + + - let output = Command::new("target/debug/nufmt") + + let output = Command::new("target/@target_triple@/release/nufmt") + .arg("--config") + .arg(config_file.to_str().unwrap()) + .arg(dir.path().to_str().unwrap()) + @@ -29,7 +29,7 @@ fn failure_with_invalid_config() { + + #[test] + fn failure_with_invalid_config_file() { + - let output = Command::new("target/debug/nufmt") + + let output = Command::new("target/@target_triple@/release/nufmt") + .arg("--config") + .arg("path/that/does/not/exist/nufmt.nuon") + .output() + @@ -42,7 +42,7 @@ fn failure_with_invalid_config_file() { + + #[test] + fn failure_with_invalid_file_to_format() { + - let output = Command::new("target/debug/nufmt") + + let output = Command::new("target/@target_triple@/release/nufmt") + .arg("path/that/does/not/exist/a.nu") + .output() + .unwrap(); + @@ -56,7 +56,7 @@ fn failure_with_invalid_file_to_format() { + fn warning_when_no_files_are_detected() { + let dir = tempdir().unwrap(); + + - let output = Command::new("target/debug/nufmt") + + let output = Command::new("target/@target_triple@/release/nufmt") + .arg("--dry-run") + .arg(dir.path().to_str().unwrap()) + .output() + @@ -75,7 +75,7 @@ fn warning_is_displayed_when_no_files_are_detected_with_excluded_files() { + fs::write(&config_file, r#"{exclude: ["a*"]}"#).unwrap(); + fs::write(&file_a, INVALID).unwrap(); + + - let output = Command::new("target/debug/nufmt") + + let output = Command::new("target/@target_triple@/release/nufmt") + .arg("--config") + .arg(config_file.to_str().unwrap()) + .arg("--dry-run") + @@ -98,7 +98,7 @@ fn files_are_reformatted() { + fs::write(&file_a, INVALID).unwrap(); + fs::write(&file_b, INVALID).unwrap(); + + - let output = Command::new("target/debug/nufmt") + + let output = Command::new("target/@target_triple@/release/nufmt") + .arg("--config") + .arg(config_file.to_str().unwrap()) + .arg(dir.path().to_str().unwrap()) + @@ -122,7 +122,7 @@ fn files_are_checked() { + fs::write(&file_a, INVALID).unwrap(); + fs::write(&file_b, INVALID).unwrap(); + + - let output = Command::new("target/debug/nufmt") + + let output = Command::new("target/@target_triple@/release/nufmt") + .arg("--config") + .arg(config_file.to_str().unwrap()) + .arg("--dry-run") + ''; + }; in { devShells.default = pkgs.mkShell { @@ -41,6 +115,11 @@ packages.nufmt = pkgs.rustPlatform.buildRustPackage { name = "nufmt"; src = ./.; + patches = [ + (pkgs.replaceVars "${integrationTestPatch}" { + target_triple = pkgs.stdenv.hostPlatform.rust.rustcTarget; + }) + ]; cargoLock.lockFile = ./Cargo.lock; }; From d7723623c5545783b047cede4447014fb5435c7c Mon Sep 17 00:00:00 2001 From: x10an14 Date: Thu, 22 May 2025 22:50:50 +0200 Subject: [PATCH 6/9] refactor(flake/tests): Minimize required patch For when you'd want to test non-debug build binaries --- flake.nix | 70 ++++++--------------------------------------------- tests/main.rs | 15 +++++------ 2 files changed, 16 insertions(+), 69 deletions(-) diff --git a/flake.nix b/flake.nix index c949aa8..faa7952 100644 --- a/flake.nix +++ b/flake.nix @@ -31,72 +31,18 @@ # patch '' diff --git a/tests/main.rs b/tests/main.rs - index c19811d..33c3829 100644 + index c3a2b64..173aea0 100644 --- a/tests/main.rs +++ b/tests/main.rs - @@ -15,7 +15,7 @@ fn failure_with_invalid_config() { - let config_file = dir.path().join("nufmt.nuon"); - fs::write(&config_file, r#"{unknown: 1}"#).unwrap(); - - - let output = Command::new("target/debug/nufmt") - + let output = Command::new("target/@target_triple@/release/nufmt") - .arg("--config") - .arg(config_file.to_str().unwrap()) - .arg(dir.path().to_str().unwrap()) - @@ -29,7 +29,7 @@ fn failure_with_invalid_config() { - - #[test] - fn failure_with_invalid_config_file() { - - let output = Command::new("target/debug/nufmt") - + let output = Command::new("target/@target_triple@/release/nufmt") - .arg("--config") - .arg("path/that/does/not/exist/nufmt.nuon") - .output() - @@ -42,7 +42,7 @@ fn failure_with_invalid_config_file() { + @@ -8,7 +8,7 @@ let one = 1 + const VALID: &str = "# beginning of script comment + let one = 1 + "; + -const TEST_BINARY: &'static str = "target/debug/nufmt"; + +const TEST_BINARY: &'static str = "target/@target_triple@/release/nufmt"; #[test] - fn failure_with_invalid_file_to_format() { - - let output = Command::new("target/debug/nufmt") - + let output = Command::new("target/@target_triple@/release/nufmt") - .arg("path/that/does/not/exist/a.nu") - .output() - .unwrap(); - @@ -56,7 +56,7 @@ fn failure_with_invalid_file_to_format() { - fn warning_when_no_files_are_detected() { - let dir = tempdir().unwrap(); - - - let output = Command::new("target/debug/nufmt") - + let output = Command::new("target/@target_triple@/release/nufmt") - .arg("--dry-run") - .arg(dir.path().to_str().unwrap()) - .output() - @@ -75,7 +75,7 @@ fn warning_is_displayed_when_no_files_are_detected_with_excluded_files() { - fs::write(&config_file, r#"{exclude: ["a*"]}"#).unwrap(); - fs::write(&file_a, INVALID).unwrap(); - - - let output = Command::new("target/debug/nufmt") - + let output = Command::new("target/@target_triple@/release/nufmt") - .arg("--config") - .arg(config_file.to_str().unwrap()) - .arg("--dry-run") - @@ -98,7 +98,7 @@ fn files_are_reformatted() { - fs::write(&file_a, INVALID).unwrap(); - fs::write(&file_b, INVALID).unwrap(); - - - let output = Command::new("target/debug/nufmt") - + let output = Command::new("target/@target_triple@/release/nufmt") - .arg("--config") - .arg(config_file.to_str().unwrap()) - .arg(dir.path().to_str().unwrap()) - @@ -122,7 +122,7 @@ fn files_are_checked() { - fs::write(&file_a, INVALID).unwrap(); - fs::write(&file_b, INVALID).unwrap(); - - - let output = Command::new("target/debug/nufmt") - + let output = Command::new("target/@target_triple@/release/nufmt") - .arg("--config") - .arg(config_file.to_str().unwrap()) - .arg("--dry-run") + fn failure_with_invalid_config() { ''; }; in diff --git a/tests/main.rs b/tests/main.rs index c19811d..c3a2b64 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -8,6 +8,7 @@ let one = 1 const VALID: &str = "# beginning of script comment let one = 1 "; +const TEST_BINARY: &'static str = "target/debug/nufmt"; #[test] fn failure_with_invalid_config() { @@ -15,7 +16,7 @@ fn failure_with_invalid_config() { let config_file = dir.path().join("nufmt.nuon"); fs::write(&config_file, r#"{unknown: 1}"#).unwrap(); - let output = Command::new("target/debug/nufmt") + let output = Command::new(TEST_BINARY) .arg("--config") .arg(config_file.to_str().unwrap()) .arg(dir.path().to_str().unwrap()) @@ -29,7 +30,7 @@ fn failure_with_invalid_config() { #[test] fn failure_with_invalid_config_file() { - let output = Command::new("target/debug/nufmt") + let output = Command::new(TEST_BINARY) .arg("--config") .arg("path/that/does/not/exist/nufmt.nuon") .output() @@ -42,7 +43,7 @@ fn failure_with_invalid_config_file() { #[test] fn failure_with_invalid_file_to_format() { - let output = Command::new("target/debug/nufmt") + let output = Command::new(TEST_BINARY) .arg("path/that/does/not/exist/a.nu") .output() .unwrap(); @@ -56,7 +57,7 @@ fn failure_with_invalid_file_to_format() { fn warning_when_no_files_are_detected() { let dir = tempdir().unwrap(); - let output = Command::new("target/debug/nufmt") + let output = Command::new(TEST_BINARY) .arg("--dry-run") .arg(dir.path().to_str().unwrap()) .output() @@ -75,7 +76,7 @@ fn warning_is_displayed_when_no_files_are_detected_with_excluded_files() { fs::write(&config_file, r#"{exclude: ["a*"]}"#).unwrap(); fs::write(&file_a, INVALID).unwrap(); - let output = Command::new("target/debug/nufmt") + let output = Command::new(TEST_BINARY) .arg("--config") .arg(config_file.to_str().unwrap()) .arg("--dry-run") @@ -98,7 +99,7 @@ fn files_are_reformatted() { fs::write(&file_a, INVALID).unwrap(); fs::write(&file_b, INVALID).unwrap(); - let output = Command::new("target/debug/nufmt") + let output = Command::new(TEST_BINARY) .arg("--config") .arg(config_file.to_str().unwrap()) .arg(dir.path().to_str().unwrap()) @@ -122,7 +123,7 @@ fn files_are_checked() { fs::write(&file_a, INVALID).unwrap(); fs::write(&file_b, INVALID).unwrap(); - let output = Command::new("target/debug/nufmt") + let output = Command::new(TEST_BINARY) .arg("--config") .arg(config_file.to_str().unwrap()) .arg("--dry-run") From 36c31e84dd1a4cd2d898f421292750bbd22af519 Mon Sep 17 00:00:00 2001 From: x10an14 Date: Fri, 23 May 2025 16:50:35 +0200 Subject: [PATCH 7/9] fix(clippy): Satisfy clippy --- flake.nix | 3 +++ src/config.rs | 24 +++++++++++++------ src/config_error.rs | 6 ++--- src/formatting.rs | 30 ++++++++++++++---------- src/lib.rs | 7 +++--- src/main.rs | 57 ++++++++++++++++++++++++++------------------- 6 files changed, 78 insertions(+), 49 deletions(-) diff --git a/flake.nix b/flake.nix index faa7952..b90e597 100644 --- a/flake.nix +++ b/flake.nix @@ -53,6 +53,9 @@ nushell # Not included in the package dependencies, but used for development + cargo-watch + clippy + rustfmt rust-analyzer ]; }; diff --git a/src/config.rs b/src/config.rs index b4cb598..b6b5010 100644 --- a/src/config.rs +++ b/src/config.rs @@ -15,7 +15,7 @@ pub struct Config { impl Default for Config { fn default() -> Self { - Config { + Self { indent: 4, line_length: 80, margin: 1, @@ -25,8 +25,9 @@ impl Default for Config { } impl Config { - pub fn new(tab_spaces: usize, max_width: usize, margin: usize) -> Self { - Config { + #[must_use] + pub const fn new(tab_spaces: usize, max_width: usize, margin: usize) -> Self { + Self { indent: tab_spaces, line_length: max_width, margin, @@ -39,7 +40,7 @@ impl TryFrom for Config { type Error = ConfigError; fn try_from(value: Value) -> Result { - let mut config = Config::default(); + let mut config = Self::default(); match value { Value::Nothing { .. } => (), Value::Record { val: record, .. } => { @@ -68,7 +69,7 @@ impl TryFrom for Config { _ => { return Err(ConfigError::InvalidFormat); } - }; + } Ok(config) } } @@ -79,11 +80,20 @@ fn parse_value_to_usize(key: &str, value: &Value) -> Result if *val <= 0 { return Err(ConfigError::InvalidOptionValue( key.to_string(), - format!("{}", val), + format!("{val}"), "a positive number", )); } - Ok(*val as usize) + usize::try_from(*val).map_or_else( + |_| { + Err(ConfigError::InvalidOptionValue( + key.to_string(), + format!("{val}"), + "Not castable as usize", + )) + }, + Ok, + ) } other => Err(ConfigError::InvalidOptionType( key.to_string(), diff --git a/src/config_error.rs b/src/config_error.rs index 8b0d5e6..746c7e2 100644 --- a/src/config_error.rs +++ b/src/config_error.rs @@ -18,18 +18,18 @@ pub enum ConfigError { impl From for ConfigError { fn from(value: std::io::Error) -> Self { - ConfigError::IOError(value.to_string()) + Self::IOError(value.to_string()) } } impl From for ConfigError { fn from(_value: nu_protocol::ShellError) -> Self { - ConfigError::InvalidFormat + Self::InvalidFormat } } impl From for ConfigError { fn from(_value: ignore::Error) -> Self { - ConfigError::InvalidExcludePattern + Self::InvalidExcludePattern } } diff --git a/src/formatting.rs b/src/formatting.rs index 84397d2..a537e60 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -1,6 +1,8 @@ //! In this module occurs most of the magic in `nufmt`. //! //! It has functions to format slice of bytes and some help functions to separate concerns while doing the job. +use std::str::Utf8Error; + use crate::config::Config; use crate::format_error::FormatError; use log::{debug, trace}; @@ -12,17 +14,17 @@ use nu_protocol::{ }; trait DeclsExt { - fn get_decl_name(&self, decl_id: usize) -> Option<&str>; + fn get_decl_name(&self, decl_id: usize) -> Result, Utf8Error>; } impl DeclsExt for Vec<(Vec, DeclId)> { - fn get_decl_name(&self, decl_id: usize) -> Option<&str> { + fn get_decl_name(&self, decl_id: usize) -> Result, Utf8Error> { for decl in self { if decl_id == decl.1.get() { - return Some(std::str::from_utf8(&decl.0).expect("Failed to parse DeclId's name")); + return Some(std::str::from_utf8(&decl.0)).transpose(); } } - None + Ok(None) } } @@ -68,7 +70,7 @@ pub(crate) fn format_inner(contents: &[u8], _config: &Config) -> Result, let skipped_contents = &contents[start..span.start]; let printable = String::from_utf8_lossy(skipped_contents).to_string(); - trace!("contents: {:?}", printable); + trace!("contents: {printable:?}"); out = write_only_if_have_hastag_or_equal(skipped_contents, out, true); } @@ -107,7 +109,7 @@ pub(crate) fn format_inner(contents: &[u8], _config: &Config) -> Result, trace!("Called Internal call with {declid}"); - if let Some(decl_name) = decl_name { + if let Ok(Some(decl_name)) = decl_name { out = resolve_call(bytes, decl_name, out); after_a_def = decl_name == "def"; } @@ -139,7 +141,7 @@ pub(crate) fn format_inner(contents: &[u8], _config: &Config) -> Result, let remaining_contents = &contents[span.end..end_of_file]; let printable = String::from_utf8_lossy(remaining_contents).to_string(); - trace!("contents: {:?}", printable); + trace!("contents: {printable:?}"); out = write_only_if_have_hastag_or_equal(remaining_contents, out, false); } @@ -194,8 +196,7 @@ fn write_only_if_have_hastag_or_equal( #[allow(clippy::wildcard_in_or_patterns)] fn resolve_call(c_bytes: &[u8], decl_name: &str, mut out: Vec) -> Vec { out = match decl_name { - "if" => insert_newline(out), - "def" => insert_newline(out), + "if" | "def" => insert_newline(out), "export def" | _ => out, }; out.extend(c_bytes); @@ -231,10 +232,12 @@ fn trim_ascii_whitespace(x: &[u8]) -> &[u8] { let Some(from) = x.iter().position(|x| !x.is_ascii_whitespace()) else { return &x[0..0]; }; - let to = x.iter().rposition(|x| !x.is_ascii_whitespace()).unwrap(); + let Some(to) = x.iter().rposition(|x| !x.is_ascii_whitespace()) else { + panic!("Unhandled Option::None") + }; let result = &x[from..=to]; let printable = String::from_utf8_lossy(result).to_string(); - trace!("stripped the whitespace, result: {:?}", printable); + trace!("stripped the whitespace, result: {printable:?}"); result } @@ -253,5 +256,8 @@ fn block_has_pipelines(block: &Block) -> bool { /// return true if the given span is the last one fn is_last_span(span: Span, flat: &[(Span, FlatShape)]) -> bool { - span == flat.last().unwrap().0 + let Some(last_element) = flat.last() else { + return false; + }; + span == last_element.0 } diff --git a/src/lib.rs b/src/lib.rs index 5f4353f..cb8ae00 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,6 +34,7 @@ pub enum FileDiagnostic { } /// format a Nushell file in place. Do not write in dry-run mode. +#[must_use] pub fn format_single_file( file: PathBuf, config: &Config, @@ -75,12 +76,12 @@ pub fn format_single_file( } debug!("File formatted."); } - }; + } (file, FileDiagnostic::Reformatted) } /// format a string of Nushell code -pub fn format_string(input_string: &String, config: &Config) -> Result { +pub fn format_string(input_string: &str, config: &Config) -> Result { let contents = input_string.as_bytes(); let formatted_bytes = format_inner(contents, config)?; Ok(String::from_utf8(formatted_bytes) @@ -95,7 +96,7 @@ mod test { /// 1. formatting the input gives the expected result /// 2. formatting the output of `nufmt` a second time does not change the content fn run_test(input: &str, expected: &str) { - let formatted = format_string(&input.to_string(), &Config::default()).unwrap(); + let formatted = format_string(input, &Config::default()).unwrap(); assert_eq!(expected.to_string(), formatted); assert_eq!( diff --git a/src/main.rs b/src/main.rs index 1ca5488..03b9767 100644 --- a/src/main.rs +++ b/src/main.rs @@ -40,11 +40,11 @@ impl ExitCode { /// Return the exit code to use. /// If check mode is off: return 2 if at least one file could not be formatted, 0 otherwise (regardless of whether any files were formatted). /// If check mode is on: return 1 if some files would be formatted if check mode was off, 0 otherwise. - fn code(&self) -> i32 { + const fn code(&self) -> i32 { match self { - ExitCode::Success => 0, - ExitCode::CheckFailed => 1, - ExitCode::Failure => 2, + Self::Success => 0, + Self::CheckFailed => 1, + Self::Failure => 2, } } } @@ -80,7 +80,7 @@ struct Cli { config: Option, } -fn exit_with_code(exit_code: ExitCode) { +fn exit_with_code(exit_code: &ExitCode) { let code = exit_code.code(); trace!("exit code: {code}"); @@ -97,27 +97,36 @@ fn main() { trace!("recieved cli.stdin: {:?}", cli.stdin); trace!("recieved cli.config: {:?}", cli.config); - let config_file = cli.config.or(find_in_parent_dirs(DEFAULT_CONFIG_FILE)); + let config_file = cli + .config + .or_else(|| find_in_parent_dirs(DEFAULT_CONFIG_FILE)); let config = match config_file { None => Config::default(), Some(cli_config) => match read_config(&cli_config) { Ok(config) => config, Err(err) => { eprintln!("{}: {}", Color::LightRed.paint("error"), &err); - return exit_with_code(ExitCode::Failure); + return exit_with_code(&ExitCode::Failure); } }, }; let exit_code = if cli.stdin { - let stdin_input = io::stdin().lines().map(|x| x.unwrap()).collect(); - format_string(stdin_input, &config) + let mut input = String::new(); + for line in io::stdin().lines() { + let l = match line { + Ok(l) => l, + Err(e) => panic!("Encountered error reading from stdin: {:?}", e), + }; + input = format!("{input}\n{l}"); + } + format_string(&input, &config) } else { let (target_files, invalid_files) = match discover_nu_files(cli.files, &config.excludes) { Ok(files) => files, Err(err) => { eprintln!("{}: {}", Color::LightRed.paint("error"), err); - return exit_with_code(ExitCode::Failure); + return exit_with_code(&ExitCode::Failure); } }; let mode = if cli.dry_run { @@ -130,11 +139,11 @@ fn main() { display_diagnostic_and_compute_exit_code(&results, cli.dry_run) }; - std::io::stdout() - .flush() - .expect("Unexpected error occurred when flushing stdout"); + let Ok(()) = std::io::stdout().flush() else { + panic!("Unexpected error occurred when flushing stdout"); + }; - exit_with_code(exit_code); + exit_with_code(&exit_code); } fn read_config(path: &PathBuf) -> Result { @@ -144,8 +153,8 @@ fn read_config(path: &PathBuf) -> Result { } /// format a string passed via stdin and output it directly to stdout -fn format_string(string: String, options: &Config) -> ExitCode { - match nu_formatter::format_string(&string, options) { +fn format_string(string: &str, options: &Config) -> ExitCode { + match nu_formatter::format_string(string, options) { Ok(output) => { println!("{output}"); ExitCode::Success @@ -215,7 +224,7 @@ fn display_diagnostic_and_compute_exit_code( "Would reformat: {}", Style::new().bold().paint(make_relative(file)) )); - }; + } } FileDiagnostic::Failure(reason) => { failures += 1; @@ -232,7 +241,7 @@ fn display_diagnostic_and_compute_exit_code( } for msg in warning_messages { - println!("{}", msg); + println!("{msg}"); } if already_formatted + reformatted_or_would_reformat + failures == 0 { @@ -266,7 +275,7 @@ fn display_diagnostic_and_compute_exit_code( already_formatted, if already_formatted == 1 { "" } else { "s" } ); - }; + } if at_least_one_failure { ExitCode::Failure } else if check_mode && reformatted_or_would_reformat > 0 { @@ -295,7 +304,7 @@ fn discover_nu_files( let mut overrides = OverrideBuilder::new("."); for pattern in excludes { - overrides.add(&format!("!{}", pattern))?; + overrides.add(&format!("!{pattern}"))?; } let overrides = overrides.build()?; @@ -307,7 +316,7 @@ fn discover_nu_files( .build() .filter_map(Result::ok) .filter(is_nu_file) - .map(|path| path.into_path()) + .map(ignore::DirEntry::into_path) .collect::>() }) .collect(); @@ -317,12 +326,12 @@ fn discover_nu_files( /// Return whether a `DirEntry` is a .nu file or not fn is_nu_file(entry: &DirEntry) -> bool { - entry.file_type().map(|ft| ft.is_file()).unwrap_or(false) + entry.file_type().is_some_and(|ft| ft.is_file()) && entry.path().extension().is_some_and(|ext| ext == "nu") } fn make_relative(path: &Path) -> String { - let current = std::env::current_dir().unwrap_or(PathBuf::from(".")); + let current = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); path.strip_prefix(¤t) .unwrap_or(path) .display() @@ -335,7 +344,7 @@ fn make_relative(path: &Path) -> String { /// Search for `filename` in current or any parent directories. /// If `start_dir` is not provided, the current directory is used fn find_in_parent_dirs(filename: &str) -> Option { - let start_dir = std::env::current_dir().unwrap_or(PathBuf::from(".")); + let start_dir = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let mut dir = Some(start_dir.as_path()); while let Some(current) = dir { From 7487a1f5731626f9296b6e2359c3bb8392f36f7d Mon Sep 17 00:00:00 2001 From: x10an14 Date: Fri, 23 May 2025 17:11:35 +0200 Subject: [PATCH 8/9] fix(tests): Satisfy clippy --- tests/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/main.rs b/tests/main.rs index c3a2b64..569f791 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -8,7 +8,7 @@ let one = 1 const VALID: &str = "# beginning of script comment let one = 1 "; -const TEST_BINARY: &'static str = "target/debug/nufmt"; +const TEST_BINARY: &str = "target/debug/nufmt"; #[test] fn failure_with_invalid_config() { From 2809ce255afda23501cc7e968d589cb046d427ce Mon Sep 17 00:00:00 2001 From: x10an14 Date: Fri, 23 May 2025 17:25:32 +0200 Subject: [PATCH 9/9] Revert "fix(clippy): Satisfy clippy" This reverts commit 36c31e84dd1a4cd2d898f421292750bbd22af519. --- flake.nix | 3 --- src/config.rs | 24 ++++++------------- src/config_error.rs | 6 ++--- src/formatting.rs | 30 ++++++++++-------------- src/lib.rs | 7 +++--- src/main.rs | 57 +++++++++++++++++++-------------------------- 6 files changed, 49 insertions(+), 78 deletions(-) diff --git a/flake.nix b/flake.nix index b90e597..faa7952 100644 --- a/flake.nix +++ b/flake.nix @@ -53,9 +53,6 @@ nushell # Not included in the package dependencies, but used for development - cargo-watch - clippy - rustfmt rust-analyzer ]; }; diff --git a/src/config.rs b/src/config.rs index b6b5010..b4cb598 100644 --- a/src/config.rs +++ b/src/config.rs @@ -15,7 +15,7 @@ pub struct Config { impl Default for Config { fn default() -> Self { - Self { + Config { indent: 4, line_length: 80, margin: 1, @@ -25,9 +25,8 @@ impl Default for Config { } impl Config { - #[must_use] - pub const fn new(tab_spaces: usize, max_width: usize, margin: usize) -> Self { - Self { + pub fn new(tab_spaces: usize, max_width: usize, margin: usize) -> Self { + Config { indent: tab_spaces, line_length: max_width, margin, @@ -40,7 +39,7 @@ impl TryFrom for Config { type Error = ConfigError; fn try_from(value: Value) -> Result { - let mut config = Self::default(); + let mut config = Config::default(); match value { Value::Nothing { .. } => (), Value::Record { val: record, .. } => { @@ -69,7 +68,7 @@ impl TryFrom for Config { _ => { return Err(ConfigError::InvalidFormat); } - } + }; Ok(config) } } @@ -80,20 +79,11 @@ fn parse_value_to_usize(key: &str, value: &Value) -> Result if *val <= 0 { return Err(ConfigError::InvalidOptionValue( key.to_string(), - format!("{val}"), + format!("{}", val), "a positive number", )); } - usize::try_from(*val).map_or_else( - |_| { - Err(ConfigError::InvalidOptionValue( - key.to_string(), - format!("{val}"), - "Not castable as usize", - )) - }, - Ok, - ) + Ok(*val as usize) } other => Err(ConfigError::InvalidOptionType( key.to_string(), diff --git a/src/config_error.rs b/src/config_error.rs index 746c7e2..8b0d5e6 100644 --- a/src/config_error.rs +++ b/src/config_error.rs @@ -18,18 +18,18 @@ pub enum ConfigError { impl From for ConfigError { fn from(value: std::io::Error) -> Self { - Self::IOError(value.to_string()) + ConfigError::IOError(value.to_string()) } } impl From for ConfigError { fn from(_value: nu_protocol::ShellError) -> Self { - Self::InvalidFormat + ConfigError::InvalidFormat } } impl From for ConfigError { fn from(_value: ignore::Error) -> Self { - Self::InvalidExcludePattern + ConfigError::InvalidExcludePattern } } diff --git a/src/formatting.rs b/src/formatting.rs index a537e60..84397d2 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -1,8 +1,6 @@ //! In this module occurs most of the magic in `nufmt`. //! //! It has functions to format slice of bytes and some help functions to separate concerns while doing the job. -use std::str::Utf8Error; - use crate::config::Config; use crate::format_error::FormatError; use log::{debug, trace}; @@ -14,17 +12,17 @@ use nu_protocol::{ }; trait DeclsExt { - fn get_decl_name(&self, decl_id: usize) -> Result, Utf8Error>; + fn get_decl_name(&self, decl_id: usize) -> Option<&str>; } impl DeclsExt for Vec<(Vec, DeclId)> { - fn get_decl_name(&self, decl_id: usize) -> Result, Utf8Error> { + fn get_decl_name(&self, decl_id: usize) -> Option<&str> { for decl in self { if decl_id == decl.1.get() { - return Some(std::str::from_utf8(&decl.0)).transpose(); + return Some(std::str::from_utf8(&decl.0).expect("Failed to parse DeclId's name")); } } - Ok(None) + None } } @@ -70,7 +68,7 @@ pub(crate) fn format_inner(contents: &[u8], _config: &Config) -> Result, let skipped_contents = &contents[start..span.start]; let printable = String::from_utf8_lossy(skipped_contents).to_string(); - trace!("contents: {printable:?}"); + trace!("contents: {:?}", printable); out = write_only_if_have_hastag_or_equal(skipped_contents, out, true); } @@ -109,7 +107,7 @@ pub(crate) fn format_inner(contents: &[u8], _config: &Config) -> Result, trace!("Called Internal call with {declid}"); - if let Ok(Some(decl_name)) = decl_name { + if let Some(decl_name) = decl_name { out = resolve_call(bytes, decl_name, out); after_a_def = decl_name == "def"; } @@ -141,7 +139,7 @@ pub(crate) fn format_inner(contents: &[u8], _config: &Config) -> Result, let remaining_contents = &contents[span.end..end_of_file]; let printable = String::from_utf8_lossy(remaining_contents).to_string(); - trace!("contents: {printable:?}"); + trace!("contents: {:?}", printable); out = write_only_if_have_hastag_or_equal(remaining_contents, out, false); } @@ -196,7 +194,8 @@ fn write_only_if_have_hastag_or_equal( #[allow(clippy::wildcard_in_or_patterns)] fn resolve_call(c_bytes: &[u8], decl_name: &str, mut out: Vec) -> Vec { out = match decl_name { - "if" | "def" => insert_newline(out), + "if" => insert_newline(out), + "def" => insert_newline(out), "export def" | _ => out, }; out.extend(c_bytes); @@ -232,12 +231,10 @@ fn trim_ascii_whitespace(x: &[u8]) -> &[u8] { let Some(from) = x.iter().position(|x| !x.is_ascii_whitespace()) else { return &x[0..0]; }; - let Some(to) = x.iter().rposition(|x| !x.is_ascii_whitespace()) else { - panic!("Unhandled Option::None") - }; + let to = x.iter().rposition(|x| !x.is_ascii_whitespace()).unwrap(); let result = &x[from..=to]; let printable = String::from_utf8_lossy(result).to_string(); - trace!("stripped the whitespace, result: {printable:?}"); + trace!("stripped the whitespace, result: {:?}", printable); result } @@ -256,8 +253,5 @@ fn block_has_pipelines(block: &Block) -> bool { /// return true if the given span is the last one fn is_last_span(span: Span, flat: &[(Span, FlatShape)]) -> bool { - let Some(last_element) = flat.last() else { - return false; - }; - span == last_element.0 + span == flat.last().unwrap().0 } diff --git a/src/lib.rs b/src/lib.rs index cb8ae00..5f4353f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,7 +34,6 @@ pub enum FileDiagnostic { } /// format a Nushell file in place. Do not write in dry-run mode. -#[must_use] pub fn format_single_file( file: PathBuf, config: &Config, @@ -76,12 +75,12 @@ pub fn format_single_file( } debug!("File formatted."); } - } + }; (file, FileDiagnostic::Reformatted) } /// format a string of Nushell code -pub fn format_string(input_string: &str, config: &Config) -> Result { +pub fn format_string(input_string: &String, config: &Config) -> Result { let contents = input_string.as_bytes(); let formatted_bytes = format_inner(contents, config)?; Ok(String::from_utf8(formatted_bytes) @@ -96,7 +95,7 @@ mod test { /// 1. formatting the input gives the expected result /// 2. formatting the output of `nufmt` a second time does not change the content fn run_test(input: &str, expected: &str) { - let formatted = format_string(input, &Config::default()).unwrap(); + let formatted = format_string(&input.to_string(), &Config::default()).unwrap(); assert_eq!(expected.to_string(), formatted); assert_eq!( diff --git a/src/main.rs b/src/main.rs index 03b9767..1ca5488 100644 --- a/src/main.rs +++ b/src/main.rs @@ -40,11 +40,11 @@ impl ExitCode { /// Return the exit code to use. /// If check mode is off: return 2 if at least one file could not be formatted, 0 otherwise (regardless of whether any files were formatted). /// If check mode is on: return 1 if some files would be formatted if check mode was off, 0 otherwise. - const fn code(&self) -> i32 { + fn code(&self) -> i32 { match self { - Self::Success => 0, - Self::CheckFailed => 1, - Self::Failure => 2, + ExitCode::Success => 0, + ExitCode::CheckFailed => 1, + ExitCode::Failure => 2, } } } @@ -80,7 +80,7 @@ struct Cli { config: Option, } -fn exit_with_code(exit_code: &ExitCode) { +fn exit_with_code(exit_code: ExitCode) { let code = exit_code.code(); trace!("exit code: {code}"); @@ -97,36 +97,27 @@ fn main() { trace!("recieved cli.stdin: {:?}", cli.stdin); trace!("recieved cli.config: {:?}", cli.config); - let config_file = cli - .config - .or_else(|| find_in_parent_dirs(DEFAULT_CONFIG_FILE)); + let config_file = cli.config.or(find_in_parent_dirs(DEFAULT_CONFIG_FILE)); let config = match config_file { None => Config::default(), Some(cli_config) => match read_config(&cli_config) { Ok(config) => config, Err(err) => { eprintln!("{}: {}", Color::LightRed.paint("error"), &err); - return exit_with_code(&ExitCode::Failure); + return exit_with_code(ExitCode::Failure); } }, }; let exit_code = if cli.stdin { - let mut input = String::new(); - for line in io::stdin().lines() { - let l = match line { - Ok(l) => l, - Err(e) => panic!("Encountered error reading from stdin: {:?}", e), - }; - input = format!("{input}\n{l}"); - } - format_string(&input, &config) + let stdin_input = io::stdin().lines().map(|x| x.unwrap()).collect(); + format_string(stdin_input, &config) } else { let (target_files, invalid_files) = match discover_nu_files(cli.files, &config.excludes) { Ok(files) => files, Err(err) => { eprintln!("{}: {}", Color::LightRed.paint("error"), err); - return exit_with_code(&ExitCode::Failure); + return exit_with_code(ExitCode::Failure); } }; let mode = if cli.dry_run { @@ -139,11 +130,11 @@ fn main() { display_diagnostic_and_compute_exit_code(&results, cli.dry_run) }; - let Ok(()) = std::io::stdout().flush() else { - panic!("Unexpected error occurred when flushing stdout"); - }; + std::io::stdout() + .flush() + .expect("Unexpected error occurred when flushing stdout"); - exit_with_code(&exit_code); + exit_with_code(exit_code); } fn read_config(path: &PathBuf) -> Result { @@ -153,8 +144,8 @@ fn read_config(path: &PathBuf) -> Result { } /// format a string passed via stdin and output it directly to stdout -fn format_string(string: &str, options: &Config) -> ExitCode { - match nu_formatter::format_string(string, options) { +fn format_string(string: String, options: &Config) -> ExitCode { + match nu_formatter::format_string(&string, options) { Ok(output) => { println!("{output}"); ExitCode::Success @@ -224,7 +215,7 @@ fn display_diagnostic_and_compute_exit_code( "Would reformat: {}", Style::new().bold().paint(make_relative(file)) )); - } + }; } FileDiagnostic::Failure(reason) => { failures += 1; @@ -241,7 +232,7 @@ fn display_diagnostic_and_compute_exit_code( } for msg in warning_messages { - println!("{msg}"); + println!("{}", msg); } if already_formatted + reformatted_or_would_reformat + failures == 0 { @@ -275,7 +266,7 @@ fn display_diagnostic_and_compute_exit_code( already_formatted, if already_formatted == 1 { "" } else { "s" } ); - } + }; if at_least_one_failure { ExitCode::Failure } else if check_mode && reformatted_or_would_reformat > 0 { @@ -304,7 +295,7 @@ fn discover_nu_files( let mut overrides = OverrideBuilder::new("."); for pattern in excludes { - overrides.add(&format!("!{pattern}"))?; + overrides.add(&format!("!{}", pattern))?; } let overrides = overrides.build()?; @@ -316,7 +307,7 @@ fn discover_nu_files( .build() .filter_map(Result::ok) .filter(is_nu_file) - .map(ignore::DirEntry::into_path) + .map(|path| path.into_path()) .collect::>() }) .collect(); @@ -326,12 +317,12 @@ fn discover_nu_files( /// Return whether a `DirEntry` is a .nu file or not fn is_nu_file(entry: &DirEntry) -> bool { - entry.file_type().is_some_and(|ft| ft.is_file()) + entry.file_type().map(|ft| ft.is_file()).unwrap_or(false) && entry.path().extension().is_some_and(|ext| ext == "nu") } fn make_relative(path: &Path) -> String { - let current = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + let current = std::env::current_dir().unwrap_or(PathBuf::from(".")); path.strip_prefix(¤t) .unwrap_or(path) .display() @@ -344,7 +335,7 @@ fn make_relative(path: &Path) -> String { /// Search for `filename` in current or any parent directories. /// If `start_dir` is not provided, the current directory is used fn find_in_parent_dirs(filename: &str) -> Option { - let start_dir = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + let start_dir = std::env::current_dir().unwrap_or(PathBuf::from(".")); let mut dir = Some(start_dir.as_path()); while let Some(current) = dir {