about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJason Newcomb <jsnewcomb@pm.me>2024-07-28 00:42:28 -0400
committerJason Newcomb <jsnewcomb@pm.me>2024-07-28 00:55:46 -0400
commit78a750e8904bbb75656c05eda18afef27c77a276 (patch)
treeab79ed5799bfda6c2a0370804e9f85154350f207
parent1d06ad55999b29b658f0a885bd8fb0a3be0282fd (diff)
downloadrust-78a750e8904bbb75656c05eda18afef27c77a276.tar.gz
rust-78a750e8904bbb75656c05eda18afef27c77a276.zip
Sort the config list using `dev fmt`
-rw-r--r--clippy_dev/src/fmt.rs392
1 files changed, 291 insertions, 101 deletions
diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs
index 25623144181..5fc4365c6e7 100644
--- a/clippy_dev/src/fmt.rs
+++ b/clippy_dev/src/fmt.rs
@@ -1,30 +1,65 @@
 use crate::clippy_project_root;
 use itertools::Itertools;
+use rustc_lexer::{tokenize, TokenKind};
 use shell_escape::escape;
 use std::ffi::{OsStr, OsString};
-use std::path::Path;
+use std::ops::ControlFlow;
+use std::path::{Path, PathBuf};
 use std::process::{self, Command, Stdio};
 use std::{fs, io};
 use walkdir::WalkDir;
 
-#[derive(Debug)]
-pub enum CliError {
+pub enum Error {
     CommandFailed(String, String),
-    IoError(io::Error),
+    Io(io::Error),
     RustfmtNotInstalled,
-    WalkDirError(walkdir::Error),
+    WalkDir(walkdir::Error),
     IntellijSetupActive,
+    Parse(PathBuf, usize, String),
+    CheckFailed,
 }
 
-impl From<io::Error> for CliError {
+impl From<io::Error> for Error {
     fn from(error: io::Error) -> Self {
-        Self::IoError(error)
+        Self::Io(error)
     }
 }
 
-impl From<walkdir::Error> for CliError {
+impl From<walkdir::Error> for Error {
     fn from(error: walkdir::Error) -> Self {
-        Self::WalkDirError(error)
+        Self::WalkDir(error)
+    }
+}
+
+impl Error {
+    fn display(&self) {
+        match self {
+            Self::CheckFailed => {
+                eprintln!("Formatting check failed!\nRun `cargo dev fmt` to update.");
+            },
+            Self::CommandFailed(command, stderr) => {
+                eprintln!("error: command `{command}` failed!\nstderr: {stderr}");
+            },
+            Self::Io(err) => {
+                eprintln!("error: {err}");
+            },
+            Self::RustfmtNotInstalled => {
+                eprintln!("error: rustfmt nightly is not installed.");
+            },
+            Self::WalkDir(err) => {
+                eprintln!("error: {err}");
+            },
+            Self::IntellijSetupActive => {
+                eprintln!(
+                    "error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`.\n\
+                    Not formatting because that would format the local repo as well!\n\
+                    Please revert the changes to `Cargo.toml`s with `cargo dev remove intellij`."
+                );
+            },
+            Self::Parse(path, line, msg) => {
+                eprintln!("error parsing `{}:{line}`: {msg}", path.display());
+            },
+        }
     }
 }
 
@@ -34,75 +69,244 @@ struct FmtContext {
     rustfmt_path: String,
 }
 
-// the "main" function of cargo dev fmt
-pub fn run(check: bool, verbose: bool) {
-    fn try_run(context: &FmtContext) -> Result<bool, CliError> {
-        let mut success = true;
-
-        let project_root = clippy_project_root();
-
-        // if we added a local rustc repo as path dependency to clippy for rust analyzer, we do NOT want to
-        // format because rustfmt would also format the entire rustc repo as it is a local
-        // dependency
-        if fs::read_to_string(project_root.join("Cargo.toml"))
-            .expect("Failed to read clippy Cargo.toml")
-            .contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]")
-        {
-            return Err(CliError::IntellijSetupActive);
-        }
-
-        rustfmt_test(context)?;
+struct ClippyConf<'a> {
+    name: &'a str,
+    attrs: &'a str,
+    lints: Vec<&'a str>,
+    field: &'a str,
+}
 
-        success &= cargo_fmt(context, project_root.as_path())?;
-        success &= cargo_fmt(context, &project_root.join("clippy_dev"))?;
-        success &= cargo_fmt(context, &project_root.join("rustc_tools_util"))?;
-        success &= cargo_fmt(context, &project_root.join("lintcheck"))?;
+fn offset_to_line(text: &str, offset: usize) -> usize {
+    match text.split('\n').try_fold((1usize, 0usize), |(line, pos), s| {
+        let pos = pos + s.len() + 1;
+        if pos > offset {
+            ControlFlow::Break(line)
+        } else {
+            ControlFlow::Continue((line + 1, pos))
+        }
+    }) {
+        ControlFlow::Break(x) | ControlFlow::Continue((x, _)) => x,
+    }
+}
 
-        let chunks = WalkDir::new(project_root.join("tests"))
-            .into_iter()
-            .filter_map(|entry| {
-                let entry = entry.expect("failed to find tests");
-                let path = entry.path();
+/// Formats the configuration list in `clippy_config/src/conf.rs`
+#[expect(clippy::too_many_lines)]
+fn fmt_conf(check: bool) -> Result<(), Error> {
+    #[derive(Clone, Copy)]
+    enum State {
+        Start,
+        Docs,
+        Pound,
+        OpenBracket,
+        Attr(u32),
+        Lints,
+        EndLints,
+        Field,
+    }
 
-                if path.extension() != Some("rs".as_ref()) || entry.file_name() == "ice-3891.rs" {
-                    None
-                } else {
-                    Some(entry.into_path().into_os_string())
-                }
-            })
-            .chunks(250);
+    let path: PathBuf = [
+        clippy_project_root().as_path(),
+        "clippy_config".as_ref(),
+        "src".as_ref(),
+        "conf.rs".as_ref(),
+    ]
+    .into_iter()
+    .collect();
+    let text = fs::read_to_string(&path)?;
 
-        for chunk in &chunks {
-            success &= rustfmt(context, chunk)?;
-        }
+    let (pre, conf) = text
+        .split_once("define_Conf! {\n")
+        .expect("can't find config definition");
+    let (conf, post) = conf.split_once("\n}\n").expect("can't find config definition");
+    let conf_offset = pre.len() + 15;
 
-        Ok(success)
-    }
+    let mut pos = 0u32;
+    let mut attrs_start = 0;
+    let mut attrs_end = 0;
+    let mut field_start = 0;
+    let mut lints = Vec::new();
+    let mut name = "";
+    let mut fields = Vec::new();
+    let mut state = State::Start;
 
-    fn output_err(err: CliError) {
-        match err {
-            CliError::CommandFailed(command, stderr) => {
-                eprintln!("error: A command failed! `{command}`\nstderr: {stderr}");
+    for (i, t) in tokenize(conf)
+        .map(|x| {
+            let start = pos;
+            pos += x.len;
+            (start as usize, x)
+        })
+        .filter(|(_, t)| !matches!(t.kind, TokenKind::Whitespace))
+    {
+        match (state, t.kind) {
+            (State::Start, TokenKind::LineComment { doc_style: Some(_) }) => {
+                attrs_start = i;
+                attrs_end = i + t.len as usize;
+                state = State::Docs;
             },
-            CliError::IoError(err) => {
-                eprintln!("error: {err}");
+            (State::Start, TokenKind::Pound) => {
+                attrs_start = i;
+                attrs_end = i;
+                state = State::Pound;
             },
-            CliError::RustfmtNotInstalled => {
-                eprintln!("error: rustfmt nightly is not installed.");
+            (State::Docs, TokenKind::LineComment { doc_style: Some(_) }) => attrs_end = i + t.len as usize,
+            (State::Docs, TokenKind::Pound) => state = State::Pound,
+            (State::Pound, TokenKind::OpenBracket) => state = State::OpenBracket,
+            (State::OpenBracket, TokenKind::Ident) => {
+                state = if conf[i..i + t.len as usize] == *"lints" {
+                    State::Lints
+                } else {
+                    State::Attr(0)
+                };
             },
-            CliError::WalkDirError(err) => {
-                eprintln!("error: {err}");
+            (State::Attr(0), TokenKind::CloseBracket) => {
+                attrs_end = i + 1;
+                state = State::Docs;
             },
-            CliError::IntellijSetupActive => {
-                eprintln!(
-                    "error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`.
-Not formatting because that would format the local repo as well!
-Please revert the changes to Cargo.tomls with `cargo dev remove intellij`."
-                );
+            (State::Attr(x), TokenKind::OpenParen | TokenKind::OpenBracket | TokenKind::OpenBrace) => {
+                state = State::Attr(x + 1);
+            },
+            (State::Attr(x), TokenKind::CloseParen | TokenKind::CloseBracket | TokenKind::CloseBrace) => {
+                state = State::Attr(x - 1);
+            },
+            (State::Lints, TokenKind::Ident) => lints.push(&conf[i..i + t.len as usize]),
+            (State::Lints, TokenKind::CloseBracket) => state = State::EndLints,
+            (State::EndLints | State::Docs, TokenKind::Ident) => {
+                field_start = i;
+                name = &conf[i..i + t.len as usize];
+                state = State::Field;
+            },
+            (State::Field, TokenKind::LineComment { doc_style: Some(_) }) => {
+                #[expect(clippy::drain_collect)]
+                fields.push(ClippyConf {
+                    name,
+                    lints: lints.drain(..).collect(),
+                    attrs: &conf[attrs_start..attrs_end],
+                    field: conf[field_start..i].trim_end(),
+                });
+                attrs_start = i;
+                attrs_end = i + t.len as usize;
+                state = State::Docs;
+            },
+            (State::Field, TokenKind::Pound) => {
+                #[expect(clippy::drain_collect)]
+                fields.push(ClippyConf {
+                    name,
+                    lints: lints.drain(..).collect(),
+                    attrs: &conf[attrs_start..attrs_end],
+                    field: conf[field_start..i].trim_end(),
+                });
+                attrs_start = i;
+                attrs_end = i;
+                state = State::Pound;
+            },
+            (State::Field | State::Attr(_), _)
+            | (State::Lints, TokenKind::Comma | TokenKind::OpenParen | TokenKind::CloseParen) => {},
+            _ => {
+                return Err(Error::Parse(
+                    path,
+                    offset_to_line(&text, conf_offset + i),
+                    format!("unexpected token `{}`", &conf[i..i + t.len as usize]),
+                ));
             },
         }
     }
 
+    if !matches!(state, State::Field) {
+        return Err(Error::Parse(
+            path,
+            offset_to_line(&text, conf_offset + conf.len()),
+            "incomplete field".into(),
+        ));
+    }
+    fields.push(ClippyConf {
+        name,
+        lints,
+        attrs: &conf[attrs_start..attrs_end],
+        field: conf[field_start..].trim_end(),
+    });
+
+    for field in &mut fields {
+        field.lints.sort_unstable();
+    }
+    fields.sort_by_key(|x| x.name);
+
+    let new_text = format!(
+        "{pre}define_Conf! {{\n{}}}\n{post}",
+        fields.iter().format_with("", |field, f| {
+            if field.lints.is_empty() {
+                f(&format_args!("    {}\n    {}\n", field.attrs, field.field))
+            } else if field.lints.iter().map(|x| x.len() + 2).sum::<usize>() < 120 - 14 {
+                f(&format_args!(
+                    "    {}\n    #[lints({})]\n    {}\n",
+                    field.attrs,
+                    field.lints.iter().join(", "),
+                    field.field,
+                ))
+            } else {
+                f(&format_args!(
+                    "    {}\n    #[lints({}\n    )]\n    {}\n",
+                    field.attrs,
+                    field
+                        .lints
+                        .iter()
+                        .format_with("", |x, f| f(&format_args!("\n        {x},"))),
+                    field.field,
+                ))
+            }
+        })
+    );
+
+    if text != new_text {
+        if check {
+            return Err(Error::CheckFailed);
+        }
+        fs::write(&path, new_text.as_bytes())?;
+    }
+    Ok(())
+}
+
+fn run_rustfmt(context: &FmtContext) -> Result<(), Error> {
+    let project_root = clippy_project_root();
+
+    // if we added a local rustc repo as path dependency to clippy for rust analyzer, we do NOT want to
+    // format because rustfmt would also format the entire rustc repo as it is a local
+    // dependency
+    if fs::read_to_string(project_root.join("Cargo.toml"))
+        .expect("Failed to read clippy Cargo.toml")
+        .contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]")
+    {
+        return Err(Error::IntellijSetupActive);
+    }
+
+    check_for_rustfmt(context)?;
+
+    cargo_fmt(context, project_root.as_path())?;
+    cargo_fmt(context, &project_root.join("clippy_dev"))?;
+    cargo_fmt(context, &project_root.join("rustc_tools_util"))?;
+    cargo_fmt(context, &project_root.join("lintcheck"))?;
+
+    let chunks = WalkDir::new(project_root.join("tests"))
+        .into_iter()
+        .filter_map(|entry| {
+            let entry = entry.expect("failed to find tests");
+            let path = entry.path();
+
+            if path.extension() != Some("rs".as_ref()) || entry.file_name() == "ice-3891.rs" {
+                None
+            } else {
+                Some(entry.into_path().into_os_string())
+            }
+        })
+        .chunks(250);
+
+    for chunk in &chunks {
+        rustfmt(context, chunk)?;
+    }
+    Ok(())
+}
+
+// the "main" function of cargo dev fmt
+pub fn run(check: bool, verbose: bool) {
     let output = Command::new("rustup")
         .args(["which", "rustfmt"])
         .stderr(Stdio::inherit())
@@ -120,21 +324,10 @@ Please revert the changes to Cargo.tomls with `cargo dev remove intellij`."
         verbose,
         rustfmt_path,
     };
-    let result = try_run(&context);
-    let code = match result {
-        Ok(true) => 0,
-        Ok(false) => {
-            eprintln!();
-            eprintln!("Formatting check failed.");
-            eprintln!("Run `cargo dev fmt` to update formatting.");
-            1
-        },
-        Err(err) => {
-            output_err(err);
-            1
-        },
-    };
-    process::exit(code);
+    if let Err(e) = run_rustfmt(&context).and_then(|()| fmt_conf(check)) {
+        e.display();
+        process::exit(1);
+    }
 }
 
 fn format_command(program: impl AsRef<OsStr>, dir: impl AsRef<Path>, args: &[impl AsRef<OsStr>]) -> String {
@@ -148,12 +341,12 @@ fn format_command(program: impl AsRef<OsStr>, dir: impl AsRef<Path>, args: &[imp
     )
 }
 
-fn exec(
+fn exec_fmt_command(
     context: &FmtContext,
     program: impl AsRef<OsStr>,
     dir: impl AsRef<Path>,
     args: &[impl AsRef<OsStr>],
-) -> Result<bool, CliError> {
+) -> Result<(), Error> {
     if context.verbose {
         println!("{}", format_command(&program, &dir, args));
     }
@@ -166,28 +359,28 @@ fn exec(
         .unwrap();
     let success = output.status.success();
 
-    if !context.check && !success {
-        let stderr = std::str::from_utf8(&output.stderr).unwrap_or("");
-        return Err(CliError::CommandFailed(
-            format_command(&program, &dir, args),
-            String::from(stderr),
-        ));
+    match (context.check, success) {
+        (_, true) => Ok(()),
+        (true, false) => Err(Error::CheckFailed),
+        (false, false) => {
+            let stderr = std::str::from_utf8(&output.stderr).unwrap_or("");
+            Err(Error::CommandFailed(
+                format_command(&program, &dir, args),
+                String::from(stderr),
+            ))
+        },
     }
-
-    Ok(success)
 }
 
-fn cargo_fmt(context: &FmtContext, path: &Path) -> Result<bool, CliError> {
+fn cargo_fmt(context: &FmtContext, path: &Path) -> Result<(), Error> {
     let mut args = vec!["fmt", "--all"];
     if context.check {
         args.push("--check");
     }
-    let success = exec(context, "cargo", path, &args)?;
-
-    Ok(success)
+    exec_fmt_command(context, "cargo", path, &args)
 }
 
-fn rustfmt_test(context: &FmtContext) -> Result<(), CliError> {
+fn check_for_rustfmt(context: &FmtContext) -> Result<(), Error> {
     let program = "rustfmt";
     let dir = std::env::current_dir()?;
     let args = &["--version"];
@@ -204,23 +397,20 @@ fn rustfmt_test(context: &FmtContext) -> Result<(), CliError> {
         .unwrap_or("")
         .starts_with("error: 'rustfmt' is not installed")
     {
-        Err(CliError::RustfmtNotInstalled)
+        Err(Error::RustfmtNotInstalled)
     } else {
-        Err(CliError::CommandFailed(
+        Err(Error::CommandFailed(
             format_command(program, &dir, args),
             std::str::from_utf8(&output.stderr).unwrap_or("").to_string(),
         ))
     }
 }
 
-fn rustfmt(context: &FmtContext, paths: impl Iterator<Item = OsString>) -> Result<bool, CliError> {
+fn rustfmt(context: &FmtContext, paths: impl Iterator<Item = OsString>) -> Result<(), Error> {
     let mut args = Vec::new();
     if context.check {
         args.push(OsString::from("--check"));
     }
     args.extend(paths);
-
-    let success = exec(context, &context.rustfmt_path, std::env::current_dir()?, &args)?;
-
-    Ok(success)
+    exec_fmt_command(context, &context.rustfmt_path, std::env::current_dir()?, &args)
 }