Add ink! linting MVP stage

[cmichi]

Closes #281.

This is an MVP, there are a couple shortcomings for which I want to create follow-up issues:

• As it is now, we unfortunately have to require users to install dylint and dylint-driver manually. We should look into making those cargo dependencies instead. However, this is not straight forward. Especially for the dylint-driver (which is used as a linker) I'm not sure if this is even possible.

• I haven't yet managed to get rid of some driver build overhead, this happens every time the linting is applied:

  [1/2] Checking ink! lints                                                     
  Building driver for toolchain `nightly-2021-11-04-x86_64-unknown-linux-gnu`        
  

• We should render and publish a proper website displaying info about the linting rules. Similar to https://rust-lang.github.io/rust-clippy/master/.

• We should recognize if the call to ink_lang::utils::initialize_contract is nested within the constructor. So if it's not called directly in the constructor, but instead in a sub-function.

How it works:

• ink_linting/ contains the ink! linting driver (i.e. the linting rules).
• build.rs builds these rules into a *.so/*.dll, this file is included via include_bytes! into the cargo-contract executable.

Try it out:

git checkout cmichi-implemnt-dylint-mvp
RUST_LOG=info cargo run -- contract check --manifest-path ../ink/examples/flipper/Cargo.toml --verbose

[xgreenx]

We should recognize if the call to ink_lang::codegen::initialize_contract is nested within the constructor. So if it's not called directly in the constructor, but instead in a sub-function.

Maybe better to require the user to always implement SpreadAllocate and we will use ink_lang::codegen::initialize_contract in the codegen?

If the contract implements SpreadAllocate we can create an instance of the contract. After we can pass that instance into the constructor and the user will do all the necessary stuff.

Example of the idea in the playground.

impl Contract {
    #[ink(constrcutor)]
    fn new(mut self/* we will require `self` here*/, total_supply: u32) -> Self {
        self.total_supply = total_supply;
        self
    }
}

[cmichi]

@xgreenx Thanks for your comment! So the thing is that we don't want to change the ink! constructor function signature, we would really like to stay as close to Rust syntax as possible.

Putting the initialize_contract call into the codegen would result in a changed function signature though. This has implications for e.g. the off-chain testing, so there some special API be needed so that it's still possible to e.g. write Rust unit tests for the contract.

Our thinking right now is that we will likely find a better way going forward and by requiring the initalize_contract in the meantime those contracts would be forward compatible. If we change the function signature now though, then those contracts can't be easily used with newer versions of ink! if we change the function signature back.

[xgreenx]

Thanks for the response. I see you are right. I forgot about off-chain unit tests that the problem still exists for them.

Our thinking right now is that we will likely find a better way going forward and by requiring the initalize_contract in the meantime those contracts would be forward compatible. If we change the function signature now though, then those contracts can't be easily used with newer versions of ink! if we change the function signature back.

Yesterday I tried to implement your idea regarding the new structure of the contract. Here is results. Maybe you can check it in your free time=)

[ascjones]

LGTM

[ascjones]

Not a fan of mapping into an io side effect, but now I am just nitpicking

[athei]

I am wondering: We are writing our custom link but don't even run the stock clippy lints. Can we do that at the same time as we are running dylint so we don't have multiple passes?

[cmichi]

We are writing our custom link but don't even run the stock clippy lints. Can we do that at the same time as we are running dylint so we don't have multiple passes?

So dylint doesn't support the stock clippy lints out of the box, it would be an additional invocation of cargo clippy. We could do that, but I'm on the fence since we then would become way more opinionated.

[xgreenx]

Is that possible to use CARGO_TARGET_DIR with dylint to not build it each time?
Also maybe make dylint configurable? For example, I don't want to build it during integration tests=)