Merge pull request #5726 from jetlime/clippy_suppress_cognitive_complexity_warning

Address cognitive complexity

src/uu/cat/src/cat.rs

@@ -463,7 +463,6 @@ fn write_fast<R: FdReadable>(handle: &mut InputHandle<R>) -> CatResult<()> {
 
 /// Outputs file contents to stdout in a line-by-line fashion,
 /// propagating any errors that might occur.
-#[allow(clippy::cognitive_complexity)]
 fn write_lines<R: FdReadable>(
     handle: &mut InputHandle<R>,
     options: &OutputOptions,
@@ -482,22 +481,7 @@ fn write_lines<R: FdReadable>(
         while pos < n {
             // skip empty line_number enumerating them if needed
             if in_buf[pos] == b'\n' {
-                // \r followed by \n is printed as ^M when show_ends is enabled, so that \r\n prints as ^M$
+                write_new_line(&mut writer, options, state, handle.is_interactive)?;
-                if state.skipped_carriage_return && options.show_ends {
-                    writer.write_all(b"^M")?;
-                    state.skipped_carriage_return = false;
-                }
-                if !state.at_line_start || !options.squeeze_blank || !state.one_blank_kept {
-                    state.one_blank_kept = true;
-                    if state.at_line_start && options.number == NumberingMode::All {
-                        write!(writer, "{0:6}\t", state.line_number)?;
-                        state.line_number += 1;
-                    }
-                    writer.write_all(options.end_of_line().as_bytes())?;
-                    if handle.is_interactive {
-                        writer.flush()?;
-                    }
-                }
                 state.at_line_start = true;
                 pos += 1;
                 continue;
@@ -514,13 +498,8 @@ fn write_lines<R: FdReadable>(
             }
 
             // print to end of line or end of buffer
-            let offset = if options.show_nonprint {
+            let offset = write_end(&mut writer, &in_buf[pos..], options);
-                write_nonprint_to_end(&in_buf[pos..], &mut writer, options.tab().as_bytes())
+
-            } else if options.show_tabs {
-                write_tab_to_end(&in_buf[pos..], &mut writer)
-            } else {
-                write_to_end(&in_buf[pos..], &mut writer)
-            };
             // end of buffer?
             if offset + pos == in_buf.len() {
                 state.at_line_start = false;
@@ -531,10 +510,11 @@ fn write_lines<R: FdReadable>(
             } else {
                 assert_eq!(in_buf[pos + offset], b'\n');
                 // print suitable end of line
-                writer.write_all(options.end_of_line().as_bytes())?;
+                write_end_of_line(
-                if handle.is_interactive {
+                    &mut writer,
-                    writer.flush()?;
+                    options.end_of_line().as_bytes(),
-                }
+                    handle.is_interactive,
+                )?;
                 state.at_line_start = true;
             }
             pos += offset + 1;
@@ -544,6 +524,41 @@ fn write_lines<R: FdReadable>(
     Ok(())
 }
 
+// \r followed by \n is printed as ^M when show_ends is enabled, so that \r\n prints as ^M$
+fn write_new_line<W: Write>(
+    writer: &mut W,
+    options: &OutputOptions,
+    state: &mut OutputState,
+    is_interactive: bool,
+) -> CatResult<()> {
+    if state.skipped_carriage_return && options.show_ends {
+        writer.write_all(b"^M")?;
+        state.skipped_carriage_return = false;
+    }
+    if !state.at_line_start || !options.squeeze_blank || !state.one_blank_kept {
+        state.one_blank_kept = true;
+        if state.at_line_start && options.number == NumberingMode::All {
+            write!(writer, "{0:6}\t", state.line_number)?;
+            state.line_number += 1;
+        }
+        writer.write_all(options.end_of_line().as_bytes())?;
+        if is_interactive {
+            writer.flush()?;
+        }
+    }
+    Ok(())
+}
+
+fn write_end<W: Write>(writer: &mut W, in_buf: &[u8], options: &OutputOptions) -> usize {
+    if options.show_nonprint {
+        write_nonprint_to_end(in_buf, writer, options.tab().as_bytes())
+    } else if options.show_tabs {
+        write_tab_to_end(in_buf, writer)
+    } else {
+        write_to_end(in_buf, writer)
+    }
+}
+
 // write***_to_end methods
 // Write all symbols till \n or \r or end of buffer is reached
 // We need to stop at \r because it may be written as ^M depending on the byte after and settings;
@@ -610,6 +625,18 @@ fn write_nonprint_to_end<W: Write>(in_buf: &[u8], writer: &mut W, tab: &[u8]) ->
     count
 }
 
+fn write_end_of_line<W: Write>(
+    writer: &mut W,
+    end_of_line: &[u8],
+    is_interactive: bool,
+) -> CatResult<()> {
+    writer.write_all(end_of_line)?;
+    if is_interactive {
+        writer.flush()?;
+    }
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use std::io::{stdout, BufWriter};

src/uu/touch/src/touch.rs

@@ -11,7 +11,7 @@ use chrono::{
     TimeZone, Timelike,
 };
 use clap::builder::ValueParser;
-use clap::{crate_version, Arg, ArgAction, ArgGroup, Command};
+use clap::{crate_version, Arg, ArgAction, ArgGroup, ArgMatches, Command};
 use filetime::{set_file_times, set_symlink_file_times, FileTime};
 use std::ffi::OsString;
 use std::fs::{self, File};
@@ -73,7 +73,6 @@ fn filetime_to_datetime(ft: &FileTime) -> Option<DateTime<Local>> {
 }
 
 #[uucore::main]
-#[allow(clippy::cognitive_complexity)]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let matches = uu_app().try_get_matches_from(args)?;
 
@@ -86,37 +85,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
             ),
         )
     })?;
-    let (mut atime, mut mtime) = match (
+
-        matches.get_one::<OsString>(options::sources::REFERENCE),
+    let (atime, mtime) = determine_times(&matches)?;
-        matches.get_one::<String>(options::sources::DATE),
-    ) {
-        (Some(reference), Some(date)) => {
-            let (atime, mtime) = stat(Path::new(reference), !matches.get_flag(options::NO_DEREF))?;
-            let atime = filetime_to_datetime(&atime).ok_or_else(|| {
-                USimpleError::new(1, "Could not process the reference access time")
-            })?;
-            let mtime = filetime_to_datetime(&mtime).ok_or_else(|| {
-                USimpleError::new(1, "Could not process the reference modification time")
-            })?;
-            (parse_date(atime, date)?, parse_date(mtime, date)?)
-        }
-        (Some(reference), None) => {
-            stat(Path::new(reference), !matches.get_flag(options::NO_DEREF))?
-        }
-        (None, Some(date)) => {
-            let timestamp = parse_date(Local::now(), date)?;
-            (timestamp, timestamp)
-        }
-        (None, None) => {
-            let timestamp = if let Some(ts) = matches.get_one::<String>(options::sources::TIMESTAMP)
-            {
-                parse_timestamp(ts)?
-            } else {
-                datetime_to_filetime(&Local::now())
-            };
-            (timestamp, timestamp)
-        }
-    };
 
     for filename in files {
         // FIXME: find a way to avoid having to clone the path
@@ -165,48 +135,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
             }
         }
 
-        // If changing "only" atime or mtime, grab the existing value of the other.
+        update_times(&matches, path, atime, mtime, filename)?;
-        // Note that "-a" and "-m" may be passed together; this is not an xor.
-        if matches.get_flag(options::ACCESS)
-            || matches.get_flag(options::MODIFICATION)
-            || matches.contains_id(options::TIME)
-        {
-            let st = stat(path, !matches.get_flag(options::NO_DEREF))?;
-            let time = matches
-                .get_one::<String>(options::TIME)
-                .map(|s| s.as_str())
-                .unwrap_or("");
-
-            if !(matches.get_flag(options::ACCESS)
-                || time.contains(&"access".to_owned())
-                || time.contains(&"atime".to_owned())
-                || time.contains(&"use".to_owned()))
-            {
-                atime = st.0;
-            }
-
-            if !(matches.get_flag(options::MODIFICATION)
-                || time.contains(&"modify".to_owned())
-                || time.contains(&"mtime".to_owned()))
-            {
-                mtime = st.1;
-            }
-        }
-
-        // sets the file access and modification times for a file or a symbolic link.
-        // The filename, access time (atime), and modification time (mtime) are provided as inputs.
-
-        // If the filename is not "-", indicating a special case for touch -h -,
-        // the code checks if the NO_DEREF flag is set, which means the user wants to
-        // set the times for a symbolic link itself, rather than the file it points to.
-        if filename == "-" {
-            filetime::set_file_times(path, atime, mtime)
-        } else if matches.get_flag(options::NO_DEREF) {
-            set_symlink_file_times(path, atime, mtime)
-        } else {
-            set_file_times(path, atime, mtime)
-        }
-        .map_err_context(|| format!("setting times of {}", path.quote()))?;
     }
     Ok(())
 }
@@ -307,6 +236,92 @@ pub fn uu_app() -> Command {
         )
 }
 
+// Determine the access and modification time
+fn determine_times(matches: &ArgMatches) -> UResult<(FileTime, FileTime)> {
+    match (
+        matches.get_one::<OsString>(options::sources::REFERENCE),
+        matches.get_one::<String>(options::sources::DATE),
+    ) {
+        (Some(reference), Some(date)) => {
+            let (atime, mtime) = stat(Path::new(&reference), !matches.get_flag(options::NO_DEREF))?;
+            let atime = filetime_to_datetime(&atime).ok_or_else(|| {
+                USimpleError::new(1, "Could not process the reference access time")
+            })?;
+            let mtime = filetime_to_datetime(&mtime).ok_or_else(|| {
+                USimpleError::new(1, "Could not process the reference modification time")
+            })?;
+            Ok((parse_date(atime, date)?, parse_date(mtime, date)?))
+        }
+        (Some(reference), None) => {
+            stat(Path::new(&reference), !matches.get_flag(options::NO_DEREF))
+        }
+        (None, Some(date)) => {
+            let timestamp = parse_date(Local::now(), date)?;
+            Ok((timestamp, timestamp))
+        }
+        (None, None) => {
+            let timestamp = if let Some(ts) = matches.get_one::<String>(options::sources::TIMESTAMP)
+            {
+                parse_timestamp(ts)?
+            } else {
+                datetime_to_filetime(&Local::now())
+            };
+            Ok((timestamp, timestamp))
+        }
+    }
+}
+
+// Updating file access and modification times based on user-specified options
+fn update_times(
+    matches: &ArgMatches,
+    path: &Path,
+    mut atime: FileTime,
+    mut mtime: FileTime,
+    filename: &OsString,
+) -> UResult<()> {
+    // If changing "only" atime or mtime, grab the existing value of the other.
+    // Note that "-a" and "-m" may be passed together; this is not an xor.
+    if matches.get_flag(options::ACCESS)
+        || matches.get_flag(options::MODIFICATION)
+        || matches.contains_id(options::TIME)
+    {
+        let st = stat(path, !matches.get_flag(options::NO_DEREF))?;
+        let time = matches
+            .get_one::<String>(options::TIME)
+            .map(|s| s.as_str())
+            .unwrap_or("");
+
+        if !(matches.get_flag(options::ACCESS)
+            || time.contains(&"access".to_owned())
+            || time.contains(&"atime".to_owned())
+            || time.contains(&"use".to_owned()))
+        {
+            atime = st.0;
+        }
+
+        if !(matches.get_flag(options::MODIFICATION)
+            || time.contains(&"modify".to_owned())
+            || time.contains(&"mtime".to_owned()))
+        {
+            mtime = st.1;
+        }
+    }
+
+    // sets the file access and modification times for a file or a symbolic link.
+    // The filename, access time (atime), and modification time (mtime) are provided as inputs.
+
+    // If the filename is not "-", indicating a special case for touch -h -,
+    // the code checks if the NO_DEREF flag is set, which means the user wants to
+    // set the times for a symbolic link itself, rather than the file it points to.
+    if filename == "-" {
+        filetime::set_file_times(path, atime, mtime)
+    } else if matches.get_flag(options::NO_DEREF) {
+        set_symlink_file_times(path, atime, mtime)
+    } else {
+        set_file_times(path, atime, mtime)
+    }
+    .map_err_context(|| format!("setting times of {}", path.quote()))
+}
 // Get metadata of the provided path
 // If `follow` is `true`, the function will try to follow symlinks
 // If `follow` is `false` or the symlink is broken, the function will return metadata of the symlink itself