Do not fail silently (#4450)

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

@@ -37,6 +37,7 @@ public abstract class AbstractBridgeSensor implements Sensor {
 
  private static final Logger LOG = Loggers.get(AbstractBridgeSensor.class);
 
+ protected final String lang;
  protected final BridgeServer bridgeServer;
  protected List<String> exclusions;
  private final AnalysisWarningsWrapper analysisWarnings;
@@ -50,11 +51,13 @@ public abstract class AbstractBridgeSensor implements Sensor {
  protected AbstractBridgeSensor(
  BridgeServer bridgeServer,
  AnalysisWarningsWrapper analysisWarnings,
- Monitoring monitoring
+ Monitoring monitoring,
+ String lang
  ) {
  this.bridgeServer = bridgeServer;
  this.analysisWarnings = analysisWarnings;
  this.monitoring = monitoring;
+ this.lang = lang;
  }
 
  @Override
@@ -85,23 +88,18 @@ public void execute(SensorContext context) {
  LOG.debug("No rules will be executed");
  } catch (NodeCommandException e) {
  logErrorOrWarn(e.getMessage(), e);
- analysisWarnings.addUnique(
- "JavaScript/TypeScript/CSS rules were not executed. " + e.getMessage()
+ throw new IllegalStateException(
+ "Error while running Node.js. A supported version of Node.js is required for running the analysis of " +
+ this.lang +
+ " files. Please make sure a supported version of Node.js is available in the PATH. Alternatively, you can exclude " +
+ this.lang +
+ " files from your analysis using the 'sonar.exclusions' configuration property. " +
+ "See the docs for configuring the analysis environment: https://docs.sonarsource.com/sonarqube/latest/analyzing-source-code/languages/javascript-typescript-css/",
+ e
  );
- if (contextUtils.failFast()) {
- throw new IllegalStateException(
- "Analysis failed (\"sonar.internal.analysis.failFast\"=true)",
- e
- );
- }
  } catch (Exception e) {
  LOG.error("Failure during analysis", e);
- if (contextUtils.failFast()) {
- throw new IllegalStateException(
- "Analysis failed (\"sonar.internal.analysis.failFast\"=true)",
- e
- );
- }
+ throw new IllegalStateException("Analysis of " + this.lang + " files failed", e);
  } finally {
  CacheStrategies.logReport();
  monitoring.stopSensor();

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

@@ -62,7 +62,7 @@ public CssRuleSensor(
  Monitoring monitoring,
  CheckFactory checkFactory
  ) {
- super(bridgeServer, analysisWarnings, monitoring);
+ super(bridgeServer, analysisWarnings, monitoring, "CSS");
  this.sonarRuntime = sonarRuntime;
  this.cssRules = new CssRules(checkFactory);
  }

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

@@ -56,7 +56,7 @@ public HtmlSensor(
  ) {
  // The monitoring sensor remains inactive during HTML files analysis, as the
  // bridge doesn't provide nor compute metrics for such files.
- super(bridgeServer, analysisWarnings, monitoring);
+ super(bridgeServer, analysisWarnings, monitoring, "JS in HTML");
  this.analysisProcessor = processAnalysis;
  this.checks = checks;
  }

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

@@ -77,7 +77,7 @@ public JsTsSensor(
  AnalysisWithProgram analysisWithProgram,
  AnalysisWithWatchProgram analysisWithWatchProgram
  ) {
- super(bridgeServer, analysisWarnings, monitoring);
+ super(bridgeServer, analysisWarnings, monitoring, "JS/TS");
  this.tempFolder = tempFolder;
  this.analysisWithProgram = analysisWithProgram;
  this.analysisWithWatchProgram = analysisWithWatchProgram;

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

@@ -59,7 +59,7 @@ public YamlSensor(
  ) {
  // The monitoring sensor remains inactive during YAML files analysis, as the
  // bridge doesn't provide nor compute metrics for such files.
- super(bridgeServer, analysisWarnings, monitoring);
+ super(bridgeServer, analysisWarnings, monitoring, "JS in YAML");
  this.checks = checks;
  this.analysisProcessor = processAnalysis;
  }

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

@@ -108,7 +108,7 @@ public void setUp() throws IOException {
  when(bridgeServerMock.analyzeCss(any()))
  .thenReturn(
  response(
- "{ issues: [{\"line\":2,\"ruleId\":\"block-no-empty\",\"message\":\"Unexpected empty block\"}]}"
+ "{ issues: [{\"line\":1,\"ruleId\":\"block-no-empty\",\"message\":\"Unexpected empty block\"}]}"
  )
  );
  when(bridgeServerMock.getCommandInfo()).thenReturn("bridgeServerMock command info");
@@ -277,28 +277,18 @@ void test_no_file_to_analyze() {
  );
  }
 
- @Test
- void failed_server_should_log_warn_no_css() throws IOException {
- context.settings().setProperty("sonar.internal.analysis.failFast", "true");
- doThrow(new NodeCommandException("Exception Message"))
- .when(bridgeServerMock)
- .startServerLazily(any());
- addInputFile("file.html");
-
- assertThatThrownBy(() -> sensor.execute(context))
- .isInstanceOf(IllegalStateException.class)
- .hasMessageContaining("Analysis failed");
- assertThat(logTester.logs(LoggerLevel.WARN)).contains("Exception Message");
- }
-
  @Test
  void failed_server_should_log_error_with_css() throws IOException {
  doThrow(new NodeCommandException("Exception Message"))
  .when(bridgeServerMock)
  .startServerLazily(any());
  addInputFile("file.css");
 
- sensor.execute(context);
+ assertThatThrownBy(() -> sensor.execute(context))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage(
+ "Error while running Node.js. A supported version of Node.js is required for running the analysis of CSS files. Please make sure a supported version of Node.js is available in the PATH. Alternatively, you can exclude CSS files from your analysis using the 'sonar.exclusions' configuration property. See the docs for configuring the analysis environment: https://docs.sonarsource.com/sonarqube/latest/analyzing-source-code/languages/javascript-typescript-css/"
+ );
  assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Exception Message");
  }
 

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

@@ -20,6 +20,7 @@
 package org.sonar.plugins.javascript.bridge;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -225,12 +226,14 @@ void should_raise_a_parsing_error() throws IOException {
  }
 
  @Test
- void should_not_explode_if_no_response() throws Exception {
+ void should_explode_if_no_response() throws Exception {
  when(bridgeServerMock.analyzeHtml(any())).thenThrow(new IOException("error"));
 
  HtmlSensor sensor = createSensor();
  DefaultInputFile inputFile = createInputFile(context);
- sensor.execute(context);
+ assertThatThrownBy(() -> sensor.execute(context))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Analysis of JS in HTML files failed");
 
  assertThat(logTester.logs(LoggerLevel.ERROR))
  .contains("Failed to get response while analyzing " + inputFile.uri());

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

@@ -108,6 +108,9 @@ class JavaScriptEslintBasedSensorTest {
 
  private SensorContextTester context;
 
+ private String nodeExceptionMessage =
+ "Error while running Node.js. A supported version of Node.js is required for running the analysis of JS/TS files. Please make sure a supported version of Node.js is available in the PATH. Alternatively, you can exclude JS/TS files from your analysis using the 'sonar.exclusions' configuration property. See the docs for configuring the analysis environment: https://docs.sonarsource.com/sonarqube/latest/analyzing-source-code/languages/javascript-typescript-css/";
+
  @TempDir
  Path workDir;
 
@@ -451,18 +454,24 @@ void should_catch_if_bridge_server_not_started() throws Exception {
 
  var sensor = createSensor();
  createInputFile(context);
- sensor.execute(context);
+
+ assertThatThrownBy(() -> sensor.execute(context))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Analysis of JS/TS files failed");
 
  assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Failure during analysis");
  assertThat(context.allIssues()).isEmpty();
  }
 
  @Test
- void should_not_explode_if_no_response() throws Exception {
+ void should_explode_if_no_response() throws Exception {
  when(bridgeServerMock.analyzeJavaScript(any())).thenThrow(new IOException("error"));
  var sensor = createSensor();
  DefaultInputFile inputFile = createInputFile(context);
- sensor.execute(context);
+
+ assertThatThrownBy(() -> sensor.execute(context))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Analysis of JS/TS files failed");
 
  assertThat(logTester.logs(LoggerLevel.ERROR))
  .contains("Failed to get response while analyzing " + inputFile);
@@ -537,21 +546,22 @@ void handle_missing_node() throws Exception {
  .when(bridgeServerMock)
  .startServerLazily(any());
 
- TestAnalysisWarnings analysisWarnings = new TestAnalysisWarnings();
  var javaScriptEslintBasedSensor = new JsTsSensor(
  checks(ESLINT_BASED_RULE),
  bridgeServerMock,
- analysisWarnings,
+ new AnalysisWarningsWrapper(),
  tempFolder,
  monitoring,
  analysisWithProgram,
  analysisWithWatchProgram
  );
  createInputFile(context);
- javaScriptEslintBasedSensor.execute(context);
+
+ assertThatThrownBy(() -> javaScriptEslintBasedSensor.execute(context))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage(nodeExceptionMessage);
+
  assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Exception Message");
- assertThat(analysisWarnings.warnings)
- .containsExactly("JavaScript/TypeScript/CSS rules were not executed. Exception Message");
  }
 
  @Test
@@ -680,24 +690,20 @@ void should_send_content_when_not_utf8() throws Exception {
  void should_fail_fast() throws Exception {
  when(bridgeServerMock.analyzeJavaScript(any())).thenThrow(new IOException("error"));
  var sensor = createSensor();
- MapSettings settings = new MapSettings().setProperty("sonar.internal.analysis.failFast", true);
- context.setSettings(settings);
  DefaultInputFile inputFile = createInputFile(context);
  assertThatThrownBy(() -> sensor.execute(context))
  .isInstanceOf(IllegalStateException.class)
- .hasMessage("Analysis failed (\"sonar.internal.analysis.failFast\"=true)");
+ .hasMessage("Analysis of JS/TS files failed");
  }
 
  @Test
  void should_fail_fast_with_nodecommandexception() throws Exception {
  doThrow(new NodeCommandException("error")).when(bridgeServerMock).startServerLazily(any());
  var sensor = createSensor();
- MapSettings settings = new MapSettings().setProperty("sonar.internal.analysis.failFast", true);
- context.setSettings(settings);
  createInputFile(context);
  assertThatThrownBy(() -> sensor.execute(context))
  .isInstanceOf(IllegalStateException.class)
- .hasMessage("Analysis failed (\"sonar.internal.analysis.failFast\"=true)");
+ .hasMessage(nodeExceptionMessage);
  }
 
  @Test

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

@@ -172,14 +172,15 @@ void should_have_descriptor() throws Exception {
 
  @Test
  void should_analyse() throws Exception {
- AnalysisResponse expectedResponse = createResponse();
- when(bridgeServerMock.analyzeTypeScript(any())).thenReturn(expectedResponse);
-
  JsTsSensor sensor = createSensor();
  DefaultInputFile inputFile = createInputFile(context);
- createVueInputFile();
  createTsConfigFile();
 
+ AnalysisResponse expectedResponse = createResponse();
+ when(bridgeServerMock.analyzeTypeScript(any())).thenReturn(expectedResponse);
+ var tsProgram = new TsProgram("1", List.of(inputFile.absolutePath()), List.of());
+ when(bridgeServerMock.createProgram(any())).thenReturn(tsProgram);
+
  sensor.execute(context);
  verify(bridgeServerMock, times(1)).initLinter(any(), any(), any(), any(), any(), any());
  assertThat(context.allIssues()).hasSize(expectedResponse.issues.size());
@@ -236,13 +237,15 @@ void should_analyse() throws Exception {
  }
 
  @Test
- void should_not_explode_if_no_response() throws Exception {
+ void should_explode_if_no_response() throws Exception {
  createVueInputFile();
  when(bridgeServerMock.analyzeTypeScript(any())).thenThrow(new IOException("error"));
 
  JsTsSensor sensor = createSensor();
  DefaultInputFile inputFile = createInputFile(context);
- sensor.execute(context);
+ assertThatThrownBy(() -> sensor.execute(context))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Analysis of JS/TS files failed");
 
  assertThat(logTester.logs(LoggerLevel.ERROR))
  .contains("Failed to get response while analyzing " + inputFile.uri());
@@ -667,13 +670,11 @@ void should_fail_fast() throws Exception {
  createTsConfigFile();
  when(bridgeServerMock.analyzeTypeScript(any())).thenThrow(new IOException("error"));
  JsTsSensor sensor = createSensor();
- MapSettings settings = new MapSettings().setProperty("sonar.internal.analysis.failFast", true);
- context.setSettings(settings);
  createInputFile(context);
 
  assertThatThrownBy(() -> sensor.execute(context))
  .isInstanceOf(IllegalStateException.class)
- .hasMessage("Analysis failed (\"sonar.internal.analysis.failFast\"=true)");
+ .hasMessage("Analysis of JS/TS files failed");
  }
 
  @Test
@@ -689,7 +690,7 @@ void should_fail_fast_with_parsing_error_without_line() throws IOException {
  createInputFile(context);
  assertThatThrownBy(() -> createSensor().execute(context))
  .isInstanceOf(IllegalStateException.class)
- .hasMessage("Analysis failed (\"sonar.internal.analysis.failFast\"=true)");
+ .hasMessage("Analysis of JS/TS files failed");
  assertThat(logTester.logs(LoggerLevel.ERROR))
  .contains("Failed to analyze file [dir/file.ts]: Parse error message");
  }
@@ -902,7 +903,7 @@ private void createVueInputFile(SensorContextTester context) {
  )
  .setLanguage("js")
  .setCharset(StandardCharsets.UTF_8)
- .setContents("<script lang=\"ts\"></script>")
+ .setContents("<script lang=\"ts\">\nif (cond)\ndoFoo(); \nelse \ndoFoo();\n</script>")
  .build();
  context.fileSystem().add(vueFile);
  }

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

@@ -20,6 +20,7 @@
 package org.sonar.plugins.javascript.bridge;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -194,12 +195,15 @@ void should_raise_a_parsing_error() throws IOException {
  }
 
  @Test
- void should_not_explode_if_no_response() throws Exception {
+ void should_explode_if_no_response() throws Exception {
  when(bridgeServerMock.analyzeYaml(any())).thenThrow(new IOException("error"));
 
  YamlSensor sensor = createSensor();
  DefaultInputFile inputFile = createInputFile(context);
- sensor.execute(context);
+
+ assertThatThrownBy(() -> sensor.execute(context))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Analysis of JS in YAML files failed");
 
  assertThat(logTester.logs(LoggerLevel.ERROR))
  .contains("Failed to get response while analyzing " + inputFile.uri());