Skip to content

Conversation

@jag-j
Copy link
Contributor

@jag-j jag-j commented Nov 7, 2024

No description provided.

Comment on lines 240 to 248
const resultsByEngine: Map<string, Violation[]> = new Map<string, Violation[]>();

for (const engine of results.getEngineNames()) {
resultsByEngine.set(engine, []);
}

for (const violation of results.getViolations()) {
resultsByEngine.get(violation.getRule().getEngineName())?.push(violation);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't needed. The results are already available per engine via results.getEngineRunResults(engineName)

So just loop over the engine names and then in the loop get the results for that engine.

}
};
const relatedLocations:sarif.Location[] = [];
if(typeSupportsMultipleLocations(violation.getRule().getType())) {
Copy link
Contributor

@stephen-carter-at-sf stephen-carter-at-sf Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cyclometic complexity is quite high.

One trick to address this, is to separate each transformation into its smallest unit.

The resulting code could look like:

class SarifOutputFormatter implements OutputFormatter {
    format(results: RunResults): string {
        const runDir = results.getRunDirectory();

        const sarifRuns: sarif.Run[] = results.getEngineNames()
            .map(engineName => results.getEngineRunResults(engineName))
            .filter(engineRunResults => engineRunResults.getViolationCount() > 0)
            .map(engineRunResults => toSarifRun(engineRunResults, runDir));

        // Construct SARIF log
        const sarifLog: sarif.Log = {
            version: "2.1.0",
            $schema: 'http://json.schemastore.org/sarif-2.1.0',
            runs: sarifRuns,
        };

        // Return formatted SARIF JSON string
        return JSON.stringify(sarifLog, null, 2);
    }
}

function toSarifRun(engineRunResults: EngineRunResults, runDir: string): sarif.Run {
    const violations: Violation[] = engineRunResults.getViolations();
    const rules: Rule[] = [... new Set(violations.map(v => v.getRule()))]; 
    const ruleNames: string[] = rules.map(r => r.getName());

    return {
        tool: {
            driver: {
                name: engineRunResults.getEngineName(),
                informationUri: "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/guide/version-5.html",
                rules: rules.map(toSarifReportingDescriptor),
            }
        },
        results: violations.map(v => toSarifResult(v, ruleNames.indexOf(v.getRule().getName()))),
        invocations: [
            {
                executionSuccessful: true,
                workingDirectory: {
                    uri: runDir,
                },
            },
        ],
    };
}

function toSarifResult(violation: Violation, ruleIndex: number) : sarif.Result {
    const primaryCodeLocation = violation.getCodeLocations()[violation.getPrimaryLocationIndex()];
    const result: sarif.Result = {
        ruleId: violation.getRule().getName(),
        ruleIndex: ruleIndex,
        message: { text: violation.getMessage() },
        locations: [toSarifLocation(primaryCodeLocation)],
        level: toSarifNotificationLevel(violation.getRule().getSeverityLevel()),
    };
    if(typeSupportsMultipleLocations(violation.getRule().getType())) {
        result.relatedLocations = violation.getCodeLocations().map(toSarifLocation);
    }
    return result;
}

function toSarifLocation(codeLocation: CodeLocation): sarif.Location {
    return {
        physicalLocation: {
            artifactLocation: {
                uri: codeLocation.getFile(),
            },
            region: {
                startLine: codeLocation.getStartLine(),
                startColumn: codeLocation.getStartColumn(),
                endLine: codeLocation.getEndLine(),
                endColumn: codeLocation.getEndColumn()
            } as sarif.Region
        }
    }
}

function toSarifReportingDescriptor(rule: Rule): sarif.ReportingDescriptor {
    return sarif.ReportingDescriptor = {
        id: rule.getName(),
        properties: {
            category: rule.getTags(),
            severity: rule.getSeverityLevel()
        },
        helpUri: rule.getResourceUrls().length > 0 ? rule.getResourceUrls()[0] : '';
    }
}

function toSarifNotificationLevel(severity: SeverityLevel): sarif.Notification.level {
    return severity < 3 ? 'error' : 'warning'; // IF satif.Notification.level is an enum then please return the num instead of the string.
}

Comment on lines 249 to 258
private groupViolationsByEngine(results: RunResults): Map<string, Violation[]> {
const resultsByEngine = new Map<string, Violation[]>();
for (const engine of results.getEngineNames()) {
resultsByEngine.set(engine, []);
}
for (const violation of results.getViolations()) {
resultsByEngine.get(violation.getRule().getEngineName())?.push(violation);
}
return resultsByEngine;
}
Copy link
Contributor

@stephen-carter-at-sf stephen-carter-at-sf Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again this isn't needed. The violations are already grouped by engine with results.getEngineRunResults(engineName)

@jag-j jag-j merged commit 5444607 into dev Nov 11, 2024
5 checks passed
@jag-j jag-j deleted the jj/W-17100356 branch November 11, 2024 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants