-
Notifications
You must be signed in to change notification settings - Fork 11.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[move][move-linter] implement unnecessary while loop rules
- Loading branch information
Showing
8 changed files
with
217 additions
and
151 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
109 changes: 0 additions & 109 deletions
109
external-crates/move/crates/move-compiler/src/linters/shift_overflow.rs
This file was deleted.
Oops, something went wrong.
78 changes: 78 additions & 0 deletions
78
external-crates/move/crates/move-compiler/src/linters/unnecessary_while_loop.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
//! Encourages replacing `while(true)` with `loop` for infinite loops in Move for clarity and conciseness. | ||
//! Identifies `while(true)` patterns, suggesting a more idiomatic approach using `loop`. | ||
//! Aims to enhance code readability and adherence to Rust idioms. | ||
use crate::{ | ||
diag, | ||
diagnostics::{ | ||
codes::{custom, DiagnosticInfo, Severity}, | ||
WarningFilters, | ||
}, | ||
expansion::ast::Value_, | ||
shared::CompilationEnv, | ||
typing::{ | ||
ast::{self as T, UnannotatedExp_}, | ||
visitor::{TypingVisitorConstructor, TypingVisitorContext}, | ||
}, | ||
}; | ||
use move_ir_types::location::Loc; | ||
|
||
use super::{LinterDiagnosticCategory, LINT_WARNING_PREFIX, WHILE_TRUE_TO_LOOP_DIAG_CODE}; | ||
|
||
const WHILE_TRUE_TO_LOOP_DIAG: DiagnosticInfo = custom( | ||
LINT_WARNING_PREFIX, | ||
Severity::Warning, | ||
LinterDiagnosticCategory::Complexity as u8, | ||
WHILE_TRUE_TO_LOOP_DIAG_CODE, | ||
"", | ||
); | ||
|
||
pub struct WhileTrueToLoop; | ||
|
||
pub struct Context<'a> { | ||
env: &'a mut CompilationEnv, | ||
} | ||
|
||
impl TypingVisitorConstructor for WhileTrueToLoop { | ||
type Context<'a> = Context<'a>; | ||
|
||
fn context<'a>(env: &'a mut CompilationEnv, _program: &T::Program) -> Self::Context<'a> { | ||
Context { env } | ||
} | ||
} | ||
|
||
impl TypingVisitorContext for Context<'_> { | ||
fn add_warning_filter_scope(&mut self, filter: WarningFilters) { | ||
self.env.add_warning_filter_scope(filter) | ||
} | ||
fn pop_warning_filter_scope(&mut self) { | ||
self.env.pop_warning_filter_scope() | ||
} | ||
|
||
fn visit_exp_custom(&mut self, exp: &mut T::Exp) -> bool { | ||
if let UnannotatedExp_::While(_, cond, _) = &exp.exp.value { | ||
if is_condition_always_true(&cond.exp.value) { | ||
report_while_true_to_loop(self.env, exp.exp.loc); | ||
} | ||
} | ||
false | ||
} | ||
} | ||
|
||
fn is_condition_always_true(condition: &UnannotatedExp_) -> bool { | ||
if let UnannotatedExp_::Value(val) = condition { | ||
if let Value_::Bool(b) = &val.value { | ||
return *b; | ||
} | ||
} | ||
false | ||
} | ||
fn report_while_true_to_loop(env: &mut CompilationEnv, loc: Loc) { | ||
let diag = diag!( | ||
WHILE_TRUE_TO_LOOP_DIAG, | ||
( | ||
loc, | ||
"Detected `while(true) {}` loop. Consider replacing with `loop {}`" | ||
) | ||
); | ||
env.add_diag(diag); | ||
} |
24 changes: 0 additions & 24 deletions
24
external-crates/move/crates/move-compiler/tests/custom_rules/shift_overflow.exp
This file was deleted.
Oops, something went wrong.
9 changes: 0 additions & 9 deletions
9
external-crates/move/crates/move-compiler/tests/custom_rules/shift_overflow.move
This file was deleted.
Oops, something went wrong.
50 changes: 50 additions & 0 deletions
50
external-crates/move/crates/move-compiler/tests/linter/incorrect_unnecessary_while_loop.exp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
warning[Lint W01004]: | ||
┌─ tests/linter/incorrect_unnecessary_while_loop.move:6:9 | ||
│ | ||
6 │ ╭ while (true) { | ||
7 │ │ // This loop will run forever | ||
8 │ │ } | ||
│ ╰─────────^ Detected `while(true) {}` loop. Consider replacing with `loop {}` | ||
│ | ||
= This warning can be suppressed with '#[allow(lint(while_true_to_loop))]' applied to the 'module' or module member ('const', 'fun', or 'struct') | ||
|
||
warning[Lint W01004]: | ||
┌─ tests/linter/incorrect_unnecessary_while_loop.move:14:9 | ||
│ | ||
14 │ ╭ while (true) { | ||
15 │ │ if (counter == 10) { | ||
16 │ │ break | ||
17 │ │ }; | ||
18 │ │ counter = counter + 1; | ||
19 │ │ } | ||
│ ╰─────────^ Detected `while(true) {}` loop. Consider replacing with `loop {}` | ||
│ | ||
= This warning can be suppressed with '#[allow(lint(while_true_to_loop))]' applied to the 'module' or module member ('const', 'fun', or 'struct') | ||
|
||
warning[Lint W01004]: | ||
┌─ tests/linter/incorrect_unnecessary_while_loop.move:43:9 | ||
│ | ||
43 │ ╭ while (true) { | ||
44 │ │ if (vector::length(&vec1) == 5) break; | ||
45 │ │ vector::push_back(&mut vec1, vector::length(&vec1)); | ||
46 │ │ }; | ||
│ ╰─────────^ Detected `while(true) {}` loop. Consider replacing with `loop {}` | ||
│ | ||
= This warning can be suppressed with '#[allow(lint(while_true_to_loop))]' applied to the 'module' or module member ('const', 'fun', or 'struct') | ||
|
||
error[E07001]: referential transparency violated | ||
┌─ tests/linter/incorrect_unnecessary_while_loop.move:45:57 | ||
│ | ||
45 │ vector::push_back(&mut vec1, vector::length(&vec1)); | ||
│ --------- ^^^^^ Invalid borrow of variable 'vec1' | ||
│ │ | ||
│ It is still being mutably borrowed by this reference | ||
|
||
error[E07001]: referential transparency violated | ||
┌─ tests/linter/incorrect_unnecessary_while_loop.move:51:57 | ||
│ | ||
51 │ vector::push_back(&mut vec2, vector::length(&vec2)); | ||
│ --------- ^^^^^ Invalid borrow of variable 'vec2' | ||
│ │ | ||
│ It is still being mutably borrowed by this reference | ||
|
54 changes: 54 additions & 0 deletions
54
external-crates/move/crates/move-compiler/tests/linter/incorrect_unnecessary_while_loop.move
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
module 0x42::loop_test { | ||
use std::vector; | ||
|
||
// This function should trigger the linter | ||
public fun infinite_loop() { | ||
while (true) { | ||
// This loop will run forever | ||
} | ||
} | ||
|
||
// This function should also trigger the linter | ||
public fun finite_loop() { | ||
let counter = 0; | ||
while (true) { | ||
if (counter == 10) { | ||
break | ||
}; | ||
counter = counter + 1; | ||
} | ||
} | ||
|
||
// This function should not trigger the linter | ||
public fun correct_infinite_loop() { | ||
loop { | ||
// This is the correct way to write an infinite loop | ||
} | ||
} | ||
|
||
// This function should not trigger the linter | ||
public fun while_with_condition(n: u64) { | ||
let i = 0; | ||
while (i < n) { | ||
i = i + 1; | ||
} | ||
} | ||
|
||
// This function uses both `while(true)` and `loop` for comparison | ||
public fun mixed_loops() { | ||
let vec1 = vector::empty<u64>(); | ||
let vec2 = vector::empty<u64>(); | ||
|
||
// This should trigger the linter | ||
while (true) { | ||
if (vector::length(&vec1) == 5) break; | ||
vector::push_back(&mut vec1, vector::length(&vec1)); | ||
}; | ||
|
||
// This should not trigger the linter | ||
loop { | ||
if (vector::length(&vec2) == 5) break; | ||
vector::push_back(&mut vec2, vector::length(&vec2)); | ||
}; | ||
} | ||
} |
Oops, something went wrong.