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-435 Clear dependencies cache when a package.json is modified. #4934

Merged
merged 3 commits into from
Nov 27, 2024
Merged
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
1 change: 1 addition & 0 deletions packages/jsts/src/analysis/analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export interface JsTsAnalysisInput extends AnalysisInput {
tsConfigs?: string[];
programId?: string;
skipAst?: boolean;
shouldClearDependenciesCache?: boolean;
}

export interface ParsingError {
Expand Down
4 changes: 3 additions & 1 deletion packages/jsts/src/analysis/analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { getContext } from '../../../shared/src/helpers/context.js';
import { computeMetrics, findNoSonarLines } from '../linter/visitors/metrics/index.js';
import { getSyntaxHighlighting } from '../linter/visitors/syntax-highlighting.js';
import { getCpdTokens } from '../linter/visitors/cpd.js';
import { clearDependenciesCache } from '../rules/helpers/package-json.js';

/**
* Analyzes a JavaScript / TypeScript analysis input
Expand Down Expand Up @@ -70,7 +71,8 @@ function analyzeFile(
sourceCode: SourceCode,
): JsTsAnalysisOutput {
try {
const { filePath, fileType, language } = input;
const { filePath, fileType, language, shouldClearDependenciesCache } = input;
shouldClearDependenciesCache && clearDependenciesCache();
const { issues, highlightedSymbols, cognitiveComplexity, ucfgPaths } = linter.lint(
sourceCode,
filePath,
Expand Down
8 changes: 8 additions & 0 deletions packages/jsts/src/rules/helpers/package-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ export function getDependencies(filename: string, cwd: string) {
return result;
}

/**
* In the case of SonarIDE, when a package.json file changes, the cache can become obsolete.
*/
export function clearDependenciesCache() {
console.debug('Clearing dependencies cache');
cache.clear();
}

export function getDependenciesFromPackageJson(content: PackageJson) {
const result = new Set<string | Minimatch>();
if (content.name) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void initLinter(List<EslintRule> rules, List<String> environments, List<String>
TsConfigFile createTsConfigFile(String content) throws IOException;

record JsAnalysisRequest(String filePath, String fileType, String language, @Nullable String fileContent, boolean ignoreHeaderComments,
@Nullable List<String> tsConfigs, @Nullable String programId, String linterId, boolean skipAst) {
@Nullable List<String> tsConfigs, @Nullable String programId, String linterId, boolean skipAst, boolean shouldClearDependenciesCache) {

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ void should_get_answer_from_server_for_ts_request() throws Exception {
singletonList(tsConfig.absolutePath()),
null,
DEFAULT_LINTER_ID,
false,
false
);
assertThat(bridgeServer.analyzeTypeScript(request).issues()).hasSize(1);
Expand Down Expand Up @@ -268,6 +269,7 @@ private static JsAnalysisRequest createRequest(DefaultInputFile inputFile) {
null,
null,
DEFAULT_LINTER_ID,
false,
false
);
}
Expand Down Expand Up @@ -295,6 +297,7 @@ void should_get_answer_from_server_for_program_based_requests() throws Exception
null,
programCreated.programId(),
DEFAULT_LINTER_ID,
false,
false
);
assertThat(bridgeServer.analyzeTypeScript(request).issues()).hasSize(1);
Expand Down Expand Up @@ -516,6 +519,7 @@ void should_fail_if_bad_json_response() throws Exception {
null,
null,
DEFAULT_LINTER_ID,
false,
false
);
assertThatThrownBy(() -> bridgeServer.analyzeJavaScript(request))
Expand Down Expand Up @@ -767,7 +771,8 @@ void should_omit_an_ast_if_skipAst_flag_is_set() throws Exception {
null,
null,
DEFAULT_LINTER_ID,
true
true,
false
);
var response = bridgeServer.analyzeJavaScript(request);
assertThat(response.ast()).isNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ protected boolean isJavaScript(InputFile file) {

abstract void analyzeFiles(List<InputFile> inputFiles, List<String> tsConfigs) throws IOException;

protected void analyzeFile(InputFile file, @Nullable List<String> tsConfigs, @Nullable TsProgram tsProgram) throws IOException {
protected void analyzeFile(InputFile file, @Nullable List<String> tsConfigs, @Nullable TsProgram tsProgram, boolean dirtyPackageJSONCache) throws IOException {
if (context.isCancelled()) {
throw new CancellationException(
"Analysis interrupted because the SensorContext is in cancelled state"
Expand All @@ -100,7 +100,7 @@ protected void analyzeFile(InputFile file, @Nullable List<String> tsConfigs, @Nu
progressReport.nextFile(file.toString());
var fileContent = contextUtils.shouldSendFileContent(file) ? file.contents() : null;
var skipAst = !consumers.hasConsumers() || !(contextUtils.isSonarArmorEnabled() || contextUtils.isSonarJasminEnabled() || contextUtils.isSonarJaredEnabled());
var request = getJsAnalysisRequest(file, fileContent, tsProgram, tsConfigs, skipAst);
var request = getJsAnalysisRequest(file, fileContent, tsProgram, tsConfigs, skipAst, dirtyPackageJSONCache);

var response = isJavaScript(file)
? bridgeServer.analyzeJavaScript(request)
Expand Down Expand Up @@ -143,7 +143,8 @@ private BridgeServer.JsAnalysisRequest getJsAnalysisRequest(
@Nullable String fileContent,
@Nullable TsProgram tsProgram,
@Nullable List<String> tsConfigs,
boolean skipAst
boolean skipAst,
boolean shouldClearDependenciesCache
) {
return new BridgeServer.JsAnalysisRequest(
file.absolutePath(),
Expand All @@ -154,7 +155,8 @@ private BridgeServer.JsAnalysisRequest getJsAnalysisRequest(
tsConfigs,
tsProgram != null ? tsProgram.programId() : null,
analysisMode.getLinterIdFor(file),
skipAst
skipAst,
shouldClearDependenciesCache
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ void analyzeFiles(List<InputFile> inputFiles, List<String> tsConfigs) throws IOE
);
for (var f : skippedFiles) {
LOG.debug("File not part of any tsconfig.json: {}", f);
analyzeFile(f, null, null);
analyzeFile(f, null, null, false);
}
}
success = true;
Expand Down Expand Up @@ -131,7 +131,7 @@ private void analyzeProgram(TsProgram program, Set<InputFile> analyzedFiles) thr
continue;
}
if (analyzedFiles.add(inputFile)) {
analyzeFile(inputFile, null, program);
analyzeFile(inputFile, null, program, false);
counter++;
} else {
LOG.debug(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void analyzeFiles(List<InputFile> inputFiles, List<String> tsConfigs) thr
progressReport.start(inputFiles.size(), inputFiles.iterator().next().toString());
for (InputFile inputFile : inputFiles) {
var tsConfigFile = tsConfigCache.getTsConfigForInputFile(inputFile);
analyzeFile(inputFile, tsConfigFile == null ? List.of() : List.of(tsConfigFile.getFilename()), null);
analyzeFile(inputFile, tsConfigFile == null ? List.of() : List.of(tsConfigFile.getFilename()), null, this.tsConfigCache.getAndResetShouldClearDependenciesCache());
}
success = true;
if (analysisProcessor.parsingErrorFilesCount() > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ private void analyze(InputFile file, CacheStrategy cacheStrategy) throws IOExcep
null,
null,
analysisMode.getLinterIdFor(file),
false,
false
);
var response = bridgeServer.analyzeHtml(jsAnalysisRequest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ private void analyze(InputFile file) throws IOException {
null,
null,
analysisMode.getLinterIdFor(file),
false,
false
);
var response = bridgeServer.analyzeYaml(jsAnalysisRequest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public interface TsConfigCache {
void initializeWith(List<String> tsConfigs, TsConfigOrigin origin);
List<String> listCachedTsConfigs(TsConfigOrigin origin);
void setOrigin(TsConfigOrigin origin);
boolean getAndResetShouldClearDependenciesCache();

void setProjectSize(int projectSize);
int getProjectSize();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public class TsConfigCacheImpl implements TsConfigCache, ModuleFileListener {
BridgeServer bridgeServer;
TsConfigOrigin origin;
int projectSize;
boolean shouldClearDependenciesCache;

Map<TsConfigOrigin, Cache> cacheMap = new EnumMap<>(TsConfigOrigin.class);

Expand All @@ -55,6 +56,7 @@ public TsConfigCacheImpl(BridgeServer bridgeServer) {
cacheMap.put(TsConfigOrigin.PROPERTY, new Cache());
cacheMap.put(TsConfigOrigin.LOOKUP, new Cache());
cacheMap.put(TsConfigOrigin.FALLBACK, new Cache());
shouldClearDependenciesCache = false;
}

class Cache {
Expand Down Expand Up @@ -170,6 +172,13 @@ public void setOrigin(TsConfigOrigin tsConfigOrigin) {
origin = tsConfigOrigin;
}

@Override
public boolean getAndResetShouldClearDependenciesCache() {
var result = shouldClearDependenciesCache;
shouldClearDependenciesCache = false;
return result;
}

public void initializeWith(List<String> tsConfigPaths, TsConfigOrigin tsConfigOrigin) {
var cache = cacheMap.get(tsConfigOrigin);
if (tsConfigOrigin == TsConfigOrigin.FALLBACK && cache.initialized) {
Expand All @@ -196,6 +205,9 @@ public void process(ModuleFileEvent moduleFileEvent) {
if (cacheMap.get(TsConfigOrigin.PROPERTY).discoveredTsConfigFiles.contains(filename)) {
cacheMap.get(TsConfigOrigin.PROPERTY).clearAll();
}
} else if (filename.endsWith("package.json")) {
LOG.debug("Package json update, will clear dependency cache on next analysis request.");
shouldClearDependenciesCache = true;
} else if (moduleFileEvent.getType() == ModuleFileEvent.Type.CREATED && (JavaScriptFilePredicate.isJavaScriptFile(file) || JavaScriptFilePredicate.isTypeScriptFile(file))) {
// The file to tsconfig cache is cleared, as potentially the tsconfig file that would cover this new file
// has already been processed, and we would not be aware of it. By clearing the cache, we guarantee correctness.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,29 @@ void testPropertyTsConfigChanged() throws IOException {
assertThat(propertyCachedTsConfig).containsExactlyInAnyOrder(tsconfig1.toAbsolutePath().toString(), tsconfig2.toAbsolutePath().toString());
}

@Test
void testPackageJsonChanged() throws IOException {
Path packageJson = baseDir.resolve("package.json");
Files.createFile(packageJson);
var packageJsonFileInput = TestInputFileBuilder.create(baseDir.toString(), packageJson.getFileName().toString()).build();
var fileEvent = DefaultModuleFileEvent.of(packageJsonFileInput, ModuleFileEvent.Type.MODIFIED);
tsConfigCache.process(fileEvent);
// We should mark the dependency cache to be cleared
assertThat(tsConfigCache.getAndResetShouldClearDependenciesCache()).isTrue();
// The getAndReset... method has side effects. Calling it a second time should clear it.
assertThat(tsConfigCache.getAndResetShouldClearDependenciesCache()).isFalse();
}

@Test
void testTSConfigChangedDoesntClearDependencyCache() throws IOException {
Path tsconfig = baseDir.resolve("tsconfig.json");
Files.createFile(tsconfig);
var packageJsonFileInput = TestInputFileBuilder.create(baseDir.toString(), tsconfig.getFileName().toString()).build();
var fileEvent = DefaultModuleFileEvent.of(packageJsonFileInput, ModuleFileEvent.Type.MODIFIED);
tsConfigCache.process(fileEvent);
assertThat(tsConfigCache.getAndResetShouldClearDependenciesCache()).isFalse();
}

private Pair<InputFile, TsConfigFile> prepareFileAndTsConfig() throws IOException {
var file1 = TestInputFileBuilder.create(baseDir.toString(), "file1.ts").setLanguage(TypeScriptLanguage.KEY).build();
Path tsconfig1 = baseDir.resolve("tsconfig.json");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ public ESTree.Program parse(String code) {
null,
null,
AnalysisMode.DEFAULT_LINTER_ID,
false);
false,
false
);
try {
BridgeServer.AnalysisResponse result = bridge.analyzeJavaScript(request);
Node ast = result.ast();
Expand Down