PR feedback

backend/commons/src/main/java/org/sonarsource/sonarlint/core/commons/NewCodeDefinition.java

@@ -62,8 +62,6 @@ static NewCodeDefinition withNumberOfDaysWithDate(int days, long thresholdDate)
     return new NewCodeNumberOfDaysWithDate(days, thresholdDate);
   }
 
-  long getThresholdDate();
-
   static NewCodeDefinition withPreviousVersion(long thresholdDate, @Nullable String version) {
     return new NewCodePreviousVersion(thresholdDate, version);
   }
@@ -76,6 +74,8 @@ static NewCodeDefinition withSpecificAnalysis(long thresholdDate) {
     return new NewCodeSpecificAnalysis(thresholdDate);
   }
 
+  Instant getThresholdDate();
+
   abstract class NewCodeDefinitionWithDate implements NewCodeDefinition {
     protected final long thresholdDate;
 
@@ -91,8 +91,8 @@ public boolean isSupported() {
       return true;
     }
 
-    public long getThresholdDate() {
-      return thresholdDate;
+    public Instant getThresholdDate() {
+      return Instant.ofEpochMilli(thresholdDate);
     }
   }
 
@@ -119,8 +119,8 @@ public boolean isSupported() {
     }
 
     @Override
-    public long getThresholdDate() {
-      return Instant.now().minus(days, ChronoUnit.DAYS).toEpochMilli();
+    public Instant getThresholdDate() {
+      return Instant.now().minus(days, ChronoUnit.DAYS);
     }
 
     // Text used by IDEs in the UI. Communicate changes to IDE squad prior to changing the wording.
@@ -224,9 +224,9 @@ public String getBranchName() {
     }
 
     @Override
-    public long getThresholdDate() {
+    public Instant getThresholdDate() {
       // instead of Long.MAX_VALUE it's set for Instant.now() in case it will be used for git blame limit
-      return Instant.now().toEpochMilli();
+      return Instant.now();
     }
 
     // Text used by IDEs in the UI. Communicate changes to IDE squad prior to changing the wording.
@@ -253,9 +253,9 @@ public boolean isOnNewCode(long creationDate) {
     }
 
     @Override
-    public long getThresholdDate() {
+    public Instant getThresholdDate() {
       // instead of 0L it's set for Instant.now() in case it will be used for git blame limit (which shouldn't normally happen)
-      return Instant.now().toEpochMilli();
+      return Instant.now();
     }
 
     @Override

backend/commons/src/main/java/org/sonarsource/sonarlint/core/commons/util/git/GitUtils.java

@@ -99,12 +99,12 @@ public static List<URI> getVSCChangedFiles(@Nullable Path baseDir) {
   }
 
   public static SonarLintBlameResult getBlameResult(Path projectBaseDir, Set<Path> projectBaseRelativeFilePaths,
-    @Nullable UnaryOperator<String> fileContentProvider, long thresholdDate) {
+    @Nullable UnaryOperator<String> fileContentProvider, Instant thresholdDate) {
     return getBlameResult(projectBaseDir, projectBaseRelativeFilePaths, fileContentProvider, GitUtils::checkIfEnabled, thresholdDate);
   }
 
   static SonarLintBlameResult getBlameResult(Path projectBaseDir, Set<Path> projectBaseRelativeFilePaths, @Nullable UnaryOperator<String> fileContentProvider,
-    Predicate<Path> isEnabled, long thresholdDate) {
+    Predicate<Path> isEnabled, Instant thresholdDate) {
     if (isEnabled.test(projectBaseDir)) {
       LOG.debug("Using native git blame");
       return blameFromNativeCommand(projectBaseDir, projectBaseRelativeFilePaths, thresholdDate);
@@ -175,12 +175,12 @@ private static String executeGitCommand(Path workingDir, String... command) thro
     return String.join(System.lineSeparator(), commandResult);
   }
 
-  public static SonarLintBlameResult blameFromNativeCommand(Path projectBaseDir, Set<Path> projectBaseRelativeFilePaths, long thresholdDate) {
+  public static SonarLintBlameResult blameFromNativeCommand(Path projectBaseDir, Set<Path> projectBaseRelativeFilePaths, Instant thresholdDate) {
     var nativeExecutable = getNativeGitExecutable();
     if (nativeExecutable != null) {
       for (var relativeFilePath : projectBaseRelativeFilePaths) {
         try {
-          var blameHistoryWindow = getBlameHistoryWindow(thresholdDate);
+          var blameHistoryWindow = "--since='" + thresholdDate + "'";
           return parseBlameOutput(executeGitCommand(projectBaseDir,
               nativeExecutable, "blame", blameHistoryWindow, projectBaseDir.resolve(relativeFilePath).toString(), "--line-porcelain", "--encoding=UTF-8"),
             projectBaseDir.resolve(relativeFilePath).toString().replace("\\", "/"), projectBaseDir);
@@ -192,11 +192,6 @@ public static SonarLintBlameResult blameFromNativeCommand(Path projectBaseDir, S
     throw new IllegalStateException("There is no native Git available");
   }
 
-  private static String getBlameHistoryWindow(long thresholdDate) {
-    var blameLimit = Instant.ofEpochMilli(thresholdDate);
-    return "--since='" + blameLimit + "'";
-  }
-
   public static boolean checkIfEnabled(Path projectBaseDir) {
     var nativeExecutable = getNativeGitExecutable();
     if (nativeExecutable == null) {

backend/commons/src/test/java/org/sonarsource/sonarlint/core/commons/util/git/GitUtilsTest.java

@@ -127,7 +127,7 @@ void it_should_fallback_to_jgit_blame() throws IOException, GitAPIException {
     createFile(projectDirPath, "fileA", "line1", "line2", "line3");
     var c1 = commit(git, "fileA");
 
-    var sonarLintBlameResult = getBlameResult(projectDirPath, Set.of(Path.of("fileA")), null, path -> false, 0);
+    var sonarLintBlameResult = getBlameResult(projectDirPath, Set.of(Path.of("fileA")), null, path -> false, Instant.now());
     assertThat(IntStream.of(1, 2, 3)
       .mapToObj(lineNumber -> sonarLintBlameResult.getLatestChangeDateForLinesInFile(Path.of("fileA"), List.of(lineNumber))))
       .map(Optional::get)
@@ -138,7 +138,7 @@ void it_should_fallback_to_jgit_blame() throws IOException, GitAPIException {
   void it_should_throw_if_no_files() {
     Set<Path> files = Set.of();
 
-    assertThrows(IllegalStateException.class, () -> getBlameResult(projectDirPath, files, null, path -> true, 0));
+    assertThrows(IllegalStateException.class, () -> getBlameResult(projectDirPath, files, null, path -> true, Instant.now()));
   }
 
   @Test
@@ -384,7 +384,7 @@ void it_should_default_to_instant_now_git_blame_history_limit() throws IOExcepti
     commitAtDate(git, fourMonthsAgo, fileAStr);
     var fileA = Path.of(fileAStr);
 
-    var blameResult = blameFromNativeCommand(projectDirPath, Set.of(fileA), Instant.now().toEpochMilli());
+    var blameResult = blameFromNativeCommand(projectDirPath, Set.of(fileA), Instant.now());
 
     var line1Date = blameResult.getLatestChangeDateForLinesInFile(fileA, List.of(1)).get();
     var line2Date = blameResult.getLatestChangeDateForLinesInFile(fileA, List.of(2)).get();
@@ -422,7 +422,7 @@ void it_should_blame_file_since_provided_period() throws IOException, GitAPIExce
     commitAtDate(git, fourMonthsAgo, fileAStr);
     var fileA = Path.of(fileAStr);
 
-    var blameResult = blameFromNativeCommand(projectDirPath, Set.of(fileA), Instant.now().minus(Period.ofDays(180)).toEpochMilli());
+    var blameResult = blameFromNativeCommand(projectDirPath, Set.of(fileA), Instant.now().minus(Period.ofDays(180)));
 
     var line1Date = blameResult.getLatestChangeDateForLinesInFile(fileA, List.of(1)).get();
     var line2Date = blameResult.getLatestChangeDateForLinesInFile(fileA, List.of(2)).get();

backend/server-connection/src/main/java/org/sonarsource/sonarlint/core/serverconnection/storage/NewCodeDefinitionStorage.java

@@ -63,7 +63,7 @@ static Sonarlint.NewCodeDefinition adapt(NewCodeDefinition newCodeDefinition) {
     }
     if (newCodeDefinition.getMode() != NewCodeMode.REFERENCE_BRANCH) {
       var newCodeDefinitionWithDate = (NewCodeDefinition.NewCodeDefinitionWithDate) newCodeDefinition;
-      builder.setThresholdDate(newCodeDefinitionWithDate.getThresholdDate());
+      builder.setThresholdDate(newCodeDefinitionWithDate.getThresholdDate().toEpochMilli());
     } else {
       var newCodeReferenceBranch = (NewCodeDefinition.NewCodeReferenceBranch) newCodeDefinition;
       builder.setReferenceBranch(newCodeReferenceBranch.getBranchName());

backend/server-connection/src/test/java/org/sonarsource/sonarlint/core/serverconnection/storage/NewCodeDefinitionStorageTests.java

@@ -57,7 +57,7 @@ void shouldAdaptFromProtobuf() {
     var days = adapt(daysProto);
     assertThat(days.isSupported()).isTrue();
     assertThat(days).isInstanceOf(NewCodeDefinition.NewCodeNumberOfDaysWithDate.class);
-    assertThat(((NewCodeDefinition.NewCodeNumberOfDaysWithDate) days).getThresholdDate()).isEqualTo(1000);
+    assertThat(days.getThresholdDate().toEpochMilli()).isEqualTo(1000);
     assertThat(((NewCodeDefinition.NewCodeNumberOfDaysWithDate) days).getDays()).isEqualTo(30);
 
     var previousWithVersionProto = Sonarlint.NewCodeDefinition.newBuilder()
@@ -68,7 +68,7 @@ void shouldAdaptFromProtobuf() {
     var previousWithVersion = adapt(previousWithVersionProto);
     assertThat(previousWithVersion.isSupported()).isTrue();
     assertThat(previousWithVersion).isInstanceOf(NewCodeDefinition.NewCodePreviousVersion.class);
-    assertThat(((NewCodeDefinition.NewCodePreviousVersion) previousWithVersion).getThresholdDate()).isEqualTo(1000);
+    assertThat(previousWithVersion.getThresholdDate().toEpochMilli()).isEqualTo(1000);
     assertThat(((NewCodeDefinition.NewCodePreviousVersion) previousWithVersion).getVersion()).isEqualTo("1.0-SNAPSHOT");
 
     var previousWithoutVersionProto = Sonarlint.NewCodeDefinition.newBuilder()
@@ -78,7 +78,7 @@ void shouldAdaptFromProtobuf() {
     var previousWithoutVersion = adapt(previousWithoutVersionProto);
     assertThat(previousWithoutVersion.isSupported()).isTrue();
     assertThat(previousWithoutVersion).isInstanceOf(NewCodeDefinition.NewCodePreviousVersion.class);
-    assertThat(((NewCodeDefinition.NewCodePreviousVersion) previousWithoutVersion).getThresholdDate()).isEqualTo(1000);
+    assertThat(previousWithoutVersion.getThresholdDate().toEpochMilli()).isEqualTo(1000);
     assertThat(((NewCodeDefinition.NewCodePreviousVersion) previousWithoutVersion).getVersion()).isNull();
 
     var branchProto = Sonarlint.NewCodeDefinition.newBuilder()
@@ -97,7 +97,7 @@ void shouldAdaptFromProtobuf() {
     var analysis = adapt(analysisProto);
     assertThat(analysis.isSupported()).isTrue();
     assertThat(analysis).isInstanceOf(NewCodeDefinition.NewCodeSpecificAnalysis.class);
-    assertThat(((NewCodeDefinition.NewCodeSpecificAnalysis) analysis).getThresholdDate()).isEqualTo(1000);
+    assertThat(analysis.getThresholdDate().toEpochMilli()).isEqualTo(1000);
 
     var unknownProto = Sonarlint.NewCodeDefinition.newBuilder()
       .setMode(Sonarlint.NewCodeDefinitionMode.UNKNOWN)