Skip to content

Commit

Permalink
fix(neverAccessedVariables): False positive on contract constants
Browse files Browse the repository at this point in the history
These are not supported, so we have to remove the logic introduced here
after #22 is implemented.
  • Loading branch information
byakuren-hijiri committed Jul 26, 2024
1 parent af377e9 commit 82b03c4
Show file tree
Hide file tree
Showing 13 changed files with 226 additions and 163 deletions.
11 changes: 7 additions & 4 deletions src/detectors/builtin/neverAccessedVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,13 @@ export class NeverAccessedVariables extends Detector {
}

collectDefinedConstants(cu: CompilationUnit): Set<[ConstantName, ASTRef]> {
return Array.from(cu.ast.getConstants()).reduce((acc, constant) => {
acc.add([constant.name, constant.ref]);
return acc;
}, new Set<[ConstantName, ASTRef]>());
return Array.from(cu.ast.getConstants({ includeContract: false })).reduce(
(acc, constant) => {
acc.add([constant.name, constant.ref]);
return acc;
},
new Set<[ConstantName, ASTRef]>(),
);
}

/**
Expand Down
22 changes: 20 additions & 2 deletions src/internals/ir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export class TactASTStore {
/**
* Constructs a TactASTStore with mappings to all major AST components.
* @param stdlibIds Identifiers of AST elements defined in stdlib.
* @param contractConstants Identifiers of constants defined within contracts (XXX: remove after #22 is fixed)
* @param programEntries Identifiers of AST elements defined on the top-level.
* @param functions Functions and methods including user-defined and special methods.
* @param constants Constants defined across the compilation unit.
Expand All @@ -37,6 +38,7 @@ export class TactASTStore {
*/
constructor(
private stdlibIds = new Set<number>(),
private contractConstants = new Set<number>(),
private programEntries: Set<number>,
private functions: Map<number, ASTFunction | ASTReceive | ASTInitFunction>,
private constants: Map<number, ASTConstant>,
Expand Down Expand Up @@ -110,11 +112,27 @@ export class TactASTStore {
* and contract constants.
* @param params Additional parameters:
* - includeStdlib: If true, includes constants defined in stdlib.
* - includeContract: If true, includes contstants defined within a contract.
* XXX: This won't compile since Tact 1.4.1, remove it after #22 is merged
*/
getConstants(
params: Partial<{ includeStdlib: boolean }> = {},
params: Partial<{ includeStdlib: boolean; includeContract: boolean }> = {},
): IterableIterator<ASTConstant> {
return this.getItems(this.constants, params);
const { includeContract = false } = params;
const constants = this.getItems(this.constants, params);
function* filterIterator(
iterator: IterableIterator<ASTConstant>,
condition: (item: ASTConstant) => boolean,
): IterableIterator<ASTConstant> {
for (const item of iterator) {
if (condition(item)) {
yield item;
}
}
}
return includeContract
? constants
: filterIterator(constants, (c) => !this.contractConstants.has(c.id));
}

getContracts(): IterableIterator<ASTContract> {
Expand Down
3 changes: 3 additions & 0 deletions src/internals/tactIRBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ function hasSubdirs(filePath: string, subdirs: string[]): boolean {
export class ASTMapper {
private programEntries = new Set<number>();
private stdlibIds = new Set<number>();
private contractConstants = new Set<number>();
private functions = new Map<
number,
ASTFunction | ASTReceive | ASTInitFunction
Expand Down Expand Up @@ -149,6 +150,7 @@ export class ASTMapper {
public getASTStore(): TactASTStore {
return new TactASTStore(
this.stdlibIds,
this.contractConstants,
this.programEntries,
this.functions,
this.constants,
Expand Down Expand Up @@ -194,6 +196,7 @@ export class ASTMapper {
break;
case "def_constant":
this.constants.set(decl.id, decl);
this.contractConstants.add(decl.id);
break;
default:
throw new Error(`Unsupported contract declaration: ${decl}`);
Expand Down
25 changes: 25 additions & 0 deletions test/contracts/never-accessed-4.cfg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"projectName": "never-accessed-4",
"functions": [],
"contracts": [
{
"name": "ConstantFieldTest",
"methods": [
{
"name": "ConstantFieldTest.test",
"cfg": {
"nodes": [
{
"id": 1566,
"stmtID": 21139,
"srcEdges": [],
"dstEdges": []
}
],
"edges": []
}
}
]
}
]
}
1 change: 1 addition & 0 deletions test/contracts/never-accessed-4.expected.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

13 changes: 13 additions & 0 deletions test/contracts/never-accessed-4.tact
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// NOTE: This example won't compile after updating Tact to 1.4.1
contract ConstantFieldTest {
const val: Int = ton("0.01");
fun test() {
send(SendParameters{
to: sender(),
bounce: true,
value: self.val,
mode: SendRemainingValue + SendIgnoreErrors
});
}
}

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": 1566,
"stmtID": 21125,
"id": 1717,
"stmtID": 23340,
"srcEdges": [],
"dstEdges": [
1568
1719
]
},
{
"id": 1567,
"stmtID": 21128,
"id": 1718,
"stmtID": 23343,
"srcEdges": [
1568
1719
],
"dstEdges": [
1570
1721
]
},
{
"id": 1569,
"stmtID": 21131,
"id": 1720,
"stmtID": 23346,
"srcEdges": [
1570
1721
],
"dstEdges": [
1572
1723
]
},
{
"id": 1571,
"stmtID": 21138,
"id": 1722,
"stmtID": 23353,
"srcEdges": [
1572
1723
],
"dstEdges": [
1574,
1578
1725,
1729
]
},
{
"id": 1573,
"stmtID": 21137,
"id": 1724,
"stmtID": 23352,
"srcEdges": [
1574
1725
],
"dstEdges": [
1576
1727
]
},
{
"id": 1575,
"stmtID": 21140,
"id": 1726,
"stmtID": 23355,
"srcEdges": [
1576
1727
],
"dstEdges": []
},
{
"id": 1577,
"stmtID": 21140,
"id": 1728,
"stmtID": 23355,
"srcEdges": [
1578
1729
],
"dstEdges": []
}
],
"edges": [
{
"id": 1568,
"src": 1566,
"dst": 1567
"id": 1719,
"src": 1717,
"dst": 1718
},
{
"id": 1570,
"src": 1567,
"dst": 1569
"id": 1721,
"src": 1718,
"dst": 1720
},
{
"id": 1572,
"src": 1569,
"dst": 1571
"id": 1723,
"src": 1720,
"dst": 1722
},
{
"id": 1574,
"src": 1571,
"dst": 1573
"id": 1725,
"src": 1722,
"dst": 1724
},
{
"id": 1576,
"src": 1573,
"dst": 1575
"id": 1727,
"src": 1724,
"dst": 1726
},
{
"id": 1578,
"src": 1571,
"dst": 1577
"id": 1729,
"src": 1722,
"dst": 1728
}
]
}
Expand Down
32 changes: 16 additions & 16 deletions test/contracts/readonly-2.cfg.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,42 +6,42 @@
"cfg": {
"nodes": [
{
"id": 1728,
"stmtID": 23209,
"id": 1879,
"stmtID": 25424,
"srcEdges": [],
"dstEdges": [
1730
1881
]
},
{
"id": 1729,
"stmtID": 23212,
"id": 1880,
"stmtID": 25427,
"srcEdges": [
1730
1881
],
"dstEdges": [
1732
1883
]
},
{
"id": 1731,
"stmtID": 23214,
"id": 1882,
"stmtID": 25429,
"srcEdges": [
1732
1883
],
"dstEdges": []
}
],
"edges": [
{
"id": 1730,
"src": 1728,
"dst": 1729
"id": 1881,
"src": 1879,
"dst": 1880
},
{
"id": 1732,
"src": 1729,
"dst": 1731
"id": 1883,
"src": 1880,
"dst": 1882
}
]
}
Expand Down
Loading

0 comments on commit 82b03c4

Please sign in to comment.