Skip to content

Commit

Permalink
fix(iterators+neverAccessedVariables): Lvalue traversal
Browse files Browse the repository at this point in the history
Introduces a hack that must be fixed when #22 and #29 are finished
  • Loading branch information
byakuren-hijiri committed Jul 26, 2024
1 parent 7df805d commit af377e9
Show file tree
Hide file tree
Showing 15 changed files with 260 additions and 171 deletions.
2 changes: 1 addition & 1 deletion src/detectors/builtin/neverAccessedVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export class NeverAccessedVariables extends Detector {
).map(([_name, ref]) =>
createError(ctx, "Field is never used", Severity.MEDIUM, ref, {
docURL: makeDocURL(this.id),
suggestion: "Consider removing the field",
suggestion: "Consider creating a constant instead of field",
}),
);
}
Expand Down
70 changes: 67 additions & 3 deletions src/internals/tactASTUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,41 @@ import {
ASTNode,
ASTStatement,
ASTExpression,
ASTLvalueRef,
ASTId,
ASTOpField,
} from "@tact-lang/compiler/dist/grammar/ast";
import JSONbig from "json-bigint";

// XXX: Remove after #22 is finished
function createIdFromLvalue(lvalue: ASTLvalueRef): ASTId {
return {
kind: "id",
id: lvalue.id,
value: lvalue.name,
ref: lvalue.ref,
};
}

// Recursively create nested ASTOpField in the correct order
// XXX: Remove after #22 is finished
function createFieldFromLvalues(lvalues: ASTLvalueRef[]): ASTOpField {
if (lvalues.length === 0) {
throw new Error("The list of lvalues must not be empty.");
}
let currentField: ASTExpression = createIdFromLvalue(lvalues[0]);
for (let i = 1; i < lvalues.length; i++) {
currentField = {
kind: "op_field",
id: lvalues[i].id,
src: currentField,
name: lvalues[i].name,
ref: lvalues[i].ref,
};
}
return currentField as ASTOpField;
}

/**
* Recursively iterates over each expression in an ASTNode and applies a callback to each expression.
* @param node The node to traverse.
Expand All @@ -14,6 +46,18 @@ export function forEachExpression(
node: ASTNode,
callback: (expr: ASTExpression) => void,
): void {
/**
* Traversing lvalue as an op_field operation.
* XXX: Remove this after #22 is finished.
*/
function traverseLvalue(lvalues: ASTLvalueRef[]): void {
if (lvalues.length === 0) {
return;
}
const fieldOperation = createFieldFromLvalues(lvalues);
traverseExpression(fieldOperation);
}

function traverseExpression(expr: ASTExpression): void {
callback(expr);

Expand Down Expand Up @@ -46,12 +90,14 @@ export function forEachExpression(
traverseExpression(expr.thenBranch);
traverseExpression(expr.elseBranch);
break;
case "lvalue_ref":
// See: `traverseLvalue`
break;
case "string":
case "number":
case "boolean":
case "id":
case "null":
case "lvalue_ref":
// Primitives and non-composite expressions don't require further traversal
break;
default:
Expand All @@ -61,9 +107,12 @@ export function forEachExpression(

function traverseStatement(stmt: ASTStatement): void {
switch (stmt.kind) {
case "statement_let":
case "statement_assign":
case "statement_augmentedassign":
traverseLvalue(stmt.path);
traverseExpression(stmt.expression);
break;
case "statement_let":
case "statement_expression":
traverseExpression(stmt.expression);
break;
Expand Down Expand Up @@ -181,6 +230,18 @@ export function foldExpressions<T>(
acc: T,
callback: (acc: T, expr: ASTExpression) => T,
): T {
/**
* Traversing lvalue as an op_field operation.
* XXX: Remove this after #22 is finished.
*/
function traverseLvalue(acc: T, lvalues: ASTLvalueRef[]): T {
if (lvalues.length === 0) {
return acc;
}
const fieldOperation = createFieldFromLvalues(lvalues);
return traverseExpression(acc, fieldOperation);
}

function traverseExpression(acc: T, expr: ASTExpression): T {
acc = callback(acc, expr);

Expand Down Expand Up @@ -237,9 +298,12 @@ export function foldExpressions<T>(

function traverseStatement(acc: T, stmt: ASTStatement): T {
switch (stmt.kind) {
case "statement_let":
case "statement_assign":
case "statement_augmentedassign":
acc = traverseLvalue(acc, stmt.path);
acc = traverseExpression(acc, stmt.expression);
break;
case "statement_let":
case "statement_expression":
acc = traverseExpression(acc, stmt.expression);
break;
Expand Down
9 changes: 1 addition & 8 deletions test/contracts/never-accessed-1.expected.out
Original file line number Diff line number Diff line change
@@ -1,8 +1 @@
test/contracts/never-accessed-1.tact:2:5:
1 | fun test(): Int {
> 2 | let a: Int = 20;
^
3 | if (true) { // error: write-only variable
Write-only variable
Help: The variable value should be accessed
See: https://nowarp.github.io/docs/misti/docs/detectors/NeverAccessedVariables

2 changes: 1 addition & 1 deletion test/contracts/never-accessed-2.cfg.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"name": "FieldTest",
"methods": [
{
"name": "FieldTest.init",
"name": "FieldTest.init_16737",
"cfg": {
"nodes": [
{
Expand Down
2 changes: 1 addition & 1 deletion test/contracts/never-accessed-2.expected.out
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ test/contracts/never-accessed-2.tact:3:5:
^
4 |
Field is never used
Help: Consider removing the field
Help: Consider creating a constant instead of field
See: https://nowarp.github.io/docs/misti/docs/detectors/NeverAccessedVariables
25 changes: 25 additions & 0 deletions test/contracts/never-accessed-3.cfg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"projectName": "never-accessed-3",
"functions": [],
"contracts": [
{
"name": "FieldTest",
"methods": [
{
"name": "FieldTest.init_18927",
"cfg": {
"nodes": [
{
"id": 1415,
"stmtID": 18925,
"srcEdges": [],
"dstEdges": []
}
],
"edges": []
}
}
]
}
]
}
1 change: 1 addition & 0 deletions test/contracts/never-accessed-3.expected.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

6 changes: 6 additions & 0 deletions test/contracts/never-accessed-3.tact
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
contract FieldTest {
f1: Address;
init() {
self.f1 = sender();
}
}
88 changes: 44 additions & 44 deletions test/contracts/readonly-1.cfg.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,101 +6,101 @@
"cfg": {
"nodes": [
{
"id": 1415,
"stmtID": 18935,
"id": 1566,
"stmtID": 21125,
"srcEdges": [],
"dstEdges": [
1417
1568
]
},
{
"id": 1416,
"stmtID": 18938,
"id": 1567,
"stmtID": 21128,
"srcEdges": [
1417
1568
],
"dstEdges": [
1419
1570
]
},
{
"id": 1418,
"stmtID": 18941,
"id": 1569,
"stmtID": 21131,
"srcEdges": [
1419
1570
],
"dstEdges": [
1421
1572
]
},
{
"id": 1420,
"stmtID": 18948,
"id": 1571,
"stmtID": 21138,
"srcEdges": [
1421
1572
],
"dstEdges": [
1423,
1427
1574,
1578
]
},
{
"id": 1422,
"stmtID": 18947,
"id": 1573,
"stmtID": 21137,
"srcEdges": [
1423
1574
],
"dstEdges": [
1425
1576
]
},
{
"id": 1424,
"stmtID": 18950,
"id": 1575,
"stmtID": 21140,
"srcEdges": [
1425
1576
],
"dstEdges": []
},
{
"id": 1426,
"stmtID": 18950,
"id": 1577,
"stmtID": 21140,
"srcEdges": [
1427
1578
],
"dstEdges": []
}
],
"edges": [
{
"id": 1417,
"src": 1415,
"dst": 1416
"id": 1568,
"src": 1566,
"dst": 1567
},
{
"id": 1419,
"src": 1416,
"dst": 1418
"id": 1570,
"src": 1567,
"dst": 1569
},
{
"id": 1421,
"src": 1418,
"dst": 1420
"id": 1572,
"src": 1569,
"dst": 1571
},
{
"id": 1423,
"src": 1420,
"dst": 1422
"id": 1574,
"src": 1571,
"dst": 1573
},
{
"id": 1425,
"src": 1422,
"dst": 1424
"id": 1576,
"src": 1573,
"dst": 1575
},
{
"id": 1427,
"src": 1420,
"dst": 1426
"id": 1578,
"src": 1571,
"dst": 1577
}
]
}
Expand Down
Loading

0 comments on commit af377e9

Please sign in to comment.