Skip to content

Commit

Permalink
Use a flag to mark the issues that are part of modified code.
Browse files Browse the repository at this point in the history
  • Loading branch information
uhafner committed Jan 23, 2024
1 parent bde5f3d commit 392afeb
Show file tree
Hide file tree
Showing 8 changed files with 217 additions and 127 deletions.
2 changes: 1 addition & 1 deletion doc/dependency-graph.puml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ skinparam rectangle {
BackgroundColor<<runtime>> lightBlue
BackgroundColor<<provided>> lightGray
}
rectangle "analysis-model\n\n11.16.0" as edu_hm_hafner_analysis_model_jar
rectangle "analysis-model\n\n11.18.0-SNAPSHOT" as edu_hm_hafner_analysis_model_jar
rectangle "jsoup\n\n1.17.2" as org_jsoup_jsoup_jar
rectangle "commons-io\n\n2.15.1" as commons_io_commons_io_jar
rectangle "commons-digester3\n\n3.2" as org_apache_commons_commons_digester3_jar
Expand Down
34 changes: 28 additions & 6 deletions src/main/java/edu/hm/hafner/analysis/Issue.java
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ public static Predicate<Issue> byType(final String type) {
private String description; // fixed

private String fingerprint; // mutable, not part of equals
private boolean partOfModifiedCode; // mutable

/**
* Creates a new instance of {@link Issue} using the properties of the other issue instance. The new issue has the
Expand Down Expand Up @@ -734,7 +735,7 @@ public String getModuleName() {
}

/**
* Sets the the name of the module or project (or similar concept) that contains this issue.
* Sets the name of the module or project (or similar concept) that contains this issue.
*
* @param moduleName
* the module name to set
Expand Down Expand Up @@ -781,7 +782,7 @@ public String getOriginName() {
* @param origin
* the origin
*/
public void setOrigin(final String origin) {
void setOrigin(final String origin) {
Ensure.that(origin).isNotBlank("Issue origin ID '%s' must be not blank (%s)", id, toString());

this.origin = origin.intern();
Expand All @@ -795,7 +796,7 @@ public void setOrigin(final String origin) {
* @param name
* the name of the origin
*/
public void setOrigin(final String originId, final String name) {
void setOrigin(final String originId, final String name) {
setOrigin(originId);

Ensure.that(name).isNotBlank("Issue origin name '%s' must be not blank (%s)", name, toString());
Expand All @@ -820,13 +821,13 @@ public String getReference() {
* @param reference
* the reference
*/
public void setReference(@CheckForNull final String reference) {
void setReference(@CheckForNull final String reference) {
this.reference = stripToEmpty(reference);
}

/**
* Returns the fingerprint for this issue. Used to decide if two issues are equal even if the equals method returns
* {@code false} since some of the properties differ due to code refactorings. The fingerprint is created by
* {@code false} since some properties differ due to code refactorings. The fingerprint is created by
* analyzing the content of the affected file.
* <p>
* Note: the fingerprint is not part of the equals method since the fingerprint might change due to an unrelated
Expand Down Expand Up @@ -860,6 +861,23 @@ public boolean hasFingerprint() {
return !UNDEFINED.equals(fingerprint);
}

/**
* Returns whether this issue affects a code line that has been modified recently.
*
* @return {@code true} if this issue affects a code line that has been modified recently.
* @see IssuesInModifiedCodeMarker
*/
public boolean isPartOfModifiedCode() {
return partOfModifiedCode;
}

/**
* Marks the issue as part of a source control diff.
*/
void markAsPartOfModifiedCode() {
partOfModifiedCode = true;
}

/**
* Returns additional properties for this issue. A static analysis tool may store additional properties in this
* untyped object. This object will be serialized and is used in {@code equals} and {@code hashCode}.
Expand All @@ -883,6 +901,9 @@ public boolean equals(@CheckForNull final Object o) {

Issue issue = (Issue) o;

if (partOfModifiedCode != issue.partOfModifiedCode) {
return false;
}
if (lineStart != issue.lineStart) {
return false;
}
Expand Down Expand Up @@ -950,11 +971,12 @@ public int hashCode() {
result = 31 * result + moduleName.hashCode();
result = 31 * result + packageName.hashCode();
result = 31 * result + fileName.hashCode();
result = 31 * result + (partOfModifiedCode ? 1 : 0);
return result;
}

@Override
public String toString() {
return String.format("%s(%d,%d): %s: %s: %s", getBaseName(), lineStart, columnStart, type, category, message);
return String.format("%s%s(%d,%d): %s: %s: %s", isPartOfModifiedCode() ? "*" : StringUtils.EMPTY, getBaseName(), lineStart, columnStart, type, category, message);

Check warning on line 980 in src/main/java/edu/hm/hafner/analysis/Issue.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 980 is only partially covered, one branch is missing
}
}
14 changes: 0 additions & 14 deletions src/main/java/edu/hm/hafner/analysis/IssueDifference.java
Original file line number Diff line number Diff line change
Expand Up @@ -187,20 +187,6 @@ public Report getNewIssues() {
return newIssues;
}

/**
* Returns the new issues that are part of the changed code lines. I.e., all issues that are part of the current
* report but that have not been shown up in the previous report. If the difference is computed for a specific
* set of changed files, then this set contains only the new issues that are part of the changes. Otherwise, this
* set will be empty.
*
* @return the new issues
* @see IssueDifference#getNewIssues()
*/
@SuppressFBWarnings("EI")
public Report getNewIssuesInChangedCode() {
return newIssuesInChangedCode;
}

/**
* Returns the fixed issues. I.e., all issues that are part of the previous report but that are not present in the
* current report anymore.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package edu.hm.hafner.analysis;

import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;

import edu.hm.hafner.util.PathUtil;

/**
* Computes old, new, and fixed issues based on the reports of two consecutive static analysis runs for the same
* software artifact.
*
* @author Ullrich Hafner
*/
@SuppressWarnings("PMD.DataClass")
public class IssuesInModifiedCodeMarker {
private static final PathUtil PATH_UTIL = new PathUtil();

/**
* Finds and marks all issues that are part the changes in a source control diff.
*
* @param report
* the report with the issues to scan
* @param modifiedLinesInFilesMapping
* a mapping modified lines within files
*/
public void markIssuesInModifiedCode(final Report report, final Map<String, Set<Integer>> modifiedLinesInFilesMapping) {
for (Entry<String, Set<Integer>> include : modifiedLinesInFilesMapping.entrySet()) {
report.filter(issue -> affectsChangedLineInFile(issue, include.getKey(), include.getValue()))
.stream()
.forEach(Issue::markAsPartOfModifiedCode);
}
}

private boolean affectsChangedLineInFile(final Issue issue, final String fileName, final Set<Integer> lines) {
var normalizedPath = PATH_UTIL.getRelativePath(fileName);

if (!issue.getFileName().endsWith(normalizedPath)) { // check only the suffix
return false;
}
return lines.stream().anyMatch(issue::affectsLine);
}
}
24 changes: 24 additions & 0 deletions src/main/java/edu/hm/hafner/analysis/LineRange.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package edu.hm.hafner.analysis;

import java.io.Serializable;
import java.util.NavigableSet;
import java.util.TreeSet;

import edu.umd.cs.findbugs.annotations.CheckForNull;

Expand Down Expand Up @@ -66,6 +68,19 @@ public int getEnd() {
return end;
}

/**
* Returns the lines of this line lange in a sorted set.
*
* @return the containing lines, one by one
*/
public NavigableSet<Integer> getLines() {
var lines = new TreeSet<Integer>();
for (int line = getStart(); line <= getEnd(); line++) {
lines.add(line);
}
return lines;
}

/**
* Returns whether the specified line is contained in this range.
*
Expand All @@ -76,6 +91,15 @@ public boolean contains(final int line) {
return line >= start && line <= end;
}

/**
* Returns whether this range is just a single line.
*
* @return {@code true} if this range is just a single line, {@code false} otherwise
*/
public boolean isSingleLine() {
return start == end;

Check warning on line 100 in src/main/java/edu/hm/hafner/analysis/LineRange.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 100 is only partially covered, one branch is missing
}

@Override
public boolean equals(@CheckForNull final Object obj) {
if (this == obj) {
Expand Down
92 changes: 0 additions & 92 deletions src/test/java/edu/hm/hafner/analysis/IssueDifferenceTest.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package edu.hm.hafner.analysis;

import java.nio.charset.StandardCharsets;
import java.util.Map;

import org.junit.jupiter.api.Test;

Expand All @@ -20,97 +19,6 @@ class IssueDifferenceTest extends ResourceTest {
private static final String REFERENCE_BUILD = "100";
private static final String CURRENT_BUILD = "2";

@Test
void shouldFilterByPath() {
Report referenceIssues = new Report().addAll(
createIssue("OUTSTANDING 1", "OUT 1"),
createIssue("OUTSTANDING 2", "OUT 2"),
createIssue("OUTSTANDING 3", "OUT 3"),
createIssue("TO FIX 1", "FIX 1"),
createIssue("TO FIX 2", "FIX 2"));

Report currentIssues = new Report().addAll(
createIssue("UPD OUTSTANDING 1", "OUT 1"),
createIssue("OUTSTANDING 2", "UPD OUT 2"),
createIssue("OUTSTANDING 3", "OUT 3"),
createIssue("NEW 1", "NEW 1", "/include/me"),
createIssue("NEW 2", "NEW 2", "/remove/me"),
createIssue("NEW 3", "NEW 3", "/include/me"));

IssueDifference everything = new IssueDifference(currentIssues, CURRENT_BUILD, referenceIssues);
assertThatReportContainsExactly(everything.getFixedIssues(), "TO FIX 1", "TO FIX 2");
assertThatReferenceIsSet(everything.getFixedIssues(), REFERENCE_BUILD);
assertThatReportContainsExactly(everything.getNewIssues(), "NEW 1", "NEW 2", "NEW 3");
assertThatReferenceIsSet(everything.getNewIssues(), CURRENT_BUILD);
assertThatReportContainsExactly(everything.getNewIssuesInChangedCode());
assertThatReportContainsExactly(everything.getOutstandingIssues(),
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1");
assertThatReferenceIsSet(everything.getOutstandingIssues(), REFERENCE_BUILD);

IssueDifference included = new IssueDifference(currentIssues, CURRENT_BUILD, referenceIssues,
Map.of("/include/me", 0));
assertThatReportContainsExactly(included.getFixedIssues(), "TO FIX 1", "TO FIX 2");
assertThatReferenceIsSet(included.getFixedIssues(), REFERENCE_BUILD);
assertThatReportContainsExactly(included.getNewIssuesInChangedCode(), "NEW 1", "NEW 3");
assertThatReferenceIsSet(included.getNewIssuesInChangedCode(), CURRENT_BUILD);
assertThatReportContainsExactly(included.getNewIssues(), "NEW 2");
assertThatReferenceIsSet(included.getNewIssues(), CURRENT_BUILD);
assertThatReportContainsExactly(included.getOutstandingIssues(),
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1");
assertThatReferenceIsSet(included.getOutstandingIssues(), REFERENCE_BUILD);

IssueDifference sameSuffixForAll = new IssueDifference(currentIssues, CURRENT_BUILD, referenceIssues,
Map.of("me", 0));
assertThatReportContainsExactly(sameSuffixForAll.getFixedIssues(), "TO FIX 1", "TO FIX 2");
assertThatReferenceIsSet(sameSuffixForAll.getFixedIssues(), REFERENCE_BUILD);
assertThatReportContainsExactly(sameSuffixForAll.getNewIssuesInChangedCode(),
"NEW 1", "NEW 2", "NEW 3");
assertThatReferenceIsSet(sameSuffixForAll.getNewIssuesInChangedCode(), CURRENT_BUILD);
assertThatReportContainsExactly(sameSuffixForAll.getNewIssues());
assertThatReportContainsExactly(sameSuffixForAll.getOutstandingIssues(),
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1");
assertThatReferenceIsSet(sameSuffixForAll.getOutstandingIssues(), REFERENCE_BUILD);

IssueDifference allFiles = new IssueDifference(currentIssues, CURRENT_BUILD, referenceIssues,
Map.of("/include/me", 0, "/remove/me", 0));
assertThatReportContainsExactly(allFiles.getFixedIssues(), "TO FIX 1", "TO FIX 2");
assertThatReferenceIsSet(allFiles.getFixedIssues(), REFERENCE_BUILD);
assertThatReportContainsExactly(allFiles.getNewIssuesInChangedCode(),
"NEW 1", "NEW 2", "NEW 3");
assertThatReferenceIsSet(allFiles.getNewIssuesInChangedCode(), CURRENT_BUILD);
assertThatReportContainsExactly(allFiles.getNewIssues());
assertThatReportContainsExactly(allFiles.getOutstandingIssues(),
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1");
assertThatReferenceIsSet(allFiles.getOutstandingIssues(), REFERENCE_BUILD);

IssueDifference nothingNew = new IssueDifference(currentIssues, CURRENT_BUILD, referenceIssues,
Map.of("other", 0));
assertThatReportContainsExactly(nothingNew.getFixedIssues(),
"TO FIX 1", "TO FIX 2");
assertThatReferenceIsSet(nothingNew.getFixedIssues(), REFERENCE_BUILD);
assertThatReportContainsExactly(nothingNew.getNewIssuesInChangedCode());
assertThatReportContainsExactly(nothingNew.getNewIssues(), "NEW 1", "NEW 2", "NEW 3");
assertThatReferenceIsSet(nothingNew.getNewIssues(), CURRENT_BUILD);
assertThatReportContainsExactly(nothingNew.getOutstandingIssues(),
"OUTSTANDING 2", "OUTSTANDING 3", "UPD OUTSTANDING 1");
assertThatReferenceIsSet(nothingNew.getOutstandingIssues(), REFERENCE_BUILD);
}

private void assertThatReferenceIsSet(final Report report, final String referenceBuild) {
assertThat(report.get()).extracting(Issue::getReference).containsOnly(referenceBuild);
}

private void assertThatReportContainsExactly(final Report report, final String... messages) {
if (messages.length == 0) {
assertThat(report).isEmpty();
}
else {
assertThat(report.get())
.extracting(Issue::getMessage)
.containsExactlyInAnyOrder(messages);
}
}

@Test
void shouldCreateIssueDifferenceBasedOnPropertiesAndThenOnFingerprint() {
Report referenceIssues = new Report().addAll(
Expand Down
Loading

0 comments on commit 392afeb

Please sign in to comment.