Skip to content

Commit 78f86c3

Browse files
grandizzygrandizzy
authored
chore(coverage): cleanup creation / push item (#8616)
1 parent fbdd40d commit 78f86c3

File tree

1 file changed

+43
-104
lines changed

1 file changed

+43
-104
lines changed

‎crates/evm/coverage/src/analysis.rs‎

Lines changed: 43 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,7 @@ impl<'a> ContractVisitor<'a> {
5959

6060
match &node.body {
6161
Some(body) => {
62-
self.push_item(CoverageItem {
63-
kind: CoverageItemKind::Function { name },
64-
loc: self.source_location_for(&node.src),
65-
hits: 0,
66-
});
62+
self.push_item_kind(CoverageItemKind::Function { name }, &node.src);
6763
self.visit_block(body)
6864
}
6965
_ => Ok(()),
@@ -76,11 +72,7 @@ impl<'a> ContractVisitor<'a> {
7672

7773
match &node.body {
7874
Some(body) => {
79-
self.push_item(CoverageItem {
80-
kind: CoverageItemKind::Function { name },
81-
loc: self.source_location_for(&node.src),
82-
hits: 0,
83-
});
75+
self.push_item_kind(CoverageItemKind::Function { name }, &node.src);
8476
self.visit_block(body)
8577
}
8678
_ => Ok(()),
@@ -119,34 +111,22 @@ impl<'a> ContractVisitor<'a> {
119111
NodeType::YulContinue |
120112
NodeType::YulLeave |
121113
NodeType::YulVariableDeclaration => {
122-
self.push_item(CoverageItem {
123-
kind: CoverageItemKind::Statement,
124-
loc: self.source_location_for(&node.src),
125-
hits: 0,
126-
});
114+
self.push_item_kind(CoverageItemKind::Statement, &node.src);
127115
Ok(())
128116
}
129117
// Skip placeholder statements as they are never referenced in source maps.
130118
NodeType::PlaceholderStatement => Ok(()),
131119
// Return with eventual subcall
132120
NodeType::Return => {
133-
self.push_item(CoverageItem {
134-
kind: CoverageItemKind::Statement,
135-
loc: self.source_location_for(&node.src),
136-
hits: 0,
137-
});
121+
self.push_item_kind(CoverageItemKind::Statement, &node.src);
138122
if let Some(expr) = node.attribute("expression") {
139123
self.visit_expression(&expr)?;
140124
}
141125
Ok(())
142126
}
143127
// Variable declaration
144128
NodeType::VariableDeclarationStatement => {
145-
self.push_item(CoverageItem {
146-
kind: CoverageItemKind::Statement,
147-
loc: self.source_location_for(&node.src),
148-
hits: 0,
149-
});
129+
self.push_item_kind(CoverageItemKind::Statement, &node.src);
150130
if let Some(expr) = node.attribute("initialValue") {
151131
self.visit_expression(&expr)?;
152132
}
@@ -225,31 +205,29 @@ impl<'a> ContractVisitor<'a> {
225205
// If this source range is not processed like this, it is virtually
226206
// impossible to correctly map instructions back to branches that
227207
// include more complex logic like conditional logic.
228-
self.push_item(CoverageItem {
229-
kind: CoverageItemKind::Branch { branch_id, path_id: 0 },
230-
loc: self.source_location_for(&ast::LowFidelitySourceLocation {
208+
self.push_item_kind(
209+
CoverageItemKind::Branch { branch_id, path_id: 0 },
210+
&ast::LowFidelitySourceLocation {
231211
start: node.src.start,
232212
length: true_body.src.length.map(|length| {
233213
true_body.src.start - node.src.start + length
234214
}),
235215
index: node.src.index,
236-
}),
237-
hits: 0,
238-
});
216+
},
217+
);
239218
// Add the coverage item for branch 1 (false body).
240219
// The relevant source range for the false branch is the `else`
241220
// statement itself and the false body of the else statement.
242-
self.push_item(CoverageItem {
243-
kind: CoverageItemKind::Branch { branch_id, path_id: 1 },
244-
loc: self.source_location_for(&ast::LowFidelitySourceLocation {
221+
self.push_item_kind(
222+
CoverageItemKind::Branch { branch_id, path_id: 1 },
223+
&ast::LowFidelitySourceLocation {
245224
start: node.src.start,
246225
length: false_body.src.length.map(|length| {
247226
false_body.src.start - true_body.src.start + length
248227
}),
249228
index: node.src.index,
250-
}),
251-
hits: 0,
252-
});
229+
},
230+
);
253231

254232
// Process the true body.
255233
self.visit_block_or_statement(&true_body)?;
@@ -261,11 +239,10 @@ impl<'a> ContractVisitor<'a> {
261239
// Add single branch coverage only if it contains statements.
262240
if has_statements(&true_body) {
263241
// Add the coverage item for branch 0 (true body).
264-
self.push_item(CoverageItem {
265-
kind: CoverageItemKind::SinglePathBranch { branch_id },
266-
loc: self.source_location_for(&true_body.src),
267-
hits: 0,
268-
});
242+
self.push_item_kind(
243+
CoverageItemKind::SinglePathBranch { branch_id },
244+
&true_body.src,
245+
);
269246
// Process the true body.
270247
self.visit_block_or_statement(&true_body)?;
271248
}
@@ -293,11 +270,7 @@ impl<'a> ContractVisitor<'a> {
293270
// branch ID as we do
294271
self.branch_id += 1;
295272

296-
self.push_item(CoverageItem {
297-
kind: CoverageItemKind::Branch { branch_id, path_id: 0 },
298-
loc: self.source_location_for(&node.src),
299-
hits: 0,
300-
});
273+
self.push_item_kind(CoverageItemKind::Branch { branch_id, path_id: 0 }, &node.src);
301274
self.visit_block(body)?;
302275

303276
Ok(())
@@ -317,21 +290,13 @@ impl<'a> ContractVisitor<'a> {
317290
.ok_or_else(|| eyre::eyre!("try statement had no clause"))?
318291
{
319292
// Add coverage for clause statement.
320-
self.push_item(CoverageItem {
321-
kind: CoverageItemKind::Statement,
322-
loc: self.source_location_for(&clause.src),
323-
hits: 0,
324-
});
293+
self.push_item_kind(CoverageItemKind::Statement, &clause.src);
325294
self.visit_statement(&clause)?;
326295

327296
// Add coverage for clause body only if it is not empty.
328297
if let Some(block) = clause.attribute::<Node>("block") {
329298
if has_statements(&block) {
330-
self.push_item(CoverageItem {
331-
kind: CoverageItemKind::Statement,
332-
loc: self.source_location_for(&block.src),
333-
hits: 0,
334-
});
299+
self.push_item_kind(CoverageItemKind::Statement, &block.src);
335300
self.visit_block(&block)?;
336301
}
337302
}
@@ -345,19 +310,11 @@ impl<'a> ContractVisitor<'a> {
345310
.attribute::<Vec<Node>>("cases")
346311
.ok_or_else(|| eyre::eyre!("yul switch had no case"))?
347312
{
348-
self.push_item(CoverageItem {
349-
kind: CoverageItemKind::Statement,
350-
loc: self.source_location_for(&case.src),
351-
hits: 0,
352-
});
313+
self.push_item_kind(CoverageItemKind::Statement, &case.src);
353314
self.visit_statement(&case)?;
354315

355316
if let Some(body) = case.body {
356-
self.push_item(CoverageItem {
357-
kind: CoverageItemKind::Statement,
358-
loc: self.source_location_for(&body.src),
359-
hits: 0,
360-
});
317+
self.push_item_kind(CoverageItemKind::Statement, &body.src);
361318
self.visit_block(&body)?
362319
}
363320
}
@@ -375,11 +332,7 @@ impl<'a> ContractVisitor<'a> {
375332
}
376333

377334
if let Some(body) = &node.body {
378-
self.push_item(CoverageItem {
379-
kind: CoverageItemKind::Statement,
380-
loc: self.source_location_for(&body.src),
381-
hits: 0,
382-
});
335+
self.push_item_kind(CoverageItemKind::Statement, &body.src);
383336
self.visit_block(body)?
384337
}
385338
Ok(())
@@ -398,23 +351,15 @@ impl<'a> ContractVisitor<'a> {
398351
NodeType::UnaryOperation |
399352
NodeType::Conditional |
400353
NodeType::YulFunctionCall => {
401-
self.push_item(CoverageItem {
402-
kind: CoverageItemKind::Statement,
403-
loc: self.source_location_for(&node.src),
404-
hits: 0,
405-
});
354+
self.push_item_kind(CoverageItemKind::Statement, &node.src);
406355
Ok(())
407356
}
408357
NodeType::FunctionCall => {
409358
// Do not count other kinds of calls towards coverage (like `typeConversion`
410359
// and `structConstructorCall`).
411360
let kind: Option<String> = node.attribute("kind");
412361
if let Some("functionCall") = kind.as_deref() {
413-
self.push_item(CoverageItem {
414-
kind: CoverageItemKind::Statement,
415-
loc: self.source_location_for(&node.src),
416-
hits: 0,
417-
});
362+
self.push_item_kind(CoverageItemKind::Statement, &node.src);
418363

419364
let expr: Option<Node> = node.attribute("expression");
420365
if let Some(NodeType::Identifier) = expr.as_ref().map(|expr| &expr.node_type) {
@@ -423,28 +368,22 @@ impl<'a> ContractVisitor<'a> {
423368
if let Some("require" | "assert") = name.as_deref() {
424369
let branch_id = self.branch_id;
425370
self.branch_id += 1;
426-
self.push_item(CoverageItem {
427-
kind: CoverageItemKind::Branch { branch_id, path_id: 0 },
428-
loc: self.source_location_for(&node.src),
429-
hits: 0,
430-
});
431-
self.push_item(CoverageItem {
432-
kind: CoverageItemKind::Branch { branch_id, path_id: 1 },
433-
loc: self.source_location_for(&node.src),
434-
hits: 0,
435-
});
371+
self.push_item_kind(
372+
CoverageItemKind::Branch { branch_id, path_id: 0 },
373+
&node.src,
374+
);
375+
self.push_item_kind(
376+
CoverageItemKind::Branch { branch_id, path_id: 1 },
377+
&node.src,
378+
);
436379
}
437380
}
438381
}
439382

440383
Ok(())
441384
}
442385
NodeType::BinaryOperation => {
443-
self.push_item(CoverageItem {
444-
kind: CoverageItemKind::Statement,
445-
loc: self.source_location_for(&node.src),
446-
hits: 0,
447-
});
386+
self.push_item_kind(CoverageItemKind::Statement, &node.src);
448387

449388
// visit left and right expressions
450389
// There could possibly a function call in the left or right expression
@@ -500,24 +439,24 @@ impl<'a> ContractVisitor<'a> {
500439
}
501440
}
502441

503-
/// Pushes a coverage item to the internal collection, and might push a line item as well.
504-
fn push_item(&mut self, item: CoverageItem) {
505-
let source_location = &item.loc;
506-
442+
/// Creates a coverage item for a given kind and source location. Pushes item to the internal
443+
/// collection (plus additional coverage line if item is a statement).
444+
fn push_item_kind(&mut self, kind: CoverageItemKind, src: &ast::LowFidelitySourceLocation) {
445+
let item = CoverageItem { kind, loc: self.source_location_for(src), hits: 0 };
507446
// Push a line item if we haven't already
508447
if matches!(
509448
item.kind,
510449
CoverageItemKind::Statement |
511450
CoverageItemKind::Branch { .. } |
512451
CoverageItemKind::SinglePathBranch { .. }
513-
) && self.last_line < source_location.line
452+
) && self.last_line < item.loc.line
514453
{
515454
self.items.push(CoverageItem {
516455
kind: CoverageItemKind::Line,
517-
loc: source_location.clone(),
456+
loc: item.loc.clone(),
518457
hits: 0,
519458
});
520-
self.last_line = source_location.line;
459+
self.last_line = item.loc.line;
521460
}
522461

523462
self.items.push(item);

0 commit comments

Comments
 (0)