Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
The initial structure of the linting crate and tests
Nedeed for #1436
  • Loading branch information
Georgiy Komarov committed Jun 26, 2023
commit 7ca2cf771c28c52f3083890eda92c620188505ee
5 changes: 4 additions & 1 deletion linting/Cargo.toml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update ref of clippy_utils to 1d334696587ac22b3a9e651e7ac684ac9e0697b2 and bump dylint_linting & dylint_testing to 2.1.12

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also need to set channel to nightly-2023-08-10 in rust-toolchain.toml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

I updated clippy_utils version to 1d334696587ac22b3a9e651e7ac684ac9e0697b2 and set channel to nightly-2023-07-14 in rust-toolchain.toml. nightly-2023-07-14 is required, since this version is used in rust-clippy at commit 1d334696587ac22b3a9e651e7ac684ac9e0697b2.

How should we choose the clippy version for linting? Should we always take the latest version from the clippy master branch?

Copy link
Contributor

@SkymanOne SkymanOne Aug 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean by clippy here. If you are referring to clippy_utils, I've been following a template in dylint repo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By clippy I mean the rust-clippy repository, which contains the clippy_utils crate. In this repo they set the toolchain version to nightly-2023-07-14 for all the crates. So we need to set the same version in our toolchain file to build clippy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK we only have rust-toolchain.toml for the lining crate

Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ dylint_testing = "2.0.0"

# The following are ink! dependencies, they are only required for the `ui` tests.
ink_env = { path = "../crates/env", default-features = false }
ink = { path = "../crates/ink", default-features = false }
ink = { path = "../crates/ink", default-features = false, features = ["std"] }
ink_metadata = { path = "../crates/metadata", default-features = false }
ink_primitives = { path = "../crates/primitives", default-features = false }
ink_storage = { path = "../crates/storage", default-features = false }
Expand All @@ -45,6 +45,9 @@ scale-info = { version = "2.6", default-features = false, features = ["derive"]
#
# Those files require the ink! dependencies though, by having
# them as examples here, they inherit the `dev-dependencies`.
[[example]]
name = "primitive_topic"
path = "ui/pass/primitive_topic.rs"

[package.metadata.rust-analyzer]
rustc_private = true
Expand Down
28 changes: 15 additions & 13 deletions linting/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
// Copyright 2018-2022 Parity Technologies (UK) Ltd.
// This file is part of cargo-contract.
// Copyright 2018-2023 Parity Technologies (UK) Ltd.
//
// cargo-contract is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// cargo-contract is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
// http://www.apache.org/licenses/LICENSE-2.0
//
// You should have received a copy of the GNU General Public License
// along with cargo-contract. If not, see <http://www.gnu.org/licenses/>.
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#![doc(
html_logo_url = "https://use.ink/img/crate-docs/logo.png",
Expand All @@ -30,12 +28,16 @@ extern crate rustc_middle;
extern crate rustc_session;
extern crate rustc_span;

mod primitive_topic;

#[doc(hidden)]
#[no_mangle]
pub fn register_lints(
_sess: &rustc_session::Session,
_lint_store: &mut rustc_lint::LintStore,
lint_store: &mut rustc_lint::LintStore,
) {
lint_store.register_lints(&[primitive_topic::PRIMITIVE_TOPIC]);
lint_store.register_late_pass(|_| Box::new(primitive_topic::PrimitiveTopic));
}

#[test]
Expand Down
77 changes: 77 additions & 0 deletions linting/src/primitive_topic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright 2018-2023 Parity Technologies (UK) Ltd.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
use rustc_hir::Item;
use rustc_lint::{
LateContext,
LateLintPass,
};
use rustc_session::{
declare_lint,
declare_lint_pass,
};

declare_lint! {
/// **What it does:** Checks for ink! contracts that use
/// the [`#[ink(topic)]`](https://use.ink/macros-attributes/topic) annotation with primitive
/// number types. Topics are discrete events for which it makes sense to filter. Typical
/// examples of fields that should be filtered are `AccountId`, `bool` or `enum` variants.
///
/// **Why is this bad?** It typically doesn't make sense to annotate types like `u32` or `i32`
/// as a topic, if those fields can take continuous values that could be anywhere between
/// `::MIN` and `::MAX`. An example of a case where it doesn't make sense at all to have a
/// topic on the storage field is something like `value: Balance` in the examle below.
///
/// **Known problems:**
///
/// **Example:**
///
/// ```rust
/// // Good
/// // Filtering transactions based on source and destination addresses.
/// #[ink(event)]
/// pub struct Transaction {
/// #[ink(topic)]
/// src: Option<AccountId>,
/// #[ink(topic)]
/// dst: Option<AccountId>,
/// value: Balance,
/// }
/// ```
///
/// ```rust
/// // Bad
/// // It typically makes no sense to filter `Balance`, since its value may varies from `::MAX`
/// // to `::MIN`.
/// #[ink(event)]
/// pub struct Transaction {
/// #[ink(topic)]
/// src: Option<AccountId>,
/// #[ink(topic)]
/// dst: Option<AccountId>,
/// #[ink(topic)]
/// value: Balance,
/// }
/// ```
pub PRIMITIVE_TOPIC,
Warn,
"The `#[ink(topic)]` annotation should not be used with a number primitive"
}

declare_lint_pass!(PrimitiveTopic => [PRIMITIVE_TOPIC]);

impl<'tcx> LateLintPass<'tcx> for PrimitiveTopic {
fn check_item(&mut self, _cx: &LateContext<'tcx>, _item: &'tcx Item<'_>) {
todo!()
}
}
112 changes: 112 additions & 0 deletions linting/ui/pass/primitive_topic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// #![cfg_attr(not(feature = "std"), no_std, no_main)]

#[ink::contract]
pub mod flipper {
#[ink(storage)]
pub struct Flipper {
value: bool,
}

impl Flipper {
/// Creates a new flipper smart contract initialized with the given value.
#[ink(constructor)]
pub fn new(init_value: bool) -> Self {
Self { value: init_value }
}

/// Creates a new flipper smart contract initialized to `false`.
#[ink(constructor)]
pub fn new_default() -> Self {
Self::new(Default::default())
}

/// Flips the current value of the Flipper's boolean.
#[ink(message)]
pub fn flip(&mut self) {
self.value = !self.value;
}

/// Returns the current value of the Flipper's boolean.
#[ink(message)]
pub fn get(&self) -> bool {
self.value
}
}

#[cfg(test)]
mod tests {
use super::*;

#[ink::test]
fn default_works() {
let flipper = Flipper::new_default();
assert!(!flipper.get());
}

#[ink::test]
fn it_works() {
let mut flipper = Flipper::new(false);
assert!(!flipper.get());
flipper.flip();
assert!(flipper.get());
}
}

#[cfg(all(test, feature = "e2e-tests"))]
mod e2e_tests {
use super::*;

type E2EResult<T> = std::result::Result<T, Box<dyn std::error::Error>>;

#[ink_e2e::test]
async fn it_works(mut client: ink_e2e::Client<C, E>) -> E2EResult<()> {
// given
let constructor = FlipperRef::new(false);
let contract = client
.instantiate("flipper", &ink_e2e::alice(), constructor, 0, None)
.await
.expect("instantiate failed");
let mut call = contract.call::<Flipper>();

let get = call.get();
let get_res = client.call_dry_run(&ink_e2e::bob(), &get, 0, None).await;
assert!(matches!(get_res.return_value(), false));

// when
let flip = call.flip();
let _flip_res = client
.call(&ink_e2e::bob(), &flip, 0, None)
.await
.expect("flip failed");

// then
let get = call.get();
let get_res = client.call_dry_run(&ink_e2e::bob(), &get, 0, None).await;
assert!(matches!(get_res.return_value(), true));

Ok(())
}

#[ink_e2e::test]
async fn default_works(mut client: ink_e2e::Client<C, E>) -> E2EResult<()> {
// given
let constructor = FlipperRef::new_default();

// when
let contract = client
.instantiate("flipper", &ink_e2e::bob(), constructor, 0, None)
.await
.expect("instantiate failed");
let call = contract.call::<Flipper>();

// then
let get = call.get();
let get_res = client.call_dry_run(&ink_e2e::bob(), &get, 0, None).await;
assert!(matches!(get_res.return_value(), false));

Ok(())
}
}
}

fn main() {}
2 changes: 2 additions & 0 deletions linting/ui/pass/primitive_topic.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
thread 'rustc' panicked at 'not yet implemented', src/primitive_topic.rs:50:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace