Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more analysis warnings #4455

Merged
merged 12 commits into from
Dec 6, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@

public class JavaScriptPlugin implements Plugin {

public static final Object TYPESCRIPT_VERSION = "5.3.2";
vdiez marked this conversation as resolved.
Show resolved Hide resolved
private static final Logger LOG = Loggers.get(JavaScriptPlugin.class);

// Subcategories
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,18 @@ abstract class AbstractAnalysis {
JsTsChecks checks;
ProgressReport progressReport;
AnalysisMode analysisMode;
protected final AnalysisWarningsWrapper analysisWarnings;

AbstractAnalysis(
BridgeServer bridgeServer,
Monitoring monitoring,
AnalysisProcessor analysisProcessor
AnalysisProcessor analysisProcessor,
AnalysisWarningsWrapper analysisWarnings
) {
this.bridgeServer = bridgeServer;
this.monitoring = monitoring;
this.analysisProcessor = analysisProcessor;
this.analysisWarnings = analysisWarnings;
}

protected static String inputFileLanguage(InputFile file) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import java.io.Serializable;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.stream.Collectors;
import org.sonar.api.batch.fs.InputFile;
Expand Down Expand Up @@ -70,6 +71,7 @@ public class AnalysisProcessor {
private ContextUtils contextUtils;
private InputFile file;
private JsTsChecks checks;
HashSet<String> parsingErrors;
vdiez marked this conversation as resolved.
Show resolved Hide resolved

public AnalysisProcessor(
NoSonarFilter noSonarFilter,
Expand All @@ -79,6 +81,7 @@ public AnalysisProcessor(
this.noSonarFilter = noSonarFilter;
this.fileLinesContextFactory = fileLinesContextFactory;
this.monitoring = monitoring;
this.parsingErrors = new HashSet<>();
}

void processResponse(
Expand All @@ -92,6 +95,7 @@ void processResponse(
this.checks = checks;
this.file = file;
if (response.parsingError != null) {
parsingErrors.add(file.absolutePath());
processParsingError(response.parsingError);
return;
}
Expand All @@ -117,6 +121,10 @@ void processResponse(
}
}

public int parsingErrorsCount() {
return parsingErrors.size();
}

void processCacheAnalysis(SensorContext context, InputFile file, CacheAnalysis cacheAnalysis) {
this.context = context;
contextUtils = new ContextUtils(context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.sonar.api.utils.log.Loggers;
import org.sonar.api.utils.log.Profiler;
import org.sonar.plugins.javascript.CancellationException;
import org.sonar.plugins.javascript.JavaScriptPlugin;
import org.sonar.plugins.javascript.bridge.BridgeServer.TsProgram;
import org.sonar.plugins.javascript.bridge.BridgeServer.TsProgramRequest;
import org.sonar.plugins.javascript.bridge.cache.CacheAnalysis;
Expand All @@ -46,16 +47,14 @@ public class AnalysisWithProgram extends AbstractAnalysis {

private static final Logger LOG = Loggers.get(AnalysisWithProgram.class);
private static final Profiler PROFILER = Profiler.create(LOG);
private final AnalysisWarningsWrapper analysisWarnings;

public AnalysisWithProgram(
BridgeServer bridgeServer,
Monitoring monitoring,
AnalysisProcessor analysisProcessor,
AnalysisWarningsWrapper analysisWarnings
) {
super(bridgeServer, monitoring, analysisProcessor);
this.analysisWarnings = analysisWarnings;
super(bridgeServer, monitoring, analysisProcessor, analysisWarnings);
}

@Override
Expand All @@ -80,6 +79,13 @@ void analyzeFiles(List<InputFile> inputFiles, List<String> tsConfigs) throws IOE
var program = bridgeServer.createProgram(new TsProgramRequest(tsConfig));
if (program.error != null) {
LOG.error("Failed to create program: " + program.error);
this.analysisWarnings.addUnique(
String.format(
"Failed to create TypeScript program with %s. Highest TypeScript supported version is %s",
vdiez marked this conversation as resolved.
Show resolved Hide resolved
tsConfig,
JavaScriptPlugin.TYPESCRIPT_VERSION
)
);
PROFILER.stopInfo();
continue;
}
Expand Down Expand Up @@ -110,6 +116,14 @@ void analyzeFiles(List<InputFile> inputFiles, List<String> tsConfigs) throws IOE
}
}
success = true;
if (analysisProcessor.parsingErrorsCount() > 0) {
this.analysisWarnings.addUnique(
String.format(
"There were %d parsing errors while analyzing the project. Check the logs for further details",
vdiez marked this conversation as resolved.
Show resolved Hide resolved
analysisProcessor.parsingErrorsCount()
)
);
}
} finally {
if (success) {
progressReport.stop();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,10 @@ public class AnalysisWithWatchProgram extends AbstractAnalysis {
public AnalysisWithWatchProgram(
BridgeServer bridgeServer,
Monitoring monitoring,
AnalysisProcessor analysisProcessor
AnalysisProcessor analysisProcessor,
AnalysisWarningsWrapper analysisWarnings
) {
super(bridgeServer, monitoring, analysisProcessor);
super(bridgeServer, monitoring, analysisProcessor, analysisWarnings);
}

@Override
Expand Down Expand Up @@ -82,6 +83,14 @@ public void analyzeFiles(List<InputFile> inputFiles, List<String> tsConfigs) thr
}
}
success = true;
if (analysisProcessor.parsingErrorsCount() > 0) {
this.analysisWarnings.addUnique(
String.format(
"There were %d parsing errors while analyzing the project. Check the logs for further details",
vdiez marked this conversation as resolved.
Show resolved Hide resolved
analysisProcessor.parsingErrorsCount()
)
);
}
} finally {
if (success) {
progressReport.stop();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,17 @@ public void setUp() throws Exception {
when(fileLinesContextFactory.createFor(any(InputFile.class))).thenReturn(fileLinesContext);
analysisProcessor =
new AnalysisProcessor(new DefaultNoSonarFilter(), fileLinesContextFactory, monitoring);
var analysisWarnings = new AnalysisWarningsWrapper();

analysisWithProgram =
new AnalysisWithProgram(
new AnalysisWithProgram(bridgeServerMock, monitoring, analysisProcessor, analysisWarnings);
analysisWithWatchProgram =
new AnalysisWithWatchProgram(
bridgeServerMock,
monitoring,
analysisProcessor,
new AnalysisWarningsWrapper()
analysisWarnings
);
analysisWithWatchProgram =
new AnalysisWithWatchProgram(bridgeServerMock, monitoring, analysisProcessor);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -83,6 +84,7 @@
import org.sonar.api.utils.Version;
import org.sonar.api.utils.log.LoggerLevel;
import org.sonar.javascript.checks.CheckList;
import org.sonar.plugins.javascript.JavaScriptPlugin;
import org.sonar.plugins.javascript.TestUtils;
import org.sonar.plugins.javascript.bridge.BridgeServer.AnalysisResponse;
import org.sonar.plugins.javascript.bridge.BridgeServer.JsAnalysisRequest;
Expand Down Expand Up @@ -506,8 +508,12 @@ void should_analyze_by_program() throws Exception {
when(bridgeServerMock.analyzeTypeScript(any())).thenReturn(new AnalysisResponse());

ArgumentCaptor<JsAnalysisRequest> captor = ArgumentCaptor.forClass(JsAnalysisRequest.class);
ArgumentCaptor<TsProgramRequest> captorProgram = ArgumentCaptor.forClass(
TsProgramRequest.class
);
createSensor().execute(context);
verify(bridgeServerMock, times(4)).analyzeTypeScript(captor.capture());
verify(bridgeServerMock, times(4)).createProgram(captorProgram.capture());
assertThat(captor.getAllValues())
.extracting(req -> req.filePath)
.containsExactlyInAnyOrder(
Expand All @@ -527,6 +533,15 @@ void should_analyze_by_program() throws Exception {
);
assertThat(logTester.logs(LoggerLevel.ERROR))
.contains("Failed to create program: something went wrong");

assertThat(analysisWarnings.warnings)
.contains(
String.format(
"Failed to create TypeScript program with %s. Highest TypeScript supported version is %s",
vdiez marked this conversation as resolved.
Show resolved Hide resolved
captorProgram.getAllValues().get(2).tsConfig,
JavaScriptPlugin.TYPESCRIPT_VERSION
)
);
}

@Test
Expand Down Expand Up @@ -795,7 +810,12 @@ private AnalysisWithProgram analysisWithProgram() {
}

private AnalysisWithWatchProgram analysisWithWatchProgram() {
return new AnalysisWithWatchProgram(bridgeServerMock, monitoring, processAnalysis);
return new AnalysisWithWatchProgram(
bridgeServerMock,
monitoring,
processAnalysis,
analysisWarnings
);
}

private AnalysisResponse createResponse() {
Expand Down