feat(clippy-ci): fix clippy running on ci (#66)

The `actions-rs/toolchain` action runs `rustup show` before installing the toolchain requested. Unfortunately, when `rustup show` is run, `rustup` will read the `rust-toolchain` file from the root of the project, and immediately download and install the toolchain mentioned there.

This means that the `nightly` toolchain is installed with no components specified, and can select a version which does not currently have the `clippy` component. As `rustup` will refuse to downgrade your rust nightly version, this leads to an incompatible version being installed, and tests failing.

This branch moves the toolchain installing step before the `actions/checkout` step, meaning that `rustup` can't see the `rust-toolchain` file, and doesn't install the incorrect toolchain version.

.github/workflows/rust.yml

@@ -6,13 +6,13 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v1
     - name: install rust toolchain
       uses: actions-rs/toolchain@v1
       with:
         profile: minimal
         toolchain: nightly
         components: rust-src, llvm-tools-preview
+    - uses: actions/checkout@v2
     - name: install dev env
       uses: actions-rs/[email protected]
       with:
@@ -26,13 +26,13 @@ jobs:
   test:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v1
     - name: install rust toolchain
       uses: actions-rs/toolchain@v1
       with:
         profile: minimal
         toolchain: nightly
         components: rust-src, llvm-tools-preview
+    - uses: actions/checkout@v2
     - name: install dev env
       uses: actions-rs/[email protected]
       with:
@@ -47,12 +47,12 @@ jobs:
   host-test:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v1
     - name: install rust toolchain
       uses: actions-rs/toolchain@v1
       with:
         profile: minimal
         toolchain: nightly
+    - uses: actions/checkout@v2
     - name: run host tests
       uses: actions-rs/[email protected]
       with:
@@ -61,14 +61,14 @@ jobs:
 
   clippy:
     runs-on: ubuntu-latest
-    needs: build
+    needs: host-test
     steps:
-    - uses: actions/checkout@v1
     - uses: actions-rs/toolchain@v1
       with:
         profile: minimal
         toolchain: nightly
         components: clippy
+    - uses: actions/checkout@v2
     - name: rust-clippy-check
       uses: actions-rs/[email protected]
       with:

alloc/src/lib.rs

@@ -28,6 +28,10 @@ unsafe impl Sync for Heap {}
 static HEAP: Heap = Heap(UnsafeCell::new(MaybeUninit::uninit()));
 static FREE: AtomicUsize = AtomicUsize::new(HEAP_SIZE);
 
+/// # Safety
+///
+/// See [`GlobalAlloc::alloc`]
+///
 /// NOTABLE INVARIANTS:
 ///  * `Layout` is non-zero sized (enforced by `GlobalAlloc`)
 ///  * `align` is a power of two (enforced by `Layout::from_size_align`)
@@ -53,6 +57,11 @@ pub unsafe fn alloc(layout: Layout) -> *mut u8 {
     }
 }
 
+/// Does nothing, because freeing is hard.
+///
+/// # Safety
+///
+/// See [`GlobalAlloc::dealloc`]
 pub unsafe fn dealloc(_ptr: *mut u8, _layout: Layout) {
     // lol
 }

build.rs

@@ -2,7 +2,7 @@ use std::env;
 use std::fs;
 use std::path::PathBuf;
 
-fn main() -> Result<(), Box<std::error::Error>> {
+fn main() -> Result<(), Box<dyn std::error::Error>> {
     let out_dir = PathBuf::from(env::var("OUT_DIR")?);
 
     // Build our helloworld.wast into binary.

hal-core/src/interrupt/ctx.rs

@@ -6,6 +6,11 @@ pub trait Context {
     type Registers: fmt::Debug + fmt::Display;
 
     fn registers(&self) -> &Self::Registers;
+
+    /// # Safety
+    ///
+    /// Mutating the value of saved interrupt registers can cause
+    /// undefined behavior.
     unsafe fn registers_mut(&mut self) -> &mut Self::Registers;
 }
 

hal-core/src/interrupt/mod.rs

@@ -1,5 +1,4 @@
 use core::fmt;
-use core::marker::PhantomData;
 
 pub mod ctx;
 pub use self::ctx::Context;
@@ -60,7 +59,7 @@ pub trait Handlers {
 
     fn keyboard_controller();
 
-    fn test_interrupt<C>(cx: C)
+    fn test_interrupt<C>(_cx: C)
     where
         C: ctx::Context,
     {

hal-x86_64/src/cpu.rs

@@ -19,12 +19,18 @@ impl Port {
         Port { num: address }
     }
 
+    /// # Safety
+    ///
+    /// Reading from a CPU port is unsafe.
     pub unsafe fn readb(&self) -> u8 {
         let result: u8;
         asm!("in al, dx" : "={al}"(result) : "{dx}"(self.num) :: "volatile", "intel");
         result
     }
 
+    /// # Safety
+    ///
+    /// Writing to a CPU port is unsafe.
     pub unsafe fn writeb(&self, value: u8) {
         asm!("out dx, al" :: "{dx}"(self.num), "{al}"(value) :: "volatile", "intel");
     }

hal-x86_64/src/interrupt/idt.rs

@@ -147,7 +147,7 @@ impl Idt {
 
     /// On modern CPUs, this interrupt is reserved; this error fires a general
     /// protection fault instead.
-    const COPROCESSOR_SEGMENT_OVERRUN: usize = 9;
+    pub const COPROCESSOR_SEGMENT_OVERRUN: usize = 9;
 
     pub const INVALID_TSS: usize = 10;
 

hal-x86_64/src/interrupt/mod.rs

@@ -1,11 +1,7 @@
 pub mod idt;
 pub mod pic;
-use crate::{cpu, segment, vga, VAddr};
-use core::{
-    fmt::{self, Write},
-    marker::PhantomData,
-    sync::atomic::{AtomicUsize, Ordering},
-};
+use crate::{segment, VAddr};
+use core::{fmt, marker::PhantomData};
 pub use idt::Idt;
 pub use pic::CascadedPic;
 
@@ -99,13 +95,13 @@ impl<'a, T> hal_core::interrupt::Context for Context<'a, T> {
     }
 }
 
-impl<'a> hal_core::interrupt::ctx::PageFault for Context<'a, PageFaultCode> {
+impl<'a> ctx::PageFault for Context<'a, PageFaultCode> {
     fn fault_vaddr(&self) -> crate::VAddr {
         unimplemented!("eliza")
     }
 }
 
-impl<'a> hal_core::interrupt::ctx::CodeFault for Context<'a, ErrorCode> {
+impl<'a> ctx::CodeFault for Context<'a, ErrorCode> {
     fn is_user_mode(&self) -> bool {
         false // TODO(eliza)
     }

hal-x86_64/src/lib.rs

@@ -6,13 +6,16 @@
 // memory, and these types don't implement Copy to ensure they aren't
 // inadvertantly copied.
 #![allow(clippy::trivially_copy_pass_by_ref)]
+// Macros generated by `tracing` often generate large amounts of code, which
+// causes this lint to complain about relatively simple methods.
+#![allow(clippy::cognitive_complexity)]
 
-pub(crate) use hal_core::{PAddr, VAddr};
+pub(crate) use hal_core::VAddr;
 pub mod cpu;
 pub mod interrupt;
 pub mod segment;
 pub mod serial;
 pub mod tracing;
 pub mod vga;
 
-pub const NAME: &'static str = "x86_64";
+pub const NAME: &str = "x86_64";

hal-x86_64/src/serial.rs

@@ -64,7 +64,7 @@ pub struct Nonblocking {
 }
 
 impl Port {
-    pub const MAX_BAUD_RATE: usize = 115200;
+    pub const MAX_BAUD_RATE: usize = 115_200;
 
     pub fn new(port: u16) -> io::Result<Self> {
         let scratch_test = unsafe {
@@ -331,11 +331,6 @@ impl<'a> LockInner<'a> {
         self.inner.is_write_ready()
     }
 
-    #[inline(always)]
-    fn is_read_ready(&self) -> bool {
-        self.inner.is_read_ready()
-    }
-
     #[inline(always)]
     fn write_nonblocking(&mut self, byte: u8) -> io::Result<()> {
         self.inner.write_nonblocking(byte)

hal-x86_64/src/tracing.rs

@@ -35,7 +35,7 @@ impl Default for Subscriber {
 
 const SERIAL_BIT: u64 = 1 << 62;
 const VGA_BIT: u64 = 1 << 63;
-const ACTUAL_ID_BITS: u64 = !(SERIAL_BIT | VGA_BIT);
+const _ACTUAL_ID_BITS: u64 = !(SERIAL_BIT | VGA_BIT);
 
 impl Subscriber {
     pub const fn vga_only(vga_max_level: LevelFilter) -> Self {
@@ -219,13 +219,11 @@ impl<'a, W: Write> field::Visit for Visitor<'a, W> {
                 let _ = write!(self.writer, "{:?}", val);
                 self.seen = true;
             }
+        } else if self.seen {
+            let _ = write!(self.writer, ", {}={:?}", field, val);
         } else {
-            if self.seen {
-                let _ = write!(self.writer, ", {}={:?}", field, val);
-            } else {
-                let _ = write!(self.writer, "{}={:?}", field, val);
-                self.seen = true;
-            }
+            let _ = write!(self.writer, "{}={:?}", field, val);
+            self.seen = true;
         }
     }
 }
@@ -262,22 +260,22 @@ impl tracing::Subscriber for Subscriber {
 
         if self.vga_enabled(level) {
             // mark that this span should be written to the VGA buffer.
-            id = id | VGA_BIT;
+            id |= VGA_BIT;
         }
 
         if self.serial_enabled(level) {
             // mark that this span should be written to the serial port buffer.
-            id = id | SERIAL_BIT;
+            id |= SERIAL_BIT;
         }
         let _ = writer.write_str("\n");
         span::Id::from_u64(id)
     }
 
-    fn record(&self, span: &span::Id, values: &span::Record) {
+    fn record(&self, _span: &span::Id, _values: &span::Record) {
         // nop for now
     }
 
-    fn record_follows_from(&self, span: &span::Id, follows: &span::Id) {
+    fn record_follows_from(&self, _span: &span::Id, _follows: &span::Id) {
         // nop for now
     }
 

src/arch/x86_64.rs

@@ -1,6 +1,6 @@
 use bootloader::bootinfo;
 use core::sync::atomic::{AtomicUsize, Ordering};
-use hal_core::{boot::BootInfo, mem, Address, PAddr};
+use hal_core::{boot::BootInfo, mem, PAddr};
 use hal_x86_64::vga;
 pub use hal_x86_64::{interrupt, NAME};
 

src/lib.rs

@@ -92,6 +92,7 @@ pub fn kernel_main(bootinfo: &impl BootInfo) -> ! {
     //
     // eventually we'll call into a kernel main loop here...
     #[allow(clippy::empty_loop)]
+    #[allow(unreachable_code)]
     loop {}
 }
 

src/main.rs

@@ -1,8 +1,10 @@
+#![no_main]
 #![cfg_attr(target_os = "none", no_std)]
-#![cfg_attr(target_os = "none", no_main)]
 #![cfg_attr(target_os = "none", feature(alloc_error_handler))]
 #![cfg_attr(target_os = "none", feature(asm))]
 #![cfg_attr(target_os = "none", feature(panic_info_message, track_caller))]
 
 // Force linking to the `mycelium_kernel` lib.
+#[allow(unused_imports)]
+#[allow(clippy::single_component_path_imports)]
 use mycelium_kernel;

src/wasm/wasi.rs

@@ -49,7 +49,8 @@ fn mem_write<T: wasmi::LittleEndianConvert>(
     let slice = mem
         .get_mut(addr as usize..addr_after as usize)
         .ok_or(wasmi::TrapKind::MemoryAccessOutOfBounds)?;
-    Ok(T::into_little_endian(value, slice))
+    T::into_little_endian(value, slice);
+    Ok(())
 }
 
 /// Reference to a subslice of memory.

util/src/io/cursor.rs

@@ -1,7 +1,10 @@
 use crate::io::prelude::*;
 use crate::io::{self, Error, ErrorKind, Initializer, SeekFrom};
 
-use core::{cmp, convert::TryInto};
+use core::cmp;
+
+#[cfg(feature = "alloc")]
+use core::convert::TryInto;
 
 #[cfg(feature = "alloc")]
 use alloc::{boxed::Box, vec::Vec};
@@ -36,10 +39,7 @@ impl<T> Cursor<T> {
     /// is not empty. So writing to cursor starts with overwriting `Vec`
     /// content, not with appending to it.
     pub fn new(inner: T) -> Cursor<T> {
-        Cursor {
-            pos: 0,
-            inner: inner,
-        }
+        Cursor { pos: 0, inner }
     }
 
     /// Consumes this cursor, returning the underlying value.

util/src/io/error.rs

@@ -115,8 +115,8 @@ impl<E: error::Error + 'static> fmt::Display for Error<E> {
 }
 
 impl ErrorKind {
-    pub(crate) fn as_str(&self) -> &'static str {
-        match *self {
+    pub(crate) fn as_str(self) -> &'static str {
+        match self {
             ErrorKind::NotFound => "entity not found",
             ErrorKind::PermissionDenied => "permission denied",
             ErrorKind::ConnectionRefused => "connection refused",

util/src/io/mod.rs

@@ -292,10 +292,7 @@ pub trait Read {
     where
         Self: Sized,
     {
-        Take {
-            inner: self,
-            limit: limit,
-        }
+        Take { inner: self, limit }
     }
 }
 
@@ -922,9 +919,9 @@ impl<B: BufRead> Iterator for Lines<B> {
         match self.buf.read_line(&mut buf) {
             Ok(0) => None,
             Ok(_n) => {
-                if buf.ends_with("\n") {
+                if buf.ends_with('\n') {
                     buf.pop();
-                    if buf.ends_with("\r") {
+                    if buf.ends_with('\r') {
                         buf.pop();
                     }
                 }
@@ -962,7 +959,7 @@ where
     let start_len = buf.len();
     let mut g = Guard {
         len: buf.len(),
-        buf: buf,
+        buf,
     };
     let ret;
     loop {
@@ -1059,7 +1056,7 @@ where
 }
 
 #[cfg(feature = "alloc")]
-fn read_until<R: BufRead + ?Sized>(r: &mut R, delim: u8, buf: &mut Vec<u8>) -> Result<usize> {
+fn read_until<R: BufRead + ?Sized>(_r: &mut R, _delim: u8, _buf: &mut Vec<u8>) -> Result<usize> {
     unimplemented!("eliza: figure out memchr!!!!")
     // let mut read = 0;
     // loop {