Skip to content

Commit

Permalink
redundant statements
Browse files Browse the repository at this point in the history
  • Loading branch information
TilakMaddy committed Jul 27, 2024
1 parent 2dbaa48 commit babda66
Show file tree
Hide file tree
Showing 8 changed files with 360 additions and 14 deletions.
13 changes: 9 additions & 4 deletions aderyn_core/src/detect/detector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ use crate::{
ContractsWithTodosDetector, DeprecatedOZFunctionsDetector,
DivisionBeforeMultiplicationDetector, EcrecoverDetector, EmptyBlockDetector,
InconsistentTypeNamesDetector, LargeLiteralValueDetector,
NonReentrantBeforeOthersDetector, PushZeroOpcodeDetector, RequireWithStringDetector,
RevertsAndRequiresInLoopsDetector, SolmateSafeTransferLibDetector,
UnindexedEventsDetector, UnsafeERC20FunctionsDetector, UnsafeERC721MintDetector,
UnspecificSolidityPragmaDetector, UselessErrorDetector,
NonReentrantBeforeOthersDetector, PushZeroOpcodeDetector, RedundantStatementsDetector,
RequireWithStringDetector, RevertsAndRequiresInLoopsDetector,
SolmateSafeTransferLibDetector, UnindexedEventsDetector, UnsafeERC20FunctionsDetector,
UnsafeERC721MintDetector, UnspecificSolidityPragmaDetector, UselessErrorDetector,
UselessInternalFunctionDetector, UselessModifierDetector,
UselessPublicFunctionDetector, ZeroAddressCheckDetector,
},
Expand Down Expand Up @@ -87,6 +87,7 @@ pub fn get_all_issue_detectors() -> Vec<Box<dyn IssueDetector>> {
Box::<RTLODetector>::default(),
Box::<UncheckedReturnDetector>::default(),
Box::<DangerousUnaryOperatorDetector>::default(),
Box::<RedundantStatementsDetector>::default(),
]
}

Expand Down Expand Up @@ -145,6 +146,7 @@ pub(crate) enum IssueDetectorNamePool {
RTLO,
UncheckedReturn,
DangerousUnaryOperator,
RedundantStatements,
// NOTE: `Undecided` will be the default name (for new bots).
// If it's accepted, a new variant will be added to this enum before normalizing it in aderyn
Undecided,
Expand Down Expand Up @@ -273,6 +275,9 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option<Box<dyn Iss
IssueDetectorNamePool::DangerousUnaryOperator => {
Some(Box::<DangerousUnaryOperatorDetector>::default())
}
IssueDetectorNamePool::RedundantStatements => {
Some(Box::<RedundantStatementsDetector>::default())
}
IssueDetectorNamePool::Undecided => None,
}
}
Expand Down
2 changes: 2 additions & 0 deletions aderyn_core/src/detect/low/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub(crate) mod inconsistent_type_names;
pub(crate) mod large_literal_value;
pub(crate) mod non_reentrant_before_others;
pub(crate) mod push_0_opcode;
pub(crate) mod redundant_statements;
pub(crate) mod require_with_string;
pub(crate) mod reverts_and_requries_in_loops;
pub(crate) mod solmate_safe_transfer_lib;
Expand All @@ -33,6 +34,7 @@ pub use inconsistent_type_names::InconsistentTypeNamesDetector;
pub use large_literal_value::LargeLiteralValueDetector;
pub use non_reentrant_before_others::NonReentrantBeforeOthersDetector;
pub use push_0_opcode::PushZeroOpcodeDetector;
pub use redundant_statements::RedundantStatementsDetector;
pub use require_with_string::RequireWithStringDetector;
pub use reverts_and_requries_in_loops::RevertsAndRequiresInLoopsDetector;
pub use solmate_safe_transfer_lib::SolmateSafeTransferLibDetector;
Expand Down
109 changes: 109 additions & 0 deletions aderyn_core/src/detect/low/redundant_statements.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
use std::collections::BTreeMap;
use std::error::Error;

use crate::ast::{Expression, NodeID, NodeType};

use crate::capture;
use crate::context::browser::GetImmediateParent;
use crate::detect::detector::IssueDetectorNamePool;
use crate::{
context::workspace_context::WorkspaceContext,
detect::detector::{IssueDetector, IssueSeverity},
};
use eyre::Result;

// HOW TO USE THIS TEMPLATE:
// 1. Copy this file and rename it to the snake_case version of the issue you are detecting.
// 2. Rename the RedundantStatementsDetector struct and impl to your new issue name.
// 3. Add this file and detector struct to the mod.rs file in the same directory.
// 4. Implement the detect function to find instances of the issue.

#[derive(Default)]
pub struct RedundantStatementsDetector {
// Keys are: [0] source file name, [1] line number, [2] character location of node.
// Do not add items manually, use `capture!` to add nodes to this BTreeMap.
found_instances: BTreeMap<(String, usize, String), NodeID>,
}

impl IssueDetector for RedundantStatementsDetector {
fn detect(&mut self, context: &WorkspaceContext) -> Result<bool, Box<dyn Error>> {
for expression_statement in context.expression_statements() {
if let Some(parent) = expression_statement.expression.parent(context) {
if parent.node_type() != NodeType::Block {
continue;
}

match &expression_statement.expression {
Expression::Identifier(identifier) => {
capture!(self, context, identifier);
}
Expression::ElementaryTypeNameExpression(elementary_type_expression) => {
capture!(self, context, elementary_type_expression);
}
_ => (),
};
}
}

Ok(!self.found_instances.is_empty())
}

fn severity(&self) -> IssueSeverity {
IssueSeverity::Low
}

fn title(&self) -> String {
String::from("Redundant statements have no effect.")
}

fn description(&self) -> String {
String::from("Remove the redundant statements because no code will be generated and it just congests the codebase.")
}

fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> {
self.found_instances.clone()
}

fn name(&self) -> String {
IssueDetectorNamePool::RedundantStatements.to_string()
}
}

#[cfg(test)]
mod redundant_statements_detector {
use crate::detect::{
detector::IssueDetector, low::redundant_statements::RedundantStatementsDetector,
};

#[test]
fn test_redundant_statements() {
let context = crate::detect::test_utils::load_solidity_source_unit(
"../tests/contract-playground/src/RedundantStatements.sol",
);

let mut detector = RedundantStatementsDetector::default();
let found = detector.detect(&context).unwrap();
// assert that the detector found an issue
assert!(found);

println!("{:#?}", detector.instances());

// assert that the detector found the correct number of instances
assert_eq!(detector.instances().len(), 6);
// assert the severity is low
assert_eq!(
detector.severity(),
crate::detect::detector::IssueSeverity::Low
);
// assert the title is correct
assert_eq!(
detector.title(),
String::from("Redundant statements have no effect.")
);
// assert the description is correct
assert_eq!(
detector.description(),
String::from("Remove the redundant statements because no code will be generated and it just congests the codebase.")
);
}
}
62 changes: 58 additions & 4 deletions reports/report.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"files_summary": {
"total_source_units": 60,
"total_sloc": 1620
"total_source_units": 61,
"total_sloc": 1634
},
"files_details": {
"files_details": [
Expand Down Expand Up @@ -105,6 +105,10 @@
"file_path": "src/RTLO.sol",
"n_sloc": 7
},
{
"file_path": "src/RedundantStatements.sol",
"n_sloc": 14
},
{
"file_path": "src/RevertsAndRequriesInLoops.sol",
"n_sloc": 27
Expand Down Expand Up @@ -249,7 +253,7 @@
},
"issue_count": {
"high": 23,
"low": 23
"low": 24
},
"high_issues": {
"issues": [
Expand Down Expand Up @@ -1664,6 +1668,12 @@
"src": "0:24",
"src_char": "0:24"
},
{
"contract_path": "src/RedundantStatements.sol",
"line_no": 2,
"src": "32:23",
"src_char": "32:23"
},
{
"contract_path": "src/UsingSelfdestruct.sol",
"line_no": 2,
Expand Down Expand Up @@ -2900,6 +2910,49 @@
"src_char": "541:5"
}
]
},
{
"title": "Redundant statements have no effect.",
"description": "Remove the redundant statements because no code will be generated and it just congests the codebase.",
"detector_name": "redundant-statements",
"instances": [
{
"contract_path": "src/RedundantStatements.sol",
"line_no": 6,
"src": "131:4",
"src_char": "131:4"
},
{
"contract_path": "src/RedundantStatements.sol",
"line_no": 7,
"src": "169:4",
"src_char": "169:4"
},
{
"contract_path": "src/RedundantStatements.sol",
"line_no": 8,
"src": "207:27",
"src_char": "207:27"
},
{
"contract_path": "src/RedundantStatements.sol",
"line_no": 12,
"src": "309:4",
"src_char": "309:4"
},
{
"contract_path": "src/RedundantStatements.sol",
"line_no": 13,
"src": "347:6",
"src_char": "347:6"
},
{
"contract_path": "src/RedundantStatements.sol",
"line_no": 14,
"src": "377:4",
"src_char": "377:4"
}
]
}
]
},
Expand Down Expand Up @@ -2949,6 +3002,7 @@
"tautological-compare",
"rtlo",
"unchecked-return",
"dangerous-unary-operator"
"dangerous-unary-operator",
"redundant-statements"
]
}
65 changes: 60 additions & 5 deletions reports/report.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
- [L-21: Unused Custom Error](#l-21-unused-custom-error)
- [L-22: Loop contains `require`/`revert` statements](#l-22-loop-contains-requirerevert-statements)
- [L-23: Incorrect Order of Division and Multiplication](#l-23-incorrect-order-of-division-and-multiplication)
- [L-24: Redundant statements have no effect.](#l-24-redundant-statements-have-no-effect)


# Summary
Expand All @@ -63,8 +64,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati

| Key | Value |
| --- | --- |
| .sol Files | 60 |
| Total nSLOC | 1620 |
| .sol Files | 61 |
| Total nSLOC | 1634 |


## Files Details
Expand Down Expand Up @@ -96,6 +97,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| src/MultipleConstructorSchemes.sol | 10 |
| src/OnceModifierExample.sol | 8 |
| src/RTLO.sol | 7 |
| src/RedundantStatements.sol | 14 |
| src/RevertsAndRequriesInLoops.sol | 27 |
| src/StateShadowing.sol | 17 |
| src/StateVariables.sol | 58 |
Expand Down Expand Up @@ -131,15 +133,15 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| src/reused_contract_name/ContractB.sol | 7 |
| src/uniswap/UniswapV2Swapper.sol | 50 |
| src/uniswap/UniswapV3Swapper.sol | 150 |
| **Total** | **1620** |
| **Total** | **1634** |


## Issue Summary

| Category | No. of Issues |
| --- | --- |
| High | 23 |
| Low | 23 |
| Low | 24 |


# High Issues
Expand Down Expand Up @@ -1639,7 +1641,7 @@ ERC20 functions may not behave as expected. For example: return values are not a

Consider using a specific version of Solidity in your contracts instead of a wide version. For example, instead of `pragma solidity ^0.8.0;`, use `pragma solidity 0.8.0;`

<details><summary>12 Found Instances</summary>
<details><summary>13 Found Instances</summary>


- Found in src/ContractWithTodo.sol [Line: 2](../tests/contract-playground/src/ContractWithTodo.sol#L2)
Expand Down Expand Up @@ -1672,6 +1674,12 @@ Consider using a specific version of Solidity in your contracts instead of a wid
pragma solidity ^0.8.24;
```

- Found in src/RedundantStatements.sol [Line: 2](../tests/contract-playground/src/RedundantStatements.sol#L2)

```solidity
pragma solidity ^0.4.0;
```

- Found in src/UsingSelfdestruct.sol [Line: 2](../tests/contract-playground/src/UsingSelfdestruct.sol#L2)

```solidity
Expand Down Expand Up @@ -2973,3 +2981,50 @@ Division operations followed directly by multiplication operations can lead to p



## L-24: Redundant statements have no effect.

Remove the redundant statements because no code will be generated and it just congests the codebase.

<details><summary>6 Found Instances</summary>


- Found in src/RedundantStatements.sol [Line: 6](../tests/contract-playground/src/RedundantStatements.sol#L6)

```solidity
uint; // Elementary Type Name
```

- Found in src/RedundantStatements.sol [Line: 7](../tests/contract-playground/src/RedundantStatements.sol#L7)

```solidity
bool; // Elementary Type Name
```

- Found in src/RedundantStatements.sol [Line: 8](../tests/contract-playground/src/RedundantStatements.sol#L8)

```solidity
RedundantStatementsContract; // Identifier
```

- Found in src/RedundantStatements.sol [Line: 12](../tests/contract-playground/src/RedundantStatements.sol#L12)

```solidity
uint; // Elementary Type Name
```

- Found in src/RedundantStatements.sol [Line: 13](../tests/contract-playground/src/RedundantStatements.sol#L13)

```solidity
assert; // Identifier
```

- Found in src/RedundantStatements.sol [Line: 14](../tests/contract-playground/src/RedundantStatements.sol#L14)

```solidity
test; // Identifier
```

</details>



Loading

0 comments on commit babda66

Please sign in to comment.