merge master to fix_patch_parser

tomasbjerre · Dec 13, 2020 · 02e44df · 02e44df
2 parents 4517af5 + 688266f
commit 02e44df
Show file tree

Hide file tree

Showing 21 changed files with 515 additions and 43 deletions.
diff --git a/.gitignore b/.gitignore
@@ -13,3 +13,5 @@ temp
 node_modules
 src/test/resources/tests
 codeclimate.json
+*.received.txt
+
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,43 @@
 
 Changelog of Violations lib.
 
+## 1.139
+### GitHub [#72](https://github.com/tomasbjerre/violations-lib/issues/72) Still comment on unchanged lines if &quot;Comment only changed part of files&quot; is checked  
+
+**Making isIgnored public jenkinsci/violation-comments-to-stash-plugin**
+
+
+[37b6d19e9884302](https://github.com/tomasbjerre/violations-lib/commit/37b6d19e9884302) Tomas Bjerre *2020-12-06 08:33:13*
+
+
+## 1.138
+### GitHub [#72](https://github.com/tomasbjerre/violations-lib/issues/72) Still comment on unchanged lines if &quot;Comment only changed part of files&quot; is checked  
+
+**ignorePaths jenkinsci/violation-comments-to-stash-plugin**
+
+
+[1508d313f91b7d2](https://github.com/tomasbjerre/violations-lib/commit/1508d313f91b7d2) Tomas Bjerre *2020-12-06 07:41:13*
+
+
+## 1.137
+### No issue
+
+**fix formatting after merge**
+
+
+[3583b539d6064fe](https://github.com/tomasbjerre/violations-lib/commit/3583b539d6064fe) Tomas Bjerre *2020-11-27 13:06:22*
+
+**fix parse gradle kotlin windows-style path**
+
+
+[80408d1630ce84f](https://github.com/tomasbjerre/violations-lib/commit/80408d1630ce84f) Nov1kov *2020-11-27 12:49:42*
+
+**fix parse gradle kotlin column line**
+
+
+[5ac9c50de3d6bbd](https://github.com/tomasbjerre/violations-lib/commit/5ac9c50de3d6bbd) Nov1kov *2020-11-26 18:38:36*
+
+
 ## 1.136
 ### GitHub [#115](https://github.com/tomasbjerre/violations-lib/issues/115) JUnit parser does not detect a failure  
 

diff --git a/build.gradle b/build.gradle
@@ -25,6 +25,7 @@ dependencies {
  testCompile 'junit:junit:4.12'
  testCompile 'org.assertj:assertj-core:2.3.0'
  testCompile 'uk.co.jemos.podam:podam:7.2.1.RELEASE'
+ testCompile 'com.approvaltests:approvaltests:9.5.0'
 }
 
 shadowJar {

diff --git a/gradle.properties b/gradle.properties
@@ -1 +1 @@
-version = 1.137-SNAPSHOT
+version = 1.140-SNAPSHOT
diff --git a/src/main/java/se/bjurr/violations/lib/ViolationsApi.java b/src/main/java/se/bjurr/violations/lib/ViolationsApi.java
@@ -8,6 +8,7 @@
 import static se.bjurr.violations.lib.util.Utils.setReporter;
 
 import java.io.File;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 import java.util.logging.Level;
@@ -23,6 +24,8 @@ public class ViolationsApi {
   private Parser parser;
   private File startFile;
   private String reporter;
+  private List<String> ignorePaths = new ArrayList<>();
+
   private ViolationsLogger violationsLogger =
       filterLevel(
           new ViolationsLogger() {
@@ -67,9 +70,14 @@ public ViolationsApi inFolder(final String folder) {
     return this;
   }
 
+  public ViolationsApi withIgnorePaths(final List<String> ignorePaths) {
+    this.ignorePaths = checkNotNull(ignorePaths, "ignorePaths");
+    return this;
+  }
+
   public Set<Violation> violations() {
     final List<File> includedFiles =
-        findAllReports(this.violationsLogger, this.startFile, this.pattern);
+        findAllReports(this.violationsLogger, this.startFile, this.pattern, this.ignorePaths);
     this.violationsLogger.log(
         INFO,
         "Found "

diff --git a/src/main/java/se/bjurr/violations/lib/parsers/KotlinGradleParser.java b/src/main/java/se/bjurr/violations/lib/parsers/KotlinGradleParser.java
@@ -21,7 +21,7 @@ public Set<Violation> parseReportOutput(final String string, ViolationsLogger vi
       throws Exception {
     final Set<Violation> violations = new TreeSet<>();
     final List<List<String>> partsPerLine =
-        getLines(string, "(w|e):([^:]*)[^\\d]+?(\\d+?)[^\\d]+?(\\d+?)[^:]+?:(.*)");
+        getLines(string, "(w|e):(.+?):[\\s(]+?(\\d+)[^\\d]+?(\\d+)[^:]+?:(.*)");
     for (final List<String> parts : partsPerLine) {
       final String severity = parts.get(1).trim();
       final String filename = parts.get(2).trim();

diff --git a/src/main/java/se/bjurr/violations/lib/reports/ReportsFinder.java b/src/main/java/se/bjurr/violations/lib/reports/ReportsFinder.java
@@ -18,7 +18,10 @@
 public class ReportsFinder {
 
   public static List<File> findAllReports(
-      final ViolationsLogger violationsLogger, final File startFile, final String pattern) {
+      final ViolationsLogger violationsLogger,
+      final File startFile,
+      final String pattern,
+      final List<String> ignorePaths) {
     final List<File> found = new ArrayList<>();
     final Path startPath = startFile.toPath();
     try {
@@ -28,6 +31,11 @@ public static List<File> findAllReports(
             @Override
             public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs)
                 throws IOException {
+              final String relativePath = startPath.relativize(file).toString();
+              if (isIgnored(relativePath, ignorePaths)) {
+                return super.visitFile(file, attrs);
+              }
+
               final String absolutePath = file.toFile().getAbsolutePath();
               if (matches(pattern, absolutePath)
                   || matches(pattern, withFrontSlashes(absolutePath))) {
@@ -49,4 +57,13 @@ public FileVisitResult visitFile(final Path file, final BasicFileAttributes attr
   static String withFrontSlashes(final String file) {
     return file.replace('\\', '/');
   }
+
+  public static boolean isIgnored(final String path, final List<String> ignorePaths) {
+    for (final String ignorePath : ignorePaths) {
+      if (withFrontSlashes(path).startsWith(withFrontSlashes(ignorePath))) {
+        return true;
+      }
+    }
+    return false;
+  }
 }
diff --git a/src/main/java/se/bjurr/violations/lib/util/PatchParserUtil.java b/src/main/java/se/bjurr/violations/lib/util/PatchParserUtil.java
@@ -18,58 +18,87 @@ public class PatchParserUtil {
   private final Map<Integer, Optional<Integer>> newLineToOldLineTable;
   private final Map<Integer, Optional<Integer>> newLineToLineInDiffTable;
 
-  public PatchParserUtil(String patchString) {
-    newLineToOldLineTable = new TreeMap<>();
-    newLineToLineInDiffTable = new TreeMap<>();
+  private String patchString;
+
+  public PatchParserUtil(final String patchString) {
+    this.patchString = patchString;
+    this.newLineToOldLineTable = new TreeMap<>();
+    this.newLineToLineInDiffTable = new TreeMap<>();
     if (patchString == null) {
       return;
     }
     int currentLine = -1;
     int patchLocation = 1;
     int diffLocation = 0;
-    for (String line : patchString.split("\n")) {
+    for (final String line : patchString.split("\n")) {
       if (line.startsWith("@")) {
-        Matcher matcher = RANGE_PATTERN.matcher(line);
+        final Matcher matcher = RANGE_PATTERN.matcher(line);
         if (!matcher.matches()) {
           throw new IllegalStateException(
               "Unable to parse patch line " + line + "\nFull patch: \n" + patchString);
         }
         currentLine = Integer.parseInt(matcher.group(2));
         patchLocation = Integer.parseInt(matcher.group(1));
       } else if (line.startsWith("+") && !line.startsWith("++")) {
-        newLineToOldLineTable.put(currentLine, empty());
+        this.newLineToOldLineTable.put(currentLine, empty());
         currentLine++;
       } else if (line.startsWith(" ")) {
-        newLineToOldLineTable.put(currentLine, of(patchLocation));
+        this.newLineToOldLineTable.put(currentLine, of(patchLocation));
         currentLine++;
         patchLocation++;
       } else {
         patchLocation++;
       }
       diffLocation++;
-      newLineToLineInDiffTable.put(currentLine, of(diffLocation));
+      this.newLineToLineInDiffTable.put(currentLine, of(diffLocation));
     }
   }
 
-  public boolean isLineInDiff(Integer newLine) {
-    return newLineToOldLineTable.containsKey(newLine);
+  public boolean isLineInDiff(final Integer newLine) {
+    return this.newLineToOldLineTable.containsKey(newLine);
   }
 
-  public Optional<Integer> findOldLine(Integer newLine) {
-    if (newLineToOldLineTable.containsKey(newLine)) {
-      return newLineToOldLineTable.get(newLine);
+  public Optional<Integer> findOldLine(final Integer newLine) {
+    if (this.newLineToOldLineTable.containsKey(newLine)) {
+      return this.newLineToOldLineTable.get(newLine);
     }
     return empty();
   }
 
   public Optional<Integer> findLineInDiff(final int newLine) {
-    if (newLineToLineInDiffTable.containsKey(newLine)) {
-      return newLineToLineInDiffTable.get(newLine);
+    if (this.newLineToLineInDiffTable.containsKey(newLine)) {
+      return this.newLineToLineInDiffTable.get(newLine);
     }
     return empty();
   }
 
   Map<Integer, Optional<Integer>> getNewLineToOldLineTable() {
-    return newLineToOldLineTable;
+    return this.newLineToOldLineTable;
+  }
+
+  @Override
+  public String toString() {
+    final StringBuilder sb = new StringBuilder();
+    sb.append("patch:\n");
+    this.withLines(sb, this.patchString);
+    sb.append("\n\n");
+    sb.append("newLineToLineInDiffTable:\n");
+    this.toString(sb, this.newLineToLineInDiffTable);
+    sb.append("\n\nnewLineToOldLineTable:\n");
+    this.toString(sb, this.newLineToOldLineTable);
+    return sb.toString();
+  }
+
+  private void withLines(final StringBuilder sb, final String str) {
+    final String[] lines = str.split("\n");
+    for (int i = 0; i < lines.length; i++) {
+      sb.append(i + "\t: " + lines[i] + "\n");
+    }
+  }
+
+  private void toString(final StringBuilder sb, final Map<Integer, Optional<Integer>> map) {
+    for (final Map.Entry<Integer, Optional<Integer>> e : map.entrySet()) {
+      sb.append(e.getKey() + " : " + e.getValue().orElse(null) + "\n");
+    }
   }
 }
diff --git a/src/test/java/se/bjurr/violations/lib/KotlinGradleTest.java b/src/test/java/se/bjurr/violations/lib/KotlinGradleTest.java
@@ -25,7 +25,7 @@ public void testThatViolationsCanBeParsed() {
             .violations();
 
     assertThat(actual) //
-        .hasSize(3);
+        .hasSize(4);
 
     final Violation violation0 = new ArrayList<>(actual).get(0);
     assertThat(violation0.getMessage()) //
@@ -39,5 +39,12 @@ public void testThatViolationsCanBeParsed() {
         .isEqualTo("");
     assertThat(violation0.getParser()) //
         .isEqualTo(KOTLINGRADLE);
+    assertThat(violation0.getColumn()).isEqualTo(87);
+
+    final Violation violation3 = new ArrayList<>(actual).get(3);
+    assertThat(violation3.getFile())
+        .isEqualTo(
+            "C:/Users/User/Documents/testProject/src/main/java/ru/novikov/maps/CameraPosition.kt");
+    assertThat(violation3.getStartLine()).isEqualTo(25);
   }
 }
diff --git a/src/test/java/se/bjurr/violations/lib/ViolationsApiTest.java b/src/test/java/se/bjurr/violations/lib/ViolationsApiTest.java
@@ -0,0 +1,44 @@
+package se.bjurr.violations.lib;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static se.bjurr.violations.lib.TestUtils.getRootFolder;
+import static se.bjurr.violations.lib.ViolationsApi.violationsApi;
+import static se.bjurr.violations.lib.reports.Parser.CHECKSTYLE;
+
+import java.util.Arrays;
+import java.util.Set;
+import org.junit.Test;
+import se.bjurr.violations.lib.model.Violation;
+
+public class ViolationsApiTest {
+  private final String rootFolder = getRootFolder();
+
+  @Test
+  public void testThatPathsCanBeIgnored() {
+    final Set<Violation> actualNotIgnored =
+        violationsApi()
+            .withPattern(".*/mixed/.*")
+            .inFolder(this.rootFolder)
+            .findAll(CHECKSTYLE)
+            .violations();
+    assertThat(actualNotIgnored).hasSize(8);
+
+    final Set<Violation> actualIgnoredCheckstyle =
+        violationsApi()
+            .withPattern(".*/mixed/.*")
+            .inFolder(this.rootFolder)
+            .withIgnorePaths(Arrays.asList("mixed/checkstyle"))
+            .findAll(CHECKSTYLE)
+            .violations();
+    assertThat(actualIgnoredCheckstyle).hasSize(4);
+
+    final Set<Violation> actualIgnoredMixed =
+        violationsApi()
+            .withPattern(".*/mixed/.*")
+            .inFolder(this.rootFolder)
+            .withIgnorePaths(Arrays.asList("mixed"))
+            .findAll(CHECKSTYLE)
+            .violations();
+    assertThat(actualIgnoredMixed).hasSize(0);
+  }
+}
diff --git a/src/test/java/se/bjurr/violations/lib/reports/ReportsFinderTest.java b/src/test/java/se/bjurr/violations/lib/reports/ReportsFinderTest.java
@@ -7,6 +7,7 @@
 import java.io.File;
 import java.net.URISyntaxException;
 import java.nio.file.Paths;
+import java.util.ArrayList;
 import java.util.logging.Handler;
 import java.util.logging.Level;
 import java.util.logging.LogRecord;
@@ -64,7 +65,7 @@ public void log(final Level level, final String string) {
           }
         };
     assertThat(
-            findAllReports(violationsLogger, path, ".*test-traversal.*txt$")
+            findAllReports(violationsLogger, path, ".*test-traversal.*txt$", new ArrayList<>())
                 .stream()
                 .map(
                     it -> {
@@ -74,7 +75,7 @@ public void log(final Level level, final String string) {
         .containsOnly("file-in-subdir.txt", "file-in-subdir2.txt", "file-in-subdir3.txt");
 
     assertThat(
-            findAllReports(violationsLogger, path, ".*subdir3\\.txt")
+            findAllReports(violationsLogger, path, ".*subdir3\\.txt", new ArrayList<>())
                 .stream()
                 .map(
                     it -> {