Auto merge of rust-lang#15071 - matklad:no-regex, r=matklad

internal: remove spurious regex dependency

- replace tokio's env-filter with a smaller&simpler targets filter
- reshuffle logging infra a bit to make sure there's only a single place where we read environmental variables
- use anyhow::Result in rust-analyzer binary

Author: bors

Cargo.lock

@@ -47,7 +47,6 @@ limit.workspace = true
 expect-test = "1.4.0"
 tracing = "0.1.35"
 tracing-subscriber = { version = "0.3.16", default-features = false, features = [
-    "env-filter",
     "registry",
 ] }
 tracing-tree = "0.2.1"

crates/hir-ty/Cargo.toml

@@ -30,7 +30,7 @@ use syntax::{
     ast::{self, AstNode, HasName},
     SyntaxNode,
 };
-use tracing_subscriber::{layer::SubscriberExt, EnvFilter, Registry};
+use tracing_subscriber::{layer::SubscriberExt, Registry};
 use tracing_tree::HierarchicalLayer;
 use triomphe::Arc;
 
@@ -52,7 +52,8 @@ fn setup_tracing() -> Option<tracing::subscriber::DefaultGuard> {
         return None;
     }
 
-    let filter = EnvFilter::from_env("CHALK_DEBUG");
+    let filter: tracing_subscriber::filter::Targets =
+        env::var("CHALK_DEBUG").ok().and_then(|it| it.parse().ok()).unwrap_or_default();
     let layer = HierarchicalLayer::default()
         .with_indent_lines(true)
         .with_ansi(false)

crates/hir-ty/src/tests.rs

@@ -37,7 +37,6 @@ mimalloc = { version = "0.1.30", default-features = false, optional = true }
 lsp-server = { version = "0.7.0", path = "../../lib/lsp-server" }
 tracing = "0.1.35"
 tracing-subscriber = { version = "0.3.16", default-features = false, features = [
-    "env-filter",
     "registry",
     "fmt",
     "tracing-log",

crates/rust-analyzer/Cargo.toml

@@ -8,24 +8,26 @@ use std::{
     sync::Arc,
 };
 
-use rust_analyzer::Result;
+use anyhow::Context;
 use tracing::{level_filters::LevelFilter, Event, Subscriber};
 use tracing_log::NormalizeEvent;
 use tracing_subscriber::{
+    filter::Targets,
     fmt::{
         format::Writer, writer::BoxMakeWriter, FmtContext, FormatEvent, FormatFields,
         FormattedFields, MakeWriter,
     },
     layer::SubscriberExt,
     registry::LookupSpan,
     util::SubscriberInitExt,
-    EnvFilter, Registry,
+    Registry,
 };
 use tracing_tree::HierarchicalLayer;
 
-pub(crate) struct Logger {
-    filter: EnvFilter,
-    file: Option<File>,
+pub(crate) struct LoggerConfig {
+    pub(crate) log_file: Option<File>,
+    pub(crate) filter: String,
+    pub(crate) chalk_filter: Option<String>,
 }
 
 struct MakeWriterStderr;
@@ -38,62 +40,42 @@ impl<'a> MakeWriter<'a> for MakeWriterStderr {
     }
 }
 
-impl Logger {
-    pub(crate) fn new(file: Option<File>, filter: Option<&str>) -> Logger {
-        let filter = filter.map_or(EnvFilter::default(), EnvFilter::new);
-
-        Logger { filter, file }
-    }
+impl LoggerConfig {
+    pub(crate) fn init(self) -> anyhow::Result<()> {
+        let mut filter: Targets = self
+            .filter
+            .parse()
+            .with_context(|| format!("invalid log filter: `{}`", self.filter))?;
+
+        let mut chalk_layer = None;
+        if let Some(chalk_filter) = self.chalk_filter {
+            let level: LevelFilter =
+                chalk_filter.parse().with_context(|| "invalid chalk log filter")?;
+            chalk_layer = Some(
+                HierarchicalLayer::default()
+                    .with_indent_lines(true)
+                    .with_ansi(false)
+                    .with_indent_amount(2)
+                    .with_writer(io::stderr),
+            );
+            filter = filter
+                .with_target("chalk_solve", level)
+                .with_target("chalk_ir", level)
+                .with_target("chalk_recursive", level);
+        };
 
-    pub(crate) fn install(self) -> Result<()> {
-        // The meaning of CHALK_DEBUG I suspected is to tell chalk crates
-        // (i.e. chalk-solve, chalk-ir, chalk-recursive) how to filter tracing
-        // logs. But now we can only have just one filter, which means we have to
-        // merge chalk filter to our main filter (from RA_LOG env).
-        //
-        // The acceptable syntax of CHALK_DEBUG is `target[span{field=value}]=level`.
-        // As the value should only affect chalk crates, we'd better manually
-        // specify the target. And for simplicity, CHALK_DEBUG only accept the value
-        // that specify level.
-        let chalk_level_dir = std::env::var("CHALK_DEBUG")
-            .map(|val| {
-                val.parse::<LevelFilter>().expect(
-                    "invalid CHALK_DEBUG value, expect right log level (like debug or trace)",
-                )
-            })
-            .ok();
-
-        let chalk_layer = HierarchicalLayer::default()
-            .with_indent_lines(true)
-            .with_ansi(false)
-            .with_indent_amount(2)
-            .with_writer(io::stderr);
-
-        let writer = match self.file {
+        let writer = match self.log_file {
             Some(file) => BoxMakeWriter::new(Arc::new(file)),
             None => BoxMakeWriter::new(io::stderr),
         };
         let ra_fmt_layer =
             tracing_subscriber::fmt::layer().event_format(LoggerFormatter).with_writer(writer);
 
-        match chalk_level_dir {
-            Some(val) => {
-                Registry::default()
-                    .with(
-                        self.filter
-                            .add_directive(format!("chalk_solve={val}").parse()?)
-                            .add_directive(format!("chalk_ir={val}").parse()?)
-                            .add_directive(format!("chalk_recursive={val}").parse()?),
-                    )
-                    .with(ra_fmt_layer)
-                    .with(chalk_layer)
-                    .init();
-            }
-            None => {
-                Registry::default().with(self.filter).with(ra_fmt_layer).init();
-            }
-        };
-
+        let registry = Registry::default().with(filter).with(ra_fmt_layer);
+        match chalk_layer {
+            Some(chalk_layer) => registry.with(chalk_layer).init(),
+            None => registry.init(),
+        }
         Ok(())
     }
 }

crates/rust-analyzer/src/bin/logger.rs

@@ -7,14 +7,11 @@
 mod logger;
 mod rustc_wrapper;
 
-use std::{
-    env, fs,
-    path::{Path, PathBuf},
-    process,
-};
+use std::{env, fs, path::PathBuf, process};
 
+use anyhow::Context;
 use lsp_server::Connection;
-use rust_analyzer::{cli::flags, config::Config, from_json, Result};
+use rust_analyzer::{cli::flags, config::Config, from_json};
 use vfs::AbsPathBuf;
 
 #[cfg(all(feature = "mimalloc"))]
@@ -25,7 +22,7 @@ static ALLOC: mimalloc::MiMalloc = mimalloc::MiMalloc;
 #[global_allocator]
 static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc;
 
-fn main() {
+fn main() -> anyhow::Result<()> {
     if std::env::var("RA_RUSTC_WRAPPER").is_ok() {
         let mut args = std::env::args_os();
         let _me = args.next().unwrap();
@@ -41,14 +38,7 @@ fn main() {
     }
 
     let flags = flags::RustAnalyzer::from_env_or_exit();
-    if let Err(err) = try_main(flags) {
-        tracing::error!("Unexpected error: {}", err);
-        eprintln!("{err}");
-        process::exit(101);
-    }
-}
 
-fn try_main(flags: flags::RustAnalyzer) -> Result<()> {
     #[cfg(debug_assertions)]
     if flags.wait_dbg || env::var("RA_WAIT_DBG").is_ok() {
         #[allow(unused_mut)]
@@ -58,14 +48,8 @@ fn try_main(flags: flags::RustAnalyzer) -> Result<()> {
         }
     }
 
-    let mut log_file = flags.log_file.as_deref();
-
-    let env_log_file = env::var("RA_LOG_FILE").ok();
-    if let Some(env_log_file) = env_log_file.as_deref() {
-        log_file = Some(Path::new(env_log_file));
-    }
+    setup_logging(flags.log_file.clone())?;
 
-    setup_logging(log_file)?;
     let verbosity = flags.verbosity();
 
     match flags.subcommand {
@@ -102,7 +86,7 @@ fn try_main(flags: flags::RustAnalyzer) -> Result<()> {
     Ok(())
 }
 
-fn setup_logging(log_file: Option<&Path>) -> Result<()> {
+fn setup_logging(log_file_flag: Option<PathBuf>) -> anyhow::Result<()> {
     if cfg!(windows) {
         // This is required so that windows finds our pdb that is placed right beside the exe.
         // By default it doesn't look at the folder the exe resides in, only in the current working
@@ -115,23 +99,42 @@ fn setup_logging(log_file: Option<&Path>) -> Result<()> {
             }
         }
     }
+
     if env::var("RUST_BACKTRACE").is_err() {
         env::set_var("RUST_BACKTRACE", "short");
     }
 
+    let log_file = env::var("RA_LOG_FILE").ok().map(PathBuf::from).or(log_file_flag);
     let log_file = match log_file {
         Some(path) => {
             if let Some(parent) = path.parent() {
                 let _ = fs::create_dir_all(parent);
             }
-            Some(fs::File::create(path)?)
+            Some(
+                fs::File::create(&path)
+                    .with_context(|| format!("can't create log file at {}", path.display()))?,
+            )
         }
         None => None,
     };
-    let filter = env::var("RA_LOG").ok();
-    // deliberately enable all `error` logs if the user has not set RA_LOG, as there is usually useful
-    // information in there for debugging
-    logger::Logger::new(log_file, filter.as_deref().or(Some("error"))).install()?;
+
+    logger::LoggerConfig {
+        log_file,
+        // Deliberately enable all `error` logs if the user has not set RA_LOG, as there is usually
+        // useful information in there for debugging.
+        filter: env::var("RA_LOG").ok().unwrap_or_else(|| "error".to_string()),
+        // The meaning of CHALK_DEBUG I suspected is to tell chalk crates
+        // (i.e. chalk-solve, chalk-ir, chalk-recursive) how to filter tracing
+        // logs. But now we can only have just one filter, which means we have to
+        // merge chalk filter to our main filter (from RA_LOG env).
+        //
+        // The acceptable syntax of CHALK_DEBUG is `target[span{field=value}]=level`.
+        // As the value should only affect chalk crates, we'd better manually
+        // specify the target. And for simplicity, CHALK_DEBUG only accept the value
+        // that specify level.
+        chalk_filter: env::var("CHALK_DEBUG").ok(),
+    }
+    .init()?;
 
     profile::init();
 
@@ -146,8 +149,8 @@ const STACK_SIZE: usize = 1024 * 1024 * 8;
 fn with_extra_thread(
     thread_name: impl Into<String>,
     thread_intent: stdx::thread::ThreadIntent,
-    f: impl FnOnce() -> Result<()> + Send + 'static,
-) -> Result<()> {
+    f: impl FnOnce() -> anyhow::Result<()> + Send + 'static,
+) -> anyhow::Result<()> {
     let handle = stdx::thread::Builder::new(thread_intent)
         .name(thread_name.into())
         .stack_size(STACK_SIZE)
@@ -158,7 +161,7 @@ fn with_extra_thread(
     Ok(())
 }
 
-fn run_server() -> Result<()> {
+fn run_server() -> anyhow::Result<()> {
     tracing::info!("server version {} will start", rust_analyzer::version());
 
     let (connection, io_threads) = Connection::stdio();