Add address_zero analyzer, some cleanup

solast/src/analysis/abi_encoding.rs

@@ -7,6 +7,22 @@ pub struct AbiEncodingVisitor {
     declaration_type_names: HashMap<NodeID, TypeName>
 }
 
+impl AbiEncodingVisitor {
+    fn print_message(
+        &mut self,
+        contract_definition: &ContractDefinition,
+        definition_node: &ContractDefinitionNode,
+        source_line: usize,
+        expression: &dyn std::fmt::Display,
+    ) {
+        println!(
+            "\t{} contains the potential for hash collisions: `{}`",
+            contract_definition.definition_node_location(source_line, definition_node),
+            expression,
+        );
+    }
+}
+
 impl AstVisitor for AbiEncodingVisitor {
     fn visit_variable_declaration<'a, 'b>(&mut self, context: &mut VariableDeclarationContext<'a, 'b>) -> std::io::Result<()> {
         //
@@ -73,41 +89,12 @@ impl AstVisitor for AbiEncodingVisitor {
         //
 
         if any_arguments_variably_sized {
-            match context.definition_node {
-                ContractDefinitionNode::FunctionDefinition(function_definition) => println!(
-                    "\tL{}: The {} {} in the `{}` {} contains the potential for hash collisions: `{}`",
-
-                    context.current_source_unit.source_line(context.function_call.src.as_str())?,
-
-                    function_definition.visibility,
-
-                    if let FunctionKind::Constructor = function_definition.kind {
-                        "constructor".to_string()
-                    } else {
-                        format!("`{}` {}", function_definition.name, function_definition.kind)
-                    },
-
-                    context.contract_definition.name,
-                    context.contract_definition.kind,
-
-                    context.function_call
-                ),
-
-                ContractDefinitionNode::ModifierDefinition(modifier_definition) => println!(
-                    "\tL{}: The `{}` modifier in the `{}` {} contains the potential for hash collisions: `{}`",
-
-                    context.current_source_unit.source_line(context.function_call.src.as_str())?,
-
-                    modifier_definition.name,
-
-                    context.contract_definition.name,
-                    context.contract_definition.kind,
-
-                    context.function_call
-                ),
-
-                _ => {}
-            }
+            self.print_message(
+                context.contract_definition,
+                context.definition_node,
+                context.current_source_unit.source_line(context.function_call.src.as_str())?,
+                context.function_call,
+            );
         }
 
         Ok(())

solast/src/analysis/address_balance.rs

@@ -11,53 +11,16 @@ impl AddressBalanceVisitor {
         expression: &dyn std::fmt::Display,
         external: bool,
     ) {
-        match definition_node {
-            ContractDefinitionNode::FunctionDefinition(function_definition) => println!(
-                "\tL{}: The {} {} in the `{}` {} contains `{}` usage, which can be optimized with assembly: `{}`",
-
-                source_line,
-
-                function_definition.visibility,
-
-                if let FunctionKind::Constructor = function_definition.kind {
-                    "constructor".to_string()
-                } else {
-                    format!("`{}` {}", function_definition.name, function_definition.kind)
-                },
-
-                contract_definition.name,
-                contract_definition.kind,
-
-                expression,
-
-                if external {
-                    "assembly { bal := balance(addr); }"
-                } else {
-                    "assembly { bal := selfbalance(); }"
-                }
-            ),
-
-            ContractDefinitionNode::ModifierDefinition(modifier_definition) => println!(
-                "\tL{}: The `{}` modifier in the `{}` {} contains `{}` usage, which can be optimized with assembly: `{}`",
-
-                source_line,
-
-                modifier_definition.name,
-
-                contract_definition.name,
-                contract_definition.kind,
-
-                expression,
-
-                if external {
-                    "assembly { bal := balance(addr); }"
-                } else {
-                    "assembly { bal := selfbalance(); }"
-                }
-            ),
-
-            _ => {}
-        }
+        println!(
+            "\t{} contains `{}` usage, which can be optimized with assembly: `{}`",
+            contract_definition.definition_node_location(source_line, definition_node),
+            expression,
+            if external {
+                "assembly { bal := balance(addr); }"
+            } else {
+                "assembly { bal := selfbalance(); }"
+            }
+        );
     }
 }
 
@@ -97,7 +60,7 @@ impl AstVisitor for AddressBalanceVisitor {
                 context.contract_definition,
                 context.definition_node,
                 context.current_source_unit.source_line(context.member_access.src.as_str())?,
-                expression,
+                context.member_access,
                 name != "this"
             );
         }

solast/src/analysis/address_zero.rs

@@ -0,0 +1,75 @@
+use solidity::ast::*;
+
+pub struct AddressZeroVisitor;
+
+impl AddressZeroVisitor {
+    fn print_message(
+        &mut self,
+        contract_definition: &ContractDefinition,
+        definition_node: &ContractDefinitionNode,
+        source_line: usize,
+        expression: &dyn std::fmt::Display,
+    ) {
+        println!(
+            "\t{} contains `{}` usage, which can be optimized with assembly: `{}`",
+            contract_definition.definition_node_location(source_line, definition_node),
+            expression,
+            "assembly { if iszero(addr) { ... } }",
+        );
+    }
+}
+
+impl AstVisitor for AddressZeroVisitor {
+    fn visit_binary_operation<'a, 'b>(
+        &mut self,
+        context: &mut BinaryOperationContext<'a, 'b>,
+    ) -> std::io::Result<()> {
+        if !matches!(context.binary_operation.operator.as_str(), "==" | "!=") {
+            return Ok(());
+        }
+
+        let check_expression = |expression: &Expression| -> bool {
+            match expression {
+                Expression::FunctionCall(FunctionCall {
+                    kind: FunctionCallKind::TypeConversion,
+                    arguments,
+                    expression,
+                    ..
+                }) if arguments.len() == 1 => match expression.as_ref() {
+                    Expression::ElementaryTypeNameExpression(ElementaryTypeNameExpression {
+                        type_name:
+                            TypeName::ElementaryTypeName(ElementaryTypeName {
+                                name: type_name, ..
+                            }),
+                        ..
+                    }) if type_name == "address" => match &arguments[0] {
+                        Expression::Literal(Literal {
+                            value: Some(value), ..
+                        }) if value == "0" => true,
+
+                        _ => false,
+                    },
+
+                    _ => false,
+                },
+
+                _ => false,
+            }
+        };
+
+        if check_expression(context.binary_operation.left_expression.as_ref())
+            || check_expression(context.binary_operation.right_expression.as_ref())
+        {
+            self.print_message(
+                context.contract_definition,
+                context.definition_node,
+                context
+                    .current_source_unit
+                    .source_line(context.binary_operation.src.as_str())?,
+                context.binary_operation,
+            );
+        }
+
+        Ok(())
+    }
+}

solast/src/analysis/assert_usage.rs

@@ -7,6 +7,22 @@ pub struct AssertUsageVisitor {
     reported_definitions: HashSet<NodeID>,
 }
 
+impl AssertUsageVisitor {
+    fn print_message(
+        &mut self,
+        contract_definition: &ContractDefinition,
+        definition_node: &ContractDefinitionNode,
+        source_line: usize,
+        expression: &dyn std::fmt::Display,
+    ) {
+        println!(
+            "\t{} contains assert usage: `{}`",
+            contract_definition.definition_node_location(source_line, definition_node),
+            expression,
+        );
+    }
+}
+
 impl AstVisitor for AssertUsageVisitor {
     fn visit_function_call<'a, 'b>(&mut self, context: &mut FunctionCallContext<'a, 'b>) -> std::io::Result<()> {
         //
@@ -47,38 +63,12 @@ impl AstVisitor for AssertUsageVisitor {
         // Print a message about the assert usage
         //
 
-        match context.definition_node {
-            ContractDefinitionNode::FunctionDefinition(function_definition) => println!(
-                "\tL{}: The {} {} in the `{}` {} contains `assert` usage",
-
-                context.current_source_unit.source_line(context.function_call.src.as_str())?,
-
-                function_definition.visibility,
-
-                if let FunctionKind::Constructor = function_definition.kind {
-                    "constructor".to_string()
-                } else {
-                    format!("`{}` {}", function_definition.name, function_definition.kind)
-                },
-
-                context.contract_definition.name,
-                context.contract_definition.kind
-            ),
-
-            ContractDefinitionNode::ModifierDefinition(modifier_definition) => println!(
-                "\tL{}: The {} `{}` modifier in the `{}` {} contains `assert` usage",
-
-                context.current_source_unit.source_line(context.function_call.src.as_str())?,
-
-                modifier_definition.visibility,
-                modifier_definition.name,
-
-                context.contract_definition.name,
-                context.contract_definition.kind
-            ),
-
-            _ => {}
-        }
+        self.print_message(
+            context.contract_definition,
+            context.definition_node,
+            context.current_source_unit.source_line(context.function_call.src.as_str())?,
+            context.function_call,
+        );
 
         Ok(())
     }

solast/src/analysis/assignment_comparisons.rs

@@ -12,45 +12,12 @@ impl AssignmentComparisonsVisitor {
         message: String,
         expression: &dyn std::fmt::Display
     ) {
-        match definition_node {
-            ContractDefinitionNode::FunctionDefinition(function_definition) => println!(
-                "\tL{}: The {} {} in the `{}` {} contains a {} that performs an assignment: `{}`",
-
-                source_line,
-
-                function_definition.visibility,
-
-                if let FunctionKind::Constructor = function_definition.kind {
-                    format!("{}", function_definition.kind)
-                } else {
-                    format!("`{}` {}", function_definition.name, function_definition.kind)
-                },
-
-                contract_definition.name,
-                contract_definition.kind,
-
-                message,
-
-                expression
-            ),
-
-            ContractDefinitionNode::ModifierDefinition(modifier_definition) => println!(
-                "\tL{}: The `{}` modifier in the `{}` {} contains a {} that performs an assignment: `{}`",
-
-                source_line,
-
-                modifier_definition.name,
-
-                contract_definition.name,
-                contract_definition.kind,
-
-                message,
-
-                expression
-            ),
-
-            _ => ()
-        }
+        println!(
+            "\t{} contains {} that performs an assignment: `{}`",
+            contract_definition.definition_node_location(source_line, definition_node),
+            message,
+            expression
+        );
     }
 }
 

solast/src/analysis/check_effects_interactions.rs

@@ -131,37 +131,10 @@ impl CheckEffectsInteractionsVisitor {
         definition_node: &ContractDefinitionNode,
         source_line: usize,
     ) {
-        match definition_node {
-            ContractDefinitionNode::FunctionDefinition(function_definition) => println!(
-                "\tL{}: The {} {} in the `{}` {} ignores the Check-Effects-Interactions pattern",
-
-                source_line,
-
-                function_definition.visibility,
-
-                if let FunctionKind::Constructor = function_definition.kind {
-                    format!("{}", function_definition.kind)
-                } else {
-                    format!("`{}` {}", function_definition.name, function_definition.kind)
-                },
-
-                contract_definition.name,
-                contract_definition.kind
-            ),
-
-            ContractDefinitionNode::ModifierDefinition(modifier_definition) => println!(
-                "\tL{}: The `{}` modifier in the `{}` {} ignores the Check-Effects-Interactions pattern",
-
-                source_line,
-
-                modifier_definition.name,
-
-                contract_definition.name,
-                contract_definition.kind
-            ),
-
-            _ => ()
-        }
+        println!(
+            "\t{} ignores the Check-Effects-Interactions pattern",
+            contract_definition.definition_node_location(source_line, definition_node),
+        );
     }
 }