Skip to content

Commit

Permalink
Try showing rule description in BaselineCli output
Browse files Browse the repository at this point in the history
  • Loading branch information
jckoenen committed Dec 5, 2023
1 parent b673bf0 commit 900577b
Show file tree
Hide file tree
Showing 5 changed files with 252 additions and 26 deletions.
20 changes: 15 additions & 5 deletions baseline-cli/src/main/kotlin/BaselineCli.kt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import com.google.gson.JsonSyntaxException
import com.jetbrains.qodana.sarif.RuleUtil
import com.jetbrains.qodana.sarif.SarifUtil
import com.jetbrains.qodana.sarif.baseline.BaselineCalculation
import com.jetbrains.qodana.sarif.model.Invocation
Expand All @@ -18,14 +19,17 @@ object BaselineCli {
errPrinter("Please provide a valid SARIF report path")
return ERROR_EXIT
}
val sarifReport: SarifReport
try {
sarifReport = SarifUtil.readReport(Paths.get(sarifPath))
val sarifReport = try {
SarifUtil.readReport(Paths.get(sarifPath))
} catch (e: Exception) {
errPrinter("Error reading SARIF report: ${e.message}")
return ERROR_EXIT
}
val printer = CommandLineResultsPrinter({ it }, cliPrinter)

val resolveInspectionName: (String) -> String = { id ->
RuleUtil.findRuleDescriptor(sarifReport, id)?.shortDescription?.text ?: id
}
val printer = CommandLineResultsPrinter(simpleMemoize(resolveInspectionName), cliPrinter)
return if (baselinePath != null) {
compareBaselineThreshold(
sarifReport,
Expand Down Expand Up @@ -115,4 +119,10 @@ object BaselineCli {
URI("https://raw.githubusercontent.com/schemastore/schemastore/master/src/schemas/json/sarif-2.1.0-rtm.5.json")
return SarifReport(SarifReport.Version._2_1_0, runs).`with$schema`(schema)
}
}

private fun <T : Any, R : Any> simpleMemoize(f: (T) -> R): (T) -> R {
val state = mutableMapOf<T, R>()

return { state.computeIfAbsent(it, f) }
}
}
15 changes: 15 additions & 0 deletions baseline-cli/src/test/kotlin/BaselineCliTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,19 @@ class BaselineCliTest {
assertFalse(stdout.contains("New problems count") && stdout.contains("is greater than the threshold"))
}

@Test
fun `test when rule description is available`() {
// Arrange
val map = mutableMapOf<String, String>().apply {
this["sarifReport"] = Path("src/test/resources/report.with-description.sarif.json").toString()
}

// Act
assertDoesNotThrow { BaselineCli.process(map, stdout::append, stderr::append) }

// Assert
assertTrue(stdout.contains("Result of method call ignored")) // the unresolved ID
assertFalse(stdout.contains("ResultOfMethodCallIgnored")) // the unresolved ID
}

}
173 changes: 173 additions & 0 deletions baseline-cli/src/test/resources/report.with-description.sarif.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
{
"runs": [
{
"tool": {
"extensions": [
{
"name": "Manually Added",
"rules": [
{
"id": "IgnoreResultOfCall",
"shortDescription": {
"text": "Result of method call ignored"
},
"fullDescription": {
"text": "Reports method calls whose result is ignored. For many methods, ignoring the result is perfectly legitimate, but for some it is almost certainly an error. Examples of methods where ignoring the result is likely an error include 'java.io.inputStream.read()', which returns the number of bytes actually read, and any method on 'java.lang.String' or 'java.math.BigInteger'. These methods do not produce side-effects and thus pointless if their result is ignored. The calls to the following methods are inspected: Simple getters (which do nothing except return a field) Methods specified in the settings of this inspection Methods annotated with 'org.jetbrains.annotations.Contract(pure=true)' Methods annotated with .*.'CheckReturnValue' Methods in a class or package annotated with 'javax.annotation.CheckReturnValue' Optionally, all non-library methods Calls to methods annotated with Error Prone's or AssertJ's '@CanIgnoreReturnValue' annotation are not reported. Use the inspection settings to specify the classes to check. Methods are matched by name or name pattern using Java regular expression syntax. For classes, use fully-qualified names. Each entry applies to both the class and all its inheritors.",
"markdown": "Reports method calls whose result is ignored.\n\nFor many methods, ignoring the result is perfectly\nlegitimate, but for some it is almost certainly an error. Examples of methods where ignoring\nthe result is likely an error include `java.io.inputStream.read()`,\nwhich returns the number of bytes actually read, and any method on\n`java.lang.String` or `java.math.BigInteger`. These methods do not produce side-effects and thus pointless\nif their result is ignored.\n\nThe calls to the following methods are inspected:\n\n* Simple getters (which do nothing except return a field)\n* Methods specified in the settings of this inspection\n* Methods annotated with `org.jetbrains.annotations.Contract(pure=true)`\n* Methods annotated with .\\*.`CheckReturnValue`\n* Methods in a class or package annotated with `javax.annotation.CheckReturnValue`\n* Optionally, all non-library methods\n\nCalls to methods annotated with Error Prone's or AssertJ's `@CanIgnoreReturnValue` annotation are not reported.\n\n\nUse the inspection settings to specify the classes to check.\nMethods are matched by name or name pattern using Java regular expression syntax.\nFor classes, use fully-qualified names. Each entry applies to both the class and all its inheritors."
},
"defaultConfiguration": {
"enabled": true,
"level": "warning",
"parameters": {
"suppressToolId": "ResultOfMethodCallIgnored",
"cweIds": [
252.0,
563.0
],
"ideaSeverity": "WARNING",
"qodanaSeverity": "High"
}
},
"relationships": [
{
"target": {
"id": "Java/Probable bugs",
"index": 14,
"toolComponent": {
"name": "QDJVM"
}
},
"kinds": [
"superset"
]
}
]
}
],
"language": "en-US",
"contents": [
"localizedData",
"nonLocalizedData"
],
"isComprehensive": false
}
]
},
"invocations": [
{
"exitCode": 0,
"executionSuccessful": true
}
],
"language": "en-US",
"results": [
{
"ruleId": "IgnoreResultOfCall",
"kind": "fail",
"level": "warning",
"message": {
"text": "Result of 'A.unusedResult()' is ignored",
"markdown": "Result of `A.unusedResult()` is ignored"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "test-module/A.java",
"uriBaseId": "SRCROOT"
},
"region": {
"startLine": 7,
"startColumn": 5,
"charOffset": 143,
"charLength": 12,
"snippet": {
"text": "unusedResult"
},
"sourceLanguage": "JAVA"
},
"contextRegion": {
"startLine": 5,
"startColumn": 1,
"charOffset": 96,
"charLength": 86,
"snippet": {
"text": " System.out.println(\"Another\");\n }\n unusedResult();\n unusedResult();\n }"
}
}
},
"logicalLocations": [
{
"fullyQualifiedName": "testBaseline_ SARIF only",
"kind": "module"
}
]
}
],
"partialFingerprints": {
"equalIndicator/v1": "16fa6aa4d9855b018a1bc04f35baf801a7a60108d14a0dba053f77ae1d55f621"
},
"properties": {
"ideaSeverity": "WARNING",
"qodanaSeverity": "High"
}
},
{
"ruleId": "IgnoreResultOfCall",
"kind": "fail",
"level": "warning",
"message": {
"text": "Result of 'A.unusedResult()' is ignored",
"markdown": "Result of `A.unusedResult()` is ignored"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "test-module/A.java",
"uriBaseId": "SRCROOT"
},
"region": {
"startLine": 8,
"startColumn": 5,
"charOffset": 163,
"charLength": 12,
"snippet": {
"text": "unusedResult"
},
"sourceLanguage": "JAVA"
},
"contextRegion": {
"startLine": 6,
"startColumn": 1,
"charOffset": 133,
"charLength": 50,
"snippet": {
"text": " }\n unusedResult();\n unusedResult();\n }\n"
}
}
},
"logicalLocations": [
{
"fullyQualifiedName": "testBaseline_ SARIF only",
"kind": "module"
}
]
}
],
"partialFingerprints": {
"equalIndicator/v1": "9ac736b5dae7725e30bdcce141bc6b126bb32f7c9ea9af7678910adc32180a55"
},
"properties": {
"ideaSeverity": "WARNING",
"qodanaSeverity": "High"
}
}
],
"newlineSequences": [
"\r\n",
"\n"
]
}
]
}
47 changes: 47 additions & 0 deletions sarif/src/main/java/com/jetbrains/qodana/sarif/RuleUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package com.jetbrains.qodana.sarif;

import com.jetbrains.qodana.sarif.model.*;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.Collection;
import java.util.Objects;
import java.util.stream.Stream;

public class RuleUtil {
private RuleUtil() {
throw new IllegalStateException("No instances.");
}

@Nullable
public static ReportingDescriptor findRuleDescriptor(@NotNull SarifReport report, @Nullable String ruleId) {
if (ruleId == null || ruleId.isEmpty()) return null;

return allRules(report)
.filter(r -> Objects.equals(r.getId(), ruleId))
.findFirst()
.orElse(null);
}

@NotNull
public static Stream<ReportingDescriptor> allRules(@NotNull SarifReport report) {
return stream(report.getRuns())
.map(Run::getTool)
.filter(Objects::nonNull)
.flatMap(tool -> {
ToolComponent driver = tool.getDriver();

Stream<ReportingDescriptor> driverRules = driver == null ? Stream.empty() : stream(driver.getRules());
Stream<ReportingDescriptor> extRules = stream(tool.getExtensions())
.flatMap(e -> stream(e.getRules()));

return Stream.concat(driverRules, extRules);
});
}

@NotNull
private static <T> Stream<T> stream(@Nullable Collection<T> c) {
if (c == null) return Stream.empty();
else return c.stream().filter(Objects::nonNull);
}
}
23 changes: 2 additions & 21 deletions sarif/src/test/java/com/jetbrains/qodana/sarif/BaselineTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import java.nio.file.Paths;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static com.jetbrains.qodana.sarif.baseline.BaselineCalculation.Options.DEFAULT;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -180,16 +179,7 @@ public void testAbsentResultWithChangedIdAndSameVersion() throws IOException {

doTest(report, baseline, 17, 1, 1, new BaselineCalculation.Options(true));

Set<String> knownDescriptorIds = report.getRuns().stream()
.map(Run::getTool)
.flatMap(tool -> {
Stream<ReportingDescriptor> driverRules = tool.getDriver().getRules().stream();
Stream<ReportingDescriptor> extRules = tool.getExtensions().stream()
.map(ToolComponent::getRules)
.flatMap(Collection::stream);

return Stream.concat(driverRules, extRules);
})
Set<String> knownDescriptorIds = RuleUtil.allRules(report)
.map(ReportingDescriptor::getId)
.collect(Collectors.toSet());

Expand All @@ -210,16 +200,7 @@ public void testAbsentResultWithChangedIdAndOldVersion() throws IOException {

doTest(report, baseline, 17, 1, 1, new BaselineCalculation.Options(true));

Set<String> knownDescriptorIds = report.getRuns().stream()
.map(Run::getTool)
.flatMap(tool -> {
Stream<ReportingDescriptor> driverRules = tool.getDriver().getRules().stream();
Stream<ReportingDescriptor> extRules = tool.getExtensions().stream()
.map(ToolComponent::getRules)
.flatMap(Collection::stream);

return Stream.concat(driverRules, extRules);
})
Set<String> knownDescriptorIds = RuleUtil.allRules(report)
.map(ReportingDescriptor::getId)
.collect(Collectors.toSet());

Expand Down

0 comments on commit 900577b

Please sign in to comment.