SLCORE-1142 Improve Git blaming for the analysis (#1247)

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

@@ -74,6 +74,8 @@ static NewCodeDefinition withSpecificAnalysis(long thresholdDate) {
     return new NewCodeSpecificAnalysis(thresholdDate);
   }
 
+  Instant getThresholdDate();
+
   abstract class NewCodeDefinitionWithDate implements NewCodeDefinition {
     protected final long thresholdDate;
 
@@ -89,8 +91,8 @@ public boolean isSupported() {
       return true;
     }
 
-    public long getThresholdDate() {
-      return thresholdDate;
+    public Instant getThresholdDate() {
+      return Instant.ofEpochMilli(thresholdDate);
     }
   }
 
@@ -116,6 +118,11 @@ public boolean isSupported() {
       return true;
     }
 
+    @Override
+    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.
     @Override
     public String toString() {
@@ -216,6 +223,12 @@ public String getBranchName() {
       return branchName;
     }
 
+    @Override
+    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();
+    }
+
     // Text used by IDEs in the UI. Communicate changes to IDE squad prior to changing the wording.
     @Override
     public String toString() {
@@ -239,6 +252,12 @@ public boolean isOnNewCode(long creationDate) {
       return true;
     }
 
+    @Override
+    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();
+    }
+
     @Override
     public boolean isSupported() {
       return true;

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

@@ -26,6 +26,7 @@
 import java.io.InputStream;
 import java.net.URI;
 import java.nio.file.Path;
+import java.time.Instant;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Optional;
@@ -97,15 +98,16 @@ public static List<URI> getVSCChangedFiles(@Nullable Path baseDir) {
     }
   }
 
-  public static SonarLintBlameResult getBlameResult(Path projectBaseDir, Set<Path> projectBaseRelativeFilePaths, @Nullable UnaryOperator<String> fileContentProvider) {
-    return getBlameResult(projectBaseDir, projectBaseRelativeFilePaths, fileContentProvider, GitUtils::checkIfEnabled);
+  public static SonarLintBlameResult getBlameResult(Path projectBaseDir, Set<Path> projectBaseRelativeFilePaths,
+    @Nullable UnaryOperator<String> fileContentProvider, Instant thresholdDate) {
+    return getBlameResult(projectBaseDir, projectBaseRelativeFilePaths, fileContentProvider, GitUtils::checkIfEnabled, thresholdDate);
   }
 
-  public static SonarLintBlameResult getBlameResult(Path projectBaseDir, Set<Path> projectBaseRelativeFilePaths, @Nullable UnaryOperator<String> fileContentProvider,
-    Predicate<Path> isEnabled) {
+  static SonarLintBlameResult getBlameResult(Path projectBaseDir, Set<Path> projectBaseRelativeFilePaths, @Nullable UnaryOperator<String> fileContentProvider,
+    Predicate<Path> isEnabled, Instant thresholdDate) {
     if (isEnabled.test(projectBaseDir)) {
       LOG.debug("Using native git blame");
-      return blameFromNativeCommand(projectBaseDir, projectBaseRelativeFilePaths);
+      return blameFromNativeCommand(projectBaseDir, projectBaseRelativeFilePaths, thresholdDate);
     } else {
       LOG.debug("Falling back to JGit git blame");
       return blameWithFilesGitCommand(projectBaseDir, projectBaseRelativeFilePaths, fileContentProvider);
@@ -173,13 +175,14 @@ private static String executeGitCommand(Path workingDir, String... command) thro
     return String.join(System.lineSeparator(), commandResult);
   }
 
-  public static SonarLintBlameResult blameFromNativeCommand(Path projectBaseDir, Set<Path> projectBaseRelativeFilePaths) {
+  public static SonarLintBlameResult blameFromNativeCommand(Path projectBaseDir, Set<Path> projectBaseRelativeFilePaths, Instant thresholdDate) {
     var nativeExecutable = getNativeGitExecutable();
     if (nativeExecutable != null) {
       for (var relativeFilePath : projectBaseRelativeFilePaths) {
         try {
+          var blameHistoryWindow = "--since='" + thresholdDate + "'";
           return parseBlameOutput(executeGitCommand(projectBaseDir,
-              nativeExecutable, "blame", projectBaseDir.resolve(relativeFilePath).toString(), "--line-porcelain", "--encoding=UTF-8"),
+              nativeExecutable, "blame", blameHistoryWindow, projectBaseDir.resolve(relativeFilePath).toString(), "--line-porcelain", "--encoding=UTF-8"),
             projectBaseDir.resolve(relativeFilePath).toString().replace("\\", "/"), projectBaseDir);
         } catch (IOException e) {
           throw new IllegalStateException("Failed to blame repository files", e);

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

@@ -23,10 +23,15 @@
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.time.Instant;
+import java.time.Period;
+import java.time.temporal.ChronoUnit;
+import java.util.Calendar;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.TimeZone;
 import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
@@ -55,9 +60,11 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.sonarsource.sonarlint.core.commons.testutils.GitUtils.addFileToGitIgnoreAndCommit;
 import static org.sonarsource.sonarlint.core.commons.testutils.GitUtils.commit;
+import static org.sonarsource.sonarlint.core.commons.testutils.GitUtils.commitAtDate;
 import static org.sonarsource.sonarlint.core.commons.testutils.GitUtils.createFile;
 import static org.sonarsource.sonarlint.core.commons.testutils.GitUtils.createRepository;
 import static org.sonarsource.sonarlint.core.commons.testutils.GitUtils.modifyFile;
+import static org.sonarsource.sonarlint.core.commons.util.git.GitUtils.blameFromNativeCommand;
 import static org.sonarsource.sonarlint.core.commons.util.git.GitUtils.blameWithFilesGitCommand;
 import static org.sonarsource.sonarlint.core.commons.util.git.GitUtils.getBlameResult;
 import static org.sonarsource.sonarlint.core.commons.util.git.GitUtils.getVSCChangedFiles;
@@ -120,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);
+    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)
@@ -131,7 +138,8 @@ 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));
+    var now = Instant.now();
+    assertThrows(IllegalStateException.class, () -> getBlameResult(projectDirPath, files, null, path -> true, now));
   }
 
   @Test
@@ -350,6 +358,85 @@ void git_blame_works_for_bare_repos_too() {
     assertThat(sonarLintBlameResult.getLatestChangeDateForLinesInFile(Path.of("fileB"), List.of(3))).isEmpty();
   }
 
+  @Test
+  void it_should_default_to_instant_now_git_blame_history_limit() throws IOException, GitAPIException {
+    var calendar = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
+    calendar.add(Calendar.YEAR, -1);
+    var fileAStr = "fileA";
+    createFile(projectDirPath, fileAStr, "line1");
+    var yearAgo = calendar.toInstant();
+    // initial commit 1 year ago
+    commitAtDate(git, yearAgo, fileAStr);
+    var lines = new String[3];
+
+    // second commit 4 months after initial commit
+    calendar.add(Calendar.MONTH, 4);
+    lines[0] = "line1";
+    lines[1] = "line2";
+    var eightMonthsAgo = calendar.toInstant();
+    modifyFile(projectDirPath.resolve(fileAStr), lines);
+    commitAtDate(git, eightMonthsAgo, fileAStr);
+
+    // third commit 4 months after second commit
+    calendar.add(Calendar.MONTH, 4);
+    lines[2] = "line3";
+    var fourMonthsAgo = calendar.toInstant();
+    modifyFile(projectDirPath.resolve(fileAStr), lines);
+    commitAtDate(git, fourMonthsAgo, fileAStr);
+    var fileA = Path.of(fileAStr);
+
+    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();
+    var line3Date = blameResult.getLatestChangeDateForLinesInFile(fileA, List.of(3)).get();
+    // provided blame time limit is 30 days, so all lines are blamed by the latest commit that happened 4 months ago
+    assertThat(ChronoUnit.MINUTES.between(line1Date.toInstant(), fourMonthsAgo)).isZero();
+    assertThat(ChronoUnit.MINUTES.between(line2Date.toInstant(), fourMonthsAgo)).isZero();
+    assertThat(ChronoUnit.MINUTES.between(line3Date.toInstant(), fourMonthsAgo)).isZero();
+  }
+
+  @Test
+  void it_should_blame_file_since_provided_period() throws IOException, GitAPIException {
+    var calendar = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
+    calendar.add(Calendar.YEAR, -1);
+    var fileAStr = "fileA";
+    createFile(projectDirPath, fileAStr, "line1");
+    var yearAgo = calendar.toInstant();
+    // initial commit 1 year ago
+    commitAtDate(git, yearAgo, fileAStr);
+    var lines = new String[3];
+
+    // second commit 4 months after initial commit
+    calendar.add(Calendar.MONTH, 4);
+    lines[0] = "line1";
+    lines[1] = "line2";
+    var eightMonthsAgo = calendar.toInstant();
+    modifyFile(projectDirPath.resolve(fileAStr), lines);
+    commitAtDate(git, eightMonthsAgo, fileAStr);
+
+    // third commit 4 months after second commit
+    calendar.add(Calendar.MONTH, 4);
+    lines[2] = "line3";
+    var fourMonthsAgo = calendar.toInstant();
+    modifyFile(projectDirPath.resolve(fileAStr), lines);
+    commitAtDate(git, fourMonthsAgo, fileAStr);
+    var fileA = Path.of(fileAStr);
+
+    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();
+    var line3Date = blameResult.getLatestChangeDateForLinesInFile(fileA, List.of(3)).get();
+    // provided blame time limit is 180 days
+    // line 1 was committed 1 year ago but should have commit date of the first commit made earlier than blame time window - 8 months ago
+    assertThat(ChronoUnit.MINUTES.between(line2Date.toInstant(), line1Date.toInstant())).isZero();
+    // line 2 was committed 8 months ago, it's outside the blame time window, but it's a first commit outside the range, so it has real commit date
+    assertThat(ChronoUnit.MINUTES.between(line2Date.toInstant(), eightMonthsAgo)).isZero();
+    // line 3 was committed 4 months ago, it's inside the blame time window, so it has real commit date
+    assertThat(ChronoUnit.MINUTES.between(line3Date.toInstant(), fourMonthsAgo)).isZero();
+  }
+
   private static void setUpBareRepo(Map<String, String> filePathContentMap) throws IOException, GitAPIException {
     bareRepoPath = Files.createTempDirectory("bare-repo");
     workingRepoPath = Files.createTempDirectory("working-repo");

backend/core/src/main/java/org/sonarsource/sonarlint/core/tracking/TrackingService.java

@@ -39,12 +39,14 @@
 import org.sonarsource.sonarlint.core.branch.SonarProjectBranchTrackingService;
 import org.sonarsource.sonarlint.core.commons.KnownFinding;
 import org.sonarsource.sonarlint.core.commons.LocalOnlyIssue;
+import org.sonarsource.sonarlint.core.commons.NewCodeDefinition;
 import org.sonarsource.sonarlint.core.commons.RuleType;
 import org.sonarsource.sonarlint.core.commons.SonarLintBlameResult;
 import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger;
 import org.sonarsource.sonarlint.core.commons.util.git.GitRepoNotFoundException;
 import org.sonarsource.sonarlint.core.file.PathTranslationService;
 import org.sonarsource.sonarlint.core.local.only.LocalOnlyIssueStorageService;
+import org.sonarsource.sonarlint.core.newcode.NewCodeService;
 import org.sonarsource.sonarlint.core.reporting.FindingReportingService;
 import org.sonarsource.sonarlint.core.repository.config.ConfigurationRepository;
 import org.sonarsource.sonarlint.core.rpc.protocol.SonarLintRpcClient;
@@ -79,10 +81,12 @@ public class TrackingService {
   private final LocalOnlyIssueRepository localOnlyIssueRepository;
   private final LocalOnlyIssueStorageService localOnlyIssueStorageService;
   private final FindingsSynchronizationService findingsSynchronizationService;
+  private final NewCodeService newCodeService;
 
   public TrackingService(SonarLintRpcClient client, ConfigurationRepository configurationRepository, SonarProjectBranchTrackingService branchTrackingService,
     PathTranslationService pathTranslationService, FindingReportingService reportingService, KnownFindingsStorageService knownFindingsStorageService, StorageService storageService,
-    LocalOnlyIssueRepository localOnlyIssueRepository, LocalOnlyIssueStorageService localOnlyIssueStorageService, FindingsSynchronizationService findingsSynchronizationService) {
+    LocalOnlyIssueRepository localOnlyIssueRepository, LocalOnlyIssueStorageService localOnlyIssueStorageService, FindingsSynchronizationService findingsSynchronizationService,
+    NewCodeService newCodeService) {
     this.client = client;
     this.configurationRepository = configurationRepository;
     this.branchTrackingService = branchTrackingService;
@@ -93,6 +97,7 @@ public TrackingService(SonarLintRpcClient client, ConfigurationRepository config
     this.localOnlyIssueRepository = localOnlyIssueRepository;
     this.localOnlyIssueStorageService = localOnlyIssueStorageService;
     this.findingsSynchronizationService = findingsSynchronizationService;
+    this.newCodeService = newCodeService;
   }
 
   @EventListener
@@ -271,7 +276,9 @@ private IntroductionDateProvider getIntroductionDateProvider(String configuratio
     var baseDir = getBaseDir(configurationScopeId);
     if (baseDir != null) {
       try {
-        var sonarLintBlameResult = getBlameResult(baseDir, fileRelativePaths, fileContentProvider);
+        var newCodeDefinition = newCodeService.getFullNewCodeDefinition(configurationScopeId);
+        var thresholdDate = newCodeDefinition.map(NewCodeDefinition::getThresholdDate).orElse(NewCodeDefinition.withAlwaysNew().getThresholdDate());
+        var sonarLintBlameResult = getBlameResult(baseDir, fileRelativePaths, fileContentProvider, thresholdDate);
         return (filePath, lineNumbers) -> determineIntroductionDate(filePath, lineNumbers, sonarLintBlameResult);
       } catch (GitRepoNotFoundException e) {
         LOG.info("Git Repository not found for {}. The path {} is not in a Git repository", configurationScopeId, e.getPath());

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)