Skip to content

Commit

Permalink
Fix clippy warnings & add CI build job for clippy (#445)
Browse files Browse the repository at this point in the history
* Fix clippy warnings

- `clippy::manual_pattern_char_comparison`
- `clippy::needless_borrows_for_generic_args`
- `clippy::range_plus_one`
- `clippy::redundant_closure_for_method_calls`
- `clippy::ref_binding_to_reference`
- `clippy::semicolon_if_nothing_returned`
- `clippy::single_char_pattern`
- `clippy::uninlined_format_args`

- allow `clippy::result_large_err`
- allow `clippy::large_enum_variant`

* ci: add build job for clippy
  • Loading branch information
nickelc authored Sep 8, 2024
1 parent 3a030b4 commit 2bc5387
Show file tree
Hide file tree
Showing 20 changed files with 47 additions and 32 deletions.
10 changes: 10 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ jobs:
components: rustfmt
- run: cargo fmt --all --check

clippy:
name: Clippy
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
with:
components: clippy
- run: cargo clippy --all --tests --all-features -- -D warnings

test:
name: Test
runs-on: ubuntu-latest
Expand Down
6 changes: 3 additions & 3 deletions benches/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl<const CHUNK_SIZE: usize> Buf for StackReadBuffer<CHUNK_SIZE> {
}

fn advance(&mut self, cnt: usize) {
Buf::advance(self.as_cursor_mut(), cnt)
Buf::advance(self.as_cursor_mut(), cnt);
}
}

Expand Down Expand Up @@ -114,10 +114,10 @@ fn benchmark(c: &mut Criterion) {
group.throughput(Throughput::Bytes(STREAM_SIZE as u64));
group.bench_function("InputBuffer", |b| b.iter(|| input_buffer(black_box(stream.clone()))));
group.bench_function("ReadBuffer (stack)", |b| {
b.iter(|| stack_read_buffer(black_box(stream.clone())))
b.iter(|| stack_read_buffer(black_box(stream.clone())));
});
group.bench_function("ReadBuffer (heap)", |b| {
b.iter(|| heap_read_buffer(black_box(stream.clone())))
b.iter(|| heap_read_buffer(black_box(stream.clone())));
});
group.finish();
}
Expand Down
2 changes: 1 addition & 1 deletion benches/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ fn benchmark(c: &mut Criterion) {
ws.flush().unwrap();
},
BatchSize::SmallInput,
)
);
});
}

Expand Down
4 changes: 2 additions & 2 deletions examples/autobahn-client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ fn get_case_count() -> Result<u32> {
}

fn update_reports() -> Result<()> {
let (mut socket, _) = connect(&format!("ws://localhost:9001/updateReports?agent={AGENT}"))?;
let (mut socket, _) = connect(format!("ws://localhost:9001/updateReports?agent={AGENT}"))?;
socket.close(None)?;
Ok(())
}

fn run_test(case: u32) -> Result<()> {
info!("Running test case {}", case);
let case_url = &format!("ws://localhost:9001/runCase?case={case}&agent={AGENT}");
let case_url = format!("ws://localhost:9001/runCase?case={case}&agent={AGENT}");
let (mut socket, _) = connect(case_url)?;
loop {
match socket.read()? {
Expand Down
2 changes: 1 addition & 1 deletion examples/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fn main() {
println!("Connected to the server");
println!("Response HTTP code: {}", response.status());
println!("Response contains the following headers:");
for (ref header, _value) in response.headers() {
for (header, _value) in response.headers() {
println!("* {header}");
}

Expand Down
2 changes: 1 addition & 1 deletion examples/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn main() {
println!("Received a new ws handshake");
println!("The request's path is: {}", req.uri().path());
println!("The request's headers are:");
for (ref header, _value) in req.headers() {
for (header, _value) in req.headers() {
println!("* {header}");
}

Expand Down
2 changes: 1 addition & 1 deletion examples/srv_accept_unmasked_frames.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn main() {
println!("Received a new ws handshake");
println!("The request's path is: {}", req.uri().path());
println!("The request's headers are:");
for (ref header, _value) in req.headers() {
for (header, _value) in req.headers() {
println!("* {header}");
}

Expand Down
2 changes: 1 addition & 1 deletion src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl<const CHUNK_SIZE: usize> Buf for ReadBuffer<CHUNK_SIZE> {
}

fn advance(&mut self, cnt: usize) {
Buf::advance(self.as_cursor_mut(), cnt)
Buf::advance(self.as_cursor_mut(), cnt);
}
}

Expand Down
8 changes: 5 additions & 3 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,14 @@ pub fn connect_with_config<Req: IntoClientRequest>(
let (parts, _) = request.into_client_request()?.into_parts();
let mut uri = parts.uri.clone();

for attempt in 0..(max_redirects + 1) {
for attempt in 0..=max_redirects {
let request = create_request(&parts, &uri);

match try_client_handshake(request, config) {
Err(Error::Http(res)) if res.status().is_redirection() && attempt < max_redirects => {
if let Some(location) = res.headers().get("Location") {
uri = location.to_str()?.parse::<Uri>()?;
debug!("Redirecting to {:?}", uri);
debug!("Redirecting to {uri:?}");
continue;
} else {
warn!("No `Location` found in redirect");
Expand Down Expand Up @@ -130,7 +130,7 @@ pub fn connect<Req: IntoClientRequest>(

fn connect_to_some(addrs: &[SocketAddr], uri: &Uri) -> Result<TcpStream> {
for addr in addrs {
debug!("Trying to contact {} at {}...", uri, addr);
debug!("Trying to contact {uri} at {addr}...");
if let Ok(stream) = TcpStream::connect(addr) {
return Ok(stream);
}
Expand All @@ -156,6 +156,7 @@ pub fn uri_mode(uri: &Uri) -> Result<Mode> {
/// Use this function if you need a nonblocking handshake support or if you
/// want to use a custom stream like `mio::net::TcpStream` or `openssl::ssl::SslStream`.
/// Any stream supporting `Read + Write` will do.
#[allow(clippy::result_large_err)]
pub fn client_with_config<Stream, Req>(
request: Req,
stream: Stream,
Expand All @@ -173,6 +174,7 @@ where
/// Use this function if you need a nonblocking handshake support or if you
/// want to use a custom stream like `mio::net::TcpStream` or `openssl::ssl::SslStream`.
/// Any stream supporting `Read + Write` will do.
#[allow(clippy::result_large_err)]
pub fn client<Stream, Req>(
request: Req,
stream: Stream,
Expand Down
2 changes: 1 addition & 1 deletion src/handshake/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ pub fn generate_request(mut request: Request) -> Result<(Vec<u8>, String)> {

fn extract_subprotocols_from_request(request: &Request) -> Result<Option<Vec<String>>> {
if let Some(subprotocols) = request.headers().get("Sec-WebSocket-Protocol") {
Ok(Some(subprotocols.to_str()?.split(",").map(|s| s.to_string()).collect()))
Ok(Some(subprotocols.to_str()?.split(',').map(ToString::to_string).collect()))
} else {
Ok(None)
}
Expand Down
2 changes: 1 addition & 1 deletion src/handshake/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ fn create_parts<T>(request: &HttpRequest<T>) -> Result<Builder> {
.headers()
.get("Connection")
.and_then(|h| h.to_str().ok())
.map(|h| h.split(|c| c == ' ' || c == ',').any(|p| p.eq_ignore_ascii_case("Upgrade")))
.map(|h| h.split([' ', ',']).any(|p| p.eq_ignore_ascii_case("Upgrade")))
.unwrap_or(false)
{
return Err(Error::Protocol(ProtocolError::MissingConnectionUpgradeHeader));
Expand Down
8 changes: 4 additions & 4 deletions src/protocol/frame/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl FrameHeader {
}

if let Some(ref mask) = self.mask {
output.write_all(mask)?
output.write_all(mask)?;
}

Ok(())
Expand All @@ -127,7 +127,7 @@ impl FrameHeader {
///
/// Of course this does not change frame contents. It just generates a mask.
pub(crate) fn set_random_mask(&mut self) {
self.mask = Some(generate_mask())
self.mask = Some(generate_mask());
}
}

Expand Down Expand Up @@ -261,15 +261,15 @@ impl Frame {
/// either on `format()` or on `apply_mask()` call.
#[inline]
pub(crate) fn set_random_mask(&mut self) {
self.header.set_random_mask()
self.header.set_random_mask();
}

/// This method unmasks the payload and should only be called on frames that are actually
/// masked. In other words, those frames that have just been received from a client endpoint.
#[inline]
pub(crate) fn apply_mask(&mut self) {
if let Some(mask) = self.header.mask.take() {
apply_mask(&mut self.payload, mask)
apply_mask(&mut self.payload, mask);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/protocol/frame/mask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub fn generate_mask() -> [u8; 4] {
/// Mask/unmask a frame.
#[inline]
pub fn apply_mask(buf: &mut [u8], mask: [u8; 4]) {
apply_mask_fast32(buf, mask)
apply_mask_fast32(buf, mask);
}

/// A safe unoptimized mask application.
Expand Down
4 changes: 2 additions & 2 deletions src/protocol/frame/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ impl FrameCodec {
let (header, length) = self.header.take().expect("Bug: no frame header");
debug_assert_eq!(payload.len() as u64, length);
let frame = Frame::from_payload(header, payload);
trace!("received frame {}", frame);
trace!("received frame {frame}");
Ok(Some(frame))
}

Expand All @@ -216,7 +216,7 @@ impl FrameCodec {
return Err(Error::WriteBufferFull(Message::Frame(frame)));
}

trace!("writing frame {}", frame);
trace!("writing frame {frame}");

self.out_buffer.reserve(frame.len());
frame.format(&mut self.out_buffer).expect("Bug: can't write to vector");
Expand Down
12 changes: 6 additions & 6 deletions src/protocol/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ impl<Stream> WebSocket<Stream> {
/// # Panics
/// Panics if config is invalid e.g. `max_write_buffer_size <= write_buffer_size`.
pub fn set_config(&mut self, set_func: impl FnOnce(&mut WebSocketConfig)) {
self.context.set_config(set_func)
self.context.set_config(set_func);
}

/// Read the configuration.
Expand Down Expand Up @@ -410,7 +410,7 @@ impl WebSocketContext {
// If we get here, either write blocks or we have nothing to write.
// Thus if read blocks, just let it return WouldBlock.
if let Some(message) = self.read_message_frame(stream)? {
trace!("Received message {}", message);
trace!("Received message {message}");
return Ok(message);
}
}
Expand Down Expand Up @@ -570,7 +570,7 @@ impl WebSocketContext {
if frame.is_masked() {
// A server MUST remove masking for data frames received from a client
// as described in Section 5.3. (RFC 6455)
frame.apply_mask()
frame.apply_mask();
} else if !self.config.accept_unmasked_frames {
// The server MUST close the connection upon receiving a
// frame that is not masked. (RFC 6455)
Expand Down Expand Up @@ -672,7 +672,7 @@ impl WebSocketContext {
/// Received a close frame. Tells if we need to return a close frame to the user.
#[allow(clippy::option_option)]
fn do_close<'t>(&mut self, close: Option<CloseFrame<'t>>) -> Option<Option<CloseFrame<'t>>> {
debug!("Received close frame: {:?}", close);
debug!("Received close frame: {close:?}");
match self.state {
WebSocketState::Active => {
self.state = WebSocketState::ClosedByPeer;
Expand All @@ -689,7 +689,7 @@ impl WebSocketContext {
});

let reply = Frame::close(close.clone());
debug!("Replying to close with {:?}", reply);
debug!("Replying to close with {reply:?}");
self.set_additional(reply);

Some(close)
Expand Down Expand Up @@ -721,7 +721,7 @@ impl WebSocketContext {
}
}

trace!("Sending frame: {:?}", frame);
trace!("Sending frame: {frame:?}");
self.frame.buffer_frame(stream, frame).check_connection_reset(self.state)
}

Expand Down
1 change: 1 addition & 0 deletions src/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ where

/// A stream that might be protected with TLS.
#[non_exhaustive]
#[allow(clippy::large_enum_variant)]
pub enum MaybeTlsStream<S: Read + Write> {
/// Unencrypted socket stream.
Plain(S),
Expand Down
2 changes: 2 additions & 0 deletions src/tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ type TlsHandshakeError<S> = HandshakeError<ClientHandshake<MaybeTlsStream<S>>>;

/// Creates a WebSocket handshake from a request and a stream,
/// upgrading the stream to TLS if required.
#[allow(clippy::result_large_err)]
pub fn client_tls<R, S>(
request: R,
stream: S,
Expand All @@ -192,6 +193,7 @@ where
/// be created.
///
/// Please refer to [`client_tls()`] for more details.
#[allow(clippy::result_large_err)]
pub fn client_tls_with_config<R, S>(
request: R,
stream: S,
Expand Down
2 changes: 1 addition & 1 deletion src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl NonBlockingError for IoError {
impl NonBlockingError for Error {
fn into_non_blocking(self) -> Option<Self> {
match self {
Error::Io(e) => e.into_non_blocking().map(|e| e.into()),
Error::Io(e) => e.into_non_blocking().map(Into::into),
x => Some(x),
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/connection_reset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ where

let client_thread = spawn(move || {
let (client, _) =
connect(&format!("ws://localhost:{}/socket", port)).expect("Can't connect to port");
connect(format!("ws://localhost:{}/socket", port)).expect("Can't connect to port");

client_task(client);
});
Expand Down
4 changes: 2 additions & 2 deletions tests/wss_fails_when_no_tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ use tungstenite::{connect, error::UrlError, Error};
#[test]
fn wss_url_fails_when_no_tls_support() {
let ws = connect("wss://127.0.0.1/ws");
eprintln!("{:?}", ws);
assert!(matches!(ws, Err(Error::Url(UrlError::TlsFeatureNotEnabled))))
eprintln!("{ws:?}");
assert!(matches!(ws, Err(Error::Url(UrlError::TlsFeatureNotEnabled))));
}

0 comments on commit 2bc5387

Please sign in to comment.