v0.8 release

[dalance]

Today, I merged #247.
I want to bump up version to v0.8 because this PR has big changes.
In addition, I want to include #248/#251 in this release because it has a breaking change of plugin binray ABI.

I think there are the following items to do.

• before release

• merge Allow Plugins to return a list of Rules #251
• setup CI for https://github.com/dalance/svlint-plugin-sample

• svlint v0.8 release

• after release

• merge Updated example for Vec of Rule svlint-plugin-sample#5
• merge textrules Rename Rule -> SyntaxRule. svlint-plugin-sample#3

@DaveMcEwan @skjdbg
Is there any idea or suggestion?

[dalance]

Plugin's API reconsideration is probably required becasuse TextRule plugin ( or other types ) support will cause a breaking change again.

[DaveMcEwan]

Apologies for my delayed response (summer vacation). This sounds like a sensible plan to me :)
It sounds like the API should look for Vec<*mut dyn Rule> with enum Rule {TextRule, SyntaxRule, ...}. Later, when more types of rules are added, we can just extend the enum. That should remain source compatible, if not binary compatible (I'm unsure).

@skjdbg Would you like to do this? If not, I'm happy to.

[skjdbg]

Sure, I'll do it tomorrow but, unless I missed a feature of Rust, we can't make enums of traits, can we ? So I think it should look like this:

Vec<Rule>

with

enum Rule {
    TextRule(*mut dyn TextRule),
    SyntaxRule(*mut dyn TextRule)
}

[DaveMcEwan]

Pretty much, yes.
I have a pair of matching branches I started yesterday:
https://github.com/DaveMcEwan/svlint-plugin-sample/blob/api/src/lib.rs#L16-L17
https://github.com/DaveMcEwan/svlint/blob/api/src/linter.rs#L253-L272.

[skjdbg]

It seems you've already done it all. Should I pull your changes in #251 and dalance/svlint-plugin-sample#5 ?

Also, wouldn't it be more practical to have each trait have a default implementation for its conversion to Rule ? This would save us the trouble of repeating the Rule variant every time we add it to the vec as in https://github.com/DaveMcEwan/svlint-plugin-sample/blob/api/src/lib.rs#L16-L17

I'm thinking about something like this :

pub trait TextRule: Sync + Send {
    fn check(&mut self, text: &str, config: &ConfigOption) -> TextRuleResult;
    fn name(&self) -> String;
    fn hint(&self, config: &ConfigOption) -> String;
    fn reason(&self) -> String;
    fn into_rule(self) -> Rule
    where
        Self: Sized + 'static,
    {
        let temp = Box::new(self);
        Rule::Text(Box::into_raw(temp))
    }
}

and on the plugin's side:

pub extern "C" fn get_plugin() -> Vec<Rule> {
    combine_rules!(
        SamplePlugin,
        AnotherPlugin,
    )
}

#[macro_export]
macro_rules! combine_rules {
    ( $( $x:ty ),* $(,)? ) => {
        {
            let mut vec: Vec<Rule> = Vec::new();
            $(
                let rule = <$x>::default();
                vec.push(rule.into_rule());
            )*
            vec
        }
    };
}

[DaveMcEwan]

It seems you've already done it all. Should I pull your changes in #251 and dalance/svlint-plugin-sample#5?

Not quite yet! I'm still working on a test infrastructure that exercises the new API.

Fine to pull my changes into your PRs, but perhaps wait for the test infrastructure commits before merging (I'll mention this issue in the commit message).

[DaveMcEwan]

Also, wouldn't it be more practical to have each trait have a default implementation for its conversion to Rule ? This would save us the trouble of repeating the Rule variant every time we add it to the vec as in https://github.com/DaveMcEwan/svlint-plugin-sample/blob/api/src/lib.rs#L16-L17

Good idea :) It would be nice to have the macro exported from svlint though. Perhaps named pluginrules to make the usecase obvious?

[DaveMcEwan]

@skjdbg I've made those changes to the pair of branches (but there's no TextRule sample).
It's a bit less admin for me to just open 2 new PRs if merging the changes into #251 and dalance/svlint-plugin-sample#5 is annoying. What's your preference?

[skjdbg]

I don't mind, you can go ahead and open them.

If you want an example of TextRule, how about forbidding uppercase characters ? Might as well add it from the get go, so nobody thinks a plugin can only have one kind of Rule at a time.

[DaveMcEwan]

@dalance I think the PRs are in place now - #255, and dalance/svlint-plugin-sample#7 (will wait for v0.8.0 release before changing from draft to proper).
Would you consider merging #254 and #250 before the v0.8.0 release? They should all merge cleanly (I've checked by making this branch).

[dalance]

I merged #250, #254, and #255.
I think v0.8.0 release is ready, and you?

[DaveMcEwan]

Nice one man :) I think ready.

[skjdbg]

I think it's good to go too!

[dalance]

I released v0.8.0.

[DaveMcEwan]

Nice one :) I've updated the corresponding PR (dalance/svlint-plugin-sample#7) and made it ready for review/merge.