Fix global state in apex module

This commit is contained in:
Clément Fournier
2022-07-23 12:39:03 +02:00
parent 12bf4cf521
commit 17040599ef
13 changed files with 69 additions and 91 deletions

View File

@ -21,7 +21,7 @@ public class ApexLanguageModule extends LanguageModuleBase {
@Override
public ApexLanguageProperties newPropertyBundle() {
return new ApexLanguageProperties(this);
return new ApexLanguageProperties();
}
@Override

View File

@ -12,6 +12,7 @@ import net.sourceforge.pmd.lang.LanguageVersionHandler;
import net.sourceforge.pmd.lang.apex.ast.ApexParser;
import net.sourceforge.pmd.lang.apex.internal.ApexDesignerBindings;
import net.sourceforge.pmd.lang.apex.metrics.ApexMetrics;
import net.sourceforge.pmd.lang.apex.multifile.ApexMultifileAnalysis;
import net.sourceforge.pmd.lang.apex.rule.internal.ApexRuleViolationFactory;
import net.sourceforge.pmd.lang.ast.Parser;
import net.sourceforge.pmd.lang.metrics.LanguageMetricsProvider;
@ -25,9 +26,11 @@ public class ApexLanguageProcessor
implements LanguageVersionHandler {
private final ApexMetricsProvider myMetricsProvider = new ApexMetricsProvider();
private final ApexMultifileAnalysis multifileAnalysis;
ApexLanguageProcessor(ApexLanguageProperties bundle) {
super(bundle);
this.multifileAnalysis = new ApexMultifileAnalysis(bundle);
}
@Override
@ -42,7 +45,7 @@ public class ApexLanguageProcessor
@Override
public Parser getParser() {
return new ApexParser(getProperties());
return new ApexParser(this);
}
@Override
@ -55,6 +58,10 @@ public class ApexLanguageProcessor
return ApexDesignerBindings.INSTANCE;
}
public ApexMultifileAnalysis getMultiFileState() {
return multifileAnalysis;
}
private static final class ApexMetricsProvider implements LanguageMetricsProvider {
private final Set<Metric<?, ?>> metrics = setOf(

View File

@ -20,8 +20,8 @@ public class ApexLanguageProperties extends LanguagePropertyBundle {
.defaultValue("") // is this ok?
.build();
public ApexLanguageProperties(ApexLanguageModule languageModule) {
super(languageModule);
public ApexLanguageProperties() {
super(ApexLanguageModule.getInstance());
definePropertyDescriptor(MULTIFILE_DIRECTORY);
}

View File

@ -9,6 +9,7 @@ import java.util.Map;
import org.checkerframework.checker.nullness.qual.NonNull;
import net.sourceforge.pmd.lang.apex.ApexLanguageProcessor;
import net.sourceforge.pmd.lang.apex.multifile.ApexMultifileAnalysis;
import net.sourceforge.pmd.lang.ast.AstInfo;
import net.sourceforge.pmd.lang.ast.Parser.ParserTask;
@ -27,10 +28,10 @@ public final class ASTApexFile extends AbstractApexNode<AstNode> implements Root
ASTApexFile(ParserTask task,
Compilation jorjeNode,
Map<Integer, String> suppressMap,
@NonNull ApexMultifileAnalysis multifileAnalysis) {
@NonNull ApexLanguageProcessor apexLang) {
super(jorjeNode);
this.astInfo = new AstInfo<>(task, this, suppressMap);
this.multifileAnalysis = multifileAnalysis;
this.multifileAnalysis = apexLang.getMultiFileState();
this.setRegion(TextRegion.fromOffsetLength(0, task.getTextDocument().getLength()));
}

View File

@ -6,8 +6,7 @@ package net.sourceforge.pmd.lang.apex.ast;
import net.sourceforge.pmd.annotation.InternalApi;
import net.sourceforge.pmd.lang.apex.ApexJorjeLogging;
import net.sourceforge.pmd.lang.apex.ApexLanguageProperties;
import net.sourceforge.pmd.lang.apex.multifile.ApexMultifileAnalysis;
import net.sourceforge.pmd.lang.apex.ApexLanguageProcessor;
import net.sourceforge.pmd.lang.ast.ParseException;
import net.sourceforge.pmd.lang.ast.Parser;
@ -17,10 +16,10 @@ import apex.jorje.semantic.ast.compilation.Compilation;
@InternalApi
public final class ApexParser implements Parser {
private final ApexLanguageProperties apexProperties;
private final ApexLanguageProcessor proc;
public ApexParser(ApexLanguageProperties apexProperties) {
this.apexProperties = apexProperties;
public ApexParser(ApexLanguageProcessor proc) {
this.proc = proc;
ApexJorjeLogging.disableLogging();
Locations.useIndexFactory();
}
@ -35,12 +34,8 @@ public final class ApexParser implements Parser {
throw new ParseException("Couldn't parse the source - there is not root node - Syntax Error??");
}
String property = apexProperties.getProperty(ApexLanguageProperties.MULTIFILE_DIRECTORY);
ApexMultifileAnalysis analysisHandler = ApexMultifileAnalysis.getAnalysisInstance(property);
final ApexTreeBuilder treeBuilder = new ApexTreeBuilder(task, apexProperties);
return treeBuilder.buildTree(astRoot, analysisHandler);
final ApexTreeBuilder treeBuilder = new ApexTreeBuilder(task, proc);
return treeBuilder.buildTree(astRoot);
} catch (apex.jorje.services.exception.ParseException e) {
throw new ParseException(e);
}

View File

@ -17,8 +17,7 @@ import java.util.function.Function;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import net.sourceforge.pmd.lang.apex.ApexLanguageProperties;
import net.sourceforge.pmd.lang.apex.multifile.ApexMultifileAnalysis;
import net.sourceforge.pmd.lang.apex.ApexLanguageProcessor;
import net.sourceforge.pmd.lang.ast.Parser.ParserTask;
import net.sourceforge.pmd.lang.document.Chars;
import net.sourceforge.pmd.lang.document.TextDocument;
@ -253,12 +252,14 @@ final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
private final TextDocument sourceCode;
private final ParserTask task;
private final ApexLanguageProcessor proc;
private final CommentInformation commentInfo;
ApexTreeBuilder(ParserTask task, ApexLanguageProperties apexProperties) {
ApexTreeBuilder(ParserTask task, ApexLanguageProcessor proc) {
this.sourceCode = task.getTextDocument();
this.task = task;
commentInfo = extractInformationFromComments(sourceCode, apexProperties.getSuppressMarker());
this.proc = proc;
commentInfo = extractInformationFromComments(sourceCode, proc.getProperties().getSuppressMarker());
}
static <T extends AstNode> AbstractApexNode<T> createNodeAdapter(T node) {
@ -274,9 +275,9 @@ final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
return constructor.apply(node);
}
ASTApexFile buildTree(Compilation astNode, ApexMultifileAnalysis analysisHandler) {
ASTApexFile buildTree(Compilation astNode) {
assert nodes.isEmpty() : "stack should be empty";
ASTApexFile root = new ASTApexFile(task, astNode, commentInfo.suppressMap, analysisHandler);
ASTApexFile root = new ASTApexFile(task, astNode, commentInfo.suppressMap, proc);
nodes.push(root);
parents.push(astNode);

View File

@ -7,15 +7,14 @@ package net.sourceforge.pmd.lang.apex.multifile;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import net.sourceforge.pmd.annotation.Experimental;
import net.sourceforge.pmd.annotation.InternalApi;
import net.sourceforge.pmd.lang.apex.ApexLanguageProcessor;
import net.sourceforge.pmd.lang.apex.ApexLanguageProperties;
import com.nawforce.common.api.FileIssueOptions;
@ -43,34 +42,46 @@ public final class ApexMultifileAnalysis {
* Instances of the apexlink index and data structures ({@link Org})
* are stored statically for now. TODO make that language-wide (#2518).
*/
private static final Map<String, ApexMultifileAnalysis> INSTANCE_MAP = new ConcurrentHashMap<>();
// An arbitrary large number of errors to report
private static final Integer MAX_ERRORS_PER_FILE = 100;
private static final int MAX_ERRORS_PER_FILE = 100;
// Create a new org for each analysis
// Null if failed.
private final @Nullable Org org;
private final FileIssueOptions options = makeOptions();
private static final ApexMultifileAnalysis FAILED_INSTANCE = new ApexMultifileAnalysis();
/** Ctor for the failed instance. */
private ApexMultifileAnalysis() {
org = null;
static {
// Default some library wide settings
ServerOps.setAutoFlush(false);
ServerOps.setLogger(new AnalysisLogger());
ServerOps.setDebugLogging(new String[] { "ALL" });
}
private ApexMultifileAnalysis(String multiFileAnalysisDirectory) {
LOG.debug("MultiFile Analysis created for {}", multiFileAnalysisDirectory);
org = Org.newOrg();
if (multiFileAnalysisDirectory != null && !multiFileAnalysisDirectory.isEmpty()) {
// Load the package into the org, this can take some time!
org.newSFDXPackage(multiFileAnalysisDirectory); // this may fail if the config is wrong
org.flush();
// FIXME: Syntax & Semantic errors found during Org loading are not currently being reported. These
// should be routed to the new SemanticErrorReporter but that is not available for use just yet.
@InternalApi
public ApexMultifileAnalysis(ApexLanguageProperties properties) {
String rootDir = properties.getProperty(ApexLanguageProperties.MULTIFILE_DIRECTORY);
LOG.debug("MultiFile Analysis created for {}", rootDir);
Org org;
try {
org = Org.newOrg();
if (rootDir != null && !rootDir.isEmpty()) {
// Load the package into the org, this can take some time!
org.newSFDXPackage(rootDir); // this may fail if the config is wrong
org.flush();
// FIXME: Syntax & Semantic errors found during Org loading are not currently being reported. These
// should be routed to the new SemanticErrorReporter but that is not available for use just yet.
}
} catch (Exception e) {
LOG.error("Exception while initializing Apexlink ({})", e.getMessage(), e);
LOG.error("PMD will not attempt to initialize Apexlink further, this can cause rules like UnusedMethod to be dysfunctional");
org = null;
}
this.org = org;
}
private static FileIssueOptions makeOptions() {
@ -84,8 +95,8 @@ public final class ApexMultifileAnalysis {
/**
* Returns true if this is analysis index is in a failed state.
* This object is then useless. The failed instance is returned
* from {@link #getAnalysisInstance(String)} if loading the org
* failed, maybe because of malformed configuration.
* from {@link ApexLanguageProcessor#getMultiFileState()} if
* loading the org failed, maybe because of malformed configuration.
*/
public boolean isFailed() {
return org == null;
@ -97,33 +108,6 @@ public final class ApexMultifileAnalysis {
: Collections.unmodifiableList(Arrays.asList(org.getFileIssues(filename, options)));
}
/**
* Returns the analysis instance. Returns a {@linkplain #isFailed() failed instance}
* if this fails.
*
* @param multiFileAnalysisDirectory Root directory of the configuration (see {@link ApexLanguageProperties#MULTIFILE_DIRECTORY}).
*/
public static @NonNull ApexMultifileAnalysis getAnalysisInstance(String multiFileAnalysisDirectory) {
if (INSTANCE_MAP.isEmpty()) {
// Default some library wide settings
ServerOps.setAutoFlush(false);
ServerOps.setLogger(new AnalysisLogger());
ServerOps.setDebugLogging(new String[] { "ALL" });
}
return INSTANCE_MAP.computeIfAbsent(
multiFileAnalysisDirectory,
dir -> {
try {
return new ApexMultifileAnalysis(dir);
} catch (Exception e) {
LOG.error("Exception while initializing Apexlink ({})", e.getMessage(), e);
LOG.error("PMD will not attempt to initialize Apexlink further, this can cause rules like UnusedMethod to be dysfunctional");
return FAILED_INSTANCE;
}
});
}
/*
* Very simple logger to aid debugging, relays ApexLink logging into PMD
*/

View File

@ -21,6 +21,7 @@ import org.junit.Test;
import org.junit.contrib.java.lang.system.SystemErrRule;
import org.junit.rules.TemporaryFolder;
import net.sourceforge.pmd.lang.apex.ApexLanguageProperties;
import net.sourceforge.pmd.util.IOUtil;
public class ApexMultifileAnalysisTest {
@ -63,7 +64,9 @@ public class ApexMultifileAnalysisTest {
}
private @NonNull ApexMultifileAnalysis getAnalysisForTempFolder() {
return ApexMultifileAnalysis.getAnalysisInstance(tempFolder.getRoot().getAbsolutePath());
ApexLanguageProperties props = new ApexLanguageProperties();
props.setProperty(ApexLanguageProperties.MULTIFILE_DIRECTORY, tempFolder.getRoot().getAbsolutePath());
return new ApexMultifileAnalysis(props);
}
private void copyResource(String resourcePath, String relativePathInTempDir) throws IOException {

View File

@ -49,8 +49,9 @@ import net.sourceforge.pmd.util.log.internal.SimpleMessageReporter;
*/
public final class PMD {
private static final String PMD_PACKAGE = "net.sourceforge.pmd";
// not final, in order to re-initialize logging
private static Logger log = LoggerFactory.getLogger(PMD.class);
private static Logger log = LoggerFactory.getLogger(PMD_PACKAGE);
/**
* The line delimiter used by PMD in outputs. Usually the platform specific
@ -229,7 +230,7 @@ public final class PMD {
if (configuration.isDebug()) {
Slf4jSimpleConfiguration.reconfigureDefaultLogLevel(Level.TRACE);
// need to reload the logger with the new configuration
log = LoggerFactory.getLogger(PMD.class);
log = LoggerFactory.getLogger(PMD_PACKAGE);
}
// create a top-level reporter
// TODO CLI errors should also be reported through this

View File

@ -194,14 +194,14 @@ class CoreCliTest {
// restoring system properties: --debug might change logging properties
SystemLambda.restoreSystemProperties(() -> {
String log = runPmdSuccessfully("--debug", "--no-cache", "--dir", srcDir, "--rulesets", DUMMY_RULESET);
assertThat(log, containsString("[main] INFO net.sourceforge.pmd.PMD - Log level is at TRACE"));
assertThat(log, containsString("[main] INFO net.sourceforge.pmd - Log level is at TRACE"));
});
}
@Test
void defaultLogging() throws Exception {
String log = runPmdSuccessfully("--no-cache", "--dir", srcDir, "--rulesets", DUMMY_RULESET);
assertThat(log, containsString("[main] INFO net.sourceforge.pmd.PMD - Log level is at INFO"));
assertThat(log, containsString("[main] INFO net.sourceforge.pmd - Log level is at INFO"));
}
@Test

View File

@ -7,11 +7,11 @@ package net.sourceforge.pmd.cli;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.contrib.java.lang.system.RestoreSystemProperties;
import org.junit.jupiter.api.BeforeEach;
import org.junit.rules.TestRule;
import net.sourceforge.pmd.PMD.StatusCode;
@ -29,7 +29,7 @@ public class CLITest extends BaseCLITest {
@Rule
public final TestRule restoreSystemProperties = new RestoreSystemProperties();
@AfterClass
@BeforeEach
public static void resetLogging() {
Slf4jSimpleConfiguration.reconfigureDefaultLogLevel(null);
}
@ -67,7 +67,6 @@ public class CLITest extends BaseCLITest {
public void changeJavaVersion() {
String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", RSET_NO_VIOLATION, "-version", "1.5", "-language", "java", "--debug", "--no-progress", };
String log = runTest(args);
assertThat(log, containsString(".java (lang: java 1.5)"));
assertThat(log, containsPattern("Adding file .*\\.java \\(lang: java 1\\.5\\)"));
}

View File

@ -12,10 +12,6 @@ import net.sourceforge.pmd.lang.scala.ast.ScalaParser;
*/
public class ScalaLanguageHandler extends AbstractPmdLanguageVersionHandler {
public ScalaLanguageHandler() {
}
@Override
public ScalaParser getParser() {
return new ScalaParser();

View File

@ -20,15 +20,6 @@ import scala.meta.internal.parsers.ScalametaParser;
*/
public final class ScalaParser implements Parser {
/**
* Create a parser using the given Scala Dialect and set of parser options.
*
* @param scalaDialect
* the Scala Dialect for this parser
*/
public ScalaParser() {
}
@Override
public ASTSource parse(ParserTask task) throws ParseException {
Input.VirtualFile virtualFile = new Input.VirtualFile(task.getFileDisplayName(), task.getSourceText());