From 2952fe46341194ea03b79048470186aaecaf9d82 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Wed, 12 Jun 2024 17:29:08 +0200 Subject: [PATCH 01/11] add option to skip AST serialization --- packages/bridge/src/worker.js | 25 +++++++++---------- packages/jsts/src/analysis/analysis.ts | 3 ++- packages/jsts/src/analysis/analyzer.ts | 24 +++++++++++------- packages/jsts/tests/analysis/analyzer.test.ts | 17 +++++++++++++ packages/jsts/tests/tools/helpers/input.ts | 2 ++ 5 files changed, 48 insertions(+), 23 deletions(-) diff --git a/packages/bridge/src/worker.js b/packages/bridge/src/worker.js index 9d1e74c68b4..25a5435e79f 100644 --- a/packages/bridge/src/worker.js +++ b/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); } diff --git a/packages/jsts/src/analysis/analysis.ts b/packages/jsts/src/analysis/analysis.ts index 9df116d22fe..7e5961deca2 100644 --- a/packages/jsts/src/analysis/analysis.ts +++ b/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; } diff --git a/packages/jsts/src/analysis/analyzer.ts b/packages/jsts/src/analysis/analyzer.ts index 1c59779b5cf..e2de88e9c70 100644 --- a/packages/jsts/src/analysis/analyzer.ts +++ b/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, input.skipAst); + if (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'; +function serializeAst(sourceCode: SourceCode, filePath: string, skipAst: boolean = false) { + if (!isSupported(filePath) || skipAst) { + 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'); } diff --git a/packages/jsts/tests/analysis/analyzer.test.ts b/packages/jsts/tests/analysis/analyzer.test.ts index 92622e38918..bb37fb106f6 100644 --- a/packages/jsts/tests/analysis/analyzer.test.ts +++ b/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(); + }); }); diff --git a/packages/jsts/tests/tools/helpers/input.ts b/packages/jsts/tests/tools/helpers/input.ts index 9587c9516dc..98a781c98e9 100644 --- a/packages/jsts/tests/tools/helpers/input.ts +++ b/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 { @@ -38,6 +39,7 @@ export async function jsTsInput(input: allOptional): Promise programId: input.programId, linterId: input.linterId ?? 'default', tsConfigs: input.tsConfigs ?? [], + skipAst: input.skipAst ?? false, }; } From fe3e12dac006b3738063d3959edc28ea392dee6c Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Wed, 12 Jun 2024 17:46:09 +0200 Subject: [PATCH 02/11] add `skipAst` flag to JsAnalysisRequest --- .../javascript/bridge/BridgeServer.java | 6 +- .../bridge/BridgeServerImplTest.java | 37 ++++++++-- .../test/resources/mock-bridge/startServer.js | 69 +++++++++++-------- 3 files changed, 76 insertions(+), 36 deletions(-) diff --git a/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java b/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java index fc3efa625fc..033f8ee13ab 100644 --- a/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java +++ b/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java @@ -19,6 +19,8 @@ */ package org.sonar.plugins.javascript.bridge; +import static org.sonarsource.api.sonarlint.SonarLintSide.INSTANCE; + import java.io.IOException; import java.util.List; import javax.annotation.Nullable; @@ -30,8 +32,6 @@ import org.sonar.plugins.javascript.bridge.protobuf.Node; import org.sonarsource.api.sonarlint.SonarLintSide; -import static org.sonarsource.api.sonarlint.SonarLintSide.INSTANCE; - @ScannerSide @SonarLintSide(lifespan = INSTANCE) public interface BridgeServer extends Startable { @@ -67,7 +67,7 @@ void initLinter(List rules, List environments, List TsConfigFile createTsConfigFile(String content) throws IOException; record JsAnalysisRequest(String filePath, String fileType, String language, @Nullable String fileContent, boolean ignoreHeaderComments, - @Nullable List tsConfigs, @Nullable String programId, String linterId) { + @Nullable List tsConfigs, @Nullable String programId, String linterId, boolean skipAst) { } diff --git a/sonar-plugin/bridge/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java b/sonar-plugin/bridge/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java index 0a9789706c2..3e6c356f23f 100644 --- a/sonar-plugin/bridge/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java +++ b/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"; diff --git a/sonar-plugin/bridge/src/test/resources/mock-bridge/startServer.js b/sonar-plugin/bridge/src/test/resources/mock-bridge/startServer.js index 6d787d6362d..80e540faa4b 100644 --- a/sonar-plugin/bridge/src/test/resources/mock-bridge/startServer.js +++ b/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 From bcdcfa292a3dd37c65198ea26c6d725e1cabada3 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Thu, 13 Jun 2024 08:35:10 +0200 Subject: [PATCH 03/11] fix compilation --- .../sonar/plugins/javascript/analysis/AbstractAnalysis.java | 5 +++-- .../org/sonar/plugins/javascript/analysis/HtmlSensor.java | 3 ++- .../org/sonar/plugins/javascript/analysis/YamlSensor.java | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java index ce57bb0e254..3c4a8a8c54f 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java @@ -28,6 +28,7 @@ import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.plugins.javascript.CancellationException; +import org.sonar.plugins.javascript.JavaScriptFilePredicate; import org.sonar.plugins.javascript.JavaScriptLanguage; import org.sonar.plugins.javascript.TypeScriptLanguage; import org.sonar.plugins.javascript.analysis.cache.CacheAnalysis; @@ -37,7 +38,6 @@ import org.sonar.plugins.javascript.bridge.AnalysisMode; import org.sonar.plugins.javascript.bridge.AnalysisWarningsWrapper; import org.sonar.plugins.javascript.bridge.BridgeServer; -import org.sonar.plugins.javascript.JavaScriptFilePredicate; import org.sonar.plugins.javascript.bridge.BridgeServer.TsProgram; import org.sonar.plugins.javascript.bridge.ESTreeFactory; import org.sonar.plugins.javascript.bridge.protobuf.Node; @@ -145,7 +145,8 @@ private BridgeServer.JsAnalysisRequest getJsAnalysisRequest( contextUtils.ignoreHeaderComments(), tsConfigs, tsProgram != null ? tsProgram.programId() : null, - analysisMode.getLinterIdFor(file) + analysisMode.getLinterIdFor(file), + false ); } } diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/HtmlSensor.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/HtmlSensor.java index 5c13ff2144f..59e2f155fdc 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/HtmlSensor.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/HtmlSensor.java @@ -131,7 +131,8 @@ private void analyze(InputFile file, CacheStrategy cacheStrategy) throws IOExcep contextUtils.ignoreHeaderComments(), null, null, - analysisMode.getLinterIdFor(file) + analysisMode.getLinterIdFor(file), + false ); var response = bridgeServer.analyzeHtml(jsAnalysisRequest); analysisProcessor.processResponse(context, checks, file, response); diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/YamlSensor.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/YamlSensor.java index 5b1ca2e3e44..24febb8a95c 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/YamlSensor.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/YamlSensor.java @@ -164,7 +164,8 @@ private void analyze(InputFile file) throws IOException { contextUtils.ignoreHeaderComments(), null, null, - analysisMode.getLinterIdFor(file) + analysisMode.getLinterIdFor(file), + false ); var response = bridgeServer.analyzeYaml(jsAnalysisRequest); analysisProcessor.processResponse(context, checks, file, response); From a57a217176b78a79731ed4e2e4dfb9d3b292f583 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Thu, 13 Jun 2024 09:59:25 +0200 Subject: [PATCH 04/11] fix CaYC issue --- packages/jsts/src/analysis/analyzer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jsts/src/analysis/analyzer.ts b/packages/jsts/src/analysis/analyzer.ts index e2de88e9c70..00c805dd983 100644 --- a/packages/jsts/src/analysis/analyzer.ts +++ b/packages/jsts/src/analysis/analyzer.ts @@ -108,7 +108,7 @@ function analyzeFile( } } -function serializeAst(sourceCode: SourceCode, filePath: string, skipAst: boolean = false) { +function serializeAst(sourceCode: SourceCode, filePath: string, skipAst = false) { if (!isSupported(filePath) || skipAst) { return null; } From 2db903a90633b228ec5419e853334633907ca9c4 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Thu, 13 Jun 2024 12:14:12 +0200 Subject: [PATCH 05/11] send `skipAst` flag if there are no consumers --- .../plugins/javascript/analysis/AbstractAnalysis.java | 8 +++++--- .../plugins/javascript/analysis/AnalysisConsumers.java | 4 ++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java index 3c4a8a8c54f..8a937c0ae9f 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java @@ -102,7 +102,8 @@ protected void analyzeFile(InputFile file, @Nullable List tsConfigs, @Nu LOG.debug("Analyzing file: {}", file.uri()); progressReport.nextFile(file.toString()); var fileContent = contextUtils.shouldSendFileContent(file) ? file.contents() : null; - var request = getJsAnalysisRequest(file, fileContent, tsProgram, tsConfigs); + var skipAst = consumers.getSize() < 1; + var request = getJsAnalysisRequest(file, fileContent, tsProgram, tsConfigs, skipAst); var response = isJavaScript(file) ? bridgeServer.analyzeJavaScript(request) @@ -135,7 +136,8 @@ private BridgeServer.JsAnalysisRequest getJsAnalysisRequest( InputFile file, @Nullable String fileContent, @Nullable TsProgram tsProgram, - @Nullable List tsConfigs + @Nullable List tsConfigs, + boolean skipAst ) { return new BridgeServer.JsAnalysisRequest( file.absolutePath(), @@ -146,7 +148,7 @@ private BridgeServer.JsAnalysisRequest getJsAnalysisRequest( tsConfigs, tsProgram != null ? tsProgram.programId() : null, analysisMode.getLinterIdFor(file), - false + skipAst ); } } diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisConsumers.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisConsumers.java index 12213eaeacb..de4dd72801b 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisConsumers.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisConsumers.java @@ -54,4 +54,8 @@ public void accept(JsFile jsFile) { public void doneAnalysis() { consumers.forEach(JsAnalysisConsumer::doneAnalysis); } + + public int getSize() { + return consumers.size(); + } } From 6b48bd315eadfe7332fe66cf0d5975da08b4ea05 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Thu, 13 Jun 2024 14:19:28 +0200 Subject: [PATCH 06/11] rollback changes when implementing formData --- .../it/plugin/SonarJsIntegrationTest.java | 39 +++---------------- 1 file changed, 5 insertions(+), 34 deletions(-) diff --git a/its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java b/its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java index e4253692602..06613b2156c 100644 --- a/its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java +++ b/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 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 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 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 { From 1734af5fe1db006695ee4005f550d3c6532f9597 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Thu, 13 Jun 2024 14:19:42 +0200 Subject: [PATCH 07/11] disable serializing AST here --- .../com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java b/its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java index 06613b2156c..ed47ac806f3 100644 --- a/its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java +++ b/its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java @@ -226,6 +226,8 @@ static class AnalysisRequest { String filePath; String fileContent; String fileType = "MAIN"; + + boolean skipAst = false; } static class InitLinter { From b7bc5579b29112780497acd9a0935993a59dd2cf Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Thu, 13 Jun 2024 14:35:49 +0200 Subject: [PATCH 08/11] skip ast correctly --- .../sonar/javascript/it/plugin/SonarJsIntegrationTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java b/its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java index ed47ac806f3..5a53d8de2cb 100644 --- a/its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java +++ b/its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/SonarJsIntegrationTest.java @@ -227,7 +227,7 @@ static class AnalysisRequest { String fileContent; String fileType = "MAIN"; - boolean skipAst = false; + boolean skipAst = true; } static class InitLinter { @@ -251,6 +251,4 @@ static class Rule { List configurations = Collections.emptyList(); String fileTypeTarget = "MAIN"; } - - record BridgeResponse(String json, String ast) {} } From fc96eba9f0c80e22688edfd76214b8e82a0baa4b Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Thu, 13 Jun 2024 16:49:31 +0200 Subject: [PATCH 09/11] add test that we send skipAst --- .../plugins/javascript/analysis/JsTsSensorTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java index 76c576f12c5..a71bf225ee7 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java @@ -370,6 +370,19 @@ void should_not_send_content() throws Exception { assertThat(deleteCaptor.getValue().programId()).isEqualTo(tsProgram.programId()); } + @Test + void should_send_skipAst_flag_when_there_are_no_consumers() throws Exception { + var ctx = createSensorContext(baseDir); + var inputFile = createInputFile(ctx); + var tsProgram = new TsProgram("1", List.of(inputFile.absolutePath()), List.of()); + when(bridgeServerMock.createProgram(any())).thenReturn(tsProgram); + when(bridgeServerMock.analyzeTypeScript(any())).thenReturn(new AnalysisResponse()); + createSensor().execute(ctx); + var captor = ArgumentCaptor.forClass(JsAnalysisRequest.class); + verify(bridgeServerMock).analyzeTypeScript(captor.capture()); + assertThat(captor.getValue().skipAst()).isTrue(); + } + @Test void should_send_content_when_not_utf8() throws Exception { var ctx = createSensorContext(baseDir); From 956ac5af9184fd73da18319d1e1f2780c774b5cc Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Thu, 13 Jun 2024 17:52:50 +0200 Subject: [PATCH 10/11] add test to verify that we do not send the `skipAst` flag when there are consumers --- .../javascript/analysis/JsTsSensorTest.java | 80 ++++++++++++++----- 1 file changed, 60 insertions(+), 20 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java index 28fbc56afd5..8430d2e8a59 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java @@ -383,6 +383,23 @@ void should_send_skipAst_flag_when_there_are_no_consumers() throws Exception { assertThat(captor.getValue().skipAst()).isTrue(); } + @Test + void should_not_send_the_skipAst_flag_when_there_are_consumers() throws Exception { + var ctx = createSensorContext(baseDir); + var inputFile = createInputFile(ctx); + var tsProgram = new TsProgram("1", List.of(inputFile.absolutePath()), List.of()); + var consumer = createConsumer(); + var sensor = createSensorWithConsumer(consumer); + when(bridgeServerMock.createProgram(any())).thenReturn(tsProgram); + when(bridgeServerMock.analyzeTypeScript(any())).thenReturn(new AnalysisResponse()); + sensor.execute(ctx); + + createSensor().execute(context); + var captor = ArgumentCaptor.forClass(JsAnalysisRequest.class); + verify(bridgeServerMock).analyzeTypeScript(captor.capture()); + assertThat(captor.getValue().skipAst()).isFalse(); + } + @Test void should_send_content_when_not_utf8() throws Exception { var ctx = createSensorContext(baseDir); @@ -849,6 +866,17 @@ public void doneAnalysis() { @Test void should_not_invoke_analysis_consumers_when_cannot_deserialize() throws Exception { + var inputFile = createInputFile(context); + var tsProgram = new TsProgram("1", List.of(inputFile.absolutePath()), List.of(), false, null); + when(bridgeServerMock.createProgram(any())).thenReturn(tsProgram); + + Node erroneousNode = Node.newBuilder() + .setType(NodeType.BlockStatementType) + .build(); + + when(bridgeServerMock.analyzeTypeScript(any())).thenReturn( + new AnalysisResponse(null, List.of(), List.of(), List.of(), new BridgeServer.Metrics(), List.of(), List.of(), erroneousNode) + ); var consumer = new JsAnalysisConsumer() { final List files = new ArrayList<>(); boolean done; @@ -863,33 +891,45 @@ public void doneAnalysis() { done = true; } }; + var sensor = createSensorWithConsumer(consumer); + sensor.execute(context); + assertThat(consumer.files).isEmpty(); + assertThat(consumer.done).isTrue(); - var sensor = new JsTsSensor( + assertThat(logTester.logs(Level.DEBUG)) + .contains("Failed to deserialize AST for file: " + inputFile.uri()); + } + + private JsAnalysisConsumer createConsumer() { + return new JsAnalysisConsumer() { + final List files = new ArrayList<>(); + boolean done; + + @Override + public void accept(JsFile jsFile) { + files.add(jsFile); + } + + @Override + public void doneAnalysis() { + done = true; + } + + public List getFiles() { + return this.files; + } + }; + } + + private JsTsSensor createSensorWithConsumer(JsAnalysisConsumer consumer) { + + return new JsTsSensor( checks(ESLINT_BASED_RULE, "S2260"), bridgeServerMock, analysisWithProgram(), analysisWithWatchProgram(), new AnalysisConsumers(List.of(consumer)) ); - - var inputFile = createInputFile(context); - var tsProgram = new TsProgram("1", List.of(inputFile.absolutePath()), List.of(), false, null); - when(bridgeServerMock.createProgram(any())).thenReturn(tsProgram); - - Node erroneousNode = Node.newBuilder() - .setType(NodeType.BlockStatementType) - .build(); - - when(bridgeServerMock.analyzeTypeScript(any())).thenReturn( - new AnalysisResponse(null, List.of(), List.of(), List.of(), new BridgeServer.Metrics(), List.of(), List.of(), erroneousNode) - ); - - sensor.execute(context); - assertThat(consumer.files).isEmpty(); - assertThat(consumer.done).isTrue(); - - assertThat(logTester.logs(Level.DEBUG)) - .contains("Failed to deserialize AST for file: " + inputFile.uri()); } private JsTsSensor createSensor() { From bf5277ae9af15a5382f70dd34407f824efdd0124 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Thu, 13 Jun 2024 18:03:50 +0200 Subject: [PATCH 11/11] apply changes from review --- packages/jsts/src/analysis/analyzer.ts | 8 ++++---- .../org/sonar/plugins/javascript/bridge/BridgeServer.java | 4 ++-- .../plugins/javascript/analysis/AbstractAnalysis.java | 2 +- .../plugins/javascript/analysis/AnalysisConsumers.java | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/jsts/src/analysis/analyzer.ts b/packages/jsts/src/analysis/analyzer.ts index 00c805dd983..98340c4d33f 100644 --- a/packages/jsts/src/analysis/analyzer.ts +++ b/packages/jsts/src/analysis/analyzer.ts @@ -92,8 +92,8 @@ function analyzeFile( ...extendedMetrics, }; - const ast = serializeAst(sourceCode, filePath, input.skipAst); - if (ast) { + const ast = serializeAst(sourceCode, filePath); + if (!input.skipAst && ast) { result.ast = ast; } @@ -108,8 +108,8 @@ function analyzeFile( } } -function serializeAst(sourceCode: SourceCode, filePath: string, skipAst = false) { - if (!isSupported(filePath) || skipAst) { +function serializeAst(sourceCode: SourceCode, filePath: string) { + if (!isSupported(filePath)) { return null; } diff --git a/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java b/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java index 033f8ee13ab..484a8befa7d 100644 --- a/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java +++ b/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java @@ -19,8 +19,6 @@ */ package org.sonar.plugins.javascript.bridge; -import static org.sonarsource.api.sonarlint.SonarLintSide.INSTANCE; - import java.io.IOException; import java.util.List; import javax.annotation.Nullable; @@ -32,6 +30,8 @@ import org.sonar.plugins.javascript.bridge.protobuf.Node; import org.sonarsource.api.sonarlint.SonarLintSide; +import static org.sonarsource.api.sonarlint.SonarLintSide.INSTANCE; + @ScannerSide @SonarLintSide(lifespan = INSTANCE) public interface BridgeServer extends Startable { diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java index 1d5bcdecec2..d9f4557cf7f 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java @@ -102,7 +102,7 @@ protected void analyzeFile(InputFile file, @Nullable List tsConfigs, @Nu LOG.debug("Analyzing file: {}", file.uri()); progressReport.nextFile(file.toString()); var fileContent = contextUtils.shouldSendFileContent(file) ? file.contents() : null; - var skipAst = consumers.getSize() < 1; + var skipAst = !consumers.hasConsumers(); var request = getJsAnalysisRequest(file, fileContent, tsProgram, tsConfigs, skipAst); var response = isJavaScript(file) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisConsumers.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisConsumers.java index de4dd72801b..c491141b0ed 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisConsumers.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisConsumers.java @@ -55,7 +55,7 @@ public void doneAnalysis() { consumers.forEach(JsAnalysisConsumer::doneAnalysis); } - public int getSize() { - return consumers.size(); + public boolean hasConsumers() { + return ! consumers.isEmpty(); } }