Rework Python bridge

Author: simoncozens

.gitmodules

@@ -0,0 +1,3 @@
+[submodule "fontbakery-bridge/fontbakery"]
+	path = fontbakery-bridge/fontbakery
+	url = https://github.com/googlefonts/fontbakery

.vscode/settings.json

@@ -0,0 +1,4 @@
+{
+  "rust-analyzer.cargo.features": "all",
+  "rust-analyzer.cargo.targetDir": true
+}

Cargo.toml

@@ -5,12 +5,14 @@ members = [
     "fontspector-cli",
     "fontspector-checkapi",
     "fontspector-checkhelper",
-    "profile-universal", "profile-testplugin", "profile-googlefonts",
+    "profile-universal",
+    "profile-testplugin",
+    "profile-googlefonts",
     "fontspector-web",
     "fontbakery-bridge",
 ]
 
-default-members = [ "fontspector-cli" ]
+default-members = ["fontspector-cli"]
 
 [workspace.dependencies]
 pluginator = "1.0.1"
@@ -22,5 +24,8 @@ write-fonts = "0.27.0"
 font-types = "0.5.5"
 
 # Serialization
-serde =  {version = "1.0.130", features=["derive"] }
+serde = { version = "1.0.130", features = ["derive"] }
+serde_json = "1.0"
 fontspector-checkhelper = { path = "./fontspector-checkhelper" }
+
+log = "0.4.14"

fontbakery-bridge/Cargo.toml

@@ -4,12 +4,17 @@ version = "0.1.0"
 edition = "2021"
 
 [lib]
-crate-type=["cdylib"]
+# When running as a plugin:
+# crate-type = ["cdylib"]
+# When running as a library:
+crate-type = ["lib"]
 
 [dependencies]
 fontspector-checkapi = { path = "../fontspector-checkapi" }
 pyo3 = "0.22"
+serde_json = { workspace = true }
+log = { workspace = true }
 
 [target.'cfg(not(target_family = "wasm"))'.dependencies]
 # Plugin architecture
-pluginator = {workspace = true}
+pluginator = { workspace = true }

fontbakery-bridge/fontbakery

@@ -1,12 +1,17 @@
 #![allow(non_upper_case_globals)]
 #![deny(clippy::unwrap_used, clippy::expect_used)]
+
 use fontspector_checkapi::{prelude::*, StatusCode};
-use pyo3::prelude::*;
-mod checks;
-struct FontbakeryBridge;
+use pyo3::{prelude::*, types::PyList};
+use serde_json::json;
+pub struct FontbakeryBridge;
 
 // We isolate the Python part to avoid type/result madness.
-fn call_python(module: &str, function: &str, testable: &Testable) -> PyResult<CheckFnResult> {
+fn python_checkrunner_impl(
+    module: &str,
+    function: &str,
+    testable: &Testable,
+) -> PyResult<CheckFnResult> {
     let filename = testable.filename.to_string_lossy();
     Python::with_gil(|py| {
         let module = PyModule::import_bound(py, module)?;
@@ -47,12 +52,22 @@ fn call_python(module: &str, function: &str, testable: &Testable) -> PyResult<Ch
                     "Fontbakery returned unknown status code".to_string(),
                 )
             })?;
-            let code = value.get_item(1)?.getattr("code")?.extract::<String>()?;
-            let message = value.get_item(1)?.getattr("message")?.extract::<String>()?;
+            let code = if value.get_item(1)?.hasattr("code")? {
+                Some(value.get_item(1)?.getattr("code")?.extract::<String>()?)
+            } else {
+                None
+            };
+            let message = if value.get_item(1)?.hasattr("message")? {
+                value.get_item(1)?.getattr("message")?
+            } else {
+                value.get_item(1)?
+            }
+            .extract::<String>()?;
+
             messages.push(Status {
                 message: Some(message),
                 severity: status,
-                code: Some(code),
+                code,
             });
         }
         Ok(return_result(messages))
@@ -61,7 +76,7 @@ fn call_python(module: &str, function: &str, testable: &Testable) -> PyResult<Ch
 
 // This wrapper will work for any fontbakery check that takes a single
 // Font or ttFont object as an argument.
-fn run_a_python_test(c: &Testable, context: &Context) -> CheckFnResult {
+fn python_checkrunner(c: &Testable, context: &Context) -> CheckFnResult {
     let module = context
         .check_metadata
         .get("module")
@@ -74,27 +89,149 @@ fn run_a_python_test(c: &Testable, context: &Context) -> CheckFnResult {
         .ok_or_else(|| CheckError::Error("No function specified".to_string()))?
         .as_str()
         .ok_or_else(|| CheckError::Error("function in metadata was not a string!".to_string()))?;
-    call_python(module, function, c)
+    python_checkrunner_impl(module, function, c)
         .unwrap_or_else(|e| Err(CheckError::Error(format!("Python error: {}", e))))
 }
 
+fn register_python_checks(modulename: &str, source: &str, cr: &mut Registry) -> Result<(), String> {
+    Python::with_gil(|py| {
+        // Assert that we have loaded the FB prelude
+        let _prelude = PyModule::import_bound(py, "fontbakery.prelude")?;
+        let callable = PyModule::import_bound(py, "fontbakery.callable")?;
+        let full_source = "from fontbakery.prelude import *\n\n".to_string() + source;
+        let module =
+            PyModule::from_code_bound(py, &full_source, &format!("{}.py", modulename), modulename)?;
+        // let check = module.getattr("check_hinting_impact")?;
+        // Find all functions in the module which are checks
+        let checktype = callable.getattr("FontBakeryCheck")?;
+        for name in module.dir()?.iter() {
+            let name_str: String = name.extract()?;
+            let obj = module.getattr(name.downcast()?)?;
+            if let Ok(true) = obj.is_instance(&checktype) {
+                let id: String = obj.getattr("id")?.extract()?;
+                // Check the mandatory arguments
+                let args = obj.getattr("mandatoryArgs")?.extract::<Vec<String>>()?;
+                if args.len() != 1 || !(args[0] == "font" || args[0] == "ttFont") {
+                    log::warn!(
+                        "Can't load check {}; unable to support arguments: {}",
+                        id,
+                        args.join(", ")
+                    );
+                    continue;
+                }
+                let title: String = obj.getattr("__doc__")?.extract()?;
+                let py_rationale = obj.getattr("rationale")?;
+                let rationale: String = if py_rationale.is_instance_of::<PyList>() {
+                    let r: Vec<String> = py_rationale.extract()?;
+                    r.join(", ")
+                } else {
+                    py_rationale.extract()?
+                };
+                let py_proposal = obj.getattr("proposal")?;
+                let proposal: String = if py_proposal.is_instance_of::<PyList>() {
+                    let r: Vec<String> = py_proposal.extract()?;
+                    r.join(", ")
+                } else {
+                    py_proposal.extract()?
+                };
+                log::info!("Registered check: {}", id);
+                let metadata = json!({
+                    "module": modulename,
+                    "function": name_str,
+                });
+                cr.register_check(Check {
+                    id: id.leak(),
+                    title: title.leak(),
+                    rationale: rationale.leak(),
+                    proposal: proposal.leak(),
+                    hotfix: None,
+                    fix_source: None,
+                    applies_to: "TTF",
+                    flags: CheckFlags::default(),
+                    implementation: CheckImplementation::CheckOne(&python_checkrunner),
+                    _metadata: Some(metadata.to_string().leak()),
+                })
+            }
+        }
+        Ok(())
+    })
+    .map_err(|e: PyErr| format!("Error loading checks: {}", e))
+}
+
 impl fontspector_checkapi::Plugin for FontbakeryBridge {
     fn register(&self, cr: &mut Registry) -> Result<(), String> {
-        cr.register_check(checks::hinting_impact);
-        cr.register_check(checks::opentype_name_empty_records);
-        cr.register_check(checks::monospace);
         pyo3::prepare_freethreaded_python();
+        // Load needed FB modules
+        let ok: PyResult<()> = Python::with_gil(|py| {
+            PyModule::from_code_bound(
+                py,
+                include_str!("../fontbakery/Lib/fontbakery/callable.py"),
+                "callable.py",
+                "fontbakery.callable",
+            )?;
+            PyModule::from_code_bound(
+                py,
+                include_str!("../fontbakery/Lib/fontbakery/status.py"),
+                "status.py",
+                "fontbakery.status",
+            )?;
+            PyModule::from_code_bound(
+                py,
+                include_str!("../fontbakery/Lib/fontbakery/message.py"),
+                "message.py",
+                "fontbakery.message",
+            )?;
+            Ok(())
+        });
+        ok.map_err(|e| format!("Error loading FB modules: {}", e))?;
+
+        register_python_checks(
+            "fontbakery.checks.opentype.kern",
+            include_str!("../fontbakery/Lib/fontbakery/checks/opentype/kern.py"),
+            cr,
+        )?;
+        register_python_checks(
+            "fontbakery.checks.opentype.cff",
+            include_str!("../fontbakery/Lib/fontbakery/checks/opentype/cff.py"),
+            cr,
+        )?;
+        register_python_checks(
+            "fontbakery.checks.opentype.fvar",
+            include_str!("../fontbakery/Lib/fontbakery/checks/opentype/fvar.py"),
+            cr,
+        )?;
+        register_python_checks(
+            "fontbakery.checks.opentype.gdef",
+            include_str!("../fontbakery/Lib/fontbakery/checks/opentype/gdef.py"),
+            cr,
+        )?;
+        register_python_checks(
+            "fontbakery.checks.opentype.gpos",
+            include_str!("../fontbakery/Lib/fontbakery/checks/opentype/gpos.py"),
+            cr,
+        )?;
+        register_python_checks(
+            "fontbakery.checks.opentype.head",
+            include_str!("../fontbakery/Lib/fontbakery/checks/opentype/head.py"),
+            cr,
+        )?;
+        register_python_checks(
+            "fontbakery.checks.opentype.hhea",
+            include_str!("../fontbakery/Lib/fontbakery/checks/opentype/hhea.py"),
+            cr,
+        )?;
         cr.register_profile(
             "fontbakery",
             Profile::from_toml(
                 r#"
-[sections]
-"Test profile" = [
-    "hinting_impact",
-    "opentype/name/empty_records",
-    "opentype/monospace",
-]
-"#,
+        [sections]
+        "Test profile" = [
+            "hinting_impact",
+            "opentype/name/empty_records",
+            "opentype/monospace",
+            "opentype/cff_call_depth",
+        ]
+        "#,
             )
             .map_err(|_| "Couldn't parse profile")?,
         )

fontbakery-bridge/src/checks.rs

@@ -8,21 +8,22 @@ crate-type = ["cdylib", "rlib"]
 
 [target.'cfg(not(target_family = "wasm"))'.dependencies]
 # Plugin architecture
-pluginator = {workspace = true}
+pluginator = { workspace = true }
 
 [dependencies]
 font-types = { workspace = true }
-read-fonts = {workspace = true}
+read-fonts = { workspace = true }
 write-fonts = { workspace = true }
-skrifa = {workspace = true}
-fontspector-checkhelper = {workspace = true}
+skrifa = { workspace = true }
+fontspector-checkhelper = { workspace = true }
+log = { workspace = true }
 
 # Filetype
 glob-match = "0.2.1"
-glob="0.3.1"
+glob = "0.3.1"
 
 # Needed so that we can refer to status codes on the command line
-clap = {version = "3.2.5", features = ["derive"]}
+clap = { version = "3.2.5", features = ["derive"] }
 
 # Serializing and deserializing profiles
 toml = "0.8.14"