feat: Lint skeleton and tests for use-ink#2017

linting/Cargo.toml

@@ -66,6 +66,12 @@ path = "ui/fail/strict_balance_equality.rs"
 [[example]]
 name = "no_main_pass"
 path = "ui/pass/no_main.rs"
+[example]]
+ame = "non_fallible_api"
+ath = "ui/pass/non_fallible_api.rs"
+[[example]]
+name = "non_fallible_api"
+path = "ui/fail/non_fallible_api.rs"
 
 [package.metadata.rust-analyzer]
 rustc_private = true

linting/src/lib.rs

@@ -36,6 +36,7 @@ mod no_main;
 mod primitive_topic;
 mod storage_never_freed;
 mod strict_balance_equality;
+mod non_fallible_api;
 
 #[doc(hidden)]
 #[no_mangle]
@@ -54,6 +55,7 @@ pub fn register_lints(
     lint_store
         .register_late_pass(|_| Box::new(strict_balance_equality::StrictBalanceEquality));
     lint_store.register_early_pass(|| Box::new(no_main::NoMain));
+    lint_store.register_late_pass(|_| Box::new(non_fallible_api::NonFallibleAPI));
 }
 
 #[test]

linting/src/non_fallible_api.rs

@@ -0,0 +1,118 @@
+// Copyright (C) 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 clippy_utils::{
+    diagnostics::span_lint_and_then,
+    is_lint_allowed,
+    match_def_path,
+    source::snippet_opt,
+};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{
+    self,
+    def::{
+        DefKind,
+        Res,
+    },
+    def_id::DefId,
+    Arm,
+    AssocItemKind,
+    ExprKind,
+    Impl,
+    ImplItemKind,
+    ImplItemRef,
+    Item,
+    ItemKind,
+    Node,
+    PatKind,
+    QPath,
+};
+use rustc_lint::{
+    LateContext,
+    LateLintPass,
+};
+use rustc_middle::ty::{
+    self,
+    Ty,
+    TyKind,
+};
+use rustc_session::{
+    declare_lint,
+    declare_lint_pass,
+};
+
+declare_lint! {
+    /// ## What it does
+    ///
+    /// The lint detects potentially unsafe uses of methods for which there are safer alternatives.
+    ///
+    /// ## Why is this bad?
+    ///
+    /// In some standard collections in ink!, there are two types of implementations: non-fallible
+    /// (e.g. `get`) and fallible (e.g. `try_get`). Fallible alternatives are considered safer,
+    /// as they perform additional checks for incorrect input parameters and return `Result::Err`
+    /// when they are used improperly. On the other hand, non-fallible methods do not provide these
+    /// checks and will panic on incorrect input, placing the responsibility on the user to
+    /// implement these checks.
+    ///
+    /// For more context, see: [ink#1910](https://github.com/paritytech/ink/pull/1910).
+    ///
+    /// ## Example
+    ///
+    /// Consider the contract that has the following `Mapping` field:
+    ///
+    /// ```rust
+    /// #[ink(storage)]
+    /// pub struct Example { map: Mapping<String, AccountId> }
+    /// ```
+    ///
+    /// The following usage of the non-fallible API is unsafe:
+    ///
+    /// ```rust
+    /// pub fn test(&mut self, a: String, b: AccountId) {
+    ///   // Bad: can panic on incorrect input
+    ///   self.map.insert(a, &b);
+    /// }
+    /// ```
+    ///
+    /// It could be replaced with the fallible version of `Mapping::insert`:
+    ///
+    /// ```rust
+    /// pub fn test(&mut self, a: String, b: AccountId) {
+    ///   // Good: returns Result::Err on incorrect input
+    ///   self.map.try_insert(a, &b)?;
+    /// }
+    /// ```
+    ///
+    /// Otherwise, the user could explicitly check the encoded size of the argument in their code:
+    ///
+    /// ```rust
+    /// pub fn test(&mut self, a: String, b: AccountId) {
+    ///   // Good: explicitly checked encoded size of the input
+    ///   if String::encoded_size(&a) < ink_env::BUFFER_SIZE {
+    ///     self.map.insert(a, &b);
+    ///   }
+    /// }
+    /// ```
+    pub NON_FALLIBLE_API,
+    Warn,
+    "using non-fallible API"
+}
+
+declare_lint_pass!(NonFallibleAPI => [NON_FALLIBLE_API]);
+
+impl<'tcx> LateLintPass<'tcx> for NonFallibleAPI {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {}
+}

linting/ui/fail/non_fallible_api.rs

@@ -0,0 +1,44 @@
+#![cfg_attr(not(feature = "std"), no_std, no_main)]
+
+#[ink::contract]
+pub mod non_fallible_api {
+    use ink::{
+        prelude::string::String,
+        storage::{
+            Lazy,
+            Mapping,
+        },
+    };
+
+    #[ink(storage)]
+    pub struct NonFallibleAPI {
+        map_1: Mapping<String, String>,
+        lazy_1: Lazy<String>,
+    }
+
+    impl NonFallibleAPI {
+        #[ink(constructor)]
+        pub fn new() -> Self {
+            Self {
+                map_1: Mapping::new(),
+                lazy_1: Lazy::new(),
+            }
+        }
+
+        // Raise warnings when using non-fallible API with argument which encoded size is
+        // statically unknown.
+        #[ink(message)]
+        pub fn non_fallible_not_statically_known(&mut self, a: String, b: String) {
+            // Mapping
+            let _ = self.map_1.insert(a.clone(), &b);
+            let _ = self.map_1.get(a.clone());
+            let _ = self.map_1.take(a.clone());
+
+            // Lazy
+            let _ = self.lazy_1.get();
+            self.lazy_1.set(&a);
+        }
+    }
+}
+
+fn main() {}

linting/ui/pass/non_fallible_api.rs

@@ -0,0 +1,54 @@
+#![cfg_attr(not(feature = "std"), no_std, no_main)]
+
+#[ink::contract]
+pub mod non_fallible_api {
+    use ink::storage::{
+        Lazy,
+        Mapping,
+    };
+
+    #[ink(storage)]
+    pub struct NonFallibleAPI {
+        map_1: Mapping<AccountId, AccountId>,
+        lazy_1: Lazy<AccountId>,
+    }
+
+    impl NonFallibleAPI {
+        #[ink(constructor)]
+        pub fn new() -> Self {
+            Self {
+                map_1: Mapping::new(),
+                lazy_1: Lazy::new(),
+            }
+        }
+
+        // Don't generate warnings when using the fallible API
+        #[ink(message)]
+        pub fn fallible(&mut self, a: AccountId, b: AccountId) {
+            // Mapping
+            let _ = self.map_1.try_insert(a, &b);
+            let _ = self.map_1.try_get(a);
+            let _ = self.map_1.try_take(a);
+
+            // Lazy
+            let _ = self.lazy_1.try_get();
+            let _ = self.lazy_1.try_set(&a);
+        }
+
+        // Don't raise warnings when using non-fallible API with argument which encoded
+        // size is statically known.
+        #[ink(message)]
+        pub fn non_fallible_statically_known(&mut self, a: AccountId, b: AccountId) {
+            // Mapping
+            let _ = self.map_1.insert(a, &b);
+            let _ = self.map_1.get(a);
+            let _ = self.map_1.take(a);
+
+            // Lazy
+            let _ = self.lazy_1.get();
+            self.lazy_1.set(&a);
+        }
+    }
+}
+
+fn main() {}