Skip to content

Commit

Permalink
SONARJAVA-4988: Use SonarLintCache component and make it accessible t…
Browse files Browse the repository at this point in the history
…o custom rules via the caching APIs (#4792)

To enable DBD support in SonarLint for Java in VSCode, DBD needs to be able to access the intermediate representation (IR) files it generates for the Java code under analysis.
This IR is generated by custom rules for sonar-java which are provided by DBD, and usually it is stored in the file system. However, no file system is available in a SonarLint context.

Hence, the IR needs to be transferred in memory. For DBD Python analysis, this has been achieved by utilizing a cache context.
I.e. a component SonarLintCache is injected into the Python analyzer frontend, a CacheContext is constructed from it, and DBD’s custom rules store the IR in this cache.
Then, when the DBD plugin is executed, it can retrieve the IR from the cache.

This PR applies the same change to sonar-java.

---

* SONARJAVA-4988: Expose SonarProduct on ModuleScannerContext

DBD custom rules need this information to turn off saving IR to the
filesystem in a SonarLint context

* SONARJAVA-4988: Always provide CacheContext if SonarLintCache is available

* SONARJAVA-4988: CacheContexts based on SonarLintCache should not report as a proper cache

* SONARJAVA-4988: Permit sensor execution ordering using @DependedUpon annotations
  • Loading branch information
anton-haubner-sonarsource committed Jun 4, 2024
1 parent aad7d6f commit 0a5e7a4
Show file tree
Hide file tree
Showing 18 changed files with 707 additions and 141 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ long getBatchModeSizeInKB() {
}

private boolean isCacheEnabled() {
return sonarComponents != null && CacheContextImpl.of(sonarComponents.context()).isCacheEnabled();
return sonarComponents != null && CacheContextImpl.of(sonarComponents).isCacheEnabled();
}

private boolean canOptimizeScanning() {
Expand Down
91 changes: 72 additions & 19 deletions java-frontend/src/main/java/org/sonar/java/SonarComponents.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
import org.sonar.plugins.java.api.CheckRegistrar;
import org.sonar.plugins.java.api.JavaCheck;
import org.sonar.plugins.java.api.JspCodeVisitor;
import org.sonar.plugins.java.api.caching.SonarLintCache;
import org.sonarsource.api.sonarlint.SonarLintSide;
import org.sonarsource.sonarlint.plugin.api.SonarLintRuntime;

Expand Down Expand Up @@ -117,6 +118,8 @@ public class SonarComponents extends CheckRegistrar.RegistrarContext {
private final ActiveRules activeRules;
@Nullable
private final ProjectDefinition projectDefinition;
@Nullable
private final SonarLintCache sonarLintCache;
private final FileSystem fs;
private final List<JavaCheck> mainChecks;
private final List<JavaCheck> testChecks;
Expand All @@ -129,43 +132,85 @@ public class SonarComponents extends CheckRegistrar.RegistrarContext {
private boolean alreadyLoggedSkipStatus = false;

public SonarComponents(FileLinesContextFactory fileLinesContextFactory, FileSystem fs,
ClasspathForMain javaClasspath, ClasspathForTest javaTestClasspath,
CheckFactory checkFactory, ActiveRules activeRules) {
this(fileLinesContextFactory, fs, javaClasspath, javaTestClasspath, checkFactory, activeRules, null, null);
ClasspathForMain javaClasspath, ClasspathForTest javaTestClasspath,
CheckFactory checkFactory, ActiveRules activeRules) {
this(fileLinesContextFactory, fs, javaClasspath, javaTestClasspath, checkFactory, activeRules, null, null, null);
}

/**
* Can be called in SonarLint context when custom rules are present.
*/
public SonarComponents(FileLinesContextFactory fileLinesContextFactory, FileSystem fs,
ClasspathForMain javaClasspath, ClasspathForTest javaTestClasspath, CheckFactory checkFactory,
ActiveRules activeRules, @Nullable CheckRegistrar[] checkRegistrars) {
this(fileLinesContextFactory, fs, javaClasspath, javaTestClasspath, checkFactory, activeRules, checkRegistrars, null, null);
}

/**
* Will *only* be called in SonarLint context and when custom rules are present.
* <p>
* This is because {@link SonarLintCache} is only added as an extension in a SonarLint context.
* See also {@code JavaPlugin#define} in the {@code sonar-java-plugin} module.
* <p>
* {@code SonarLintCache} is used only by newer custom rules, e.g. DBD.
* Thus, for this constructor, we can also assume the presence of {@code CheckRegistrar} instances.
*/
public SonarComponents(FileLinesContextFactory fileLinesContextFactory, FileSystem fs,
ClasspathForMain javaClasspath, ClasspathForTest javaTestClasspath, CheckFactory checkFactory,
ActiveRules activeRules, @Nullable CheckRegistrar[] checkRegistrars, SonarLintCache sonarLintCache) {
this(fileLinesContextFactory, fs, javaClasspath, javaTestClasspath, checkFactory, activeRules, checkRegistrars, null, sonarLintCache);
}

/**
* Will be called in SonarLint context when custom rules are present
* Will be called in SonarScanner context when no custom rules are present.
* May be called in some SonarLint contexts, but not others, since ProjectDefinition might not be available.
*/
public SonarComponents(FileLinesContextFactory fileLinesContextFactory, FileSystem fs,
ClasspathForMain javaClasspath, ClasspathForTest javaTestClasspath, CheckFactory checkFactory,
ActiveRules activeRules, @Nullable CheckRegistrar[] checkRegistrars) {
this(fileLinesContextFactory, fs, javaClasspath, javaTestClasspath, checkFactory, activeRules, checkRegistrars, null);
ClasspathForMain javaClasspath, ClasspathForTest javaTestClasspath, CheckFactory checkFactory,
ActiveRules activeRules, @Nullable ProjectDefinition projectDefinition) {
this(fileLinesContextFactory, fs, javaClasspath, javaTestClasspath, checkFactory, activeRules, null, projectDefinition, null);
}

/**
* Will be called in SonarScanner context when no custom rules is present
* May be called in some SonarLint contexts, but not others, since ProjectDefinition might not be available.
*/
public SonarComponents(FileLinesContextFactory fileLinesContextFactory, FileSystem fs,
ClasspathForMain javaClasspath, ClasspathForTest javaTestClasspath, CheckFactory checkFactory,
ActiveRules activeRules, @Nullable ProjectDefinition projectDefinition) {
this(fileLinesContextFactory, fs, javaClasspath, javaTestClasspath, checkFactory, activeRules,null, projectDefinition);
ClasspathForMain javaClasspath, ClasspathForTest javaTestClasspath, CheckFactory checkFactory,
ActiveRules activeRules, @Nullable CheckRegistrar[] checkRegistrars,
@Nullable ProjectDefinition projectDefinition) {
this(
fileLinesContextFactory,
fs,
javaClasspath,
javaTestClasspath,
checkFactory,
activeRules,
checkRegistrars,
projectDefinition,
null
);
}


/**
* ProjectDefinition class is not available in SonarLint context, so this constructor will never be called when using SonarLint
* All other constructors delegate to this one.
* <p>
* It will also be called directly when constructing a SonarComponents instance for injection if all parameters are available.
* This is for example the case for SonarLint in IntelliJ when DBD is present
* (because ProjectDefinition can be available in recent SonarLint versions, and DBD provides a CheckRegistrar.)
*/
public SonarComponents(FileLinesContextFactory fileLinesContextFactory, FileSystem fs,
ClasspathForMain javaClasspath, ClasspathForTest javaTestClasspath, CheckFactory checkFactory,
ActiveRules activeRules, @Nullable CheckRegistrar[] checkRegistrars,
@Nullable ProjectDefinition projectDefinition) {
ClasspathForMain javaClasspath, ClasspathForTest javaTestClasspath, CheckFactory checkFactory,
ActiveRules activeRules, @Nullable CheckRegistrar[] checkRegistrars,
@Nullable ProjectDefinition projectDefinition, @Nullable SonarLintCache sonarLintCache) {
this.fileLinesContextFactory = fileLinesContextFactory;
this.fs = fs;
this.javaClasspath = javaClasspath;
this.javaTestClasspath = javaTestClasspath;
this.checkFactory = checkFactory;
this.activeRules = activeRules;
this.projectDefinition = projectDefinition;
this.sonarLintCache = sonarLintCache;
this.mainChecks = new ArrayList<>();
this.testChecks = new ArrayList<>();
this.jspChecks = new ArrayList<>();
Expand Down Expand Up @@ -341,7 +386,8 @@ void reportIssue(AnalyzerMessage analyzerMessage, RuleKey key, InputComponent fi
if (!textSpan.onLine()) {
Preconditions.checkState(!textSpan.isEmpty(), "Issue location should not be empty");
}
issue.setPrimaryLocation((InputFile) fileOrProject, analyzerMessage.getMessage(), textSpan.startLine, textSpan.startCharacter, textSpan.endLine, textSpan.endCharacter);
issue.setPrimaryLocation((InputFile) fileOrProject, analyzerMessage.getMessage(), textSpan.startLine, textSpan.startCharacter,
textSpan.endLine, textSpan.endCharacter);
}
if (!analyzerMessage.flows.isEmpty()) {
issue.addFlow((InputFile) analyzerMessage.getInputComponent(), analyzerMessage.flows);
Expand Down Expand Up @@ -493,7 +539,7 @@ public boolean canSkipUnchangedFiles() throws ApiMismatchException {


public boolean fileCanBeSkipped(InputFile inputFile) {
var contentHashCache = new ContentHashCache(context);
var contentHashCache = new ContentHashCache(this);
if (inputFile instanceof GeneratedFile) {
// Generated files should not be skipped as we cannot assess the change status of the source file
return false;
Expand All @@ -513,7 +559,8 @@ public boolean fileCanBeSkipped(InputFile inputFile) {
} catch (ApiMismatchException e) {
if (!alreadyLoggedSkipStatus) {
LOG.info(
"Cannot determine whether the context allows skipping unchanged files: canSkipUnchangedFiles not part of sonar-plugin-api. Not skipping. {}",
"Cannot determine whether the context allows skipping unchanged files: canSkipUnchangedFiles not part of sonar-plugin-api. " +
"Not skipping. {}",
e.getCause().getMessage()
);
alreadyLoggedSkipStatus = true;
Expand Down Expand Up @@ -572,7 +619,8 @@ private void logUndefinedTypes(int maxLines) {
);
}

private static void logParserMessages(Stream<Map.Entry<JProblem, List<String>>> messages, int maxProblems, String warningMessage, String debugMessage) {
private static void logParserMessages(Stream<Map.Entry<JProblem, List<String>>> messages, int maxProblems, String warningMessage,
String debugMessage) {
String problemDelimiter = System.lineSeparator() + "- ";
List<List<String>> messagesList = messages
.sorted(Comparator.comparing(entry -> entry.getKey().toString()))
Expand Down Expand Up @@ -608,4 +656,9 @@ private static void logParserMessages(Stream<Map.Entry<JProblem, List<String>>>
public SensorContext context() {
return context;
}

@CheckForNull
public SonarLintCache sonarLintCache() {
return sonarLintCache;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.java.SonarComponents;
import org.sonar.plugins.java.api.caching.CacheContext;
import org.sonar.plugins.java.api.caching.JavaReadCache;
import org.sonar.plugins.java.api.caching.JavaWriteCache;
import org.sonar.plugins.java.api.caching.SonarLintCache;

public class CacheContextImpl implements CacheContext {
/**
Expand All @@ -47,36 +49,75 @@ private CacheContextImpl(boolean isCacheEnabled, JavaReadCache readCache, JavaWr
this.writeCache = writeCache;
}

public static CacheContextImpl of(@Nullable SensorContext context) {

if (context != null) {
try {
boolean cacheEnabled =
(context.config() == null ? Optional.<Boolean>empty() : context.config().getBoolean(SONAR_CACHING_ENABLED_KEY))
.map(flag -> {
LOGGER.debug("Forcing caching behavior. Caching will be enabled: {}", flag);
return flag;
})
.orElse(context.isCacheEnabled());

LOGGER.trace("Caching is enabled: {}", cacheEnabled);

if (cacheEnabled) {
return new CacheContextImpl(
true,
new JavaReadCacheImpl(context.previousCache()),
new JavaWriteCacheImpl(context.nextCache())
);
}
} catch (NoSuchMethodError error) {
LOGGER.debug("Missing cache related method from sonar-plugin-api: {}.", error.getMessage());
public static CacheContextImpl of(@Nullable SonarComponents sonarComponents) {
if (sonarComponents == null) {
return dummyCache();
}

// If a SonarLintCache is available, it means we must be running in a SonarLint context, and we should use it,
// regardless of whether settings for caching are enabled or not.
// This is because custom rules (i.e. DBD rules) are depending on SonarLintCache in a SonarLint context.
var sonarLintCache = sonarComponents.sonarLintCache();
if (sonarLintCache != null) {
return fromSonarLintCache(sonarLintCache);
}

var sensorContext = sonarComponents.context();
if (sensorContext == null) {
return dummyCache();
}

try {
var isCachingEnabled = isCachingEnabled(sensorContext);
LOGGER.trace("Caching is enabled: {}", isCachingEnabled);
if (!isCachingEnabled) {
return dummyCache();
}

return fromSensorContext(sensorContext);
} catch (NoSuchMethodError error) {
LOGGER.debug("Missing cache related method from sonar-plugin-api: {}.", error.getMessage());
return dummyCache();
}
}

DummyCache dummyCache = new DummyCache();
private static CacheContextImpl dummyCache() {
var dummyCache = new DummyCache();
return new CacheContextImpl(false, dummyCache, dummyCache);
}

private static CacheContextImpl fromSensorContext(SensorContext context) {
return new CacheContextImpl(
true,
new JavaReadCacheImpl(context.previousCache()),
new JavaWriteCacheImpl(context.nextCache())
);
}

private static CacheContextImpl fromSonarLintCache(SonarLintCache sonarLintCache) {
return new CacheContextImpl(
// SonarLintCache is not an actual cache, but a temporary solution to transferring data between plugins in SonarLint.
// Hence, it should not report that caching is enabled so that no logic which is not aware of SonarLintCache tries to use it like
// a regular cache.
// (However, this means code which is aware of SonarLintCache needs to consciously ignore the `isCacheEnabled` setting where
// appropriate.)
false,
new JavaReadCacheImpl(sonarLintCache),
new JavaWriteCacheImpl(sonarLintCache)
);
}

private static boolean isCachingEnabled(SensorContext context) {
return
Optional.ofNullable(context.config())
.flatMap(config -> config.getBoolean(SONAR_CACHING_ENABLED_KEY))
.map(flag -> {
LOGGER.debug("Forcing caching behavior. Caching will be enabled: {}", flag);
return flag;
})
.orElse(context.isCacheEnabled());
}

@Override
public boolean isCacheEnabled() {
return isCacheEnabled;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.api.batch.sensor.cache.ReadCache;
import org.sonar.api.batch.sensor.cache.WriteCache;
import org.sonar.java.SonarComponents;

public class ContentHashCache {

Expand All @@ -39,13 +39,14 @@ public class ContentHashCache {
private WriteCache writeCache;
private final boolean enabled;

public ContentHashCache(SensorContext context) {
CacheContextImpl cacheContext = CacheContextImpl.of(context);
public ContentHashCache(SonarComponents sonarComponents) {
CacheContextImpl cacheContext = CacheContextImpl.of(sonarComponents);
enabled = cacheContext.isCacheEnabled();

var sensorContext = sonarComponents.context();
if (enabled) {
readCache = context.previousCache();
writeCache = context.nextCache();
readCache = sensorContext.previousCache();
writeCache = sensorContext.nextCache();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
package org.sonar.java.model;

import java.io.File;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
import org.sonar.api.SonarProduct;
import org.sonar.api.batch.fs.InputComponent;
import org.sonar.java.SonarComponents;
import org.sonar.java.caching.CacheContextImpl;
Expand All @@ -36,14 +38,15 @@ public class DefaultModuleScannerContext implements ModuleScannerContext {
protected final boolean inAndroidContext;
protected final CacheContext cacheContext;

public DefaultModuleScannerContext(@Nullable SonarComponents sonarComponents, JavaVersion javaVersion, boolean inAndroidContext, @Nullable CacheContext cacheContext) {
public DefaultModuleScannerContext(@Nullable SonarComponents sonarComponents, JavaVersion javaVersion, boolean inAndroidContext,
@Nullable CacheContext cacheContext) {
this.sonarComponents = sonarComponents;
this.javaVersion = javaVersion;
this.inAndroidContext = inAndroidContext;
if (cacheContext != null) {
this.cacheContext = cacheContext;
} else {
this.cacheContext = CacheContextImpl.of(sonarComponents != null ? sonarComponents.context() : null);
this.cacheContext = CacheContextImpl.of(sonarComponents);
}
}

Expand Down Expand Up @@ -85,4 +88,21 @@ public File getRootProjectWorkingDirectory() {
public String getModuleKey() {
return sonarComponents.getModuleKey();
}

@CheckForNull
@Override
public SonarProduct sonarProduct() {
// In production, sonarComponents and sonarComponents.context() should never be null.
// However, in testing contexts, this can happen and calling this method should not cause tests to fail.
if (sonarComponents == null) {
return null;
}

var context = sonarComponents.context();
if (context == null) {
return null;
}

return context.runtime().getProduct();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.sonar.java.CheckFailureException;
import org.sonar.java.ExceptionHandler;
import org.sonar.java.IllegalRuleParameterException;
import org.sonar.plugins.java.api.JavaVersionAwareVisitor;
import org.sonar.java.SonarComponents;
import org.sonar.java.annotations.VisibleForTesting;
import org.sonar.java.ast.visitors.SonarSymbolTableVisitor;
Expand All @@ -55,6 +54,7 @@
import org.sonar.plugins.java.api.JavaFileScanner;
import org.sonar.plugins.java.api.JavaFileScannerContext;
import org.sonar.plugins.java.api.JavaVersion;
import org.sonar.plugins.java.api.JavaVersionAwareVisitor;
import org.sonar.plugins.java.api.ModuleScannerContext;
import org.sonar.plugins.java.api.caching.CacheContext;
import org.sonar.plugins.java.api.internal.EndOfAnalysis;
Expand Down Expand Up @@ -97,7 +97,8 @@ public VisitorsBridge(Iterable<? extends JavaCheck> visitors, List<File> project
this.scannersThatCannotBeSkipped = new ArrayList<>();
this.classpath = projectClasspath;
this.sonarComponents = sonarComponents;
this.cacheContext = CacheContextImpl.of(sonarComponents != null ? sonarComponents.context() : null);
this.cacheContext = CacheContextImpl.of(sonarComponents);

this.javaVersion = javaVersion;
updateScanners();
}
Expand Down
Loading

0 comments on commit 0a5e7a4

Please sign in to comment.