JS-166 The AST should be serialized and sent to the plugin only when at least one consumer is listening

its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java

@@ -97,9 +97,8 @@ private void assertAnalyzeJs(Bridge bridge) throws IOException, InterruptedExcep
     r.fileContent =
       "function foo() { \n" + "  var a; \n" + "  var c; // NOSONAR\n" + "  var b = 42; \n" + "} \n";
     r.filePath = temp.resolve("file.js").toAbsolutePath().toString();
-    HttpResponse<String> response = bridge.request(gson.toJson(r), "analyze-js");
-    var parsedResponse = parseFormData(response);
-    JsonObject jsonObject = gson.fromJson(parsedResponse.json(), JsonObject.class);
+    String response = bridge.request(gson.toJson(r), "analyze-js");
+    JsonObject jsonObject = gson.fromJson(response, JsonObject.class);
     JsonArray issues = jsonObject.getAsJsonArray("issues");
     assertThat(issues).hasSize(3);
     assertThat(issues)
@@ -109,35 +108,6 @@ private void assertAnalyzeJs(Bridge bridge) throws IOException, InterruptedExcep
     JsonObject metrics = jsonObject.getAsJsonObject("metrics");
     assertThat(metrics.entrySet()).hasSize(1);
     assertThat(metrics.get("nosonarLines").getAsJsonArray()).containsExactly(new JsonPrimitive(3));
-    // put back new assertion when we know how to parse this
-    //assertThat(parsedResponse.ast()).contains("plop");
-  }
-
-  private static BridgeResponse parseFormData(HttpResponse<String> response) {
-    String boundary = "--" + response.headers().firstValue("Content-Type")
-      .orElseThrow(() -> new IllegalStateException("No Content-Type header"))
-      .split("boundary=")[1];
-    String[] parts = response.body().split(boundary);
-    String json = null;
-    String ast = null;
-    for (String part : parts) {
-      // Split the part into headers and body
-      String[] splitPart = part.split("\r\n\r\n", 2);
-      if (splitPart.length < 2) {
-        // Skip if there's no body
-        continue;
-      }
-
-      String headers = splitPart[0];
-      String partBody = splitPart[1];
-
-      if (headers.contains("json")) {
-        json = partBody;
-      } else if (headers.contains("ast")) {
-        ast = partBody;
-      }
-    }
-    return new BridgeResponse(json, ast);
   }
 
   private void assertStatus(Bridge bridge) {
@@ -216,14 +186,15 @@ void start(Path dest) throws IOException {
       process = pb.start();
     }
 
-    HttpResponse<String> request(String json, String endpoint) throws IOException, InterruptedException {
+    String request(String json, String endpoint) throws IOException, InterruptedException {
       var request = HttpRequest
         .newBuilder(url(endpoint))
         .header("Content-Type", "application/json")
         .POST(HttpRequest.BodyPublishers.ofString(json))
         .build();
 
-      return client.send(request, HttpResponse.BodyHandlers.ofString());
+      var response = client.send(request, HttpResponse.BodyHandlers.ofString());
+      return response.body();
     }
 
     String status() throws IOException, InterruptedException {
@@ -255,6 +226,8 @@ static class AnalysisRequest {
     String filePath;
     String fileContent;
     String fileType = "MAIN";
+
+    boolean skipAst = true;
   }
 
   static class InitLinter {
@@ -278,6 +251,4 @@ static class Rule {
     List<Object> configurations = Collections.emptyList();
     String fileTypeTarget = "MAIN";
   }
-
-  record BridgeResponse(String json, String ast) {}
 }

packages/bridge/src/worker.js

@@ -49,16 +49,8 @@ exports.delegate = function (worker, type) {
       switch (message.type) {
         case 'success':
           if (message.format === 'multipart') {
-            const fd = new formData();
-            fd.append('ast', Buffer.from(message.result.ast));
-            delete message.result.ast;
-            fd.append('json', JSON.stringify(message.result));
-            // this adds the boundary string that will be used to separate the parts
-            response.set('Content-Type', fd.getHeaders()['content-type']);
-            response.set('Content-Length', fd.getLengthSync());
-            fd.pipe(response);
+            sendFormData(message.result, response);
           } else {
-            delete message.result.ast;
             response.send(message.result);
           }
           break;
@@ -109,7 +101,7 @@ if (parentPort) {
           parentThread.postMessage({
             type: 'success',
             result: output,
-            format: isSupported(output.ast) ? 'multipart' : 'json',
+            format: output.ast ? 'multipart' : 'json',
           });
           break;
         }
@@ -128,7 +120,7 @@ if (parentPort) {
           parentThread.postMessage({
             type: 'success',
             result: output,
-            format: isSupported(output.ast) ? 'multipart' : 'json',
+            format: output.ast ? 'multipart' : 'json',
           });
           break;
         }
@@ -232,6 +224,13 @@ if (parentPort) {
   }
 }
 
-function isSupported(ast) {
-  return ast !== 'not-supported';
+function sendFormData(result, response) {
+  const fd = new formData();
+  fd.append('ast', Buffer.from(result.ast));
+  delete result.ast;
+  fd.append('json', JSON.stringify(result));
+  // this adds the boundary string that will be used to separate the parts
+  response.set('Content-Type', fd.getHeaders()['content-type']);
+  response.set('Content-Length', fd.getLengthSync());
+  fd.pipe(response);
 }

packages/jsts/src/analysis/analysis.ts

@@ -44,6 +44,7 @@ export interface JsTsAnalysisInput extends AnalysisInput {
   ignoreHeaderComments?: boolean;
   tsConfigs?: string[];
   programId?: string;
+  skipAst?: boolean;
 }
 
 export interface ParsingError {
@@ -63,5 +64,5 @@ export interface JsTsAnalysisOutput extends AnalysisOutput {
   metrics?: Metrics;
   cpdTokens?: CpdToken[];
   ucfgPaths?: string[];
-  ast: Uint8Array | string;
+  ast?: Uint8Array;
 }

packages/jsts/src/analysis/analyzer.ts

@@ -86,12 +86,18 @@ function analyzeFile(
       cognitiveComplexity,
     );
 
-    return {
+    const result: JsTsAnalysisOutput = {
       issues,
       ucfgPaths,
       ...extendedMetrics,
-      ast: serializeAst(sourceCode, filePath),
     };
+
+    const ast = serializeAst(sourceCode, filePath);
+    if (!input.skipAst && ast) {
+      result.ast = ast;
+    }
+
+    return result;
   } catch (e) {
     /** Turns exceptions from TypeScript compiler into "parsing" errors */
     if (e.stack.indexOf('typescript.js:') > -1) {
@@ -102,21 +108,21 @@ function analyzeFile(
   }
 }
 
-/**
- * Remove this when we figure out how to serialize the TypeScript AST
- */
 function serializeAst(sourceCode: SourceCode, filePath: string) {
   if (!isSupported(filePath)) {
-    return 'not-supported';
+    return null;
   }
 
   try {
     return serializeInProtobuf(sourceCode.ast);
   } catch (e) {
     info(`Failed to serialize AST for file "${filePath}"`);
-    return 'not-supported';
+    return null;
   }
 
+  /**
+   * Remove this when we figure out how to serialize the TypeScript AST
+   */
   function isSupported(filePath: string) {
     return filePath.endsWith('.js');
   }

packages/jsts/tests/analysis/analyzer.test.ts

@@ -948,4 +948,21 @@ describe('analyzeJSTS', () => {
     expect(protoMessage.program.body).toHaveLength(1);
     expect(protoMessage.program.body[0].functionDeclaration.id.identifier.name).toEqual('f');
   });
+
+  it('should not return the AST if the skipAst flag is set', async () => {
+    const rules = [
+      { key: 'prefer-default-last', configurations: [], fileTypeTarget: ['MAIN'] },
+    ] as RuleConfig[];
+    initializeLinter(rules);
+    initializeLinter([], [], [], 'empty');
+
+    const filePath = path.join(__dirname, 'fixtures', 'code.js');
+    const language = 'js';
+
+    const { ast } = analyzeJSTS(
+      await jsTsInput({ filePath, skipAst: true }),
+      language,
+    ) as JsTsAnalysisOutput;
+    expect(ast).toBeUndefined();
+  });
 });

packages/jsts/tests/tools/helpers/input.ts

@@ -28,6 +28,7 @@ type allOptional = {
   tsConfigs?: string[];
   programId?: string;
   linterId?: string;
+  skipAst?: boolean;
 };
 
 export async function jsTsInput(input: allOptional): Promise<JsTsAnalysisInput> {
@@ -38,6 +39,7 @@ export async function jsTsInput(input: allOptional): Promise<JsTsAnalysisInput>
     programId: input.programId,
     linterId: input.linterId ?? 'default',
     tsConfigs: input.tsConfigs ?? [],
+    skipAst: input.skipAst ?? false,
   };
 }
 

sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java

@@ -67,7 +67,7 @@ void initLinter(List<EslintRule> rules, List<String> environments, List<String>
   TsConfigFile createTsConfigFile(String content) throws IOException;
 
   record JsAnalysisRequest(String filePath, String fileType, String language, @Nullable String fileContent, boolean ignoreHeaderComments,
-                           @Nullable List<String> tsConfigs, @Nullable String programId, String linterId) {
+                           @Nullable List<String> tsConfigs, @Nullable String programId, String linterId, boolean skipAst) {
 
   }
 

sonar-plugin/bridge/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java

@@ -237,7 +237,8 @@ void should_get_answer_from_server_for_ts_request() throws Exception {
       true,
       singletonList(tsConfig.absolutePath()),
       null,
-      DEFAULT_LINTER_ID
+      DEFAULT_LINTER_ID,
+      false
     );
     assertThat(bridgeServer.analyzeTypeScript(request).issues()).hasSize(1);
   }
@@ -265,7 +266,8 @@ private static JsAnalysisRequest createRequest(DefaultInputFile inputFile) {
       true,
       null,
       null,
-      DEFAULT_LINTER_ID
+      DEFAULT_LINTER_ID,
+      false
     );
   }
 
@@ -291,7 +293,8 @@ void should_get_answer_from_server_for_program_based_requests() throws Exception
       true,
       null,
       programCreated.programId(),
-      DEFAULT_LINTER_ID
+      DEFAULT_LINTER_ID,
+      false
     );
     assertThat(bridgeServer.analyzeTypeScript(request).issues()).hasSize(1);
 
@@ -509,7 +512,8 @@ void should_fail_if_bad_json_response() throws Exception {
       true,
       null,
       null,
-      DEFAULT_LINTER_ID
+      DEFAULT_LINTER_ID,
+      false
     );
     assertThatThrownBy(() -> bridgeServer.analyzeJavaScript(request))
       .isInstanceOf(IllegalStateException.class);
@@ -739,6 +743,31 @@ void should_return_an_ast() throws Exception {
     assertThat(node.getProgram()).isNotNull();
     assertThat(node.getProgram().getBodyList().get(0).getExpressionStatement()).isNotNull();
   }
+
+  @Test
+  void should_omit_an_ast_if_skipAst_flag_is_set() throws Exception {
+    bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
+    bridgeServer.startServer(context, emptyList());
+
+    DefaultInputFile inputFile = TestInputFileBuilder
+      .create("foo", "foo.js")
+      .setContents("alert('Fly, you fools!')")
+      .build();
+    JsAnalysisRequest request = new JsAnalysisRequest(
+      inputFile.absolutePath(),
+      inputFile.type().toString(),
+      "js",
+      null,
+      true,
+      null,
+      null,
+      DEFAULT_LINTER_ID,
+      true
+    );
+    var response = bridgeServer.analyzeJavaScript(request);
+    assertThat(response.ast()).isNull();
+  }
+
   @Test
   void should_not_deploy_runtime_if_sonar_nodejs_executable_is_set() {
     var existingDoesntMatterScript = "logging.js";

sonar-plugin/bridge/src/test/resources/mock-bridge/startServer.js

@@ -75,39 +75,50 @@ const requestHandler = (request, response) => {
         highlightedSymbols: [{}],
         cpdTokens: [{}],
       };
-      const boundary = '---------9051914041544843365972754266';
-      const contentTypeHeader = `multipart/form-data; boundary=${boundary}`;
-      let firstPart = '';
-      firstPart += `--${boundary}`;
-      firstPart += `\r\n`;
-      firstPart += `Content-Disposition: form-data; name="json"`;
-      firstPart += `\r\n`;
-      firstPart += `\r\n`;
-      firstPart += `${JSON.stringify(res)}`;
-      firstPart += `\r\n`;
-      firstPart += `--${boundary}`;
-      firstPart += `\r\n`;
-      firstPart += `Content-Disposition: application/octet-stream; name="ast"`;
-      firstPart += `\r\n`;
-      firstPart += `\r\n`;
-      // the clear version of serialized.proto is `packages/jsts/tests/parsers/fixtures/ast/base.js`
-      // it was generated by writing to a file the serialized data in the test `packages/jsts/tests/parsers/ast.test.ts`
-      const protoData = fs.readFileSync(path.join(__dirname, '..', 'files', 'serialized.proto'));
-      let lastPart = '';
-      lastPart += `\r\n`;
-      lastPart += `--${boundary}--`;
-      lastPart += `\r\n`;
-      const body = Buffer.concat([Buffer.from(firstPart), protoData, Buffer.from(lastPart)]);
-      const contentLength = body.length;
-      response.writeHead(200, {
-        'Content-Type': contentTypeHeader,
-        'Content-Length': contentLength,
-      });
-      response.end(body);
+
+      data = JSON.parse(data);
+
+      if (data.skipAst) {
+        response.end(JSON.stringify(res));
+      } else {
+        replyWithAst(res, response);
+      }
     }
   });
 };
 
+function replyWithAst(res, response) {
+  const boundary = '---------9051914041544843365972754266';
+  const contentTypeHeader = `multipart/form-data; boundary=${boundary}`;
+  let firstPart = '';
+  firstPart += `--${boundary}`;
+  firstPart += `\r\n`;
+  firstPart += `Content-Disposition: form-data; name="json"`;
+  firstPart += `\r\n`;
+  firstPart += `\r\n`;
+  firstPart += `${JSON.stringify(res)}`;
+  firstPart += `\r\n`;
+  firstPart += `--${boundary}`;
+  firstPart += `\r\n`;
+  firstPart += `Content-Disposition: application/octet-stream; name="ast"`;
+  firstPart += `\r\n`;
+  firstPart += `\r\n`;
+  // the clear version of serialized.proto is `packages/jsts/tests/parsers/fixtures/ast/base.js`
+  // it was generated by writing to a file the serialized data in the test `packages/jsts/tests/parsers/ast.test.ts`
+  const protoData = fs.readFileSync(path.join(__dirname, '..', 'files', 'serialized.proto'));
+  let lastPart = '';
+  lastPart += `\r\n`;
+  lastPart += `--${boundary}--`;
+  lastPart += `\r\n`;
+  const body = Buffer.concat([Buffer.from(firstPart), protoData, Buffer.from(lastPart)]);
+  const contentLength = body.length;
+  response.writeHead(200, {
+    'Content-Type': contentTypeHeader,
+    'Content-Length': contentLength,
+  });
+  response.end(body);
+}
+
 const server = http.createServer(requestHandler);
 server.keepAliveTimeout = 100; // this is used so server disconnects faster