SONARPHP-1250 Secondary locations for regex rules are not added to Ne…

…wIssue (#869)

* SONARPHP-1250 Secondary locations for regex rules are not added to NewIssue

* Address review suggestion

php-frontend/src/main/java/org/sonar/php/regex/PhpAnalyzerRegexSource.java

@@ -83,7 +83,7 @@ private static String leadingWhitespaces(String s) {
   public LocationInFile locationInFileFor(IndexRange range) {
     int[] startLineAndOffset = lineAndOffset(range.getBeginningOffset());
     int[] endLineAndOffset = lineAndOffset(range.getEndingOffset());
-    return new LocationInFileImpl(UnknownLocationInFile.UNKNOWN_LOCATION.filePath(), startLineAndOffset[0], startLineAndOffset[1], endLineAndOffset[0], endLineAndOffset[1]);
+    return new LocationInFileImpl(null, startLineAndOffset[0], startLineAndOffset[1], endLineAndOffset[0], endLineAndOffset[1]);
   }
 
   private int[] lineAndOffset(int index) {

php-frontend/src/main/java/org/sonar/php/symbols/LocationInFileImpl.java

@@ -20,17 +20,20 @@
 package org.sonar.php.symbols;
 
 import java.util.Objects;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
 import org.sonar.plugins.php.api.visitors.LocationInFile;
 
 public class LocationInFileImpl implements LocationInFile {
 
+  @Nullable
   private final String filePath;
   private final int startLine;
   private final int startLineOffset;
   private final int endLine;
   private final int endLineOffset;
 
-  public LocationInFileImpl(String filePath, int startLine, int startLineOffset, int endLine, int endLineOffset) {
+  public LocationInFileImpl(@Nullable String filePath, int startLine, int startLineOffset, int endLine, int endLineOffset) {
     this.filePath = filePath;
     this.startLine = startLine;
     this.startLineOffset = startLineOffset;
@@ -39,6 +42,7 @@ public LocationInFileImpl(String filePath, int startLine, int startLineOffset, i
   }
 
   @Override
+  @CheckForNull
   public String filePath() {
     return filePath;
   }

php-frontend/src/main/java/org/sonar/plugins/php/api/visitors/LocationInFile.java

@@ -19,8 +19,11 @@
  */
 package org.sonar.plugins.php.api.visitors;
 
+import javax.annotation.CheckForNull;
+
 public interface LocationInFile {
 
+  @CheckForNull
   String filePath();
 
   int startLine();

sonar-php-plugin/src/test/java/org/sonar/plugins/php/PHPSensorTest.java

@@ -49,6 +49,7 @@
 import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor;
 import org.sonar.api.batch.sensor.internal.SensorContextTester;
 import org.sonar.api.batch.sensor.issue.Issue;
+import org.sonar.api.batch.sensor.issue.IssueLocation;
 import org.sonar.api.config.internal.MapSettings;
 import org.sonar.api.internal.SonarRuntimeImpl;
 import org.sonar.api.issue.NoSonarFilter;
@@ -99,6 +100,7 @@ public class PHPSensorTest {
 
   private static final String PARSE_ERROR_FILE = "parseError.php";
   private static final String ANALYZED_FILE = "PHPSquidSensor.php";
+  private static final String REGEX_FILE = "regexIssue.php";
   private static final String TEST_FILE = "Test.php";
 
   private Set<File> tempReportFiles = new HashSet<>();
@@ -333,6 +335,50 @@ public void test_issues() throws Exception {
       tuple("S1124", 22));
   }
 
+  @Test
+  public void test_regex_issues() {
+    // S5855: Regex alternatives should not be redundant
+    ActiveRules rules = new ActiveRulesBuilder()
+      .addRule(newActiveRule("S5855"))
+      .build();
+
+    checkFactory = new CheckFactory(rules);
+    analyseSingleFile(createSensor(), REGEX_FILE);
+
+    Collection<Issue> issues = context.allIssues();
+    assertThat(issues).hasSize(1);
+    Issue issue = issues.iterator().next();
+    assertLocation("Remove or rework this redundant alternative.", 3, 18, 3, 19,   issue.primaryLocation());
+
+    assertThat(issue.flows()).hasSize(1);
+    Issue.Flow secondaryFlow = issue.flows().get(0);
+    assertThat(secondaryFlow.locations()).hasSize(1);
+    assertLocation("Alternative to keep", 3, 13, 3, 17, secondaryFlow.locations().get(0));
+  }
+
+  private void assertLocation(String message, int startLine, int startLineOffset, int endLine, int endLineOffset, IssueLocation location) {
+    assertThat(location.message()).isEqualTo(message);
+    TextRange range = location.textRange();
+    assertThat(range).isNotNull();
+    assertRange(startLine, startLineOffset, endLine, endLineOffset,  range);
+  }
+
+  private void assertRange(int startLine, int startLineOffset, int endLine, int endLineOffset, TextRange textRange) {
+    assertThat(textRange.start().line())
+      .withFailMessage(String.format("Start line is expected to be %s, but get %s.", startLine, textRange.start().line()))
+      .isEqualTo(startLine);
+    assertThat(textRange.start().lineOffset())
+      .withFailMessage(String.format("Start line offset is expected to be %s, but get %s.", startLineOffset, textRange.start().lineOffset()))
+      .isEqualTo(startLineOffset);
+    assertThat(textRange.end().line())
+      .withFailMessage(String.format("End line is expected to be %s, but get %s.", endLine, textRange.end().line()))
+      .isEqualTo(endLine);
+    assertThat(textRange.end().lineOffset())
+      .withFailMessage(String.format("End line offset is expected to be %s, but get %s.", endLineOffset, textRange.end().lineOffset()))
+      .isEqualTo(endLineOffset);
+  }
+
+
   @Test
   public void cross_file_issue() {
     checkFactory = new CheckFactory(new ActiveRulesBuilder().addRule(newActiveRule("S1045")).build());

sonar-php-plugin/src/test/resources/regexIssue.php

@@ -0,0 +1,3 @@
+<?php
+
+preg_match("/[ab]|a/", $input);