From decd3fcb430d2d40b4ba4ff1f438cbd1e8471479 Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Mon, 18 Jul 2022 09:44:42 +0200
Subject: [PATCH] feat(bindings/crypto-js) Simplify code for `feature =
 "tracing"`.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch creates one `inner` module for when `feature = "tracing"`,
and one for when `no(feature = "tracing")`. Then, let's expose
everything from `inner::*`.

This patch also replaces `Tracing.install` by `new Tracing`. In case
of `not(feature = "tracing")`, `new Tracing` raises an error.

The goal is to remove all the `#[cfg(…)]` annotations everywhere. Now
there is only 2 of them.
---
 bindings/matrix-sdk-crypto-js/src/tracing.rs  | 423 +++++++++---------
 .../tests/tracing.test.js                     |  10 +-
 2 files changed, 223 insertions(+), 210 deletions(-)

diff --git a/bindings/matrix-sdk-crypto-js/src/tracing.rs b/bindings/matrix-sdk-crypto-js/src/tracing.rs
index d7ed61d5b..cf1f98d63 100644
--- a/bindings/matrix-sdk-crypto-js/src/tracing.rs
+++ b/bindings/matrix-sdk-crypto-js/src/tracing.rs
@@ -1,108 +1,5 @@
-use std::{fmt, sync::Arc};
-#[cfg(feature = "tracing")]
-use std::{fmt::Write as _, sync::Once};
-
-#[cfg(feature = "tracing")]
-use tracing::{
-    field::{Field, Visit},
-    metadata::LevelFilter,
-    Event, Level, Metadata, Subscriber,
-};
-#[cfg(feature = "tracing")]
-use tracing_subscriber::{
-    layer::{Context, Layer as TracingLayer},
-    prelude::*,
-    reload, Registry,
-};
 use wasm_bindgen::prelude::*;
 
-type TracingInner = Arc<reload::Handle<Layer, Registry>>;
-
-/// Type to install and to manipulate the tracing layer.
-#[wasm_bindgen]
-pub struct Tracing {
-    #[cfg(feature = "tracing")]
-    handle: TracingInner,
-}
-
-impl fmt::Debug for Tracing {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_struct("Tracing").finish_non_exhaustive()
-    }
-}
-
-#[wasm_bindgen]
-impl Tracing {
-    /// Check whether the `tracing` feature has been enabled.
-    #[wasm_bindgen(js_name = "isAvailable")]
-    pub fn is_available() -> bool {
-        cfg!(feature = "tracing")
-    }
-
-    /// Install the tracing layer.
-    ///
-    /// `Tracing` is a singleton. Once it is installed, consecutive
-    /// calls to `install` will construct a new `Tracing` object but
-    /// with the exact same inner state. Calling `install` with a new
-    /// `min_level` will just update the `min_level` parameter; in
-    /// that regard, it is similar to calling the `min_level` method
-    /// on an existing `Tracing` object.
-    pub fn install(_min_level: LoggerLevel) -> Option<Tracing> {
-        #[cfg(not(feature = "tracing"))]
-        {
-            None
-        }
-
-        #[cfg(feature = "tracing")]
-        {
-            static mut INSTALL: Option<TracingInner> = None;
-            static INSTALLED: Once = Once::new();
-
-            INSTALLED.call_once(|| {
-                let (filter, reload_handle) = reload::Layer::new(Layer::new(_min_level.clone()));
-
-                tracing_subscriber::registry().with(filter).init();
-
-                unsafe { INSTALL = Some(Arc::new(reload_handle)) };
-            });
-
-            let tracing = Tracing {
-                handle: unsafe { INSTALL.as_ref() }
-                    .cloned()
-                    .expect("`Tracing` has not been installed correctly"),
-            };
-
-            // If it's not the first call to `install`, the
-            // `min_level` can be different. Let's update it.
-            tracing.min_level(_min_level);
-
-            Some(tracing)
-        }
-    }
-
-    /// Re-define the minimum logger level.
-    #[cfg(feature = "tracing")]
-    #[wasm_bindgen(setter, js_name = "minLevel")]
-    pub fn min_level(&self, min_level: LoggerLevel) {
-        let _ = self.handle.modify(|layer| layer.min_level = min_level.into());
-    }
-
-    /// Turn the logger on, i.e. it emits logs again if it was turned
-    /// off.
-    #[cfg(feature = "tracing")]
-    #[wasm_bindgen(js_name = "turnOn")]
-    pub fn turn_on(&self) {
-        let _ = self.handle.modify(|layer| layer.turn_on());
-    }
-
-    /// Turn the logger off, i.e. it no long emits logs.
-    #[cfg(feature = "tracing")]
-    #[wasm_bindgen(js_name = "turnOff")]
-    pub fn turn_off(&self) {
-        let _ = self.handle.modify(|layer| layer.turn_off());
-    }
-}
-
 /// Logger level.
 #[wasm_bindgen]
 #[derive(Debug, Clone)]
@@ -135,141 +32,257 @@ pub enum LoggerLevel {
 }
 
 #[cfg(feature = "tracing")]
-impl From<LoggerLevel> for Level {
-    fn from(value: LoggerLevel) -> Self {
-        use LoggerLevel::*;
+mod inner {
+    use std::{
+        fmt,
+        fmt::Write as _,
+        sync::{Arc, Once},
+    };
 
-        match value {
-            Trace => Self::TRACE,
-            Debug => Self::DEBUG,
-            Info => Self::INFO,
-            Warn => Self::WARN,
-            Error => Self::ERROR,
+    use super::*;
+    use tracing::{
+        field::{Field, Visit},
+        metadata::LevelFilter,
+        Event, Level, Metadata, Subscriber,
+    };
+    use tracing_subscriber::{
+        layer::{Context, Layer as TracingLayer},
+        prelude::*,
+        reload, Registry,
+    };
+
+    type TracingInner = Arc<reload::Handle<Layer, Registry>>;
+
+    /// Type to install and to manipulate the tracing layer.
+    #[wasm_bindgen]
+    pub struct Tracing {
+        handle: TracingInner,
+    }
+
+    impl fmt::Debug for Tracing {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            f.debug_struct("Tracing").finish_non_exhaustive()
         }
     }
-}
 
-#[cfg(feature = "tracing")]
-#[wasm_bindgen]
-extern "C" {
-    #[wasm_bindgen(js_namespace = console, js_name = "trace")]
-    fn log_trace(message: String);
+    #[wasm_bindgen]
+    impl Tracing {
+        /// Check whether the `tracing` feature has been enabled.
+        #[wasm_bindgen(js_name = "isAvailable")]
+        pub fn is_available() -> bool {
+            true
+        }
 
-    #[wasm_bindgen(js_namespace = console, js_name = "debug")]
-    fn log_debug(message: String);
+        /// Install the tracing layer.
+        ///
+        /// `Tracing` is a singleton. Once it is installed, consecutive
+        /// calls to `install` will construct a new `Tracing` object but
+        /// with the exact same inner state. Calling `install` with a new
+        /// `min_level` will just update the `min_level` parameter; in
+        /// that regard, it is similar to calling the `min_level` method
+        /// on an existing `Tracing` object.
+        #[wasm_bindgen(constructor)]
+        pub fn new(min_level: LoggerLevel) -> Result<Tracing, JsError> {
+            static mut INSTALL: Option<TracingInner> = None;
+            static INSTALLED: Once = Once::new();
 
-    #[wasm_bindgen(js_namespace = console, js_name = "info")]
-    fn log_info(message: String);
+            INSTALLED.call_once(|| {
+                let (filter, reload_handle) = reload::Layer::new(Layer::new(min_level.clone()));
 
-    #[wasm_bindgen(js_namespace = console, js_name = "warn")]
-    fn log_warn(message: String);
+                tracing_subscriber::registry().with(filter).init();
 
-    #[wasm_bindgen(js_namespace = console, js_name = "log")]
-    fn log(message: String);
-}
+                unsafe { INSTALL = Some(Arc::new(reload_handle)) };
+            });
 
-#[cfg(feature = "tracing")]
-struct Layer {
-    min_level: Level,
-    enabled: bool,
-}
+            let tracing = Tracing {
+                handle: unsafe { INSTALL.as_ref() }
+                    .cloned()
+                    .expect("`Tracing` has not been installed correctly"),
+            };
 
-#[cfg(feature = "tracing")]
-impl Layer {
-    fn new<L>(min_level: L) -> Self
+            // If it's not the first call to `install`, the
+            // `min_level` can be different. Let's update it.
+            tracing.min_level(min_level);
+
+            Ok(tracing)
+        }
+
+        /// Re-define the minimum logger level.
+        #[wasm_bindgen(setter, js_name = "minLevel")]
+        pub fn min_level(&self, min_level: LoggerLevel) {
+            let _ = self.handle.modify(|layer| layer.min_level = min_level.into());
+        }
+
+        /// Turn the logger on, i.e. it emits logs again if it was turned
+        /// off.
+        #[wasm_bindgen(js_name = "turnOn")]
+        pub fn turn_on(&self) {
+            let _ = self.handle.modify(|layer| layer.turn_on());
+        }
+
+        /// Turn the logger off, i.e. it no long emits logs.
+        #[wasm_bindgen(js_name = "turnOff")]
+        pub fn turn_off(&self) {
+            let _ = self.handle.modify(|layer| layer.turn_off());
+        }
+    }
+
+    impl From<LoggerLevel> for Level {
+        fn from(value: LoggerLevel) -> Self {
+            use LoggerLevel::*;
+
+            match value {
+                Trace => Self::TRACE,
+                Debug => Self::DEBUG,
+                Info => Self::INFO,
+                Warn => Self::WARN,
+                Error => Self::ERROR,
+            }
+        }
+    }
+
+    #[wasm_bindgen]
+    extern "C" {
+        #[wasm_bindgen(js_namespace = console, js_name = "trace")]
+        fn log_trace(message: String);
+
+        #[wasm_bindgen(js_namespace = console, js_name = "debug")]
+        fn log_debug(message: String);
+
+        #[wasm_bindgen(js_namespace = console, js_name = "info")]
+        fn log_info(message: String);
+
+        #[wasm_bindgen(js_namespace = console, js_name = "warn")]
+        fn log_warn(message: String);
+
+        #[wasm_bindgen(js_namespace = console, js_name = "log")]
+        fn log(message: String);
+    }
+
+    struct Layer {
+        min_level: Level,
+        enabled: bool,
+    }
+
+    impl Layer {
+        fn new<L>(min_level: L) -> Self
+        where
+            L: Into<Level>,
+        {
+            Self { min_level: min_level.into(), enabled: true }
+        }
+
+        fn turn_on(&mut self) {
+            self.enabled = true;
+        }
+
+        fn turn_off(&mut self) {
+            self.enabled = false;
+        }
+    }
+
+    impl<S> TracingLayer<S> for Layer
     where
-        L: Into<Level>,
+        S: Subscriber,
     {
-        Self { min_level: min_level.into(), enabled: true }
-    }
+        fn enabled(&self, metadata: &Metadata<'_>, _: Context<'_, S>) -> bool {
+            self.enabled && metadata.level() <= &self.min_level
+        }
 
-    fn turn_on(&mut self) {
-        self.enabled = true;
-    }
+        fn max_level_hint(&self) -> Option<LevelFilter> {
+            if !self.enabled {
+                Some(LevelFilter::OFF)
+            } else {
+                Some(LevelFilter::from_level(self.min_level))
+            }
+        }
 
-    fn turn_off(&mut self) {
-        self.enabled = false;
-    }
-}
+        fn on_event(&self, event: &Event<'_>, _: Context<'_, S>) {
+            let mut recorder = StringVisitor::new();
+            event.record(&mut recorder);
+            let metadata = event.metadata();
+            let level = metadata.level();
 
-#[cfg(feature = "tracing")]
-impl<S> TracingLayer<S> for Layer
-where
-    S: Subscriber,
-{
-    fn enabled(&self, metadata: &Metadata<'_>, _: Context<'_, S>) -> bool {
-        self.enabled && metadata.level() <= &self.min_level
-    }
+            let origin = metadata
+                .file()
+                .and_then(|file| metadata.line().map(|ln| format!("{}:{}", file, ln)))
+                .unwrap_or_default();
 
-    fn max_level_hint(&self) -> Option<LevelFilter> {
-        if !self.enabled {
-            Some(LevelFilter::OFF)
-        } else {
-            Some(LevelFilter::from_level(self.min_level))
+            let message = format!("{level} {origin}{recorder}");
+
+            match *level {
+                Level::TRACE => log_trace(message),
+                Level::DEBUG => log_debug(message),
+                Level::INFO => log_info(message),
+                Level::WARN => log_warn(message),
+                Level::ERROR => log(message),
+            }
         }
     }
 
-    fn on_event(&self, event: &Event<'_>, _: Context<'_, S>) {
-        let mut recorder = StringVisitor::new();
-        event.record(&mut recorder);
-        let metadata = event.metadata();
-        let level = metadata.level();
+    struct StringVisitor {
+        string: String,
+    }
 
-        let origin = metadata
-            .file()
-            .and_then(|file| metadata.line().map(|ln| format!("{}:{}", file, ln)))
-            .unwrap_or_default();
-
-        let message = format!("{level} {origin}{recorder}");
-
-        match *level {
-            Level::TRACE => log_trace(message),
-            Level::DEBUG => log_debug(message),
-            Level::INFO => log_info(message),
-            Level::WARN => log_warn(message),
-            Level::ERROR => log(message),
+    impl StringVisitor {
+        fn new() -> Self {
+            Self { string: String::new() }
         }
     }
-}
 
-#[cfg(feature = "tracing")]
-struct StringVisitor {
-    string: String,
-}
+    impl Visit for StringVisitor {
+        fn record_debug(&mut self, field: &Field, value: &dyn fmt::Debug) {
+            match field.name() {
+                "message" => {
+                    if !self.string.is_empty() {
+                        self.string.push('\n');
+                    }
 
-#[cfg(feature = "tracing")]
-impl StringVisitor {
-    fn new() -> Self {
-        Self { string: String::new() }
-    }
-}
-
-#[cfg(feature = "tracing")]
-impl Visit for StringVisitor {
-    fn record_debug(&mut self, field: &Field, value: &dyn fmt::Debug) {
-        match field.name() {
-            "message" => {
-                if !self.string.is_empty() {
-                    self.string.push('\n');
+                    let _ = write!(self.string, "{:?}", value);
                 }
 
-                let _ = write!(self.string, "{:?}", value);
+                field_name => {
+                    let _ = write!(self.string, "\n{} = {:?}", field_name, value);
+                }
             }
+        }
+    }
 
-            field_name => {
-                let _ = write!(self.string, "\n{} = {:?}", field_name, value);
+    impl fmt::Display for StringVisitor {
+        fn fmt(&self, mut f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            if !self.string.is_empty() {
+                write!(&mut f, " {}", self.string)
+            } else {
+                Ok(())
             }
         }
     }
 }
 
-#[cfg(feature = "tracing")]
-impl fmt::Display for StringVisitor {
-    fn fmt(&self, mut f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        if !self.string.is_empty() {
-            write!(&mut f, " {}", self.string)
-        } else {
-            Ok(())
+#[cfg(not(feature = "tracing"))]
+mod inner {
+    use super::*;
+
+    /// Type to install and to manipulate the tracing layer.
+    #[wasm_bindgen]
+    #[derive(Debug)]
+    pub struct Tracing;
+
+    #[wasm_bindgen]
+    impl Tracing {
+        /// Check whether the `tracing` feature has been enabled.
+        #[wasm_bindgen(js_name = "isAvailable")]
+        pub fn is_available() -> bool {
+            false
+        }
+
+        /// The `tracing` feature is not enabled, so this constructor
+        /// will raise an error.
+        #[wasm_bindgen(constructor)]
+        pub fn new(_min_level: LoggerLevel) -> Result<Tracing, JsError> {
+            Err(JsError::new("The `tracing` feature is disabled. Check `Tracing.isAvailable` before constructing `Tracing`"))
         }
     }
 }
+
+pub use inner::*;
diff --git a/bindings/matrix-sdk-crypto-js/tests/tracing.test.js b/bindings/matrix-sdk-crypto-js/tests/tracing.test.js
index 0ce28cc6f..bf27431ee 100644
--- a/bindings/matrix-sdk-crypto-js/tests/tracing.test.js
+++ b/bindings/matrix-sdk-crypto-js/tests/tracing.test.js
@@ -12,12 +12,12 @@ describe('LoggerLevel', () => {
 
 describe(Tracing.name, () => {
     if (Tracing.isAvailable()) {
-        let tracing = Tracing.install(LoggerLevel.Debug);
+        let tracing = new Tracing(LoggerLevel.Debug);
 
         test('can installed several times', () => {
-            Tracing.install(LoggerLevel.Debug);
-            Tracing.install(LoggerLevel.Warn);
-            Tracing.install(LoggerLevel.Debug);
+            new Tracing(LoggerLevel.Debug);
+            new Tracing(LoggerLevel.Warn);
+            new Tracing(LoggerLevel.Debug);
         });
 
         const originalConsoleDebug = console.debug;
@@ -78,7 +78,7 @@ describe(Tracing.name, () => {
         }
     } else {
         test('cannot be constructed', () => {
-            expect(Tracing.install(LoggerLevel.Error)).toBeUndefined();
+            expect(() => { new Tracing(LoggerLevel.Error) }).toThrow();
         });
     }
 });