JS-1619 Reduce Sonar coverage import log noise (#6861)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Author: zglicz

.github/workflows/build.yml

@@ -220,14 +220,8 @@ jobs:
             development/artifactory/token/{REPO_OWNER_NAME_DASH}-qa-deployer access_token | ARTIFACTORY_DEPLOY_ACCESS_TOKEN;
             development/kv/data/sign key | SIGN_KEY;
             development/kv/data/sign passphrase | PGP_PASSPHRASE;
-      - &java_coverage_cache
-        name: Cache Java coverage
-        uses: SonarSource/gh-action_cache@v1
-        with:
-          path: coverage/java
-          key: java-coverage-${{ github.sha }}
       - name: Build, test and deploy Maven artifacts
-        run: mvn deploy -Pdeploy-sonarsource,coverage,coverage-report,sign,release -T1C
+        run: mvn deploy -Pdeploy-sonarsource,coverage,sign,release -T1C
         env:
           ARTIFACTORY_DEPLOY_USERNAME: ${{ fromJSON(steps.deployer-secrets.outputs.vault).ARTIFACTORY_DEPLOY_USERNAME }}
           ARTIFACTORY_DEPLOY_PASSWORD: ${{ fromJSON(steps.deployer-secrets.outputs.vault).ARTIFACTORY_DEPLOY_ACCESS_TOKEN }}
@@ -249,6 +243,13 @@ jobs:
             **/target/
             !**/target/site/
           retention-days: 1
+      - &upload_jacoco_xml_reports
+        name: Upload JaCoCo XML reports
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+        with:
+          name: jacoco-xml-reports-${{ github.sha }}
+          path: sonar-plugin/**/target/site/jacoco/jacoco.xml
+          retention-days: 1
       # Clean up SonarJS artifacts before post-job cache save (only on default branch where cache is saved)
       - name: Clean up SonarJS artifacts before cache save
         if: github.ref_name == github.event.repository.default_branch
@@ -532,12 +533,16 @@ jobs:
         with:
           path: coverage/js
           key: js-coverage-${{ needs.setup.outputs.js-files-hash }}
-      - *java_coverage_cache
       - &download_maven_targets
         name: Download Maven target artifacts
         uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
         with:
           name: maven-targets-${{ github.sha }}
+      - &download_jacoco_xml_reports
+        name: Download JaCoCo XML reports
+        uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
+        with:
+          name: jacoco-xml-reports-${{ github.sha }}
       - *config_maven
       - id: secrets
         uses: SonarSource/vault-action-wrapper@v3
@@ -587,8 +592,8 @@ jobs:
       - *npm_cache
       - *maven_cache
       - *js_coverage_cache
-      - *java_coverage_cache
       - *download_maven_targets
+      - *download_jacoco_xml_reports
       - *config_maven
       - id: secrets
         uses: SonarSource/vault-action-wrapper@v3

packages/analysis/src/jsts/rules/README.md

@@ -125,7 +125,7 @@ If you have any questions, encounter any bugs, or have feature requests, please
 | [aws-apigateway-public-api](https://sonarsource.github.io/rspec/#/rspec/S6333/javascript)            | Creating public APIs is security-sensitive                                                                                     | ✅  |     |     |     |     |
 | [aws-ec2-rds-dms-public](https://sonarsource.github.io/rspec/#/rspec/S6329/javascript)               | Allowing public network access to cloud resources is security-sensitive                                                        | ✅  |     |     |     |     |
 | [aws-ec2-unencrypted-ebs-volume](https://sonarsource.github.io/rspec/#/rspec/S6275/javascript)       | EBS volumes should be encrypted                                                                                                | ✅  |     |     |     |     |
-| [aws-efs-unencrypted](https://sonarsource.github.io/rspec/#/rspec/S6332/javascript)                  | Using unencrypted EFS file systems is security-sensitive                                                                       | ✅  |     |     |     |     |
+| [aws-efs-unencrypted](https://sonarsource.github.io/rspec/#/rspec/S6332/javascript)                  | Amazon EFS file systems should be encrypted                                                                                    | ✅  |     |     |     |     |
 | [aws-iam-all-privileges](https://sonarsource.github.io/rspec/#/rspec/S6302/javascript)               | Policies should not grant all privileges                                                                                       | ✅  |     |     |     |     |
 | [aws-iam-all-resources-accessible](https://sonarsource.github.io/rspec/#/rspec/S6304/javascript)     | Policies granting access to all resources of an account are security-sensitive                                                 |     |     |     |     |     |
 | [aws-iam-privilege-escalation](https://sonarsource.github.io/rspec/#/rspec/S6317/javascript)         | AWS IAM policies should limit the scope of permissions given                                                                   | ✅  |     |     |     |     |
@@ -328,7 +328,7 @@ If you have any questions, encounter any bugs, or have feature requests, please
 | [null-dereference](https://sonarsource.github.io/rspec/#/rspec/S2259/javascript)                     | Properties of variables with "null" or "undefined" values should not be accessed                                               | ✅  |     |     | 💭  |     |
 | [object-alt-content](https://sonarsource.github.io/rspec/#/rspec/S5264/javascript)                   | "<object>" tags should provide an alternative content                                                                          | ✅  |     |     |     |     |
 | [operation-returning-nan](https://sonarsource.github.io/rspec/#/rspec/S3757/javascript)              | Arithmetic operations should not result in "NaN"                                                                               |     |     |     | 💭  |     |
-| [os-command](https://sonarsource.github.io/rspec/#/rspec/S4721/javascript)                           | Using shell interpreter when executing OS commands is security-sensitive                                                       | ✅  |     |     |     |     |
+| [os-command](https://sonarsource.github.io/rspec/#/rspec/S4721/javascript)                           | OS commands should not be executed using a shell interpreter                                                                   |     |     |     |     | ❌  |
 | [post-message](https://sonarsource.github.io/rspec/#/rspec/S2819/javascript)                         | Origins should be verified during cross-origin communications                                                                  | ✅  |     |     | 💭  |     |
 | [prefer-default-last](https://sonarsource.github.io/rspec/#/rspec/S4524/javascript)                  | "default" clauses should be last                                                                                               | ✅  |     |     |     |     |
 | [prefer-immediate-return](https://sonarsource.github.io/rspec/#/rspec/S1488/javascript)              | Local variables should not be declared and then immediately returned or thrown                                                 |     | 🔧  |     | 💭  |     |

pom.xml

@@ -82,15 +82,14 @@
     <maven.test.redirectTestOutputToFile>true</maven.test.redirectTestOutputToFile>
     <maven.compiler.release>17</maven.compiler.release>
     <jdk.min.version>17</jdk.min.version>
+    <jacoco.version>0.8.12</jacoco.version>
     <version.surefire.plugin>3.1.2</version.surefire.plugin>
-    <sonar.coverage.jacoco.xmlReportPaths>${maven.multiModuleProjectDirectory}/coverage/java/jacoco.xml</sonar.coverage.jacoco.xmlReportPaths>
-
     <!-- FIXME fix javadoc errors -->
     <doclint>none</doclint>
 
     <!-- sonar analysis -->
     <sonar.sources>packages/</sonar.sources>
-    <sonar.exclusions>packages/**/*.test.ts,packages/**/*.fixture.*,packages/**/fixtures/**/*,packages/**/fixtures-*/**/*,packages/grpc/src/proto/*,packages/analysis/src/jsts/parsers/estree.js,packages/analysis/src/jsts/parsers/estree.d.ts</sonar.exclusions>
+    <sonar.exclusions>packages/**/*.test.ts,packages/**/*.fixture.*,packages/**/fixtures/**/*,packages/**/fixtures-*/**/*,packages/grpc/src/proto/*,packages/analysis/src/jsts/parsers/estree.js,packages/analysis/src/jsts/parsers/estree.d.ts,packages/analysis/src/jsts/rules/**/generated-meta.ts,packages/analysis/src/jsts/rules/metas.ts,packages/analysis/src/jsts/rules/plugin-rules.ts,packages/analysis/src/jsts/rules/rules.ts</sonar.exclusions>
     <sonar.tests>packages/</sonar.tests>
     <sonar.test.inclusions>packages/**/*.test.ts</sonar.test.inclusions>
     <sonar.javascript.lcov.reportPaths>coverage/js/lcov.info</sonar.javascript.lcov.reportPaths>
@@ -99,7 +98,7 @@
       ${project.basedir}/packages/tsconfig.app.json,${project.basedir}/packages/tsconfig.test.json
     </sonar.typescript.tsconfigPath>
     <sonar.cpd.exclusions>sonar-plugin/javascript-checks/src/main/resources/**/*.html</sonar.cpd.exclusions>
-    <sonar.coverage.exclusions>packages/ruling/**/*,packages/*/tests/**/*,resources/org/sonar/l10n/javascript/rules/javascript/**</sonar.coverage.exclusions>
+    <sonar.coverage.exclusions>packages/ruling/**/*,packages/*/tests/**/*,resources/org/sonar/l10n/javascript/rules/javascript/**,packages/analysis/src/jsts/rules/**/generated-meta.ts,packages/analysis/src/jsts/rules/metas.ts,packages/analysis/src/jsts/rules/plugin-rules.ts,packages/analysis/src/jsts/rules/rules.ts</sonar.coverage.exclusions>
   </properties>
 
   <dependencyManagement>

sonar-plugin/api/pom.xml

@@ -12,6 +12,10 @@
 
   <name>SonarQube JavaScript :: API</name>
 
+  <properties>
+    <sonar.coverage.jacoco.xmlReportPaths>${project.reporting.outputDirectory}/jacoco/jacoco.xml</sonar.coverage.jacoco.xmlReportPaths>
+  </properties>
+
   <dependencies>
     <dependency>
       <groupId>org.sonarsource.api.plugin</groupId>

sonar-plugin/bridge/pom.xml

@@ -11,6 +11,7 @@
   <name>SonarQube JavaScript :: Bridge</name>
   <properties>
     <protobuf.version>4.34.1</protobuf.version>
+    <sonar.coverage.jacoco.xmlReportPaths>${project.reporting.outputDirectory}/jacoco/jacoco.xml</sonar.coverage.jacoco.xmlReportPaths>
   </properties>
   <dependencies>
     <dependency>
@@ -117,6 +118,16 @@
           </filesets>
         </configuration>
       </plugin>
+      <plugin>
+        <groupId>org.jacoco</groupId>
+        <artifactId>jacoco-maven-plugin</artifactId>
+        <version>${jacoco.version}</version>
+        <configuration>
+          <excludes>
+            <exclude>org/sonar/plugins/javascript/bridge/protobuf/*</exclude>
+          </excludes>
+        </configuration>
+      </plugin>
       <plugin>
         <groupId>org.xolstice.maven.plugins</groupId>
         <artifactId>protobuf-maven-plugin</artifactId>

sonar-plugin/coverage-report/pom.xml

@@ -12,6 +12,10 @@
 
   <name>SonarQube JavaScript :: CSS</name>
 
+  <properties>
+    <sonar.coverage.jacoco.xmlReportPaths>${project.reporting.outputDirectory}/jacoco/jacoco.xml</sonar.coverage.jacoco.xmlReportPaths>
+  </properties>
+
   <dependencies>
     <dependency>
       <groupId>${project.groupId}</groupId>

sonar-plugin/css/pom.xml

@@ -12,6 +12,12 @@
 
   <name>SonarQube JavaScript :: Checks</name>
 
+  <properties>
+    <sonar.coverage.jacoco.xmlReportPaths>${project.reporting.outputDirectory}/jacoco/jacoco.xml</sonar.coverage.jacoco.xmlReportPaths>
+    <sonar.exclusions>src/main/java/org/sonar/javascript/checks/AllChecks.java,src/main/java/org/sonar/javascript/checks/S*.java</sonar.exclusions>
+    <sonar.coverage.exclusions>src/main/java/org/sonar/javascript/checks/AllChecks.java,src/main/java/org/sonar/javascript/checks/S*.java</sonar.coverage.exclusions>
+  </properties>
+
   <dependencies>
     <dependency>
       <groupId>${project.groupId}</groupId>
@@ -65,6 +71,17 @@
           </filesets>
         </configuration>
       </plugin>
+      <plugin>
+        <groupId>org.jacoco</groupId>
+        <artifactId>jacoco-maven-plugin</artifactId>
+        <version>${jacoco.version}</version>
+        <configuration>
+          <excludes>
+            <exclude>org/sonar/javascript/checks/AllChecks*</exclude>
+            <exclude>org/sonar/javascript/checks/S*</exclude>
+          </excludes>
+        </configuration>
+      </plugin>
       <plugin>
         <groupId>org.codehaus.mojo</groupId>
         <artifactId>exec-maven-plugin</artifactId>

sonar-plugin/javascript-checks/pom.xml

@@ -21,48 +21,65 @@ public final class EscapeUtils {
   private EscapeUtils() {}
 
   public static String unescape(String value) {
-    StringBuilder result = new StringBuilder();
-    StringBuilder escapeSequence = new StringBuilder(4);
+    if (value.indexOf('\\') < 0) {
+      return value;
+    }
+
+    StringBuilder result = new StringBuilder(value.length());
     int i = 0;
     while (i < value.length()) {
       char ch = value.charAt(i);
-      if (ch == '\\') {
-        i++;
-        ch = value.charAt(i);
-        if (ch == 'u') {
-          i = consumeEscapeSequence(i, 4, value, escapeSequence, result);
-        } else if (ch == 'x') {
-          i = consumeEscapeSequence(i, 2, value, escapeSequence, result);
-        } else {
-          result.append(unescape(ch));
-          i++;
-        }
-      } else {
+      if (ch != '\\') {
         result.append(ch);
         i++;
+        continue;
+      }
+
+      if (i + 1 == value.length()) {
+        result.append(ch);
+        break;
+      }
+
+      char escaped = value.charAt(i + 1);
+      if (escaped == 'u') {
+        i = consumeEscapeSequence(i, 4, value, result);
+      } else if (escaped == 'x') {
+        i = consumeEscapeSequence(i, 2, value, result);
+      } else {
+        result.append(unescape(escaped));
+        i += 2;
       }
     }
     return result.toString();
   }
 
-  private static int consumeEscapeSequence(
-    int i,
-    int len,
-    String value,
-    StringBuilder escapeSequence,
-    StringBuilder result
-  ) {
-    int ii = i;
-    while (escapeSequence.length() != len && ii < value.length()) {
-      ii++;
-      escapeSequence.append(value.charAt(ii));
+  private static int consumeEscapeSequence(int i, int len, String value, StringBuilder result) {
+    int escapeStart = i;
+    int digitsStart = i + 2;
+    int digitsEnd = digitsStart + len;
+
+    if (digitsEnd > value.length()) {
+      result.append(value, escapeStart, value.length());
+      return value.length();
+    }
+
+    String escapeSequence = value.substring(digitsStart, digitsEnd);
+    if (isHexadecimal(escapeSequence)) {
+      result.append((char) Integer.parseInt(escapeSequence, 16));
+    } else {
+      result.append(value, escapeStart, digitsEnd);
     }
-    if (escapeSequence.length() == len) {
-      result.append((char) Integer.parseInt(escapeSequence.toString(), 16));
-      escapeSequence.setLength(0);
+
+    return digitsEnd;
+  }
+
+  private static boolean isHexadecimal(String value) {
+    for (int i = 0; i < value.length(); i++) {
+      if (Character.digit(value.charAt(i), 16) < 0) {
+        return false;
+      }
     }
-    ii++;
-    return ii;
+    return true;
   }
 
   private static char unescape(char ch) {

sonar-plugin/javascript-checks/src/main/java/org/sonar/javascript/checks/EscapeUtils.java

@@ -23,8 +23,9 @@
 class EscapeUtilsTest {
 
   @Test
-  void test() {
-    assertThat(EscapeUtils.unescape("foo")).isEqualTo("foo");
+  void should_unescape_known_sequences() {
+    String plainText = "foo";
+    assertThat(EscapeUtils.unescape(plainText)).isSameAs(plainText);
     assertThat(EscapeUtils.unescape("\\u000B")).isEqualTo("\u000B");
     assertThat(EscapeUtils.unescape("\\x0B")).isEqualTo("\u000B");
 
@@ -39,4 +40,14 @@ void test() {
     assertThat(EscapeUtils.unescape("\\\\")).isEqualTo("\\");
     assertThat(EscapeUtils.unescape("\\|")).isEqualTo("|");
   }
+
+  @Test
+  void should_preserve_incomplete_or_invalid_escape_sequences() {
+    assertThat(EscapeUtils.unescape("\\")).isEqualTo("\\");
+    assertThat(EscapeUtils.unescape("foo\\")).isEqualTo("foo\\");
+    assertThat(EscapeUtils.unescape("\\u00")).isEqualTo("\\u00");
+    assertThat(EscapeUtils.unescape("\\x0")).isEqualTo("\\x0");
+    assertThat(EscapeUtils.unescape("\\u00GG")).isEqualTo("\\u00GG");
+    assertThat(EscapeUtils.unescape("\\xGG")).isEqualTo("\\xGG");
+  }
 }