Merge pull request #7 from DaveMcEwan/api

Use updated API for svlint v0.8.0

.github/workflows/regression.yml

@@ -34,4 +34,5 @@ jobs:
             ~/.cargo/git/db/
             target/
           key: ${{ runner.os }}-cargo-${{ hashFiles('BUILDENV', '**/Cargo.lock') }}
+      - run: cargo build
       - run: cargo test

Cargo.toml

@@ -12,5 +12,7 @@ edition = "2018"
 crate-type = ["cdylib"]
 
 [dependencies]
-svlint    = "0.7.2"
+#svlint = { path = "../svlint" }
+svlint    = "0.8.0"
 sv-parser = "0.13.1"
+regex     = "1" # Only used for the rule "forbidden_regex".

README.md

@@ -1,69 +1,128 @@
 # svlint-plugin-sample
 
-This is a sample project of [svlint](https://github.com/dalance/svlint) plugin.
+This is a sample project of an [svlint](https://github.com/dalance/svlint)
+plugin.
+An svlint plugin implements one or more rules (either `TextRule` or
+`SyntaxRule`) in an externally developed project, compiling a shared object
+file which is dynamically loaded at svlint runtime.
 
-## Create plugin
 
-svlint plugin is a shared library. So crate-type of `Cargo.toml` must be `cdylib`.
-`dylib` can be used also, but it causes too large binary size.
+## Usage
+
+Use svlint's `--plugin` option with the shared object produced by `cargo build`
+in this repository.
+The shared object can be copied from `target/(debug|release)/`, but the
+filename will be platform-dependent.
+
+- Linux: `lib<name>.so`
+- MacOS: `lib<name>.dylib`
+- Windows: `<name>.dll`
 
 ```
+$ svlint --plugin libsvlint_plugin_sample.so test.sv
+Fail: sample_plugin
+   --> test.sv:2:1
+  |
+2 | initial begin
+  | ^^^^^^^ hint  : Remove the `initial` process.
+  |         reason: This example doesn't like `initial` processes.
+```
+
+The loaded plugin is automatically enabled, has access to values from svlint's
+TOML configuration, and syntax rules may be controlled using
+[special comments](https://github.com/dalance/svlint/blob/master/MANUAL.md#textrules-and-syntaxrules-sections).
+
+
+## Implementation
+
+As a plugin must create a shared object, the crate type of `Cargo.toml` should
+be `cdylib`.
+Alternatively, `dylib` could be used, but the resulting binary may be very
+large.
+
+```toml
 [lib]
 crate-type = ["cdylib"]
 ```
 
-All plugin must have `get_plugin` function to generate `Rule`.
+A plugin project must define a
+[`get_plugin`](https://github.com/dalance/svlint-plugin-sample/blob/master/src/lib.rs#L13-L21)
+function (in `src/lib.rs`) which returns the list of rules that it implements.
+Svlint provides a macro [`pluginrules`](https://github.com/dalance/svlint/blob/master/src/linter.rs#L15-L33)
+which makes this quite simple.
 
-```
+```rust
 #[allow(improper_ctypes_definitions)]
 #[no_mangle]
-pub extern "C" fn get_plugin() -> *mut dyn Rule {
-    let boxed = Box::new(SamplePlugin {});
-    Box::into_raw(boxed)
+pub extern "C" fn get_plugin() -> Vec<Rule> {
+    pluginrules!(
+        SamplePlugin,
+        AnotherPlugin,
+        ForbiddenRegex
+    )
 }
 ```
 
-The lint rule is defined as `Rule` trait.
-
-```
-pub struct SamplePlugin;
-
-impl Rule for SamplePlugin {
-    fn check(&self, _syntax_tree: &SyntaxTree, node: &RefNode) -> RuleResult {
-        match node {
-            RefNode::InitialConstruct(_) => RuleResult::Fail,
-            _ => RuleResult::Pass,
+Rules are defined by the `TextRule` or `SyntaxRule` traits, see
+[`src/forbidden_regex.rs`](https://github.com/dalance/svlint-plugin-sample/blob/master/src/forbidden_regex.rs)
+and
+[`src/another_plugin.rs`](https://github.com/dalance/svlint-plugin-sample/blob/master/src/sample_plugin.rs)
+for examples of each.
+
+```rust
+impl SyntaxRule for SamplePlugin {
+    fn check(
+        &mut self,
+        _syntax_tree: &Tree,
+        event: &NodeEvent,
+        _config: &ConfigOption,
+    ) -> SyntaxRuleResult {
+        match event {
+            NodeEvent::Enter(RefNode::InitialConstruct(_)) => SyntaxRuleResult::Fail,
+            _ => SyntaxRuleResult::Pass,
         }
     }
 
     fn name(&self) -> String {
         String::from("sample_plugin")
     }
 
-    fn hint(&self) -> String {
-        String::from("`initial` is forbidden")
+    fn hint(&self, _config: &ConfigOption) -> String {
+        String::from("Remove the `initial` process.")
     }
 
     fn reason(&self) -> String {
-        String::from("this is a sample plugin")
+        String::from("This example doesn't like `initial` processes.")
     }
 }
 ```
 
-`Rule` must implement `check`, `name`, `hint` and `reason`.
-
-## Usage
-
-svlint can load plugin by `--plugin` option.
-
-```
-$ svlint --plugin libsvlint_plugin_sample.so test.sv
-Fail: sample_plugin
-   --> test.sv:2:1
-  |
-2 | initial begin
-  | ^^^^^^^ hint  : `initial` is forbidden
-  |         reason: this is a sample plugin
-```
-
-The loaded plugin is automatically enabled.
+`TextRule` must implement `check`, `name`, `hint` and `reason`.
+`SyntaxRule` must implement `check`, `name`, `hint` and `reason`.
+
+
+## Testing
+
+This sample project includes a basic test infrastructure to test its rules.
+To run the tests, first build the shared object (`cargo build`), then run
+`cargo test`.
+If you wish to debug via `println`, run `cargo test -- --show-output`.
+
+The test infrastructure has 3 main parts:
+
+1. The `tests` module in `src/lib.rs`.
+  - `so_path()`: Return a string with the expected filesystem path of the
+    shared object.
+    If your plugin has an unusual name (specified in `Cargo.toml`), then this
+    may require modification.
+  - `execute_linter()`: Attempts to perform in the same way as svlint does.
+    If svlint is modified, then this may require modification.
+  - `plugin_test()`: Called by the functions written by `build.rs`.
+    Should not normally require modification.
+2. A collection of SystemVerilog testcase files in `testcases/(fail|pass)/`.
+  Naturally, you must create your own testcases for your own plugin rules.
+  To add a SystemVerilog test file, simply copy it to `testcases/pass/` if it
+  must pass *all* of the plugin's rules, or to `testcases/fail/` if it
+  must fail *any* of the plugin's rules.
+3. The build script (`build.rs`) which uses the testcase files to produce
+  test functions just before the main compilation.

build.rs

@@ -0,0 +1,82 @@
+use std::env;
+use std::fs::{File, read_dir};
+use std::io::{BufRead, BufReader, Write};
+use std::path::Path;
+
+fn write_test_rs(testcases: &Vec<(String, bool)>) -> () {
+    let out_dir = env::var("OUT_DIR").unwrap();
+    let o = Path::new(&out_dir).join("test.rs");
+    let mut o = File::create(&o).unwrap();
+
+    for (path, pass_not_fail) in testcases {
+        let passfail = if *pass_not_fail { "pass" } else { "fail" };
+
+        let lines = BufReader::new(File::open(path).unwrap())
+            .lines()
+            .collect::<Result<Vec<_>, _>>()
+            .unwrap();
+
+        let sep = "/".repeat(80);
+        let subtests: Vec<&[String]> = lines
+            .as_slice()
+            .split(|l| l.contains(sep.as_str()))
+            .collect();
+        let n_subtests: usize = subtests.len();
+
+        let testname = Path::new(path).file_stem().unwrap().to_str().unwrap();
+
+        for (t, subtest) in subtests.into_iter().enumerate().map(|(i, x)| (i + 1, x)) {
+            // Write subtest to its own file.
+            let subtest_path = Path::new(&out_dir)
+                .join(format!("subtest.{testname}.{passfail}.{t}of{n_subtests}.sv"));
+            let mut out_subtest = File::create(&subtest_path).unwrap();
+            for line in subtest {
+                let _ = writeln!(out_subtest, "{}", line);
+            }
+
+            // Create call to `lib.rs::tests::plugin_test()` via `tests.rs`.
+            let subtest_name = format!("{testname}_{passfail}_{t}of{n_subtests}");
+            let _ = writeln!(o, "#[test]");
+            let _ = writeln!(o, "fn {}() {{", subtest_name);
+            if *pass_not_fail {
+                let _ = writeln!(
+                    o,
+                    "    plugin_test({subtest_path:?}, true);"
+                );
+            } else {
+                let _ = writeln!(
+                    o,
+                    "    plugin_test({subtest_path:?}, false);"
+                );
+            }
+            let _ = writeln!(o, "}}");
+        }
+    }
+}
+
+fn main() {
+
+    let mut testcases: Vec<(String, bool)> = Vec::new();
+
+    if let Ok(entries) = read_dir("testcases/fail") {
+        for entry in entries {
+            if let Ok(entry) = entry {
+                let p = String::from(entry.path().to_string_lossy());
+                testcases.push((p, false));
+            }
+        }
+    }
+
+    if let Ok(entries) = read_dir("testcases/pass") {
+        for entry in entries {
+            if let Ok(entry) = entry {
+                let p = String::from(entry.path().to_string_lossy());
+                testcases.push((p, true));
+            }
+        }
+    }
+
+    testcases.sort_by(|a, b| a.0.cmp(&b.0));
+
+    write_test_rs(&testcases);
+}

src/another_plugin.rs

@@ -0,0 +1,32 @@
+use sv_parser::{NodeEvent, RefNode, SyntaxTree};
+use svlint::config::ConfigOption;
+use svlint::linter::{SyntaxRule, SyntaxRuleResult};
+
+#[derive(Default)]
+pub struct AnotherPlugin;
+
+impl SyntaxRule for AnotherPlugin {
+    fn check(
+        &mut self,
+        _syntax_tree: &SyntaxTree,
+        event: &NodeEvent,
+        _config: &ConfigOption,
+    ) -> SyntaxRuleResult {
+        match event {
+            NodeEvent::Enter(RefNode::DisableStatementFork(_)) => SyntaxRuleResult::Fail,
+            _ => SyntaxRuleResult::Pass,
+        }
+    }
+
+    fn name(&self) -> String {
+        String::from("another_plugin")
+    }
+
+    fn hint(&self, _config: &ConfigOption) -> String {
+        String::from("Do not use `disable fork`.")
+    }
+
+    fn reason(&self) -> String {
+        String::from("This example dislikes disable-fork statements.")
+    }
+}

src/forbidden_regex.rs

@@ -0,0 +1,44 @@
+use svlint::config::ConfigOption;
+use svlint::linter::{TextRule, TextRuleResult};
+use regex::Regex;
+
+#[derive(Default)]
+pub struct ForbiddenRegex {
+    re: Option<Regex>,
+}
+
+impl TextRule for ForbiddenRegex {
+    fn check(
+        &mut self,
+        line: &str,
+        _option: &ConfigOption,
+    ) -> TextRuleResult {
+        if self.re.is_none() {
+            let r = format!(r"XXX");
+            self.re = Some(Regex::new(&r).unwrap());
+        }
+        let re = self.re.as_ref().unwrap();
+
+        let is_match: bool = re.is_match(line);
+        if is_match {
+            TextRuleResult::Fail {
+                offset: 0,
+                len: line.len(),
+            }
+        } else {
+            TextRuleResult::Pass
+        }
+    }
+
+    fn name(&self) -> String {
+        String::from("forbidden_regex")
+    }
+
+    fn hint(&self, _option: &ConfigOption) -> String {
+        String::from("Remove the string 'XXX' from all lines.")
+    }
+
+    fn reason(&self) -> String {
+        String::from("XXX is not meaningful enough.")
+    }
+}