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

JS-518 Rewrite AnalyzeWithProgram for analyze-project #5044

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ export async function analyzeWithProgram(
tsConfigs: AsyncGenerator<string>,
results: ProjectAnalysisOutput,
pendingFiles: Set<string>,
skipAst: boolean,
) {
for await (const tsConfig of tsConfigs) {
await analyzeProgram(files, tsConfig, results, pendingFiles);
await analyzeProgram(files, tsConfig, results, pendingFiles, skipAst);
if (!pendingFiles.size) {
break;
}
Expand All @@ -49,6 +50,7 @@ async function analyzeProgram(
tsConfig: string,
results: ProjectAnalysisOutput,
pendingFiles: Set<string>,
skipAst: boolean,
) {
let filenames, programId, projectReferences;
try {
Expand All @@ -67,13 +69,14 @@ async function analyzeProgram(
fileType: files[filename].fileType,
language: files[filename].language ?? DEFAULT_LANGUAGE,
programId,
skipAst,
});
pendingFiles.delete(filename);
}
}
deleteProgram(programId);

for (const reference of projectReferences) {
await analyzeProgram(files, reference, results, pendingFiles);
await analyzeProgram(files, reference, results, pendingFiles, skipAst);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export type ProjectAnalysisInput = {
exclusions?: string[];
isSonarlint?: boolean;
maxFilesForTypeChecking?: number;
skipAst?: boolean;
};

export const DEFAULT_LANGUAGE: JsTsLanguage = 'ts';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export async function analyzeProject(input: ProjectAnalysisInput): Promise<Proje
exclusions = [],
isSonarlint = false,
maxFilesForTypeChecking,
skipAst = true,
} = input;
const inputFilenames = Object.keys(input.files);
const results: ProjectAnalysisOutput = {
Expand Down Expand Up @@ -76,7 +77,7 @@ export async function analyzeProject(input: ProjectAnalysisInput): Promise<Proje
await analyzeWithWatchProgram(input.files, tsConfigs, results, pendingFiles);
} else {
results.meta!.withProgram = true;
await analyzeWithProgram(input.files, tsConfigs, results, pendingFiles);
await analyzeWithProgram(input.files, tsConfigs, results, pendingFiles, skipAst);
}

await analyzeWithoutProgram(pendingFiles, input.files, results);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.io.IOException;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
import org.sonar.api.Startable;
import org.sonar.api.batch.fs.InputFile;
Expand Down Expand Up @@ -71,6 +72,13 @@ void initLinter(

TelemetryData getTelemetry();

ProjectAnalysisOutput analyzeProject(ProjectAnalysisRequest request) throws IOException;

record ProjectAnalysisOutput(
Map<String, AnalysisResponse> files,
ProjectAnalysisMetaResponse meta
) {}

record JsAnalysisRequest(
String filePath,
String fileType,
Expand All @@ -84,6 +92,32 @@ record JsAnalysisRequest(
boolean shouldClearDependenciesCache
) {}

record JsTsFile(
@Nullable String fileContent,
@Nullable Boolean ignoreHeaderComments,
String fileType,
@Nullable String language
) {}

record ProjectAnalysisRequest(
Map<String, JsTsFile> files,
List<EslintRule> rules,
List<String> environments,
List<String> globals,
String baseDir,
List<String> exclusions,
boolean isSonarlint,
@Nullable Integer maxFilesForTypeChecking,
@Nullable Boolean skipAst
) {}

record ProjectAnalysisMetaResponse(
boolean withProgram,
boolean withWatchProgram,
List<String> filesWithoutTypeChecking,
List<String> programsCreated
) {}

record CssAnalysisRequest(
String filePath,
@Nullable String fileContent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,12 @@ public TelemetryData getTelemetry() {
);
}

@Override
public ProjectAnalysisOutput analyzeProject(ProjectAnalysisRequest request) throws IOException {
var response = request(GSON.toJson(request), "analyze-project");
return GSON.fromJson(response.json(), ProjectAnalysisOutput.class);
}

private TelemetryEslintBridgeResponse getTelemetryEslintBridgeResponse() {
try {
var result = http.get(url("get-telemetry"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ public abstract class AbstractAnalysis {
ProgressReport progressReport;
AnalysisMode analysisMode;
protected final AnalysisWarningsWrapper analysisWarnings;
private AnalysisConsumers consumers;
protected AnalysisConsumers consumers;
protected List<String> exclusions;
List<String> environments;
List<String> globals;

AbstractAnalysis(
BridgeServer bridgeServer,
Expand All @@ -76,14 +79,20 @@ void initialize(
SensorContext context,
JsTsChecks checks,
AnalysisMode analysisMode,
AnalysisConsumers consumers
AnalysisConsumers consumers,
List<String> environments,
List<String> globals,
List<String> exclusions
) {
LOG.debug("Initializing {}", getClass().getName());
this.context = context;
contextUtils = new ContextUtils(context);
this.checks = checks;
this.analysisMode = analysisMode;
this.consumers = consumers;
this.environments = environments;
this.globals = globals;
this.exclusions = exclusions;
}

protected boolean isJavaScript(InputFile file) {
Expand Down Expand Up @@ -144,7 +153,7 @@ protected void analyzeFile(
}
}

private void acceptAstResponse(BridgeServer.AnalysisResponse response, InputFile file) {
protected void acceptAstResponse(BridgeServer.AnalysisResponse response, InputFile file) {
Node responseAst = response.ast();
if (responseAst != null) {
// When we haven't serialized the AST:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,19 @@
package org.sonar.plugins.javascript.analysis;

import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.HashSet;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Set;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.scanner.ScannerSide;
import org.sonar.plugins.javascript.JavaScriptPlugin;
import org.sonar.plugins.javascript.CancellationException;
import org.sonar.plugins.javascript.analysis.cache.CacheAnalysis;
import org.sonar.plugins.javascript.analysis.cache.CacheStrategies;
import org.sonar.plugins.javascript.analysis.cache.CacheStrategy;
import org.sonar.plugins.javascript.bridge.AnalysisWarningsWrapper;
import org.sonar.plugins.javascript.bridge.BridgeServer;
import org.sonar.plugins.javascript.bridge.BridgeServer.TsProgram;
import org.sonar.plugins.javascript.bridge.BridgeServer.TsProgramRequest;
import org.sonar.plugins.javascript.utils.ProgressReport;

@ScannerSide
public class AnalysisWithProgram extends AbstractAnalysis {
Expand All @@ -48,100 +45,74 @@ public AnalysisWithProgram(
}

@Override
void analyzeFiles(List<InputFile> inputFiles) throws IOException {
var tsConfigs = TsConfigProvider.getTsConfigs(contextUtils, this::createTsConfigFile);
progressReport = new ProgressReport(PROGRESS_REPORT_TITLE, PROGRESS_REPORT_PERIOD);
progressReport.start(inputFiles.size(), inputFiles.iterator().next().toString());
boolean success = false;
try {
Deque<String> workList = new ArrayDeque<>(tsConfigs);
Set<String> analyzedProjects = new HashSet<>();
Set<InputFile> analyzedFiles = new HashSet<>();
while (!workList.isEmpty()) {
var tsConfig = Path.of(workList.pop()).toString();
// Use of path.of as it normalizes Unix and Windows paths. Otherwise, project references returned by typescript may not match system slash
if (!analyzedProjects.add(tsConfig)) {
LOG.debug("tsconfig.json already analyzed: '{}'. Skipping it.", tsConfig);
continue;
}
LOG.info("Creating TypeScript program");
LOG.info("TypeScript configuration file {}", tsConfig);
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 TSConfig file %s. Highest TypeScript supported version is %s.",
tsConfig,
JavaScriptPlugin.TYPESCRIPT_VERSION
)
);
continue;
}
if (program.missingTsConfig()) {
String msg =
"At least one tsconfig.json was not found in the project. Please run 'npm install' for a more complete analysis. Check analysis logs for more details.";
LOG.warn(msg);
this.analysisWarnings.addUnique(msg);
}
analyzeProgram(program, analyzedFiles);
workList.addAll(program.projectReferences());
bridgeServer.deleteProgram(program);
}
Set<InputFile> skippedFiles = new HashSet<>(inputFiles);
skippedFiles.removeAll(analyzedFiles);
if (!skippedFiles.isEmpty()) {
// Temporarily we will analyze skipped programs without program,
// when this logic moves to Node we will have full analysis also for skipped files
LOG.info(
"Found {} file(s) not part of any tsconfig.json: they will be analyzed without type information",
skippedFiles.size()
);
for (var f : skippedFiles) {
LOG.debug("File not part of any tsconfig.json: {}", f);
analyzeFile(f, null, null, false);
}
}
success = true;
if (analysisProcessor.parsingErrorFilesCount() > 0) {
this.analysisWarnings.addUnique(
String.format(
"There were parsing errors in %d files while analyzing the project. Check the logs for further details.",
analysisProcessor.parsingErrorFilesCount()
)
);
}
new PluginTelemetry(context, bridgeServer).reportTelemetry();
} finally {
if (success) {
progressReport.stop();
public void analyzeFiles(List<InputFile> inputFiles) throws IOException {
if (context.isCancelled()) {
throw new CancellationException(
"Analysis interrupted because the SensorContext is in cancelled state"
);
}
var filesToAnalyze = new ArrayList<InputFile>();
var fileToInputFile = new HashMap<String, InputFile>();
var fileToCacheStrategy = new HashMap<String, CacheStrategy>();
for (InputFile inputFile : inputFiles) {
var cacheStrategy = CacheStrategies.getStrategyFor(context, inputFile);
if (cacheStrategy.isAnalysisRequired()) {
filesToAnalyze.add(inputFile);
fileToInputFile.put(inputFile.absolutePath(), inputFile);
fileToCacheStrategy.put(inputFile.absolutePath(), cacheStrategy);
} else {
progressReport.cancel();
LOG.debug("Processing cache analysis of file: {}", inputFile.uri());
var cacheAnalysis = cacheStrategy.readAnalysisFromCache();
analysisProcessor.processCacheAnalysis(context, inputFile, cacheAnalysis);
}
}
}
var skipAst =
!consumers.hasConsumers() ||
!(contextUtils.isSonarArmorEnabled() ||
contextUtils.isSonarJasminEnabled() ||
contextUtils.isSonarJaredEnabled());

private void analyzeProgram(TsProgram program, Set<InputFile> analyzedFiles) throws IOException {
LOG.info("Starting analysis with current program");
var fs = context.fileSystem();
var counter = 0;
for (var file : program.files()) {
var inputFile = fs.inputFile(fs.predicates().hasAbsolutePath(file));
if (inputFile == null) {
LOG.debug("File not part of the project: '{}'", file);
continue;
}
if (analyzedFiles.add(inputFile)) {
analyzeFile(inputFile, null, program, false);
counter++;
} else {
LOG.debug(
"File already analyzed: '{}'. Check your project configuration to avoid files being part of multiple projects.",
var files = new HashMap<String, BridgeServer.JsTsFile>();
for (InputFile file : filesToAnalyze) {
files.put(
file.absolutePath(),
new BridgeServer.JsTsFile(
contextUtils.shouldSendFileContent(file) ? file.contents() : null,
contextUtils.ignoreHeaderComments(),
file.type().toString(),
inputFileLanguage(file)
)
);
}
var request = new BridgeServer.ProjectAnalysisRequest(
files,
checks.eslintRules(),
environments,
globals,
context.fileSystem().baseDir().getAbsolutePath(),
exclusions,
false,
null,
skipAst
);
try {
var projectResponse = bridgeServer.analyzeProject(request);
for (var entry : projectResponse.files().entrySet()) {
var filePath = entry.getKey();
var response = entry.getValue();
var file = fileToInputFile.get(filePath);
var cacheStrategy = fileToCacheStrategy.get(filePath);
analysisProcessor.processResponse(context, checks, file, response);
cacheStrategy.writeAnalysisToCache(
CacheAnalysis.fromResponse(response.ucfgPaths(), response.cpdTokens()),
file
);
acceptAstResponse(response, file);
}
new PluginTelemetry(context, bridgeServer).reportTelemetry();
} catch (Exception e) {
LOG.error("Failed to get response from analysis", e);
throw e;
}

LOG.info("Analyzed {} file(s) with current program", counter);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,15 @@ protected void analyzeFiles(List<InputFile> inputFiles) throws IOException {
exclusions
);

analysis.initialize(context, checks, analysisMode, consumers);
analysis.initialize(
context,
checks,
analysisMode,
consumers,
environments,
globals,
exclusions
);
analysis.analyzeFiles(inputFiles);
consumers.doneAnalysis();
}
Expand Down
Loading