Improve docs

src/mode/mod.rs

@@ -2,15 +2,15 @@
 mod parser;
 use core::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign};
 
-pub use parser::{Parser, ParseError};
+pub use parser::{ParseError, Parser};
 
+#[must_use]
 pub fn get_umask() -> u32 {
     let mask = unsafe { libc::umask(0) };
-    let umask = unsafe {libc::umask(mask) };
+    unsafe { libc::umask(mask) }
-    umask
 }
 
-#[derive(Clone, Copy, Debug, PartialEq)]
+#[derive(Clone, Copy, PartialEq)]
 pub enum Bit {
     Suid = 0o4000,
     Sgid = 0o2000,

src/mode/parser.rs

@@ -1,12 +1,21 @@
-use std::{error, fmt::Display, num::ParseIntError, ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign}};
+use super::{get_umask, Bit};
-use super::{Bit, get_umask};
+use std::{
+    error,
+    fmt::Display,
+    num::ParseIntError,
+    ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign},
+};
 
 #[derive(Debug, PartialEq)]
 pub enum ParseError {
+    /// the given `Bit` cannot be set for the given `Who`
     InvalidBit,
-    InvalidDigit,
+    /// the character is not recognized
+    InvalidChar,
+    /// the specified octal mode is invalid
     OutsideRange,
     ParseIntError(ParseIntError),
+    /// no `Op` is set when parsing a `Bit`
     NoOpSet,
 }
 
@@ -24,17 +33,26 @@ impl From<ParseIntError> for ParseError {
     }
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(PartialEq)]
+/// Operations which can be performed to add, remove, or set explicitly the given
+/// `Bit` for the given `Who`
 enum Op {
+    /// `Bit`s will be added for `Who`
     Add,
+    /// `Bit`s will be remoed for `Who`
     Remove,
+    /// `Bit`s will be set to exactly the specified arrangement
     Equals,
 }
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(PartialEq)]
+/// The granularity of the given permissions
 enum Who {
+    /// applies for the current user
     User = 0o100,
+    /// applies for the current group
     Group = 0o10,
+    /// applies for everyone else
     Other = 0o1,
 }
 
@@ -66,6 +84,12 @@ impl BitOrAssign<Who> for u32 {
     }
 }
 
+/// A parser for octal and symbolic permissions. `Parser::default` creates an
+/// instance which applies the given operations to the default setting for the
+/// current user's umask. `Parser::new` creates a parser which applies the given
+/// operations to the specified beginning set of permissions. Therefore, when
+/// creating new files or directories use `Parser::default`, while `Parser::new`
+/// should be used for setting permissions of already existing files or directories.
 pub struct Parser {
     mode: u32,
     op: Option<Op>,
@@ -88,6 +112,7 @@ impl Default for Parser {
 }
 
 impl Parser {
+    #[must_use]
     pub fn new(mode: u32) -> Self {
         Self {
             mode,
@@ -97,7 +122,7 @@ impl Parser {
         }
     }
 
-    fn parse_octal(&mut self, value: &str) -> Result<u32, ParseError> {
+    fn parse_octal(value: &str) -> Result<u32, ParseError> {
         let m = u32::from_str_radix(value, 8)?;
         if m <= 0o7777 {
             Ok(m)
@@ -108,7 +133,7 @@ impl Parser {
 
     fn add_who(&mut self, who: Who) -> Result<(), ParseError> {
         if self.op.is_some() || !self.bits == 0 {
-            Err(ParseError::InvalidDigit)
+            Err(ParseError::InvalidChar)
         } else {
             self.who |= who;
             Ok(())
@@ -117,7 +142,7 @@ impl Parser {
 
     fn set_op(&mut self, op: Op) -> Result<(), ParseError> {
         if self.op.is_some() || !self.bits == 0 {
-            Err(ParseError::InvalidDigit)
+            Err(ParseError::InvalidChar)
         } else {
             self.op = Some(op);
             if self.who == 0 {
@@ -128,52 +153,66 @@ impl Parser {
     }
 
     fn push_read_bits(&mut self) -> Result<(), ParseError> {
-        if self.who & 0o100 != 0 {
+        if self.op.is_none() {
-            self.bits |= Bit::URead;
+            Err(ParseError::NoOpSet)
+        } else {
+            if self.who & 0o100 != 0 {
+                self.bits |= Bit::URead;
+            }
+            if self.who & 0o10 != 0 {
+                self.bits |= Bit::GRead;
+            }
+            if self.who & 0o1 != 0 {
+                self.bits |= Bit::ORead;
+            }
+            Ok(())
         }
-        if self.who & 0o10 != 0 {
-            self.bits |= Bit::GRead;
-        }
-        if self.who & 0o1 != 0 {
-            self.bits |= Bit::ORead;
-        }
-        Ok(())
     }
 
     fn push_write_bits(&mut self) -> Result<(), ParseError> {
-        if self.who & 0o100 != 0 {
+        if self.op.is_none() {
-            self.bits |= Bit::UWrite;
+            Err(ParseError::NoOpSet)
+        } else {
+            if self.who & 0o100 != 0 {
+                self.bits |= Bit::UWrite;
+            }
+            if self.who & 0o10 != 0 {
+                self.bits |= Bit::GWrite;
+            }
+            if self.who & 0o1 != 0 {
+                self.bits |= Bit::OWrite;
+            }
+            Ok(())
         }
-        if self.who & 0o10 != 0 {
-            self.bits |= Bit::GWrite;
-        }
-        if self.who & 0o1 != 0 {
-            self.bits |= Bit::OWrite;
-        }
-        Ok(())
     }
 
     fn push_exec_bits(&mut self) -> Result<(), ParseError> {
-        if self.who & 0o100 != 0 {
+        if self.op.is_none() {
-            self.bits |= Bit::UExec;
+            Err(ParseError::NoOpSet)
+        } else {
+            if self.who & 0o100 != 0 {
+                self.bits |= Bit::UExec;
+            }
+            if self.who & 0o10 != 0 {
+                self.bits |= Bit::GExec;
+            }
+            if self.who & 0o1 != 0 {
+                self.bits |= Bit::OExec;
+            }
+            Ok(())
         }
-        if self.who & 0o10 != 0 {
-            self.bits |= Bit::GExec;
-        }
-        if self.who & 0o1 != 0 {
-            self.bits |= Bit::OExec;
-        }
-        Ok(())
     }
 
     fn push_suid_sgid(&mut self) -> Result<(), ParseError> {
-        if self.who == 0 || self.who & 0o1 != 0{
+        if self.who == 0 || self.who & 0o1 != 0 {
             return Err(ParseError::InvalidBit);
+        } else if self.op.is_none() {
+            return Err(ParseError::NoOpSet);
         }
         if self.who & 0o100 != 0 {
             self.bits |= Bit::Suid;
         }
-        if self.who &0o10 != 0 {
+        if self.who & 0o10 != 0 {
             self.bits |= Bit::Sgid;
         }
         Ok(())
@@ -182,6 +221,8 @@ impl Parser {
     fn push_sticky(&mut self) -> Result<(), ParseError> {
         if self.who == 0 || self.who & 0o100 != 0 || self.who & 0o10 != 0 {
             return Err(ParseError::InvalidBit);
+        } else if self.op.is_none() {
+            return Err(ParseError::NoOpSet);
         }
         if self.who & 0o1 != 0 {
             self.bits |= Bit::Sticky;
@@ -208,11 +249,11 @@ impl Parser {
                 if self.who & 0o10 != 0 {
                     self.mode &= !(0o2070);
                 }
-                if self.who &0o1 != 0 {
+                if self.who & 0o1 != 0 {
                     self.mode &= !(0o1007);
                 }
                 self.add_bits();
-            },
+            }
             None => return Err(ParseError::NoOpSet),
         }
         Ok(())
@@ -224,18 +265,32 @@ impl Parser {
         self.bits = 0;
     }
 
+    #[must_use]
     pub fn mode(&self) -> u32 {
         self.mode
     }
 
+    /// Parses a numerical mode from either an octal string or symbolic representation
+    /// and applies those permissions to the starting set of permissions.
+    /// # Errors
+    /// Returns `ParseError` if:
+    /// - invalid digit
+    /// - no operation specified
+    /// - more than one operation specified at a time (multiple operations can
+    ///   be specified separated by comma)
+    /// - the specified bit cannot be applied to the specified `Who`
+    /// - bits are specified before operations
+    /// - the specified octal mode is greater than 0o7777
     pub fn parse(&mut self, value: &str) -> Result<u32, ParseError> {
-        match self.parse_octal(value) {
+        match Self::parse_octal(value) {
             Ok(mode) => {
                 self.mode = mode;
-                return Ok(mode)
+                return Ok(mode);
-            },
+            }
-            Err(e) => if e == ParseError::OutsideRange {
+            Err(e) => {
-                return Err(e);
+                if e == ParseError::OutsideRange {
+                    return Err(e);
+                }
             }
         }
         for c in value.chars() {
@@ -247,7 +302,7 @@ impl Parser {
                     self.add_who(Who::User)?;
                     self.add_who(Who::Group)?;
                     self.add_who(Who::Other)?;
-                },
+                }
                 '-' => self.set_op(Op::Remove)?,
                 '+' => self.set_op(Op::Add)?,
                 '=' => self.set_op(Op::Equals)?,
@@ -259,8 +314,8 @@ impl Parser {
                 ',' => {
                     self.set_bits()?;
                     self.reset();
-                },
+                }
-                _ => return Err(ParseError::InvalidDigit),
+                _ => return Err(ParseError::InvalidChar),
             }
         }
         self.set_bits()?;
@@ -349,4 +404,25 @@ mod test {
         let mode = parser.parse("10000");
         assert_eq!(mode, Err(ParseError::OutsideRange))
     }
+
+    #[test]
+    fn no_op() {
+        let mut parser = Parser::default();
+        let mode = parser.parse("rws");
+        assert_eq!(mode, Err(ParseError::NoOpSet));
+    }
+
+    #[test]
+    fn ordering_error() {
+        let mut parser = Parser::default();
+        let mode = parser.parse("ux+s");
+        assert_eq!(mode, Err(ParseError::NoOpSet));
+    }
+
+    #[test]
+    fn ordering_error1() {
+        let mut parser = Parser::default();
+        let mode = parser.parse("x+s");
+        assert_eq!(mode, Err(ParseError::NoOpSet));
+    }
 }