Get rid of unsafe code in the interpreter (#303)

* Get rid of unsafe code unless really necessary

* Add lint levels where appropriate

Some crates (air-beautifier, air-testing-framework) have lot of
rust_2018_idioms violations, that will be resolved later.

air/src/lib.rs

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
 #![warn(rust_2018_idioms)]
 #![deny(
     dead_code,

avm/interface/src/lib.rs

@@ -14,6 +14,18 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
+#![warn(rust_2018_idioms)]
+#![deny(
+    dead_code,
+    nonstandard_style,
+    unused_imports,
+    unused_mut,
+    unused_variables,
+    unused_unsafe,
+    unreachable_patterns
+)]
+
 mod call_request_parameters;
 mod call_service_result;
 mod outcome;

crates/air-lib/air-parser/src/ast/values/impls.rs

@@ -18,6 +18,8 @@ use super::*;
 use air_lambda_parser::LambdaAST;
 use air_lambda_parser::ValueAccessor;
 
+use std::convert::TryFrom;
+
 impl<'i> ScalarWithLambda<'i> {
     pub fn new(name: &'i str, lambda: Option<LambdaAST<'i>>, position: usize) -> Self {
         Self {
@@ -27,16 +29,15 @@ impl<'i> ScalarWithLambda<'i> {
         }
     }
 
-    // it's unsafe method that should be used only for tests
     pub(crate) fn from_raw_lambda(
         name: &'i str,
         lambda: Vec<ValueAccessor<'i>>,
         position: usize,
     ) -> Self {
-        let lambda = unsafe { LambdaAST::new_unchecked(lambda) };
+        let lambda = LambdaAST::try_from(lambda).ok();
         Self {
             name,
-            lambda: Some(lambda),
+            lambda,
             position,
         }
     }
@@ -51,17 +52,16 @@ impl<'i> StreamWithLambda<'i> {
         }
     }
 
-    // it's unsafe method that should be used only for tests
     #[allow(dead_code)]
     pub(crate) fn from_raw_lambda(
         name: &'i str,
         lambda: Vec<ValueAccessor<'i>>,
         position: usize,
     ) -> Self {
-        let lambda = unsafe { LambdaAST::new_unchecked(lambda) };
+        let lambda = LambdaAST::try_from(lambda).ok();
         Self {
             name,
-            lambda: Some(lambda),
+            lambda,
             position,
         }
     }
@@ -154,7 +154,6 @@ impl<'i> VariableWithLambda<'i> {
         }
     }
 
-    // This function is unsafe and lambda must be non-empty, although it's used only for tests
     #[allow(dead_code)]
     pub(crate) fn from_raw_lambda_scalar(
         name: &'i str,
@@ -165,7 +164,6 @@ impl<'i> VariableWithLambda<'i> {
         Self::Scalar(scalar)
     }
 
-    // This function is unsafe and lambda must be non-empty, although it's used only for tests
     #[allow(dead_code)]
     pub(crate) fn from_raw_lambda_stream(
         name: &'i str,

crates/air-lib/air-parser/src/lib.rs

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
 #![deny(
     dead_code,
     nonstandard_style,

crates/air-lib/air-parser/src/parser/lexer/tests.rs

@@ -25,6 +25,8 @@ use air_lambda_parser::ValueAccessor;
 use fstrings::f;
 use fstrings::format_args_f;
 
+use std::convert::TryFrom;
+
 fn run_lexer(input: &str) -> Vec<Spanned<Token<'_>, usize, LexerError>> {
     let lexer = AIRLexer::new(input);
     lexer.collect()
@@ -306,14 +308,13 @@ fn lambda() {
             0,
             Token::ScalarWithLambda {
                 name: "value",
-                lambda: unsafe {
+                lambda: LambdaAST::try_from(vec![
-                    LambdaAST::new_unchecked(vec![
                     ValueAccessor::FieldAccessByName {
                         field_name: "field",
                     },
                     ValueAccessor::ArrayAccess { idx: 1 },
                 ])
-                },
+                .unwrap(),
                 position: 0,
             },
             LAMBDA.len(),
@@ -413,11 +414,12 @@ fn last_error() {
 fn last_error_instruction() {
     const LAST_ERROR: &str = r#"%last_error%.$.instruction"#;
 
-    let token = Token::LastErrorWithLambda(unsafe {
+    let token = Token::LastErrorWithLambda(
-        LambdaAST::new_unchecked(vec![ValueAccessor::FieldAccessByName {
+        LambdaAST::try_from(vec![ValueAccessor::FieldAccessByName {
             field_name: "instruction",
         }])
-    });
+        .unwrap(),
+    );
 
     lexer_test(LAST_ERROR, Single(Ok((0, token, LAST_ERROR.len()))));
 }
@@ -426,11 +428,12 @@ fn last_error_instruction() {
 fn last_error_message() {
     const LAST_ERROR: &str = r#"%last_error%.$.message"#;
 
-    let token = Token::LastErrorWithLambda(unsafe {
+    let token = Token::LastErrorWithLambda(
-        LambdaAST::new_unchecked(vec![ValueAccessor::FieldAccessByName {
+        LambdaAST::try_from(vec![ValueAccessor::FieldAccessByName {
             field_name: "message",
         }])
-    });
+        .unwrap(),
+    );
     lexer_test(LAST_ERROR, Single(Ok((0, token, LAST_ERROR.len()))));
 }
 
@@ -438,11 +441,12 @@ fn last_error_message() {
 fn last_error_peer_id() {
     const LAST_ERROR: &str = r#"%last_error%.$.peer_id"#;
 
-    let token = Token::LastErrorWithLambda(unsafe {
+    let token = Token::LastErrorWithLambda(
-        LambdaAST::new_unchecked(vec![ValueAccessor::FieldAccessByName {
+        LambdaAST::try_from(vec![ValueAccessor::FieldAccessByName {
             field_name: "peer_id",
         }])
-    });
+        .unwrap(),
+    );
     lexer_test(LAST_ERROR, Single(Ok((0, token, LAST_ERROR.len()))));
 }
 
@@ -450,11 +454,12 @@ fn last_error_peer_id() {
 fn last_error_non_standard_field() {
     const LAST_ERROR: &str = r#"%last_error%.$.asdasd"#;
 
-    let token = Token::LastErrorWithLambda(unsafe {
+    let token = Token::LastErrorWithLambda(
-        LambdaAST::new_unchecked(vec![ValueAccessor::FieldAccessByName {
+        LambdaAST::try_from(vec![ValueAccessor::FieldAccessByName {
             field_name: "asdasd",
         }])
-    });
+        .unwrap(),
+    );
     lexer_test(LAST_ERROR, Single(Ok((0, token, LAST_ERROR.len()))));
 }
 

crates/air-lib/air-parser/src/parser/tests/ap.rs

@@ -22,6 +22,8 @@ use air_lambda_ast::{LambdaAST, ValueAccessor};
 use fstrings::f;
 use fstrings::format_args_f;
 
+use std::convert::TryFrom;
+
 #[test]
 fn ap_with_literal() {
     let source_code = r#"
@@ -75,11 +77,12 @@ fn ap_with_last_error() {
 
     let actual = parse(source_code);
     let expected = ap(
-        ApArgument::LastError(Some(unsafe {
+        ApArgument::LastError(Some(
-            LambdaAST::new_unchecked(vec![ValueAccessor::FieldAccessByName {
+            LambdaAST::try_from(vec![ValueAccessor::FieldAccessByName {
                 field_name: "message",
             }])
-        })),
+            .unwrap(),
+        )),
         ApResult::Stream(Stream::new("$stream", 37)),
     );
 
@@ -175,7 +178,7 @@ fn ap_with_canon_stream_with_lambda() {
     let expected = ap(
         ApArgument::CanonStream(CanonStreamWithLambda::new(
             canon_stream,
-            Some(unsafe { LambdaAST::new_unchecked(vec![ValueAccessor::ArrayAccess { idx: 0 }]) }),
+            Some(LambdaAST::try_from(vec![ValueAccessor::ArrayAccess { idx: 0 }]).unwrap()),
             13,
         )),
         ApResult::Scalar(Scalar::new(scalar, 33)),

crates/air-lib/air-parser/src/parser/tests/call.rs

@@ -24,6 +24,7 @@ use fstrings::f;
 use fstrings::format_args_f;
 use lalrpop_util::ParseError;
 
+use std::convert::TryFrom;
 use std::rc::Rc;
 
 #[test]
@@ -471,12 +472,11 @@ fn canon_stream_with_lambda_in_triplet() {
     let expected = call(
         CallInstrValue::Variable(VariableWithLambda::canon_stream_wl(
             canon_stream,
-            unsafe {
+            LambdaAST::try_from(vec![
-                LambdaAST::new_unchecked(vec![
                 ValueAccessor::ArrayAccess { idx: 0 },
                 ValueAccessor::FieldAccessByName { field_name: "path" },
             ])
-            },
+            .unwrap(),
             19,
         )),
         CallInstrValue::Literal(service_id),

crates/air-lib/air-parser/src/parser/tests/fail.rs

@@ -21,6 +21,8 @@ use crate::ast::ScalarWithLambda;
 use air_lambda_ast::LambdaAST;
 use air_lambda_ast::ValueAccessor;
 
+use std::convert::TryFrom;
+
 #[test]
 fn parse_fail_last_error() {
     let source_code = r#"
@@ -59,11 +61,12 @@ fn parse_fail_scalar_with_lambda() {
     let instruction = parse(source_code);
     let expected = fail_scalar(ScalarWithLambda::new(
         "scalar",
-        Some(unsafe {
+        Some(
-            LambdaAST::new_unchecked(vec![ValueAccessor::FieldAccessByName {
+            LambdaAST::try_from(vec![ValueAccessor::FieldAccessByName {
                 field_name: "field_accessor",
             }])
-        }),
+            .unwrap(),
+        ),
         18,
     ));
     assert_eq!(instruction, expected)

crates/air-lib/air-parser/src/parser/tests/match_.rs

@@ -22,6 +22,8 @@ use air_lambda_ast::{LambdaAST, ValueAccessor};
 use fstrings::f;
 use fstrings::format_args_f;
 
+use std::convert::TryFrom;
+
 #[test]
 fn parse_match() {
     let source_code = r#"
@@ -52,7 +54,7 @@ fn parse_match_with_canon_stream() {
     let expected = match_(
         Value::Variable(VariableWithLambda::canon_stream_wl(
             canon_stream,
-            unsafe { LambdaAST::new_unchecked(vec![ValueAccessor::ArrayAccess { idx: 0 }]) },
+            LambdaAST::try_from(vec![ValueAccessor::ArrayAccess { idx: 0 }]).unwrap(),
             16,
         )),
         Value::Variable(VariableWithLambda::scalar("v2", 36)),

crates/air-lib/execution-info-collector/src/lib.rs

@@ -14,6 +14,18 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
+#![warn(rust_2018_idioms)]
+#![deny(
+    dead_code,
+    nonstandard_style,
+    unused_imports,
+    unused_mut,
+    unused_variables,
+    unused_unsafe,
+    unreachable_patterns
+)]
+
 mod instructions_tracker;
 
 pub use instructions_tracker::*;

crates/air-lib/interpreter-data/src/lib.rs

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
 #![warn(rust_2018_idioms)]
 #![deny(
     dead_code,

crates/air-lib/interpreter-interface/src/lib.rs

@@ -14,6 +14,18 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
+#![warn(rust_2018_idioms)]
+#![deny(
+    dead_code,
+    nonstandard_style,
+    unused_imports,
+    unused_mut,
+    unused_variables,
+    unused_unsafe,
+    unreachable_patterns
+)]
+
 mod call_request_parameters;
 mod call_service_result;
 mod interpreter_outcome;

crates/air-lib/lambda/ast/src/lib.rs

@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
+#![warn(rust_2018_idioms)]
 #![deny(
     dead_code,
     nonstandard_style,

crates/air-lib/lambda/parser/src/lib.rs

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
 #![deny(
     dead_code,
     nonstandard_style,

crates/air-lib/lambda/parser/src/parser/lambda_parser.rs

@@ -23,6 +23,8 @@ use crate::ValueAccessor;
 
 use va_lambda::LambdaParser;
 
+use std::convert::TryFrom;
+
 // Caching parser to cache internal regexes, which are expensive to instantiate
 // See also https://github.com/lalrpop/lalrpop/issues/269
 thread_local!(static PARSER: LambdaParser = LambdaParser::new());
@@ -43,10 +45,5 @@ pub fn parse(lambda: &str) -> LambdaParserResult<'_, LambdaAST> {
 }
 
 fn try_to_lambda(accessors: Vec<ValueAccessor>) -> LambdaParserResult<'_, LambdaAST> {
-    if accessors.is_empty() {
+    LambdaAST::try_from(accessors).or(Err(LambdaParserError::EmptyLambda))
-        return Err(LambdaParserError::EmptyLambda);
-    }
-
-    let ast = unsafe { LambdaAST::new_unchecked(accessors) };
-    Ok(ast)
 }

crates/air-lib/polyplets/src/lib.rs

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
 #![warn(rust_2018_idioms)]
 #![deny(
     dead_code,

crates/air-lib/test-utils/src/lib.rs

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
 #![warn(rust_2018_idioms)]
 #![deny(
     dead_code,

crates/air-lib/trace-handler/src/lib.rs

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
 #![warn(rust_2018_idioms)]
 #![deny(
     dead_code,

crates/data-store/src/lib.rs

@@ -14,6 +14,18 @@
  * limitations under the License.
  */
 
+#![forbid(unsafe_code)]
+#![warn(rust_2018_idioms)]
+#![deny(
+    dead_code,
+    nonstandard_style,
+    unused_imports,
+    unused_mut,
+    unused_variables,
+    unused_unsafe,
+    unreachable_patterns
+)]
+
 use serde::Deserialize;
 use serde::Serialize;
 use std::borrow::Cow;