Skip to content

Commit

Permalink
feat(lints): Improved error reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
byakuren-hijiri committed Jul 23, 2024
1 parent 9da695f commit 9d032c4
Show file tree
Hide file tree
Showing 13 changed files with 67 additions and 26 deletions.
14 changes: 12 additions & 2 deletions src/detectors/builtin/divideBeforeMultiply.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ import {
import { Detector } from "../detector";
import { CompilationUnit, Node, CFG } from "../../internals/ir";
import { MistiContext } from "../../internals/context";
import { createError, MistiTactError, Severity } from "../../internals/errors";
import {
createError,
makeDocURL,
MistiTactError,
Severity,
} from "../../internals/errors";
import {
forEachExpression,
foldExpressions,
Expand Down Expand Up @@ -86,9 +91,14 @@ export class DivideBeforeMultiply extends Detector {
}
reportedDivIds.add(divId);
const err = createError(
"Division operation before multiplication detected. Consider rearranging the operations.",
"Divide Before Multiply",
Severity.HIGH,
fact.data,
{
docURL: makeDocURL(this.id),
suggestion:
"Consider rearranging the operations: division should follow multiplication",
},
);
acc.push(err);
return acc;
Expand Down
15 changes: 11 additions & 4 deletions src/detectors/builtin/neverAccessedVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,10 @@ export class NeverAccessedVariables extends Detector {
new Set(
[...definedFields].filter(([name, _ref]) => !usedFields.has(name)),
),
).map(([name, ref]) =>
createError(`Field ${name} is never used`, Severity.MEDIUM, ref, {
).map(([_name, ref]) =>
createError("Field is never used", Severity.MEDIUM, ref, {
docURL: makeDocURL(this.id),
suggestion: "Consider removing the field",
}),
);
}
Expand Down Expand Up @@ -170,8 +171,9 @@ export class NeverAccessedVariables extends Detector {
),
),
).map(([name, ref]) =>

Check failure on line 173 in src/detectors/builtin/neverAccessedVariables.ts

View workflow job for this annotation

GitHub Actions / test (18.x, ubuntu-latest)

'name' is defined but never used. Allowed unused args must match /^_/u
createError(`Constant ${name} is never used`, Severity.MEDIUM, ref, {
createError("Constant is never used", Severity.MEDIUM, ref, {
docURL: makeDocURL(this.id),
suggestion: "Consider removing the constant",
}),
);
}
Expand Down Expand Up @@ -229,12 +231,17 @@ export class NeverAccessedVariables extends Detector {
});
Array.from(declaredVariables.keys()).forEach((name) => {
if (!accessedVariables.has(name)) {
const msg = writtenVariables.has(name)
const isWritten = writtenVariables.has(name);
const msg = isWritten
? "Write-only variable"
: "Variable is never accessed";
const suggestion = isWritten
? "The variable value should be accessed"
: "Consider removing the variable";
errors.push(
createError(msg, Severity.MEDIUM, declaredVariables.get(name)!, {
docURL: makeDocURL(this.id),
suggestion,
}),
);
}
Expand Down
1 change: 1 addition & 0 deletions src/detectors/builtin/readOnlyVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export class ReadOnlyVariables extends Detector {
}
return createError("Read-only variable", Severity.MEDIUM, fact.data, {
docURL: makeDocURL(this.id),
suggestion: "Consider creating a constant instead",
});
});

Expand Down
13 changes: 7 additions & 6 deletions src/detectors/builtin/unboundLoops.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,13 @@ export class UnboundLoops extends Detector {
if (fact.data === undefined) {
throw new Error(`AST position for fact ${fact} is not available`);
}
return createError(
"Unbounded loop: the condition variable doesn't change within the loop",
Severity.MEDIUM,
fact.data,
{ docURL: makeDocURL(this.id) },
);
return createError("Unbounded Loop", Severity.MEDIUM, fact.data, {
docURL: makeDocURL(this.id),
suggestion:
"Consider changing the variable within the loop to ensure it terminates",
extraDescription:
"The condition variable doesn't change within the loop",
});
});

return warnings;
Expand Down
3 changes: 2 additions & 1 deletion src/detectors/builtin/zeroAddress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ function findZeroAddress(
expr.args[1].value === 0n
) {
acc.push(
createError("Using zero address", Severity.MEDIUM, expr.args[1].ref, {
createError("Using Zero Address", Severity.MEDIUM, expr.args[1].ref, {
docURL: makeDocURL("zeroAddress"),
suggestion: "Consider changing code to avoid using it",
}),
);
}
Expand Down
15 changes: 10 additions & 5 deletions src/internals/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,16 @@ export function createError(
return `${relativeFilePath}:${lc.lineNum}:${lc.colNum}:\n${lcLines.join("\n")}`;
})()
: "";
const docURLStr = docURL === undefined ? "" : ` (see: ${docURL})`;
const extraDescriptionStr =
extraDescription === undefined ? "" : `\n${extraDescription}`;
const suggestionStr =
suggestion === undefined ? "" : `\nSuggestion: ${suggestion}`;
const msg = `${pos}${description}${docURLStr}${extraDescriptionStr}${suggestionStr}`;
extraDescription === undefined ? "" : `: ${extraDescription}`;
const suggestionStr = suggestion === undefined ? "" : `\nHelp: ${suggestion}`;
const docURLStr = docURL === undefined ? "" : `\nSee: ${docURL})`;
const msg = [
pos,
description,
extraDescriptionStr,
suggestionStr,
docURLStr,
].join("");
return new MistiTactError(msg, ref, severity);
}
4 changes: 3 additions & 1 deletion test/contracts/div-before-mul-1.expected.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ test/contracts/div-before-mul-1.tact:5:21:
> 5 | let result: Int = a / b * c; // error: Division before multiplication
^
6 | return result;
Division operation before multiplication detected. Consider rearranging the operations.
Divide Before Multiply
Help: Consider rearranging the operations: division should follow multiplication
See: https://nowarp.github.io/docs/misti/docs/detectors/DivideBeforeMultiply)
4 changes: 3 additions & 1 deletion test/contracts/div-before-mul-3.expected.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ test/contracts/div-before-mul-3.tact:2:16:
> 2 | let a: Int = 10 / 3; // Division operation
^
3 | let c: Int = 2;
Division operation before multiplication detected. Consider rearranging the operations.
Divide Before Multiply
Help: Consider rearranging the operations: division should follow multiplication
See: https://nowarp.github.io/docs/misti/docs/detectors/DivideBeforeMultiply)
4 changes: 3 additions & 1 deletion test/contracts/never-accessed-1.expected.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ test/contracts/never-accessed-1.tact:2:5:
> 2 | let a: Int = 20;
^
3 | if (true) { // error: write-only variable
Write-only variable (see: https://nowarp.github.io/docs/misti/docs/detectors/NeverAccessedVariables)
Write-only variable
Help: The variable value should be accessed
See: https://nowarp.github.io/docs/misti/docs/detectors/NeverAccessedVariables)
8 changes: 6 additions & 2 deletions test/contracts/readonly-2.expected.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@ test/contracts/readonly-2.tact:2:5:
> 2 | let a: Int = 20;
^
3 | let b: Int = 22;
Read-only variable (see: https://nowarp.github.io/docs/misti/docs/detectors/ReadOnlyVariables)
Read-only variable
Help: Consider creating a constant instead
See: https://nowarp.github.io/docs/misti/docs/detectors/ReadOnlyVariables)

test/contracts/readonly-2.tact:2:5:
1 | fun test1(): Int {
> 2 | let a: Int = 20;
^
3 | let b: Int = 22;
Variable is never accessed (see: https://nowarp.github.io/docs/misti/docs/detectors/NeverAccessedVariables)
Variable is never accessed
Help: Consider removing the variable
See: https://nowarp.github.io/docs/misti/docs/detectors/NeverAccessedVariables)
4 changes: 3 additions & 1 deletion test/contracts/unbound-loop-1.expected.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ test/contracts/unbound-loop-1.tact:3:12:
> 3 | while (a > 10) { // error: a is not changing within the loop
^
4 | }
Unbounded loop: the condition variable doesn't change within the loop (see: https://nowarp.github.io/docs/misti/docs/detectors/UnboundLoops)
Unbounded Loop: The condition variable doesn't change within the loop
Help: Consider changing the variable within the loop to ensure it terminates
See: https://nowarp.github.io/docs/misti/docs/detectors/UnboundLoops)
4 changes: 3 additions & 1 deletion test/contracts/unbound-loop-2.expected.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ test/contracts/unbound-loop-2.tact:4:14:
> 4 | } until (a > 10);
^
5 | return a;
Unbounded loop: the condition variable doesn't change within the loop (see: https://nowarp.github.io/docs/misti/docs/detectors/UnboundLoops)
Unbounded Loop: The condition variable doesn't change within the loop
Help: Consider changing the variable within the loop to ensure it terminates
See: https://nowarp.github.io/docs/misti/docs/detectors/UnboundLoops)
4 changes: 3 additions & 1 deletion test/contracts/zero-address.expected.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ test/contracts/zero-address.tact:3:23:
> 3 | newAddress(1, 0x000000000000000000000000000000000000000000000000);
^
4 | }
Using zero address (see: https://nowarp.github.io/docs/misti/docs/detectors/zeroAddress)
Using Zero Address
Help: Consider changing code to avoid using it
See: https://nowarp.github.io/docs/misti/docs/detectors/zeroAddress)

0 comments on commit 9d032c4

Please sign in to comment.