diff options
| author | Elena Vilchik | 2019-12-27 11:49:52 +0100 | 
|---|---|---|
| committer | GitHub | 2019-12-27 11:49:52 +0100 | 
| commit | 0c7fadc03cca985bfc5fbae3a29f29cc71866bac (patch) | |
| tree | a68977c79532e0064f75c2657c2d3e76cbc49ccc /sonar-css-plugin/src/main | |
| parent | e65fb9a64ef2b81c3741a63d6d4a046b37659628 (diff) | |
| download | sonar-css-0c7fadc03cca985bfc5fbae3a29f29cc71866bac.tar.bz2 | |
Avoid error level logs when on project without CSS files (#231)
Diffstat (limited to 'sonar-css-plugin/src/main')
6 files changed, 177 insertions, 195 deletions
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 3b32501..db961df 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 @@ -37,7 +37,6 @@ 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;  import org.sonar.api.batch.fs.InputFile;  import org.sonar.api.batch.rule.CheckFactory; @@ -51,12 +50,10 @@ import org.sonar.api.rule.RuleKey;  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.AnalyzerBridgeServer.Request;  import org.sonar.css.plugin.server.CssAnalyzerBridgeServer; -import org.sonar.css.plugin.server.exception.ServerAlreadyFailedException; +import org.sonar.css.plugin.server.CssAnalyzerBridgeServer.Issue; +import org.sonar.css.plugin.server.CssAnalyzerBridgeServer.Request;  import org.sonarsource.analyzer.commons.ProgressReport; -import org.sonarsource.nodejs.NodeCommandException;  public class CssRuleSensor implements Sensor { @@ -89,34 +86,34 @@ public class CssRuleSensor implements Sensor {    public void execute(SensorContext context) {      reportOldNodeProperty(context); -    boolean failFast = context.config().getBoolean("sonar.internal.analysis.failFast").orElse(false); +    List<InputFile> inputFiles = getInputFiles(context); +    if (inputFiles.isEmpty()) { +      LOG.info("No CSS, PHP or HTML files are found in the project. CSS analysis is skipped."); +      return; +    } + +    File configFile = null; +    boolean serverRunning = false;      try { -      List<InputFile> inputFiles = getInputFiles(context); -      if (inputFiles.isEmpty()) { -        LOG.info("No CSS, PHP or HTML files are found in the project. CSS analysis is skipped."); -      } else { -        cssAnalyzerBridgeServer.startServerLazily(context); -        File configFile = createLinterConfig(context); -        analyzeFiles(context, inputFiles, configFile); -      } -    } catch (CancellationException e) { -      // do not propagate the exception -      LOG.info(e.toString()); -    } catch (ServerAlreadyFailedException e) { -      LOG.debug("Skipping start of css-bundle server due to the failure during first analysis"); -      LOG.debug("Skipping execution of CSS rules due to the problems with css-bundle server"); -    } catch (NodeCommandException e) { -      LOG.error(e.getMessage(), e); -      reportAnalysisWarning("CSS rules were not executed. " + e.getMessage()); -      if (failFast) { -        throw new IllegalStateException("Analysis failed (\"sonar.internal.analysis.failFast\"=true)", e); -      } +      serverRunning = cssAnalyzerBridgeServer.startServerLazily(context); +      configFile = createLinterConfig(context);      } catch (Exception e) { -      LOG.error("Failure during analysis, " + cssAnalyzerBridgeServer.getCommandInfo(), e); -      if (failFast) { -        throw new IllegalStateException("Analysis failed (\"sonar.internal.analysis.failFast\"=true)", e); -      } +      // we can end up here in the following cases: problem during bundle unpacking, or config file creation, or socket creation +      String msg = "Failure during CSS analysis preparation, " + cssAnalyzerBridgeServer.getCommandInfo(); +      logErrorOrWarn(context, msg, e); +      throwFailFast(context, e); +    } + +    if (serverRunning && configFile != null) { +      analyzeFiles(context, inputFiles, configFile); +    } +  } + +  public static void throwFailFast(SensorContext context, Exception e) { +    boolean failFast = context.config().getBoolean("sonar.internal.analysis.failFast").orElse(false); +    if (failFast) { +      throw new IllegalStateException("Analysis failed (\"sonar.internal.analysis.failFast\"=true)", e);      }    } @@ -128,34 +125,58 @@ public class CssRuleSensor implements Sensor {      }    } -  void analyzeFiles(SensorContext context, List<InputFile> inputFiles, File configFile) throws InterruptedException, IOException { +  private void analyzeFiles(SensorContext context, List<InputFile> inputFiles, File configFile) {      ProgressReport progressReport = new ProgressReport("Analysis progress", TimeUnit.SECONDS.toMillis(10));      boolean success = false; +      try {        progressReport.start(inputFiles.stream().map(InputFile::toString).collect(Collectors.toList()));        for (InputFile inputFile : inputFiles) { -        if (context.isCancelled()) { -          throw new CancellationException("Analysis interrupted because the SensorContext is in cancelled state"); -        } -        if (cssAnalyzerBridgeServer.isAlive()) { -          try { -            analyzeFile(context, inputFile, configFile); -          } catch (IOException | RuntimeException e) { -            throw new IOException("Failure during analysis of " + inputFile.uri() + ": " + e.getMessage(), e); -          } -          progressReport.nextFile(); -        } else { -          throw new IllegalStateException("css-bundle server is not answering"); -        } +        analyzeFileWithContextCheck(inputFile, context, configFile); +        progressReport.nextFile();        }        success = true; + +    } catch (CancellationException e) { +      // do not propagate the exception +      LOG.info(e.toString()); + +    } catch (Exception e) { +      // we can end up here in the following cases: file analysis request sending, or response parsing, server is not answering +      // or some unpredicted state +      String msg = "Failure during CSS analysis, " + cssAnalyzerBridgeServer.getCommandInfo(); +      logErrorOrWarn(context, msg, e); +      throwFailFast(context, e);      } finally { -      if (success) { -        progressReport.stop(); -      } else { -        progressReport.cancel(); -      } +      finishProgressReport(progressReport, success); +    } +  } + +  private static void finishProgressReport(ProgressReport progressReport, boolean success) { +    if (success) { +      progressReport.stop(); +    } else { +      progressReport.cancel(); +    } +    try {        progressReport.join(); +    } catch (InterruptedException e) { +      LOG.error(e.getMessage(), e); +      Thread.currentThread().interrupt(); +    } +  } + +  void analyzeFileWithContextCheck(InputFile inputFile, SensorContext context, File configFile) throws IOException { +    if (context.isCancelled()) { +      throw new CancellationException("Analysis interrupted because the SensorContext is in cancelled state"); +    } +    if (!cssAnalyzerBridgeServer.isAlive()) { +      throw new IllegalStateException("css-bundle server is not answering"); +    } +    try { +      analyzeFile(context, inputFile, configFile); +    } catch (IOException | RuntimeException e) { +      throw new IllegalStateException("Failure during analysis of " + inputFile.uri(), e);      }    } @@ -187,9 +208,9 @@ public class CssRuleSensor implements Sensor {        if (ruleKey == null) {          if ("CssSyntaxError".equals(issue.rule)) {            String errorMessage = issue.text.replace("(CssSyntaxError)", "").trim(); -          LOG.error("Failed to parse {}, line {}, {}", inputFile.uri(), issue.line, errorMessage); +          logErrorOrDebug(inputFile, "Failed to parse {}, line {}, {}", inputFile.uri(), issue.line, errorMessage);          } else { -          LOG.error("Unknown stylelint rule or rule not enabled: '" + issue.rule + "'"); +          logErrorOrDebug(inputFile,"Unknown stylelint rule or rule not enabled: '" + issue.rule + "'");          }        } else { @@ -206,16 +227,39 @@ public class CssRuleSensor implements Sensor {      }    } +  private static void logErrorOrDebug(InputFile file, String msg, Object ... arguments) { +    if (CssLanguage.KEY.equals(file.language())) { +      LOG.error(msg, arguments); +    } else { +      LOG.debug(msg, arguments); +    } +  } + +  private static void logErrorOrWarn(SensorContext context, String msg, Throwable e) { +    if (hasCssFiles(context)) { +      LOG.error(msg, e); +    } else { +      LOG.warn(msg); +    } +  } +    private static List<InputFile> getInputFiles(SensorContext context) {      FileSystem fileSystem = context.fileSystem(); -    FilePredicates predicates = context.fileSystem().predicates(); -    FilePredicate mainFilePredicate = predicates.and( +    FilePredicate mainFilePredicate = fileSystem.predicates().and(        fileSystem.predicates().hasType(InputFile.Type.MAIN),        fileSystem.predicates().hasLanguages(CssLanguage.KEY, "php", "web"));      return StreamSupport.stream(fileSystem.inputFiles(mainFilePredicate).spliterator(), false)        .collect(Collectors.toList());    } +  public static boolean hasCssFiles(SensorContext context) { +    FileSystem fileSystem = context.fileSystem(); +    FilePredicate mainFilePredicate = fileSystem.predicates().and( +      fileSystem.predicates().hasType(InputFile.Type.MAIN), +      fileSystem.predicates().hasLanguages(CssLanguage.KEY)); +    return fileSystem.inputFiles(mainFilePredicate).iterator().hasNext(); +  } +    private File createLinterConfig(SensorContext context) throws IOException {      StylelintConfig config = cssRules.getConfig();      final GsonBuilder gsonBuilder = new GsonBuilder(); 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 deleted file mode 100644 index 56481ec..0000000 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/AnalyzerBridgeServer.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * SonarCSS - * Copyright (C) 2018-2019 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA. - */ -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; -import org.sonarsource.api.sonarlint.SonarLintSide; - -import static org.sonarsource.api.sonarlint.SonarLintSide.MULTIPLE_ANALYSES; - -@ScannerSide -@SonarLintSide(lifespan = MULTIPLE_ANALYSES) -public interface AnalyzerBridgeServer extends Startable { - -  void startServerLazily(SensorContext context) throws IOException; - -  Issue[] analyze(Request request) throws IOException; - -  String getCommandInfo(); - -  boolean isAlive(); - -  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, @Nullable String fileContent, String configFile) { -      this.filePath = filePath; -      this.fileContent = fileContent; -      this.configFile = configFile; -    } -  } - -  class Issue { -    public final Integer line; -    public final String rule; -    public final String text; - -    public Issue(Integer line, String rule, String text) { -      this.line = line; -      this.rule = rule; -      this.text = text; -    } -  } - -} diff --git a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/CssAnalyzerBridgeServer.java b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/CssAnalyzerBridgeServer.java index 05760f8..271d8f8 100644 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/CssAnalyzerBridgeServer.java +++ b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/CssAnalyzerBridgeServer.java @@ -24,22 +24,31 @@ import com.google.gson.JsonSyntaxException;  import java.io.File;  import java.io.IOException;  import java.time.Duration; +import javax.annotation.Nullable;  import okhttp3.HttpUrl;  import okhttp3.MediaType;  import okhttp3.OkHttpClient;  import okhttp3.RequestBody;  import okhttp3.Response;  import org.sonar.api.batch.sensor.SensorContext; +import org.sonar.api.notifications.AnalysisWarnings; +import org.sonar.api.scanner.ScannerSide;  import org.sonar.api.utils.log.Logger;  import org.sonar.api.utils.log.Loggers;  import org.sonar.api.utils.log.Profiler;  import org.sonar.css.plugin.server.bundle.Bundle; -import org.sonar.css.plugin.server.exception.ServerAlreadyFailedException; +import org.sonarsource.api.sonarlint.SonarLintSide;  import org.sonarsource.nodejs.NodeCommand;  import org.sonarsource.nodejs.NodeCommandBuilder;  import org.sonarsource.nodejs.NodeCommandException; -public class CssAnalyzerBridgeServer implements AnalyzerBridgeServer { +import static org.sonar.css.plugin.CssRuleSensor.hasCssFiles; +import static org.sonar.css.plugin.CssRuleSensor.throwFailFast; +import static org.sonarsource.api.sonarlint.SonarLintSide.MULTIPLE_ANALYSES; + +@ScannerSide +@SonarLintSide(lifespan = MULTIPLE_ANALYSES) +public class CssAnalyzerBridgeServer {    private static final Logger LOG = Loggers.get(CssAnalyzerBridgeServer.class);    private static final Profiler PROFILER = Profiler.createIfDebug(LOG); @@ -53,28 +62,29 @@ public class CssAnalyzerBridgeServer implements AnalyzerBridgeServer {    private final NodeCommandBuilder nodeCommandBuilder;    final int timeoutSeconds;    private final Bundle bundle; +  private final AnalysisWarnings analysisWarnings;    private int port;    private NodeCommand nodeCommand;    private boolean failedToStart;    // Used by pico container for dependency injection    @SuppressWarnings("unused") -  public CssAnalyzerBridgeServer(Bundle bundle) { -    this(NodeCommand.builder(), DEFAULT_TIMEOUT_SECONDS, bundle); +  public CssAnalyzerBridgeServer(Bundle bundle, @Nullable AnalysisWarnings analysisWarnings) { +    this(NodeCommand.builder(), DEFAULT_TIMEOUT_SECONDS, bundle, analysisWarnings);    } -  protected CssAnalyzerBridgeServer(NodeCommandBuilder nodeCommandBuilder, int timeoutSeconds, Bundle bundle) { +  protected CssAnalyzerBridgeServer(NodeCommandBuilder nodeCommandBuilder, int timeoutSeconds, Bundle bundle, @Nullable AnalysisWarnings analysisWarnings) {      this.nodeCommandBuilder = nodeCommandBuilder;      this.timeoutSeconds = timeoutSeconds;      this.bundle = bundle; +    this.analysisWarnings = analysisWarnings;      this.client = new OkHttpClient.Builder()        .callTimeout(Duration.ofSeconds(timeoutSeconds))        .readTimeout(Duration.ofSeconds(timeoutSeconds))        .build();    } -  // for testing purposes -  public void deploy(File deployLocation) throws IOException { +  public void deploy(File deployLocation) {      bundle.deploy(deployLocation.toPath());    } @@ -121,27 +131,45 @@ public class CssAnalyzerBridgeServer implements AnalyzerBridgeServer {      nodeCommand = nodeCommandBuilder.build();    } -  @Override -  public void startServerLazily(SensorContext context) throws IOException { +  /** +   * @return true when server is up and running normally, false otherwise +   */ +  public boolean startServerLazily(SensorContext context) throws IOException {      // required for SonarLint context to avoid restarting already failed server      if (failedToStart) { -      throw new ServerAlreadyFailedException(); +      LOG.debug("Skipping start of css-bundle server due to the failure during first analysis"); +      LOG.debug("Skipping execution of CSS rules due to the problems with css-bundle server"); +      return false;      }      try {        if (isAlive()) {          LOG.debug("css-bundle server is up, no need to start."); -        return; +        return true;        }        deploy(context.fileSystem().workDir());        startServer(context); +      return true;      } catch (NodeCommandException e) {        failedToStart = true; -      throw e; +      processNodeCommandException(e, context); +      return false;      }    } -  @Override +  // happens for example when NodeJS is not available, or version is too old +  private void processNodeCommandException(NodeCommandException e, SensorContext context) { +    String message = "CSS rules were not executed. " + e.getMessage(); +    if (hasCssFiles(context)) { +      LOG.error(message, e); +      reportAnalysisWarning(message); +    } else { +      // error logs are often blocking (esp. in Azure), so we log at warning level if there is no CSS files in the project +      LOG.warn(message); +    } +    throwFailFast(context, e); +  } +    public Issue[] analyze(Request request) throws IOException {      String json = GSON.toJson(request);      return parseResponse(request(json)); @@ -164,8 +192,8 @@ public class CssAnalyzerBridgeServer implements AnalyzerBridgeServer {        return GSON.fromJson(result, Issue[].class);      } catch (JsonSyntaxException e) {        String msg = "Failed to parse response: \n-----\n" + result + "\n-----\n"; -      LOG.error(msg, e); -      throw new IllegalStateException("Failed to parse response", e); +      LOG.debug(msg); +      throw new IllegalStateException("Failed to parse response (check DEBUG logs for the response content)", e);      }    } @@ -183,12 +211,11 @@ public class CssAnalyzerBridgeServer implements AnalyzerBridgeServer {        // in this case response.body() is never null (according to docs)        return "OK!".equals(body);      } catch (IOException e) { -      LOG.error("Error requesting server status. Server is probably dead.", e); +      LOG.warn("Error requesting server status. Server is probably dead.", e);        return false;      }    } -  @Override    public String getCommandInfo() {      if (nodeCommand == null) {        return "Node.js command to start css-bundle server was not built yet."; @@ -197,12 +224,10 @@ public class CssAnalyzerBridgeServer implements AnalyzerBridgeServer {      }    } -  @Override    public void start() {      // Server is started lazily by the sensor    } -  @Override    public void stop() {      clean();    } @@ -229,4 +254,39 @@ public class CssAnalyzerBridgeServer implements AnalyzerBridgeServer {      this.port = port;    } +  private void reportAnalysisWarning(String message) { +    if (analysisWarnings != null) { +      analysisWarnings.addUnique(message); +    } +  } + +  public static 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, @Nullable String fileContent, String configFile) { +      this.filePath = filePath; +      this.fileContent = fileContent; +      this.configFile = configFile; +    } +  } + +  public static class Issue { +    public final Integer line; +    public final String rule; +    public final String text; + +    public Issue(Integer line, String rule, String text) { +      this.line = line; +      this.rule = rule; +      this.text = text; +    } +  }  } diff --git a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/bundle/Bundle.java b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/bundle/Bundle.java index 513131f..ef82a2b 100644 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/bundle/Bundle.java +++ b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/bundle/Bundle.java @@ -19,12 +19,11 @@   */  package org.sonar.css.plugin.server.bundle; -import java.io.IOException;  import java.nio.file.Path;  public interface Bundle { -  void deploy(Path deployLocation) throws IOException; +  void deploy(Path deployLocation);    /**     * should be called after deploy(Path deployLocation) diff --git a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/exception/ServerAlreadyFailedException.java b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/exception/ServerAlreadyFailedException.java deleted file mode 100644 index 0865aa4..0000000 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/exception/ServerAlreadyFailedException.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * SonarCSS - * Copyright (C) 2018-2019 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA. - */ -package org.sonar.css.plugin.server.exception; - -/** - * This exception is required to inform sensor about analyzer bridge server start up failure in SonarLint - * It is required to not try to start it again - */ -public class ServerAlreadyFailedException extends RuntimeException { -} diff --git a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/exception/package-info.java b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/exception/package-info.java deleted file mode 100644 index 4f5b753..0000000 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/server/exception/package-info.java +++ /dev/null @@ -1,21 +0,0 @@ -/* - * SonarCSS - * Copyright (C) 2018-2019 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA. - */ -@javax.annotation.ParametersAreNonnullByDefault -package org.sonar.css.plugin.server.exception;  | 
