Skip to content

Commit

Permalink
Some clippy fixes (#879)
Browse files Browse the repository at this point in the history
* clippy: Use char, not str, versions when single char patterns.

* clippy: Fix needless_return warnings.

* clippy: Fix redundant_field_names warnings.

* clippy: Fix needless_borrowed_reference warnings.

* clippy: Fix useless_conversion warnings.

* clippy: Fix most needless_borrow warnings.

* clippy: Fix needless_bool warnings.

* clippy: Suppress upper_case_acronyms lints.

These are part of Windows FFI glue code and the Windows types use
names in uppercase.

* clippy: Fix doc_markdown warnings.

* clippy: Fix if_same_then_else errors.

* ci: Add a clippy check that only fails on errors.
  • Loading branch information
waywardmonkeys committed Dec 18, 2023
1 parent bfd68f2 commit 987cd25
Show file tree
Hide file tree
Showing 11 changed files with 122 additions and 114 deletions.
12 changes: 12 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,18 @@ jobs:
- run: cargo check --lib
- run: cargo check --lib --all-features

clippy:
name: Clippy
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Install Rust
run: |
rustup toolchain install stable --no-self-update --profile minimal --component rustfmt
rustup default stable
shell: bash
- run: cargo clippy

rustfmt:
name: Rustfmt
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion cc-test/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ fn main() {
.compile("bar");

let target = std::env::var("TARGET").unwrap();
let file = target.split("-").next().unwrap();
let file = target.split('-').next().unwrap();
let file = format!(
"src/{}.{}",
file,
Expand Down
2 changes: 1 addition & 1 deletion src/com.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ where
fn as_unknown(&self) -> &IUnknown {
unsafe { &*(self.0 as *mut IUnknown) }
}
/// Performs QueryInterface fun.
/// Performs `QueryInterface` fun.
pub fn cast<U>(&self) -> Result<ComPtr<U>, i32>
where
U: Interface,
Expand Down
158 changes: 77 additions & 81 deletions src/lib.rs

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions src/os_pipe.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! Adapted from:
//! - https://doc.rust-lang.org/src/std/sys/unix/pipe.rs.html
//! - https://doc.rust-lang.org/src/std/sys/unix/fd.rs.html#385
//! - https://github.com/rust-lang/rust/blob/master/library/std/src/sys/mod.rs#L57
//! - https://github.com/oconnor663/os_pipe.rs
//! - <https://doc.rust-lang.org/src/std/sys/unix/pipe.rs.html>
//! - <https://doc.rust-lang.org/src/std/sys/unix/fd.rs.html#385>
//! - <https://github.com/rust-lang/rust/blob/master/library/std/src/sys/mod.rs#L57>
//! - <https://github.com/oconnor663/os_pipe.rs>
use std::fs::File;

/// Open a new pipe and return a pair of [`File`] objects for the reader and writer.
Expand Down
2 changes: 1 addition & 1 deletion src/os_pipe/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{fs::File, io, os::windows::prelude::*, ptr};
/// NOTE: These pipes do not support IOCP.
///
/// If IOCP is needed, then you might want to emulate
/// anonymous pipes with CreateNamedPipe, as Rust's stdlib does.
/// anonymous pipes with `CreateNamedPipe`, as Rust's stdlib does.
pub(super) fn pipe() -> io::Result<(File, File)> {
let mut read_pipe = INVALID_HANDLE_VALUE;
let mut write_pipe = INVALID_HANDLE_VALUE;
Expand Down
3 changes: 2 additions & 1 deletion src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use std::{
/// Must never be `HKEY_PERFORMANCE_DATA`.
pub(crate) struct RegistryKey(Repr);

#[allow(clippy::upper_case_acronyms)]
type DWORD = u32;

struct OwnedKey(HKEY);
Expand Down Expand Up @@ -147,7 +148,7 @@ impl RegistryKey {
if !v.is_empty() && v[v.len() - 1] == 0 {
v.pop();
}
return Ok(OsString::from_wide(&v));
Ok(OsString::from_wide(&v))
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/winapi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
// All files in the project carrying such notice may not be copied, modified, or distributed
// except according to those terms.

#![allow(bad_style)]
#![allow(bad_style, clippy::upper_case_acronyms)]

use std::os::raw;

pub type wchar_t = u16;

pub use crate::windows_sys::{FILETIME, GUID, HRESULT, SAFEARRAY, SAFEARRAYBOUND};
pub use crate::windows_sys::{FILETIME, GUID, HRESULT, SAFEARRAY};

pub type REFIID = *const IID;
pub type IID = GUID;
Expand Down
35 changes: 17 additions & 18 deletions src/windows_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
//! A helper module to probe the Windows Registry when looking for
//! windows-specific tools.

#![allow(clippy::upper_case_acronyms)]

use std::process::Command;

use crate::Tool;
Expand Down Expand Up @@ -71,11 +73,11 @@ pub fn find_tool(target: &str, tool: &str) -> Option<Tool> {
// environment variables like `LIB`, `INCLUDE`, and `PATH` to ensure that
// the tool is actually usable.

return impl_::find_msvc_environment(tool, target)
impl_::find_msvc_environment(tool, target)
.or_else(|| impl_::find_msvc_15plus(tool, target))
.or_else(|| impl_::find_msvc_14(tool, target))
.or_else(|| impl_::find_msvc_12(tool, target))
.or_else(|| impl_::find_msvc_11(tool, target));
.or_else(|| impl_::find_msvc_11(tool, target))
}

/// A version of Visual Studio
Expand Down Expand Up @@ -182,7 +184,7 @@ mod impl_ {
impl MsvcTool {
fn new(tool: PathBuf) -> MsvcTool {
MsvcTool {
tool: tool,
tool,
libs: Vec::new(),
path: Vec::new(),
include: Vec::new(),
Expand All @@ -196,7 +198,7 @@ mod impl_ {
path,
include,
} = self;
let mut tool = Tool::with_family(tool.into(), MSVC_FAMILY);
let mut tool = Tool::with_family(tool, MSVC_FAMILY);
add_env(&mut tool, "LIB", libs);
add_env(&mut tool, "PATH", path);
add_env(&mut tool, "INCLUDE", include);
Expand All @@ -210,7 +212,7 @@ mod impl_ {
fn is_vscmd_target(target: &str) -> Option<bool> {
let vscmd_arch = env::var("VSCMD_ARG_TGT_ARCH").ok()?;
// Convert the Rust target arch to its VS arch equivalent.
let arch = match target.split("-").next() {
let arch = match target.split('-').next() {
Some("x86_64") => "x64",
Some("aarch64") => "arm64",
Some("i686") | Some("i586") => "x86",
Expand Down Expand Up @@ -242,7 +244,7 @@ mod impl_ {
.map(|p| p.join(tool))
.find(|p| p.exists())
})
.map(|path| Tool::with_family(path.into(), MSVC_FAMILY))
.map(|path| Tool::with_family(path, MSVC_FAMILY))
}
}

Expand Down Expand Up @@ -394,7 +396,7 @@ mod impl_ {
.into_iter()
.filter_map(|instance| instance.installation_path())
.map(|path| path.join(tool))
.find(|ref path| path.is_file()),
.find(|path| path.is_file()),
None => None,
};

Expand Down Expand Up @@ -467,18 +469,15 @@ mod impl_ {
let path = instance_path.join(r"VC\Tools\MSVC").join(version);
// This is the path to the toolchain for a particular target, running
// on a given host
let bin_path = path
.join("bin")
.join(&format!("Host{}", host))
.join(&target);
let bin_path = path.join("bin").join(format!("Host{}", host)).join(target);
// But! we also need PATH to contain the target directory for the host
// architecture, because it contains dlls like mspdb140.dll compiled for
// the host architecture.
let host_dylib_path = path
.join("bin")
.join(&format!("Host{}", host))
.join(&host.to_lowercase());
let lib_path = path.join("lib").join(&target);
.join(format!("Host{}", host))
.join(host.to_lowercase());
let lib_path = path.join("lib").join(target);
let include_path = path.join("include");
Some((path, bin_path, host_dylib_path, lib_path, include_path))
}
Expand Down Expand Up @@ -632,7 +631,7 @@ mod impl_ {
path.join("bin").join(host),
)
})
.filter(|&(ref path, _)| path.is_file())
.filter(|(path, _)| path.is_file())
.map(|(path, host)| {
let mut tool = MsvcTool::new(path);
tool.path.push(host);
Expand Down Expand Up @@ -840,7 +839,7 @@ mod impl_ {
for subkey in key.iter().filter_map(|k| k.ok()) {
let val = subkey
.to_str()
.and_then(|s| s.trim_left_matches("v").replace(".", "").parse().ok());
.and_then(|s| s.trim_left_matches("v").replace('.', "").parse().ok());
let val = match val {
Some(s) => s,
None => continue,
Expand Down Expand Up @@ -883,7 +882,7 @@ mod impl_ {
}

pub fn find_devenv(target: &str) -> Option<Tool> {
find_devenv_vs15(&target)
find_devenv_vs15(target)
}

fn find_devenv_vs15(target: &str) -> Option<Tool> {
Expand All @@ -894,7 +893,7 @@ mod impl_ {
pub fn find_msbuild(target: &str) -> Option<Tool> {
// VS 15 (2017) changed how to locate msbuild
if let Some(r) = find_msbuild_vs17(target) {
return Some(r);
Some(r)
} else if let Some(r) = find_msbuild_vs16(target) {
return Some(r);
} else if let Some(r) = find_msbuild_vs15(target) {
Expand Down
6 changes: 3 additions & 3 deletions tests/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,16 @@ impl Test {
// lesser of the two evils.
env::remove_var("RUSTC_WRAPPER");

let mut gcc = PathBuf::from(env::current_exe().unwrap());
let mut gcc = env::current_exe().unwrap();
gcc.pop();
if gcc.ends_with("deps") {
gcc.pop();
}
let td = Builder::new().prefix("gcc-test").tempdir_in(&gcc).unwrap();
gcc.push(format!("gcc-shim{}", env::consts::EXE_SUFFIX));
Test {
td: td,
gcc: gcc,
td,
gcc,
msvc: false,
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,8 @@ fn gnu_x86_64_no_plt() {
test.gcc()
.pic(true)
.use_plt(false)
.target(&target)
.host(&target)
.target(target)
.host(target)
.file("foo.c")
.compile("foo");
test.cmd(0).must_have("-fno-plt");
Expand Down

0 comments on commit 987cd25

Please sign in to comment.