Mordant lints

Every warning ends with the lint that raised it, such as #[warn(unchecked_construction)]. Each one is below with the code it flags and the fix it wants.

State that should be a type (`mordant_state`)

`options_as_enum`

Option fields where every construction fills exactly one, so the struct also allows both and neither, which nobody builds. An enum with a variant per case allows only the real ones.

Flagged

struct Outcome {
    ok: Option<u32>,
    err: Option<String>,
}

fn success() -> Outcome {
    Outcome {
        ok: Some(1),
        err: None,
    }
}

fn failure() -> Outcome {
    Outcome {
        ok: None,
        err: Some("boom".to_owned()),
    }
}

Fixed

enum Outcome {
    Ok(u32),
    Err(String),
}

fn success() -> Outcome {
    Outcome::Ok(1)
}

fn failure() -> Outcome {
    Outcome::Err("boom".to_owned())
}

`parallel_bools`

bool fields that are always assigned together, so done goes up whenever running goes down. They are one state under two names, and a single enum field cannot be half updated.

Flagged

struct Task {
    running: bool,
    done: bool,
    retries: u32,
}

impl Task {
    fn start(&mut self) {
        self.running = true;
        self.done = false;
    }

    fn finish(&mut self) {
        self.running = false;
        self.done = true;
    }
}

Fixed

enum State {
    Running,
    Done,
}

struct Task {
    state: State,
    retries: u32,
}

impl Task {
    fn start(&mut self) {
        self.state = State::Running;
    }

    fn finish(&mut self) {
        self.state = State::Done;
    }
}

`bool_cluster`

A struct with 3 or more bool fields allows 8 or more combinations, when the code may mean only a few. Many such structs are harmless sets of options, so it is off until bool-cluster-enabled = true, and is best run once as a survey.

Flagged

struct Refusals {
    too_big: bool,
    exhausted: bool,
    fragmented: bool,
    slots_leaked: u64,
}

Fixed

enum Refusal {
    TooBig,
    Exhausted,
    Fragmented,
}

struct Refusals {
    kind: Refusal,
    slots_leaked: u64,
}

`runtime_typestate`

A bool such as ready that 2 or more methods test first and return early on. The struct is two stages sharing one type, and a type per stage makes calling too early fail to compile.

Flagged

struct Conn {
    ready: bool,
    sent: u32,
}

impl Conn {
    fn connect(&mut self) {
        self.ready = true;
    }

    fn send(&mut self) {
        if !self.ready {
            return;
        }
        self.sent += 1;
    }

    fn flush(&mut self) {
        if !self.ready {
            return;
        }
        self.sent = 0;
    }
}

Fixed

struct Idle;

struct Ready {
    sent: u32,
}

impl Idle {
    fn connect(self) -> Ready {
        Ready { sent: 0 }
    }
}

impl Ready {
    fn send(&mut self) {
        self.sent += 1;
    }

    fn flush(&mut self) {
        self.sent = 0;
    }
}

`always_unwrapped_option`

An Option field that every read unwraps. If nothing handles None, it is not a real state, so build the value once the field is known.

Flagged

struct Conn {
    sock: Option<u32>,
}

impl Conn {
    fn new() -> Conn {
        Conn { sock: None }
    }

    fn ready(&self) -> u32 {
        self.sock.unwrap()
    }

    fn doubled(&self) -> u32 {
        self.sock.unwrap() + 1
    }
}

Fixed

struct Conn {
    sock: u32,
}

impl Conn {
    fn new(sock: u32) -> Conn {
        Conn { sock }
    }

    fn ready(&self) -> u32 {
        self.sock
    }

    fn doubled(&self) -> u32 {
        self.sock + 1
    }
}

`derived_field`

A field fixed by another, so whenever ceiling is Words, limit is MAX_WORDS. The copy can drift out of step, and a method such as Ceiling::limit() cannot.

Flagged

enum Ceiling {
    Words,
    Types,
    Blocks,
}

struct Exceeded {
    ceiling: Ceiling,
    limit: u32,
    wanted: u32,
}

fn words(wanted: u32) -> Exceeded {
    Exceeded {
        ceiling: Ceiling::Words,
        limit: MAX_WORDS,
        wanted,
    }
}

fn types(wanted: u32) -> Exceeded {
    Exceeded {
        ceiling: Ceiling::Types,
        limit: MAX_TYPES,
        wanted,
    }
}

fn blocks(wanted: u32) -> Exceeded {
    Exceeded {
        ceiling: Ceiling::Blocks,
        limit: MAX_BLOCKS,
        wanted,
    }
}

Fixed

enum Ceiling {
    Words,
    Types,
    Blocks,
}

impl Ceiling {
    fn limit(self) -> u32 {
        match self {
            Ceiling::Words => MAX_WORDS,
            Ceiling::Types => MAX_TYPES,
            Ceiling::Blocks => MAX_BLOCKS,
        }
    }
}

struct Exceeded {
    ceiling: Ceiling,
    wanted: u32,
}

fn words(wanted: u32) -> Exceeded {
    Exceeded {
        ceiling: Ceiling::Words,
        wanted,
    }
}

fn types(wanted: u32) -> Exceeded {
    Exceeded {
        ceiling: Ceiling::Types,
        wanted,
    }
}

fn blocks(wanted: u32) -> Exceeded {
    Exceeded {
        ceiling: Ceiling::Blocks,
        wanted,
    }
}

`field_valid_only_when`

A field that is only read after checking a sibling, such as c.tag == Tag::Subproc, and holds a placeholder the rest of the time. Put it inside that enum variant and there is no placeholder left to misread.

Flagged

enum Tag {
    Cmd,
    Subproc,
}

struct Child {
    tag: Tag,
    node: u32,
    pid: u32,
}

fn cmd(node: u32) -> Child {
    Child {
        tag: Tag::Cmd,
        node,
        pid: 0,
    } // placeholder
}

fn subproc(pid: u32) -> Child {
    Child {
        tag: Tag::Subproc,
        node: 0,
        pid,
    }
}

fn wait_on(c: &Child) -> Option<u32> {
    match c.tag {
        Tag::Subproc => Some(c.pid), // the only read, under the test
        Tag::Cmd => None,
    }
}

Fixed

enum Child {
    Cmd { node: u32 },
    Subproc { pid: u32 },
}

fn wait_on(c: &Child) -> Option<u32> {
    match c {
        Child::Subproc { pid } => Some(*pid),
        Child::Cmd { .. } => None,
    }
}

`bool_beside_option`

A bool that always equals is_some() of the Option beside it. Delete the bool and ask the option.

Flagged

struct Conn {
    peer: Option<String>,
    connected: bool,
}

impl Conn {
    fn new() -> Self {
        Conn {
            peer: None,
            connected: false,
        }
    }

    fn open(&mut self, peer: String) {
        self.peer = Some(peer);
        self.connected = true;
    }

    fn close(&mut self) {
        self.connected = false;
        self.peer = None;
    }

    fn is_up(&self) -> bool {
        self.connected
    }
}

Fixed

struct Conn {
    peer: Option<String>,
}

impl Conn {
    fn is_up(&self) -> bool {
        self.peer.is_some()
    }
}

`parallel_vecs`

Vec fields that grow together and are read at the same index, like names[i] and ages[i]. One Vec of a struct cannot fall out of step with itself.

Flagged

struct People {
    names: Vec<String>,
    ages: Vec<u32>,
}

impl People {
    fn add(&mut self, name: String, age: u32) {
        self.names.push(name);
        self.ages.push(age);
    }

    fn forget(&mut self, keep: usize) {
        self.names.truncate(keep);
        self.ages.truncate(keep);
    }

    fn describe(&self, i: usize) -> String {
        format!("{} is {}", self.names[i], self.ages[i])
    }
}

Fixed

struct Person {
    name: String,
    age: u32,
}

struct People {
    people: Vec<Person>,
}

impl People {
    fn add(&mut self, name: String, age: u32) {
        self.people.push(Person { name, age });
    }

    fn describe(&self, i: usize) -> String {
        let p = &self.people[i];
        format!("{} is {}", p.name, p.age)
    }
}

`parallel_params`

Parameters such as w and h handed together through several private functions, a value with no type that any caller can swap. Some pairs travel together on purpose, so it is off until parallel-params-enabled = true.

Flagged

fn decode(src: &[u8], w: u32, h: u32) -> Vec<u8> {
    let out = scale(src, w, h, 2);
    let _ = checksum(w, h, &out);
    out
}

fn scale(src: &[u8], w: u32, h: u32, factor: u32) -> Vec<u8> {
    src.iter()
        .cycle()
        .take((w * h * factor) as usize)
        .copied()
        .collect()
}

fn checksum(w: u32, h: u32, px: &[u8]) -> u64 {
    u64::from(w) * u64::from(h) + px.len() as u64
}

Fixed

#[derive(Clone, Copy)]
struct Size {
    w: u32,
    h: u32,
}

fn decode(src: &[u8], size: Size) -> Vec<u8> {
    let out = scale(src, size, 2);
    let _ = checksum(size, &out);
    out
}

`stringly_state`

A string that only ever holds one of a few literals and is read by comparing it with them. A typo in a comparison still compiles, so store an enum and keep the text in an as_str method.

Flagged

struct Task {
    kind: &'static str,
    id: u32,
}

fn download(id: u32) -> Task {
    Task {
        kind: "download",
        id,
    }
}

fn extract(id: u32) -> Task {
    Task {
        kind: "extract",
        id,
    }
}

fn run(t: &Task) -> u32 {
    if t.kind == "download" {
        t.id
    } else {
        0
    }
}

Fixed

#[derive(PartialEq)]
enum Kind {
    Download,
    Extract,
}

struct Task {
    kind: Kind,
    id: u32,
}

fn run(t: &Task) -> u32 {
    if t.kind == Kind::Download {
        t.id
    } else {
        0
    }
}

`tuple_wants_struct`

A private function returning two values of one type as a tuple, which every caller unpacks under the same names. A struct puts those names in the signature, so the pair cannot be read the wrong way round.

Flagged

struct Node {
    depth: u32,
    width: u32,
}

fn measure(n: &Node) -> (u32, u32) {
    (n.depth + 1, n.width * 2)
}

fn taller(a: &Node, b: &Node) -> bool {
    let (depth, width) = measure(a);
    depth > width && depth > b.depth
}

fn wider(a: &Node) -> u32 {
    let (depth, width) = measure(a);
    width.saturating_sub(depth)
}

Fixed

struct Size {
    depth: u32,
    width: u32,
}

fn measure(n: &Node) -> Size {
    Size {
        depth: n.depth + 1,
        width: n.width * 2,
    }
}

fn wider(a: &Node) -> u32 {
    let Size { depth, width } = measure(a);
    width.saturating_sub(depth)
}

`some_still_unchecked`

A match that accepts Some only when a further test passes and sends the rest to the None arm, so Some no longer means usable. Filter where the value is made. It is off until some-still-unchecked-enabled = true.

Flagged

fn poll() -> u32 {
    match next() {
        Some(j) if j.ready => j.id,
        _ => wait(), // a Job that is not ready lands here, with None
    }
}

Fixed

fn next_ready() -> Option<Job> {
    next().filter(|j| j.ready)
}

fn poll() -> u32 {
    match next_ready() {
        Some(j) => j.id,
        None => wait(),
    }
}

Checks that some path skips (`mordant_checks`)

`unchecked_construction`

A type with a checking constructor, built some other way: a struct literal, a field write or a transmute. Everything downstream trusts a value that was never checked.

Flagged

mod port {
    pub struct Port {
        pub(crate) n: u16,
    }

    impl Port {
        pub fn new(n: u32) -> Result<Port, ()> {
            if n <= u16::MAX as u32 {
                Ok(Port { n: n as u16 })
            } else {
                Err(())
            }
        }
    }
}

use port::Port;

fn bypass() -> Port {
    Port { n: 0 }
}

Fixed

mod port {
    pub struct Port {
        n: u16,
    }

    impl Port {
        pub fn new(n: u32) -> Result<Port, ()> {
            if n <= u16::MAX as u32 {
                Ok(Port { n: n as u16 })
            } else {
                Err(())
            }
        }
    }
}

use port::Port;

fn build(n: u32) -> Result<Port, ()> {
    Port::new(n)
}

`defaulted_failure`

A rejection turned straight into a fixed value, such as parse(x).unwrap_or(0). Bad input then carries on as if it were something else, so pass the failure to the caller.

Flagged

fn install(header: &str, bytes: &[u8]) -> Result<(), Error> {
    let integrity = Integrity::parse(header).unwrap_or_default(); // malformed: "no integrity"
    verify(bytes, &integrity)?; // which verifies nothing
    Ok(())
}

Fixed

fn install(header: &str, bytes: &[u8]) -> Result<(), Error> {
    let integrity = Integrity::parse(header).map_err(Error::MalformedIntegrity)?;
    verify(bytes, &integrity)?;
    Ok(())
}

`unchecked_input_len`

A length from input that is bounded on one path and passed unchecked on another into set_len, get_unchecked or similar, where crafted input becomes an out-of-bounds access. It is off until unchecked-input-len-enabled = true.

Flagged

impl Reader {
    fn window<'a>(&self, bytes: &'a [u8], start: usize) -> &'a [u8] {
        if self.strict && start + 32 > bytes.len() {
            return &[];
        }
        unsafe { core::slice::from_raw_parts(bytes.as_ptr().add(start), 32) } // unchecked when !strict
    }
}

Fixed

impl Reader {
    fn window<'a>(&self, bytes: &'a [u8], start: usize) -> &'a [u8] {
        if start + 32 > bytes.len() {
            return &[];
        }
        unsafe { core::slice::from_raw_parts(bytes.as_ptr().add(start), 32) }
    }
}

`guard_blind_to_action`

A check such as can_donate guarding a call that changes fields the check never reads. The guard and the action have drifted apart, so make the check look at what the call touches.

Flagged

struct Sched {
    queue: Vec<u32>,
    conns: Vec<u32>,
}

impl Sched {
    fn can_donate(&self) -> bool {
        self.queue.is_empty()
    }

    fn donate(&mut self) {
        if !self.can_donate() {
            return;
        }
        self.detach();
    }

    fn detach(&mut self) {
        self.queue.clear();
        self.conns = Vec::new();
    }
}

Fixed

struct Sched {
    queue: Vec<u32>,
    conns: Vec<u32>,
}

impl Sched {
    fn can_donate(&self) -> bool {
        self.queue.is_empty() && self.conns.is_empty()
    }

    fn donate(&mut self) {
        if !self.can_donate() {
            return;
        }
        self.detach();
    }

    fn detach(&mut self) {
        self.queue.clear();
        self.conns = Vec::new();
    }
}

`stale_across_reentry`

A method saves a fact about a field, such as its len(), calls something that can run other code, then uses the saved fact. The callback may have changed the field, so read it again after the call.

Flagged

impl List {
    fn trim(&mut self) {
        let last = self.items.len() - 1;
        (self.on_event)(self); // may push to or clear self.items
        self.items.remove(last); // `last` describes the vector before the callback
    }
}

Fixed

impl List {
    fn trim(&mut self) {
        (self.on_event)(self);
        let last = self.items.len() - 1; // read after the callback
        self.items.remove(last);
    }
}

`error_collapsed_to_bool`

A function that turns a typed Err into false, called by code that ignores even that. Return the Result so the caller can see what failed.

Flagged

fn write_pidfile(buf: &mut Vec<u8>) -> bool {
    match sys_write(buf, b"1") {
        Ok(_) => true,
        Err(_) => false,
    }
}

fn start(buf: &mut Vec<u8>) {
    write_pidfile(buf); // the false goes nowhere
}

Fixed

fn write_pidfile(buf: &mut Vec<u8>) -> Result<(), SysError> {
    sys_write(buf, b"1")?;
    Ok(())
}

fn start(buf: &mut Vec<u8>) -> Result<(), SysError> {
    write_pidfile(buf)?;
    Ok(())
}

`narrowed_two_ways`

One integer narrowed with try_from in one place and a bare as in another. They cannot both be right, so store the narrow type once, checked where the value is made.

Flagged

struct Buf {
    len: usize,
}

fn wire_len(b: &Buf) -> u32 {
    b.len as u32 // wraps past 4 GiB
}

fn header_len(b: &Buf) -> u32 {
    u32::try_from(b.len).expect("length fits the header") // knows it might not
}

Fixed

struct Buf {
    len: u32, // checked once, where the Buf is made
}

fn wire_len(b: &Buf) -> u32 {
    b.len
}

`cast_bypasses_from`

A transmute or pointer cast into a type that already has a From, TryFrom or constructor for that conversion. The cast accepts any bit pattern, and an enum value with no variant is undefined behaviour.

Flagged

mod level {
    #[repr(u8)]
    pub enum Level {
        Low = 0,
        Mid = 1,
        High = 2,
    }

    impl TryFrom<u8> for Level {
        type Error = u8;
        fn try_from(n: u8) -> Result<Self, u8> {
            match n {
                0 => Ok(Level::Low),
                1 => Ok(Level::Mid),
                2 => Ok(Level::High),
                _ => Err(n),
            }
        }
    }
}

fn level_from_wire(n: u8) -> level::Level {
    unsafe { core::mem::transmute::<u8, level::Level>(n) } // 3..=255 is UB
}

Fixed

fn level_from_wire(n: u8) -> Result<level::Level, u8> {
    level::Level::try_from(n)
}

`sentinel_integer`

An integer where one value, such as u32::MAX, means "nothing here", and some code indexes with it unchecked. Store an Option and every reader has to handle the empty case.

Flagged

const INVALID_SLOT: u32 = u32::MAX;

struct Entry {
    slot: u32,
}

impl Table {
    fn name(&self, e: &Entry) -> Option<&str> {
        if e.slot == INVALID_SLOT {
            None
        } else {
            Some(self.names[e.slot as usize])
        }
    }

    fn rename(&mut self, e: &Entry, to: &'static str) {
        self.names[e.slot as usize] = to; // INVALID_SLOT indexes past the end
    }
}

Fixed

struct Entry {
    slot: Option<u32>,
}

impl Table {
    fn rename(&mut self, e: &Entry, to: &'static str) {
        if let Some(slot) = e.slot {
            self.names[slot as usize] = to;
        }
    }
}

Errors that lose their type (`mordant_errors`)

`stringly_error`

An exported function whose error is a String or &str. A caller can print it but not match on it, so return an error enum and keep the text in its Display.

Flagged

pub fn parse_port(x: u32) -> Result<u32, String> {
    if x > 0 {
        Ok(x)
    } else {
        Err("zero".to_owned())
    }
}

Fixed

pub enum PortError {
    Zero,
}

pub fn parse_port(x: u32) -> Result<u32, PortError> {
    if x > 0 {
        Ok(x)
    } else {
        Err(PortError::Zero)
    }
}

`stringified_error`

.map_err(|e| e.to_string()) on a typed error, the moment the type is lost. Pass the typed error on and make text only where a person reads it.

Flagged

fn load(r: Result<u32, ParseError>) -> Result<u32, String> {
    r.map_err(|e| e.to_string())
}

Fixed

enum Wrapped {
    Parse(ParseError),
}

fn load(r: Result<u32, ParseError>) -> Result<u32, Wrapped> {
    r.map_err(Wrapped::Parse)
}

`discarded_error`

something().ok(); on its own line, which drops the error while looking like handling. let _ = something(); says the same thing plainly.

Flagged

fn cleanup() {
    remove_socket_file().ok();
}

Fixed

fn cleanup() {
    if let Err(e) = remove_socket_file() {
        eprintln!("could not remove socket file: {e}");
    }
}

`unread_error_variant`

A variant of a private enum that is built but never matched. Fold it into a neighbour, or handle it where a catch-all arm is swallowing it.

Flagged

enum LoadError {
    NotFound,
    Corrupt(String),
}

fn handle(x: u32) -> u32 {
    match load(x) {
        Ok(n) => n,
        Err(LoadError::NotFound) => 0,
        Err(other) => {
            eprintln!("{other}");
            0
        }
    }
}

Fixed

enum LoadError {
    NotFound,
    Corrupt(String),
}

fn handle(x: u32) -> u32 {
    match load(x) {
        Ok(n) => n,
        Err(LoadError::NotFound) => 0,
        Err(LoadError::Corrupt(why)) => {
            eprintln!("rebuilding index: {why}");
            0
        }
    }
}

Enums wider than their use (`mordant_enums`)

`wildcard_over_own_enum`

A _ => arm over an enum from this crate, which will quietly take the next variant anyone adds. List the variants instead, or let cargo mordant --fix list them.

Flagged

enum Op {
    Add,
    Sub,
    Mul,
}

fn cost(o: Op) -> i32 {
    match o {
        Op::Add => 1,
        _ => 0,
    }
}

Fixed

enum Op {
    Add,
    Sub,
    Mul,
}

fn cost(o: Op) -> i32 {
    match o {
        Op::Add => 1,
        Op::Sub | Op::Mul => 0,
    }
}

`param_wider_than_callers`

A function that panics on an enum variant no caller ever passes. Narrow the parameter so passing that variant no longer compiles.

Flagged

enum Shape {
    Circle(u32),
    Square(u32),
    Line,
}

fn area(s: Shape) -> u32 {
    match s {
        Shape::Circle(r) => 3 * r * r,
        Shape::Square(w) => w * w,
        Shape::Line => unreachable!("lines have no area"),
    }
}

fn draw() -> u32 {
    area(Shape::Circle(2)) + area(Shape::Square(3))
}

Fixed

enum Area {
    Circle(u32),
    Square(u32),
}

fn area(s: Area) -> u32 {
    match s {
        Area::Circle(r) => 3 * r * r,
        Area::Square(w) => w * w,
    }
}

fn draw() -> u32 {
    area(Area::Circle(2)) + area(Area::Square(3))
}

`return_wider_than_body`

Callers that panic on a variant the function never returns. Narrow the return type and the arm cannot be written.

Flagged

enum Token {
    Word(u32),
    Space,
    Eof,
}

fn next_token(n: u32) -> Token {
    if n == 0 {
        Token::Space
    } else {
        Token::Word(n)
    }
}

fn count(n: u32) -> u32 {
    match next_token(n) {
        Token::Word(w) => w,
        Token::Space => 0,
        Token::Eof => unreachable!("the tokenizer never yields Eof here"),
    }
}

Fixed

enum Token {
    Word(u32),
    Space,
}

fn next_token(n: u32) -> Token {
    if n == 0 {
        Token::Space
    } else {
        Token::Word(n)
    }
}

fn count(n: u32) -> u32 {
    match next_token(n) {
        Token::Word(w) => w,
        Token::Space => 0,
    }
}

Code written twice (`mordant_duplication`)

`same_match_twice`

A match that repeats another one arm for arm. Make it a method on the enum, so a new variant is handled in one place.

Flagged

pub fn describe(f: &Failure) -> (u32, &'static str) {
    match f.step {
        Step::Open => (f.code, "open"),
        Step::Read => (f.code + 1, "read"),
        Step::Parse => (f.code + 2, "parse"),
    }
}

pub fn log_line(cause: &Failure) -> String {
    let (code, what) = match cause.step {
        Step::Open => (cause.code, "open"),
        Step::Read => (cause.code + 1, "read"),
        Step::Parse => (cause.code + 2, "parse"),
    };
    format!("{what} failed with {code}")
}

Fixed

impl Step {
    fn describe(&self, code: u32) -> (u32, &'static str) {
        match self {
            Step::Open => (code, "open"),
            Step::Read => (code + 1, "read"),
            Step::Parse => (code + 2, "parse"),
        }
    }
}

`reimplemented_helper`

Two functions with the same signature and body apart from names. Keep one and call it from both places.

Flagged

pub fn clamp_add(base: u32, delta: u32, limit: u32) -> u32 {
    let sum = base.saturating_add(delta);
    if sum > limit {
        limit
    } else {
        sum
    }
}

pub fn bounded_sum(a: u32, b: u32, max: u32) -> u32 {
    let total = a.saturating_add(b);
    if total > max {
        max
    } else {
        total
    }
}

Fixed

pub fn clamp_add(base: u32, delta: u32, limit: u32) -> u32 {
    let sum = base.saturating_add(delta);
    if sum > limit {
        limit
    } else {
        sum
    }
}

`generic_body_not_generic`

A long stretch of a generic function that never touches its type parameters, compiled again for every set of types. Move it into a non-generic function. It is off until generic-body-not-generic-enabled = true.

Flagged

pub fn checksum<B: AsRef<[u8]>>(bytes: B) -> u32 {
    let src = bytes.as_ref();
    let mut acc = 17u32;
    let mut run = 0u32;
    for byte in src {
        let v = u32::from(*byte);
        acc = acc.wrapping_mul(31).wrapping_add(v);
        if v & 1 == 0 {
            run += 1;
        } else {
            run = 0;
        }
        acc ^= run << 3;
    }
    (acc ^ (src.len() as u32)).rotate_left(7)
}

Fixed

pub fn checksum<B: AsRef<[u8]>>(bytes: B) -> u32 {
    checksum_bytes(bytes.as_ref())
}

fn checksum_bytes(src: &[u8]) -> u32 {
    let mut acc = 17u32;
    let mut run = 0u32;
    for byte in src {
        let v = u32::from(*byte);
        acc = acc.wrapping_mul(31).wrapping_add(v);
        if v & 1 == 0 {
            run += 1;
        } else {
            run = 0;
        }
        acc ^= run << 3;
    }
    (acc ^ (src.len() as u32)).rotate_left(7)
}

Names doing a type's job (`mordant_naming`)

`bare_bool_args`

A call like render(text, true, false) to a private function with several bool parameters. An enum per flag makes each call say what it sets.

Flagged

fn render(text: &str, wrap: bool, color: bool) -> usize {
    text.len() + usize::from(wrap) + usize::from(color)
}

fn page(body: &str, notes: &str, color: bool) -> usize {
    render(body, true, false) + render(notes, false, false) + render("", true, color)
}

Fixed

enum Wrap {
    Wrap,
    NoWrap,
}

enum Color {
    Color,
    Plain,
}

fn render(text: &str, wrap: Wrap, color: Color) -> usize {
    text.len()
        + usize::from(matches!(wrap, Wrap::Wrap))
        + usize::from(matches!(color, Color::Color))
}

fn page(body: &str, notes: &str, color: Color) -> usize {
    render(body, Wrap::Wrap, Color::Plain)
        + render(notes, Wrap::NoWrap, Color::Plain)
        + render("", Wrap::Wrap, color)
}

`arg_named_like_other_param`

resize(height, width) against fn resize(width: u32, height: u32). Either the call is swapped or the names mislead, and distinct types make the next swap a compile error.

Flagged

fn resize(width: u32, height: u32) -> u32 {
    width * 2 + height
}

fn thumbnail(width: u32, height: u32) -> u32 {
    resize(height, width)
}

Fixed

struct Width(u32);
struct Height(u32);

fn resize(width: Width, height: Height) -> u32 {
    width.0 * 2 + height.0
}

fn thumbnail(width: Width, height: Height) -> u32 {
    resize(width, height)
}

`interchangeable_aliases`

A PackageId used where a DependencyId is expected, when both are aliases of u32. An alias is only another name, so use newtypes.

Flagged

pub type PackageId = u32;
pub type DependencyId = u32;

pub fn link(package: PackageId, dependency: DependencyId) -> u32 {
    package ^ dependency
}

pub fn relink(pkg: PackageId, dep: DependencyId) -> u32 {
    link(dep, pkg)
}

Fixed

#[derive(Clone, Copy, PartialEq, Eq, Hash)]
pub struct PackageId(u32);
#[derive(Clone, Copy, PartialEq, Eq, Hash)]
pub struct DependencyId(u32);

pub fn link(package: PackageId, dependency: DependencyId) -> u32 {
    package.0 ^ dependency.0
}

pub fn relink(pkg: PackageId, dep: DependencyId) -> u32 {
    link(pkg, dep) // link(dep, pkg) no longer compiles
}

`index_of_other_kind`

An index named for one table, such as pkg_id, used on a table that the rest of the function indexes with dep_id. An index newtype per table makes it a type error.

Flagged

fn is_stale(l: &Lockfile, dep_id: u32, pkg_id: u32) -> bool {
    let resolved = l.resolutions[dep_id as usize]; // resolutions: one slot per dependency
    let name = l.packages[pkg_id as usize];
    let stale = l.resolutions[pkg_id as usize]; // a package id into the dependency table
    resolved == stale && !name.is_empty()
}

Fixed

struct DepId(u32);
struct PkgId(u32);

// Resolutions implements Index<DepId>, and Packages implements Index<PkgId>.

fn is_stale(l: &Lockfile, dep_id: DepId, pkg_id: PkgId) -> bool {
    let resolved = l.resolutions[dep_id];
    let name = l.packages[pkg_id];
    // l.resolutions[pkg_id] is now a type error
    resolved != 0 && !name.is_empty()
}

`unit_mismatch`

Adding or comparing names with different unit suffixes, like timeout_ms + deadline_ns. Convert one side first, or use Duration.

Flagged

struct Timing {
    timeout_ms: u64,
    deadline_ns: u64,
}

fn remaining(t: &Timing) -> u64 {
    t.timeout_ms + t.deadline_ns
}

Fixed

struct Timing {
    timeout_ms: u64,
    deadline_ns: u64,
}

fn remaining(t: &Timing) -> u64 {
    t.timeout_ms * 1_000_000 + t.deadline_ns
}

Map keys and locks (`mordant_keys_locks`)

`key_not_identity`

A map keyed on a type that does not identify its value, such as a source Span, where two things at one span overwrite each other. It does nothing until key-not-identity-types names at least one type.

Flagged

m.insert(*span, index);
pool.insert(constant.to_bits(), index);
ids.insert(value.as_ptr() as usize, ());

Fixed

m.insert((file, *span), index);
pool.insert(constant, index);
ids.insert(value, ());

`insert_then_unwrap`

map.insert(k, v) followed by map.get(&k).unwrap(). Keep the value you inserted, or use entry.

Flagged

fn put(m: &mut HashMap<u32, u32>) -> u32 {
    m.insert(1, 2);
    let v = m.get(&1).unwrap();
    *v
}

Fixed

fn put(m: &mut HashMap<u32, u32>) -> u32 {
    m.insert(1, 2);
    2
}

`lock_order`

Two locks that one method takes in one order and another method takes in the other, which can deadlock. Pick one order, or guard both with one lock.

Flagged

struct Pair {
    a: Mutex<u32>,
    b: Mutex<u32>,
}

impl Pair {
    fn total(&self) -> u32 {
        let ga = self.a.lock().unwrap();
        let gb = self.b.lock().unwrap();
        *ga + *gb
    }

    fn spread(&self) -> u32 {
        let gb = self.b.lock().unwrap();
        let ga = self.a.lock().unwrap();
        ga.abs_diff(*gb)
    }
}

Fixed

struct Pair {
    a: Mutex<u32>,
    b: Mutex<u32>,
}

impl Pair {
    fn total(&self) -> u32 {
        let ga = self.a.lock().unwrap();
        let gb = self.b.lock().unwrap();
        *ga + *gb
    }

    fn spread(&self) -> u32 {
        let ga = self.a.lock().unwrap();
        let gb = self.b.lock().unwrap();
        ga.abs_diff(*gb)
    }
}

Comments that have gone stale (`mordant_comments`)

`stale_safety_comment`

A // SAFETY: comment naming something in backticks that no longer exists. Names defined in C look stale too, so it is off until stale-safety-comment-enabled = true.

Flagged

fn read(p: *const u64) -> u64 {
    // SAFETY: `frames_lock` is held for the duration of this read.
    unsafe { *p }
}

Fixed

fn read(p: *const u64, guard: &std::sync::Mutex<()>) -> u64 {
    let _held = guard.lock().unwrap();
    // SAFETY: `_held` keeps `guard` locked while the pointer is read.
    unsafe { *p }
}

`stale_panic_message`

The same check for expect and panic! messages, which would send whoever reads the crash looking for code that is gone.

Flagged

fn slot(t: &Table, i: usize) -> u32 {
    *t.slots.get(i).expect("guarded upstream by `frame_lock`")
}

Fixed

fn slot(t: &Table, i: usize) -> u32 {
    *t.slots
        .get(i)
        .expect("index checked against `slots` length")
}

Public code nobody uses (`mordant_unused`)

`unused_pub`

A pub item that nothing in the workspace uses. Run it with --workspace, and disable it for a library published outside the workspace.

Flagged

/// Nothing in the workspace calls this.
pub fn legacy_split(line: &str) -> Vec<&str> {
    line.split(',').collect()
}

Your own rules (`mordant_custom`)

`forbidden_reach`

A rule you write in mordant.toml, such as "nothing reached from hot_path may call Vec::push". Mordant reports the shortest chain of calls to each banned function:

[[mordant.forbidden-reach]]
from = "hot_path"
never = ["std::vec::Vec::push"]

Flagged

fn hot_path(n: u32) -> u32 {
    helper(n)
}

fn helper(n: u32) -> u32 {
    let mut v = Vec::new();
    v.push(n);
    v[0]
}

Fixed

fn hot_path(n: u32) -> u32 {
    n + 1
}

fn helper(n: u32) -> u32 {
    let mut v = Vec::new();
    v.push(n);
    v[0]
}

Mordant lints

State that should be a type (mordant_state)

options_as_enum

parallel_bools

bool_cluster

runtime_typestate

always_unwrapped_option

derived_field

field_valid_only_when

bool_beside_option

parallel_vecs

parallel_params

stringly_state

tuple_wants_struct

some_still_unchecked

Checks that some path skips (mordant_checks)

unchecked_construction

defaulted_failure

unchecked_input_len

guard_blind_to_action

stale_across_reentry

error_collapsed_to_bool

narrowed_two_ways

cast_bypasses_from

sentinel_integer

Errors that lose their type (mordant_errors)

stringly_error

stringified_error

discarded_error

unread_error_variant

Enums wider than their use (mordant_enums)

wildcard_over_own_enum

param_wider_than_callers

return_wider_than_body

Code written twice (mordant_duplication)

same_match_twice

reimplemented_helper

generic_body_not_generic

Names doing a type's job (mordant_naming)

bare_bool_args

arg_named_like_other_param

interchangeable_aliases

index_of_other_kind

unit_mismatch

Map keys and locks (mordant_keys_locks)

key_not_identity

insert_then_unwrap

lock_order

Comments that have gone stale (mordant_comments)

stale_safety_comment

stale_panic_message

Public code nobody uses (mordant_unused)

unused_pub

Your own rules (mordant_custom)

forbidden_reach

State that should be a type (`mordant_state`)

`options_as_enum`

`parallel_bools`

`bool_cluster`

`runtime_typestate`

`always_unwrapped_option`

`derived_field`

`field_valid_only_when`

`bool_beside_option`

`parallel_vecs`

`parallel_params`

`stringly_state`

`tuple_wants_struct`

`some_still_unchecked`

Checks that some path skips (`mordant_checks`)

`unchecked_construction`

`defaulted_failure`

`unchecked_input_len`

`guard_blind_to_action`

`stale_across_reentry`

`error_collapsed_to_bool`

`narrowed_two_ways`

`cast_bypasses_from`

`sentinel_integer`

Errors that lose their type (`mordant_errors`)

`stringly_error`

`stringified_error`

`discarded_error`

`unread_error_variant`

Enums wider than their use (`mordant_enums`)

`wildcard_over_own_enum`

`param_wider_than_callers`

`return_wider_than_body`

Code written twice (`mordant_duplication`)

`same_match_twice`

`reimplemented_helper`

`generic_body_not_generic`

Names doing a type's job (`mordant_naming`)

`bare_bool_args`

`arg_named_like_other_param`

`interchangeable_aliases`

`index_of_other_kind`

`unit_mismatch`

Map keys and locks (`mordant_keys_locks`)

`key_not_identity`

`insert_then_unwrap`

`lock_order`

Comments that have gone stale (`mordant_comments`)

`stale_safety_comment`

`stale_panic_message`

Public code nobody uses (`mordant_unused`)

`unused_pub`

Your own rules (`mordant_custom`)

`forbidden_reach`