Skip to content

Commit

Permalink
feat(driver): Support suppression annotations (#203)
Browse files Browse the repository at this point in the history
Closes #43
  • Loading branch information
jubnzv authored Oct 29, 2024
1 parent 601e15e commit 3c31e7f
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `CellOverflow` detector: PR [#177](https://github.com/nowarp/misti/pull/177)
- `UnboundMap` detector: Issue [#50](https://github.com/nowarp/misti/issues/50)
- `UnusedExpressionResult` detector: PR [#190](https://github.com/nowarp/misti/pull/190)
- Warning suppressions: PR [#203](https://github.com/nowarp/misti/pull/203)
- `--list-detectors` CLI option: PR [#192](https://github.com/nowarp/misti/pull/192)
- Import Graph: PR [#180](https://github.com/nowarp/misti/pull/180)
- Leverage `ImportGraph` to resolve entry points: PR [#194](https://github.com/nowarp/misti/pull/194)
Expand Down
28 changes: 27 additions & 1 deletion src/cli/driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,11 +511,37 @@ export class Driver {
}

/**
* Filters out the warnings suppressed in the configuration files.
* Filters out the suppressed warnings.
* Mutates the input map removing suppressed warnings.
*/
private filterSuppressedWarnings(
warnings: Map<ProjectName, MistiTactWarning[]>,
): void {
this.filterSuppressedInAnnotations(warnings);
this.filterSuppressedInConfig(warnings);
}

/**
* Filters out the warnings suppressed in the code annotations.
* Mutates the input map removing suppressed warnings.
*/
private filterSuppressedInAnnotations(
warnings: Map<ProjectName, MistiTactWarning[]>,
): void {
warnings.forEach((projectWarnings, projectName) => {
const filteredWarnings = projectWarnings.filter(
(warning) => !warning.isSuppressed(),
);
warnings.set(projectName, filteredWarnings);
});
}

/**
* Filters out the warnings suppressed in the configuration files.
* Mutates the input map removing suppressed warnings.
*/
private filterSuppressedInConfig(
warnings: Map<ProjectName, MistiTactWarning[]>,
): void {
this.ctx.config.suppressions.forEach((suppression) => {
let suppressionUsed = false;
Expand Down
15 changes: 4 additions & 11 deletions src/detectors/detector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,17 +104,10 @@ export abstract class Detector {
suggestion: string;
}> = {},
): MistiTactWarning {
return MistiTactWarning.make(
this.ctx,
this.id,
description,
this.severity,
loc,
{
...data,
docURL: hasBuiltInDetector(this.id) ? makeDocURL(this.id) : undefined,
},
);
return MistiTactWarning.make(this.id, description, this.severity, loc, {
...data,
docURL: hasBuiltInDetector(this.id) ? makeDocURL(this.id) : undefined,
});
}
}

Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export * from "./version";
export * from "./detectors/detector";
export * from "./tools/";
export * from "./internals/warnings";
export * from "./internals/annotation";
export * from "./internals/transfer";
export * from "./internals/tact";
export * from "./internals/logger";
Expand Down
61 changes: 61 additions & 0 deletions src/internals/annotation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* Represents annotations in the comments processed by Misti.
*
* @packageDocumentation
*/

import { srcInfoToString } from "./tact";
import { SrcInfo } from "@tact-lang/compiler/dist/grammar/grammar";

/**
* Represents a Misti annotation.
*/
export type MistiAnnotation = {
kind: "suppress";
detectors: string[];
};

/**
* The marker used to identify Misti suppress annotations.
* Syntax: // @misti:suppress Detector1,Detector2
*/
export const SUPPRESS_MARKER = "@misti:suppress";

/**
* Retrieves the Misti annotation from the current source location if present.
*
* These can be single or multi-line comments on the current or previous line
* annotated with SUPPRESS_MARKER.
*/
export function getMistiAnnotation(loc: SrcInfo): MistiAnnotation | null {
const lines = srcInfoToString(loc).split("\n");
const currentLineIndex = lines.findIndex((line) =>
line.trim().startsWith(">"),
);
if (currentLineIndex <= 0) return null;
const previousLine = lines[currentLineIndex - 1];
const previousLineCode = getCodeFromLine(previousLine);
const annotationPattern = new RegExp(
`^\\s*(\\/\\/|\\/\\*)\\s*${SUPPRESS_MARKER}\\s+([\\w,]+)\\s*`,
);

const match = previousLineCode.match(annotationPattern);
if (match) {
const detectors = match[2].split(",").map((detector) => detector.trim());
return {
kind: "suppress",
detectors,
};
}

return null;
}

function getCodeFromLine(line: string): string {
const pipeIndex = line.indexOf("|");
if (pipeIndex !== -1) {
return line.substring(pipeIndex + 1).trim();
} else {
return line.trim();
}
}
17 changes: 17 additions & 0 deletions src/internals/tact/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -347,3 +347,20 @@ export function collectConditions(
}
return conditions;
}

/**
* Converts SrcInfo to the string representation shown to the user.
*/
export function srcInfoToString(loc: SrcInfo): string {
const lc = loc.interval.getLineAndColumn() as {
lineNum: number;
colNum: number;
};
const lcStr = `${lc}`;
const lcLines = lcStr.split("\n");
lcLines.shift();
const shownPath = loc.file
? path.relative(process.cwd(), loc.file) + ":"
: "";
return `${shownPath}${lc.lineNum}:${lc.colNum}:\n${lcLines.join("\n")}`;
}
33 changes: 16 additions & 17 deletions src/internals/warnings.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { MistiContext } from "./context";
import { getMistiAnnotation } from "./annotation";
import { InternalException } from "./exceptions";
import { srcInfoToString } from "./tact";
import { SrcInfo } from "@tact-lang/compiler/dist/grammar/ast";
import * as path from "path";

/**
* Enumerates the levels of severity that can be assigned to detected findings.
Expand Down Expand Up @@ -99,7 +99,6 @@ export class MistiTactWarning {
* @returns A new MistiTactWarning containing the warning message and source code reference.
*/
public static make(
ctx: MistiContext,
detectorId: string,
description: string,
severity: Severity,
Expand All @@ -118,33 +117,33 @@ export class MistiTactWarning {
docURL = undefined,
suggestion = undefined,
} = data;
const pos = loc.file
? (() => {
const lc = loc.interval.getLineAndColumn() as {
lineNum: number;
colNum: number;
};
const lcStr = `${lc}`;
const lcLines = lcStr.split("\n");
lcLines.shift();
const shownPath = path.relative(process.cwd(), loc.file);
return `${shownPath}:${lc.lineNum}:${lc.colNum}:\n${lcLines.join("\n")}`;
})()
: "";
const extraDescriptionStr =
extraDescription === undefined ? "" : extraDescription + "\n";
const suggestionStr = suggestion === undefined ? "" : `Help: ${suggestion}`;
const docURLStr = docURL === undefined ? "" : `\nSee: ${docURL}`;
const msg = [
description,
"\n",
pos,
srcInfoToString(loc),
extraDescriptionStr,
suggestionStr,
docURLStr,
].join("");
return new MistiTactWarning(detectorId, msg, loc, severity);
}

/**
* Checks whether this warning is suppressing using a Misti annotation.
*/
public isSuppressed(): boolean {
const annotation = getMistiAnnotation(this.loc);
if (annotation && annotation.kind === "suppress") {
return (
annotation.detectors.find((d) => d === this.detectorId) !== undefined
);
}
return false;
}
}

/**
Expand Down
4 changes: 4 additions & 0 deletions test/detectors/AsmIsUsed.tact
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
asm(-> 1 0) extends mutates fun loadRefEx(self: Slice): Cell { LDREF }

// OK: Suppressed
// @misti:suppress AsmIsUsed
asm(-> 1 0) extends mutates fun loadRefExSuppressed(self: Slice): Cell { LDREF }
10 changes: 5 additions & 5 deletions test/detectors/FieldDoubleInit.expected.out
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
[MEDIUM] FieldDoubleInit: Field a is initialized twice
test/detectors/FieldDoubleInit.tact:4:9:
3 | init(x: Int) {
> 4 | self.a = x; // Should be highlighted
[MEDIUM] FieldDoubleInit: Field a1 is initialized twice
test/detectors/FieldDoubleInit.tact:5:9:
4 | init(x: Int) {
> 5 | self.a1 = x; // Should be highlighted
^
5 | }
6 | // @misti:suppress FieldDoubleInit
Help: Consider initializing the field only in its declaration or in the `init` function
See: https://nowarp.io/tools/misti/docs/detectors/FieldDoubleInit
7 changes: 5 additions & 2 deletions test/detectors/FieldDoubleInit.tact
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
contract Test1 {
a: Int = 0;
a1: Int = 0;
a2: Int = 0;
init(x: Int) {
self.a = x; // Should be highlighted
self.a1 = x; // Should be highlighted
// @misti:suppress FieldDoubleInit
self.a2 = x; // OK: Suppressed
}
}

Expand Down

0 comments on commit 3c31e7f

Please sign in to comment.