From e1084ab0bee42625105ff332365b05ab30654d47 Mon Sep 17 00:00:00 2001 From: Alban Auzeill Date: Mon, 23 Dec 2019 14:38:46 +0100 Subject: Send file content to NodeJS process if encoding not UFT-8 (#224) --- sonar-css-plugin/css-bundle/src/server.ts | 7 +-- sonar-css-plugin/css-bundle/tests/server.test.ts | 16 +++++++ .../java/org/sonar/css/plugin/CssRuleSensor.java | 13 ++++-- .../css/plugin/server/AnalyzerBridgeServer.java | 11 ++++- .../org/sonar/css/plugin/CssRuleSensorTest.java | 53 ++++++++++++++++++++-- .../plugin/server/CssAnalyzerBridgeServerTest.java | 8 ++-- .../resources/mock-start-server/startServer.js | 11 +++-- .../test/resources/mock-start-server/testLogs.js | 19 ++++++++ 8 files changed, 120 insertions(+), 18 deletions(-) create mode 100644 sonar-css-plugin/src/test/resources/mock-start-server/testLogs.js diff --git a/sonar-css-plugin/css-bundle/src/server.ts b/sonar-css-plugin/css-bundle/src/server.ts index 05405c8..93bb6cb 100644 --- a/sonar-css-plugin/css-bundle/src/server.ts +++ b/sonar-css-plugin/css-bundle/src/server.ts @@ -53,9 +53,9 @@ function analyzeWithStylelint( response: express.Response ) { const parsedRequest = request.body as AnalysisInput; - const { filePath, configFile } = parsedRequest; - const code = getFileContent(filePath); - + const { filePath, fileContent, configFile } = parsedRequest; + const code = + typeof fileContent == "string" ? fileContent : getFileContent(filePath); const options = { code, codeFilename: filePath, @@ -106,6 +106,7 @@ function getFileContent(filePath: string) { export interface AnalysisInput { filePath: string; + fileContent: string | undefined; configFile: string; } diff --git a/sonar-css-plugin/css-bundle/tests/server.test.ts b/sonar-css-plugin/css-bundle/tests/server.test.ts index 8465810..84fc617 100644 --- a/sonar-css-plugin/css-bundle/tests/server.test.ts +++ b/sonar-css-plugin/css-bundle/tests/server.test.ts @@ -147,6 +147,22 @@ describe("server", () => { ); }); + it("should use fileContent from the request and not from the filesystem", async () => { + const request = JSON.stringify({ + filePath: path.join(__dirname, "fixtures", "file.css"), + fileContent: "\n\n a { }", // move the issue on line 3 + configFile + }); + const response = await post(request, "/analyze"); + expect(JSON.parse(response)).toEqual([ + { + line: 3, + rule: "block-no-empty", + text: "Unexpected empty block (block-no-empty)" + } + ]); + }); + function post(data: string, endpoint: string): Promise { return postToServer(data, endpoint, server); } diff --git a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssRuleSensor.java b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssRuleSensor.java index 644ece8..3b32501 100644 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssRuleSensor.java +++ b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssRuleSensor.java @@ -35,6 +35,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.StreamSupport; import javax.annotation.Nullable; +import org.sonar.api.SonarProduct; import org.sonar.api.batch.fs.FilePredicate; import org.sonar.api.batch.fs.FilePredicates; import org.sonar.api.batch.fs.FileSystem; @@ -51,8 +52,8 @@ import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.css.plugin.CssRules.StylelintConfig; import org.sonar.css.plugin.server.AnalyzerBridgeServer.Issue; -import org.sonar.css.plugin.server.CssAnalyzerBridgeServer; import org.sonar.css.plugin.server.AnalyzerBridgeServer.Request; +import org.sonar.css.plugin.server.CssAnalyzerBridgeServer; import org.sonar.css.plugin.server.exception.ServerAlreadyFailedException; import org.sonarsource.analyzer.commons.ProgressReport; import org.sonarsource.nodejs.NodeCommandException; @@ -140,7 +141,7 @@ public class CssRuleSensor implements Sensor { try { analyzeFile(context, inputFile, configFile); } catch (IOException | RuntimeException e) { - throw new IOException("Failure during analysis of " + inputFile.uri() + ": " + e.getMessage()); + throw new IOException("Failure during analysis of " + inputFile.uri() + ": " + e.getMessage(), e); } progressReport.nextFile(); } else { @@ -164,13 +165,19 @@ public class CssRuleSensor implements Sensor { LOG.debug("Skipping {} as it has not 'file' scheme", uri); return; } - Request request = new Request(new File(uri).getAbsolutePath(), configFile.toString()); + String fileContent = shouldSendFileContent(context, inputFile) ? inputFile.contents() : null; + Request request = new Request(new File(uri).getAbsolutePath(), fileContent, configFile.toString()); LOG.debug("Analyzing " + request.filePath); Issue[] issues = cssAnalyzerBridgeServer.analyze(request); LOG.debug("Found {} issue(s)", issues.length); saveIssues(context, inputFile, issues); } + private static boolean shouldSendFileContent(SensorContext context, InputFile file) { + return context.runtime().getProduct() == SonarProduct.SONARLINT + || !StandardCharsets.UTF_8.equals(file.charset()); + } + private void saveIssues(SensorContext context, InputFile inputFile, Issue[] issues) { for (Issue issue : issues) { NewIssue sonarIssue = context.newIssue(); diff --git a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/AnalyzerBridgeServer.java b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/AnalyzerBridgeServer.java index 0656b06..56481ec 100644 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/AnalyzerBridgeServer.java +++ b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/AnalyzerBridgeServer.java @@ -20,6 +20,7 @@ package org.sonar.css.plugin.server; import java.io.IOException; +import javax.annotation.Nullable; import org.sonar.api.Startable; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.scanner.ScannerSide; @@ -41,10 +42,18 @@ public interface AnalyzerBridgeServer extends Startable { class Request { public final String filePath; + /** + * The fileContent is sent only in the SonarLint context or when the encoding + * of the file is not utf-8. Otherwise, for performance reason, it's more efficient to + * not have the fileContent and let the server getting it using filePath. + */ + @Nullable + public final String fileContent; public final String configFile; - public Request(String filePath, String configFile) { + public Request(String filePath, @Nullable String fileContent, String configFile) { this.filePath = filePath; + this.fileContent = fileContent; this.configFile = configFile; } } diff --git a/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssRuleSensorTest.java b/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssRuleSensorTest.java index 0dd35d8..0b37c92 100644 --- a/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssRuleSensorTest.java +++ b/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssRuleSensorTest.java @@ -35,13 +35,14 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.junit.rules.TemporaryFolder; import org.sonar.api.batch.fs.InputFile; -import org.sonar.api.batch.fs.InputFile.Type; import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.api.batch.fs.internal.TestInputFileBuilder; import org.sonar.api.batch.rule.CheckFactory; import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor; import org.sonar.api.batch.sensor.internal.SensorContextTester; +import org.sonar.api.internal.SonarRuntimeImpl; import org.sonar.api.notifications.AnalysisWarnings; +import org.sonar.api.utils.Version; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; import org.sonar.css.plugin.server.CssAnalyzerBridgeServer; @@ -260,10 +261,56 @@ public class CssRuleSensorTest { assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Unknown stylelint rule or rule not enabled: 'unknown-rule-key'"); } + @Test + public void should_not_send_file_content_if_encoding_is_utf8_and_context_is_not_sonarlint() throws IOException { + String filePath = "copy-file-content-into-issue-message.css"; + DefaultInputFile inputFile = new TestInputFileBuilder("moduleKey", filePath) + .setLanguage(CssLanguage.KEY) + .setCharset(StandardCharsets.UTF_8) + .setContents("css content") + .build(); + context.fileSystem().add(inputFile); + sensor.execute(context); + + assertThat(context.allIssues()).hasSize(1); + assertThat(context.allIssues()).extracting("primaryLocation.message") + .containsOnly("undefined"); + } + + @Test + public void should_send_file_content_if_encoding_is_not_utf8() throws IOException { + String filePath = "copy-file-content-into-issue-message.css"; + DefaultInputFile inputFile = new TestInputFileBuilder("moduleKey", filePath) + .setLanguage(CssLanguage.KEY) + .setCharset(StandardCharsets.ISO_8859_1) + .setContents("css content") + .build(); + context.fileSystem().add(inputFile); + sensor.execute(context); + + assertThat(context.allIssues()).hasSize(1); + assertThat(context.allIssues()).extracting("primaryLocation.message") + .containsOnly("css content"); + } + + @Test + public void should_send_file_content_if_context_is_sonarlint() throws IOException { + String filePath = "copy-file-content-into-issue-message.css"; + DefaultInputFile inputFile = new TestInputFileBuilder("moduleKey", filePath) + .setLanguage(CssLanguage.KEY) + .setCharset(StandardCharsets.UTF_8) + .setContents("css content") + .build(); + context.fileSystem().add(inputFile); + context.setRuntime(SonarRuntimeImpl.forSonarLint(Version.create(7, 9))); + sensor.execute(context); + assertThat(context.allIssues()).hasSize(1); + assertThat(context.allIssues()).extracting("primaryLocation.message") + .containsOnly("css content"); + } + private DefaultInputFile addInputFile(String relativePath) { DefaultInputFile inputFile = new TestInputFileBuilder("moduleKey", relativePath) - .setModuleBaseDir(context.fileSystem().baseDirPath()) - .setType(Type.MAIN) .setLanguage(relativePath.split("\\.")[1]) .setCharset(StandardCharsets.UTF_8) .setContents("some css content\n on 2 lines") diff --git a/sonar-css-plugin/src/test/java/org/sonar/css/plugin/server/CssAnalyzerBridgeServerTest.java b/sonar-css-plugin/src/test/java/org/sonar/css/plugin/server/CssAnalyzerBridgeServerTest.java index 302b6bb..45ae8ae 100644 --- a/sonar-css-plugin/src/test/java/org/sonar/css/plugin/server/CssAnalyzerBridgeServerTest.java +++ b/sonar-css-plugin/src/test/java/org/sonar/css/plugin/server/CssAnalyzerBridgeServerTest.java @@ -120,7 +120,7 @@ public class CssAnalyzerBridgeServerTest { @Test public void should_forward_process_streams() throws Exception { - cssAnalyzerBridgeServer = createCssAnalyzerBridgeServer(); + cssAnalyzerBridgeServer = createCssAnalyzerBridgeServer("testLogs.js"); cssAnalyzerBridgeServer.startServerLazily(context); assertThat(logTester.logs(DEBUG)).contains("testing debug log"); @@ -133,14 +133,14 @@ public class CssAnalyzerBridgeServerTest { cssAnalyzerBridgeServer = createCssAnalyzerBridgeServer(); cssAnalyzerBridgeServer.startServerLazily(context); - Request request = new Request("/absolute/path/file.css", CONFIG_FILE); + Request request = new Request("/absolute/path/file.css", null, CONFIG_FILE); Issue[] issues = cssAnalyzerBridgeServer.analyze(request); assertThat(issues).hasSize(1); assertThat(issues[0].line).isEqualTo(2); assertThat(issues[0].rule).isEqualTo("block-no-empty"); assertThat(issues[0].text).isEqualTo("Unexpected empty block"); - request = new Request("/absolute/path/empty.css", CONFIG_FILE); + request = new Request("/absolute/path/empty.css", null, CONFIG_FILE); issues = cssAnalyzerBridgeServer.analyze(request); assertThat(issues).isEmpty(); } @@ -217,7 +217,7 @@ public class CssAnalyzerBridgeServerTest { DefaultInputFile inputFile = TestInputFileBuilder.create("foo", "invalid-json-response.css") .build(); - Request request = new Request(inputFile.absolutePath(), CONFIG_FILE); + Request request = new Request(inputFile.absolutePath(), null, CONFIG_FILE); assertThatThrownBy(() -> cssAnalyzerBridgeServer.analyze(request)).isInstanceOf(IllegalStateException.class); assertThat(context.allIssues()).isEmpty(); } diff --git a/sonar-css-plugin/src/test/resources/mock-start-server/startServer.js b/sonar-css-plugin/src/test/resources/mock-start-server/startServer.js index c8b8d38..f7f40c5 100644 --- a/sonar-css-plugin/src/test/resources/mock-start-server/startServer.js +++ b/sonar-css-plugin/src/test/resources/mock-start-server/startServer.js @@ -3,10 +3,6 @@ const http = require('http'); const port = process.argv[2]; -console.log(`DEBUG testing debug log`) -console.log(`WARN testing warn log`) -console.log(`testing info log`) - const requestHandler = (request, response) => { let data = []; request.on('data', chunk => { @@ -14,9 +10,11 @@ const requestHandler = (request, response) => { }); request.on('end', () => { let fileName = null; + let fileContent = null; if (data.length > 0) { const analysisRequest = JSON.parse(data.join()); fileName = analysisRequest.filePath.replace(/.*[\/\\]/g,""); + fileContent = analysisRequest.fileContent; } if (request.url === '/status') { response.writeHead(200, { 'Content-Type': 'text/plain' }); @@ -52,6 +50,11 @@ const requestHandler = (request, response) => { case "invalid-json-response.css": response.end("["); break; + case "copy-file-content-into-issue-message.css": + response.end(JSON.stringify([ + {line: 1, rule: "block-no-empty", text: "" + fileContent} + ])); + break; default: throw "Unexpected fileName: " + fileName; } diff --git a/sonar-css-plugin/src/test/resources/mock-start-server/testLogs.js b/sonar-css-plugin/src/test/resources/mock-start-server/testLogs.js new file mode 100644 index 0000000..8fb5caa --- /dev/null +++ b/sonar-css-plugin/src/test/resources/mock-start-server/testLogs.js @@ -0,0 +1,19 @@ +#!/usr/bin/env node + +const http = require('http'); +const port = process.argv[2]; + +console.log(`DEBUG testing debug log`) +console.log(`WARN testing warn log`) +console.log(`testing info log`) + + +const server = http.createServer(() => {}); + +server.listen(port, (err) => { + if (err) { + return console.log('something bad happened', err) + } + + console.log(`server is listening on ${port}`) +}); -- cgit v1.2.3