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-379 Merge file traversals to count project size and tsconfigs together #4888

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
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 @@ -141,7 +141,7 @@ void test_exclusion_filter() throws Exception {
.setSourceEncoding("UTF-8")
.setSourceDirs(".")
.setProjectDir(TestUtils.projectDirNoCopy("file-filter/excluded_dir/project"))
.setProperty("sonar.javascript.exclusions", "excluded_dir/**");
.setProperty("sonar.javascript.exclusions", "excluded_dir/**,**/node_modules");

OrchestratorStarter.setProfile(projectKey, jsProfile, "js");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
import org.sonar.plugins.javascript.analysis.HtmlSensor;
import org.sonar.plugins.javascript.analysis.JsTsChecks;
import org.sonar.plugins.javascript.analysis.JsTsSensor;
import org.sonar.plugins.javascript.analysis.TsConfigCacheImpl;
import org.sonar.plugins.javascript.sonarlint.TsConfigCacheImpl;
import org.sonar.plugins.javascript.analysis.TsConfigProvider;
import org.sonar.plugins.javascript.analysis.YamlSensor;
import org.sonar.plugins.javascript.bridge.AnalysisWarningsWrapper;
Expand All @@ -59,7 +59,6 @@
import org.sonar.plugins.javascript.rules.JavaScriptRulesDefinition;
import org.sonar.plugins.javascript.rules.TslintRulesDefinition;
import org.sonar.plugins.javascript.rules.TypeScriptRulesDefinition;
import org.sonar.plugins.javascript.sonarlint.SonarLintTypeCheckingCheckerImpl;

public class JavaScriptPlugin implements Plugin {

Expand Down Expand Up @@ -121,10 +120,15 @@ public class JavaScriptPlugin implements Plugin {
public static final String TS_EXCLUSIONS_KEY = "sonar.typescript.exclusions";
public static final String[] EXCLUSIONS_DEFAULT_VALUE = new String[] {
"**/node_modules/**",
"**/node_modules",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to rethink this. Users adding exclusions will not be aware that they need to be repeated with both globs.

"**/bower_components/**",
"**/bower_components",
"**/dist/**",
"**/dist",
"**/vendor/**",
"**/vendor",
"**/external/**",
"**/external",
"**/*.d.ts",
};

Expand Down Expand Up @@ -338,7 +342,6 @@ public void addSonarLintExtensions(
SonarLintPluginAPIVersion sonarLintPluginAPIVersion
) {
if (sonarLintPluginAPIVersion.isDependencyAvailable()) {
context.addExtension(SonarLintTypeCheckingCheckerImpl.class);
context.addExtension(TsConfigCacheImpl.class);
} else {
LOG.debug("Error while trying to inject SonarLint extensions");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.sonar.api.scanner.ScannerSide;
import org.sonar.plugins.javascript.bridge.AnalysisWarningsWrapper;
import org.sonar.plugins.javascript.bridge.BridgeServer;
import org.sonar.plugins.javascript.sonarlint.TsConfigCache;
import org.sonar.plugins.javascript.utils.ProgressReport;
import org.sonarsource.api.sonarlint.SonarLintSide;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import org.sonar.plugins.javascript.TypeScriptLanguage;
import org.sonar.plugins.javascript.bridge.AnalysisMode;
import org.sonar.plugins.javascript.bridge.BridgeServer;
import org.sonar.plugins.javascript.sonarlint.SonarLintTypeCheckingChecker;
import org.sonar.plugins.javascript.sonarlint.TsConfigCache;

@DependedUpon("js-analysis")
public class JsTsSensor extends AbstractBridgeSensor {
Expand All @@ -46,7 +46,6 @@ public class JsTsSensor extends AbstractBridgeSensor {
private final AnalysisWithProgram analysisWithProgram;
private final AnalysisWithWatchProgram analysisWithWatchProgram;
private final JsTsChecks checks;
private final SonarLintTypeCheckingChecker javaScriptProjectChecker;
private final AnalysisConsumers consumers;
private final TsConfigCache tsConfigCache;

Expand All @@ -57,30 +56,11 @@ public JsTsSensor(
AnalysisWithProgram analysisWithProgram,
AnalysisWithWatchProgram analysisWithWatchProgram,
AnalysisConsumers consumers
) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remove comment at the top of constructor

this(
checks,
bridgeServer,
null,
analysisWithProgram,
analysisWithWatchProgram,
consumers
);
}

public JsTsSensor(
JsTsChecks checks,
BridgeServer bridgeServer,
@Nullable SonarLintTypeCheckingChecker javaScriptProjectChecker,
AnalysisWithProgram analysisWithProgram,
AnalysisWithWatchProgram analysisWithWatchProgram,
AnalysisConsumers consumers
) {
super(bridgeServer, "JS/TS");
this.analysisWithProgram = analysisWithProgram;
this.analysisWithWatchProgram = analysisWithWatchProgram;
this.checks = checks;
this.javaScriptProjectChecker = javaScriptProjectChecker;
this.consumers = consumers;
this.tsConfigCache = analysisWithWatchProgram.tsConfigCache;
}
Expand Down Expand Up @@ -115,7 +95,6 @@ protected void analyzeFiles(List<InputFile> inputFiles) throws IOException {

var tsConfigs = getTsConfigs(
contextUtils,
javaScriptProjectChecker,
this::createTsConfigFile,
tsConfigCache
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,22 @@
* 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.plugins.javascript.sonarlint;
package org.sonar.plugins.javascript.analysis;

import static java.util.Arrays.stream;
import static java.util.stream.Stream.concat;
import org.sonar.api.config.Configuration;
import org.sonar.api.utils.WildcardPattern;
import org.sonar.plugins.javascript.JavaScriptLanguage;
import org.sonar.plugins.javascript.JavaScriptPlugin;
import org.sonar.plugins.javascript.TypeScriptLanguage;

import java.nio.file.Path;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Predicate;
import org.sonar.api.config.Configuration;
import org.sonar.api.utils.WildcardPattern;
import org.sonar.plugins.javascript.JavaScriptLanguage;
import org.sonar.plugins.javascript.JavaScriptPlugin;
import org.sonar.plugins.javascript.TypeScriptLanguage;
import org.sonar.plugins.javascript.filter.JavaScriptExclusionsFileFilter;

import static java.util.Arrays.stream;
import static java.util.stream.Stream.concat;

/**
* This class partially reproduces the behavior of JavaScriptExclusionsFileFilter's implementation.
Expand All @@ -48,9 +48,8 @@
*
* @see JavaScriptExclusionsFileFilter
*/
public class SonarLintTypeCheckingFilter {

private SonarLintTypeCheckingFilter() {}
public class LookupConfigProviderFilter {
private LookupConfigProviderFilter() {}

static class FileFilter implements Predicate<Path> {

Expand Down Expand Up @@ -104,7 +103,7 @@ public PathFilter(Configuration config) {
private static boolean isExclusionOverridden(Configuration config) {
return (
config.get(JavaScriptPlugin.JS_EXCLUSIONS_KEY).isPresent() ||
config.get(JavaScriptPlugin.TS_EXCLUSIONS_KEY).isPresent()
config.get(JavaScriptPlugin.TS_EXCLUSIONS_KEY).isPresent()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@
import org.sonar.api.batch.fs.FileSystem;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.sensor.SensorContext;
import static org.sonar.plugins.javascript.analysis.LookupConfigProviderFilter.FileFilter;
import static org.sonar.plugins.javascript.analysis.LookupConfigProviderFilter.PathFilter;
import org.sonar.plugins.javascript.JavaScriptFilePredicate;
import org.sonar.plugins.javascript.sonarlint.SonarLintTypeCheckingChecker;
import org.sonar.plugins.javascript.sonarlint.TsConfigCache;
import org.sonarsource.analyzer.commons.FileProvider;

public class TsConfigProvider {
Expand Down Expand Up @@ -80,12 +82,11 @@ interface TsConfigFileCreator {
*/
static List<String> getTsConfigs(
ContextUtils contextUtils,
@Nullable SonarLintTypeCheckingChecker javaScriptProjectChecker,
TsConfigProvider.TsConfigFileCreator tsConfigFileCreator,
@Nullable TsConfigCache tsConfigCache
) throws IOException {
var defaultProvider = contextUtils.isSonarLint()
? new TsConfigProvider.WildcardTsConfigProvider(javaScriptProjectChecker, tsConfigFileCreator)
? new TsConfigProvider.WildcardTsConfigProvider(tsConfigCache, tsConfigFileCreator)
: new TsConfigProvider.DefaultTsConfigProvider(tsConfigFileCreator, JavaScriptFilePredicate::getJsTsPredicate);


Expand Down Expand Up @@ -192,9 +193,11 @@ public List<String> tsconfigs(SensorContext context) {
if (tsconfigs != null) {
return tsconfigs;
}

}
var fs = context.fileSystem();
var fileCount = 0;
var fileFilter = new FileFilter(context.config());
var pathFilter = new PathFilter(context.config());
var tsconfigs = new ArrayList<String>();
var dirs = new ArrayDeque<File>();
dirs.add(fs.baseDir());
Expand All @@ -205,14 +208,21 @@ public List<String> tsconfigs(SensorContext context) {
continue;
}
for (var file : files) {
if (file.isDirectory() && !"node_modules".equals(file.getName())) {
if (file.isDirectory() && !pathFilter.test(file.toPath())) {
dirs.add(file);
} else if ("tsconfig.json".equals(file.getName())) {
tsconfigs.add(file.getAbsolutePath());
} else {
if (fileFilter.test(file.toPath())) {
fileCount++;
} else if ("tsconfig.json".equals(file.getName())) {
tsconfigs.add(file.getAbsolutePath());
}
}
}
}
LOG.info("Found {} tsconfig.json file(s): {}", tsconfigs.size(), tsconfigs);
if (cache != null) {
cache.setProjectSize(fileCount);
}
return tsconfigs;
}

Expand Down Expand Up @@ -306,31 +316,34 @@ private File writeToJsonFile(TsConfig tsConfig) throws IOException {
}

static class WildcardTsConfigProvider extends GeneratedTsConfigFileProvider {
private static String getProjectRoot(SensorContext context) {
var projectBaseDir = context.fileSystem().baseDir().getAbsolutePath();
return "/".equals(File.separator)
? projectBaseDir
: projectBaseDir.replace(File.separator, "/");
}
static final String MAX_FILES_PROPERTY = "sonar.javascript.sonarlint.typechecking.maxfiles";
static final int DEFAULT_MAX_FILES_FOR_TYPE_CHECKING = 20_000;

private static final Map<String, List<String>> defaultWildcardTsConfig =
new ConcurrentHashMap<>();

final TsConfigCache tsConfigCache;
final TsConfigFileCreator tsConfigFileCreator;
SonarLintTypeCheckingChecker checker;

WildcardTsConfigProvider(
@Nullable SonarLintTypeCheckingChecker checker,
@Nullable TsConfigCache tsConfigCache,
TsConfigFileCreator tsConfigFileCreator
) {
super(SonarProduct.SONARLINT);
this.tsConfigCache = tsConfigCache;
this.tsConfigFileCreator = tsConfigFileCreator;
this.checker = checker;
}

private static String getProjectRoot(SensorContext context) {
var projectBaseDir = context.fileSystem().baseDir().getAbsolutePath();
return "/".equals(File.separator)
? projectBaseDir
: projectBaseDir.replace(File.separator, "/");
}

@Override
List<String> getDefaultTsConfigs(SensorContext context) {
boolean deactivated = checker == null || checker.isBeyondLimit(context);
boolean deactivated = tsConfigCache == null || isBeyondLimit(context, tsConfigCache.getProjectSize());
if (deactivated) {
return emptyList();
} else {
Expand All @@ -347,5 +360,40 @@ List<String> writeTsConfigFileFor(String root) {
LOG.debug("Using generated tsconfig.json file using wildcards {}", file);
return file;
}

static boolean isBeyondLimit(SensorContext context, int projectSize) {
var typeCheckingLimit = getTypeCheckingLimit(context);

var beyondLimit = projectSize >= typeCheckingLimit;
if (!beyondLimit) {
LOG.info("Turning on type-checking of JavaScript files");
} else {
// TypeScript type checking mechanism creates performance issues for large projects. Analyzing a file can take more than a minute in
// SonarLint, and it can even lead to runtime errors due to Node.js being out of memory during the process.
LOG.warn(
"Turning off type-checking of JavaScript files due to the project size exceeding the limit ({} files)",
typeCheckingLimit
);
LOG.warn("This may cause rules dependent on type information to not behave as expected");
LOG.warn(
"Check the list of impacted rules at https://rules.sonarsource.com/javascript/tag/type-dependent"
);
LOG.warn(
"To turn type-checking back on, increase the \"{}\" property value",
MAX_FILES_PROPERTY
);
LOG.warn(
"Please be aware that this could potentially impact the performance of the analysis"
);
}
return beyondLimit;
}

static int getTypeCheckingLimit(SensorContext context) {
return Math.max(
context.config().getInt(MAX_FILES_PROPERTY).orElse(DEFAULT_MAX_FILES_FOR_TYPE_CHECKING),
0
);
}
}
}

This file was deleted.

Loading