Skip to content

chore: Avoid using classpath classloader for auxclasspath #319

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.

Sign up for GitHub

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

Open
adangel wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore: Avoid using classpath classloader for auxclasspath #319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 1 addition & 11 deletions src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import static net.sourceforge.pmd.util.fxdesigner.util.reactfx.ReactfxUtil.latestValue;

import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.util.Collections;
Expand All @@ -28,7 +27,6 @@
import org.reactfx.value.Val;
import org.reactfx.value.Var;

import net.sourceforge.pmd.internal.util.ClasspathClassLoader; // NOPMD
import net.sourceforge.pmd.lang.Language;
import net.sourceforge.pmd.lang.LanguageVersion;
import net.sourceforge.pmd.lang.ast.Node;
Expand Down Expand Up @@ -89,14 +87,6 @@ public class SourceEditorController extends AbstractController {
private static final Duration AST_REFRESH_DELAY = Duration.ofMillis(100);
private final ASTManager astManager;
private final Var<List<File>> auxclasspathFiles = Var.newSimpleVar(emptyList());
private final Val<ClassLoader> auxclasspathClassLoader = auxclasspathFiles.<ClassLoader>map(fileList -> {
try {
return new ClasspathClassLoader(fileList, SourceEditorController.class.getClassLoader());
} catch (IOException e) {
e.printStackTrace();
return null;
}
}).orElseConst(SourceEditorController.class.getClassLoader());

@FXML
private Button searchButton;
Expand Down Expand Up @@ -193,7 +183,7 @@ protected void beforeParentInit() {
.distinct()
.subscribe(nodeEditionCodeArea::updateSyntaxHighlighter);

((ASTManagerImpl) astManager).classLoaderProperty().bind(auxclasspathClassLoader);
((ASTManagerImpl) astManager).classpathProperty().bind(auxclasspathFiles);

// default text, will be overwritten by settings restore
setText(getDefaultText());
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/net/sourceforge/pmd/util/fxdesigner/app/services/ASTManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

package net.sourceforge.pmd.util.fxdesigner.app.services;

import java.io.File;
import java.util.List;
import java.util.Map;

import org.reactfx.value.SuspendableVar;
Expand Down Expand Up @@ -50,7 +52,7 @@ public interface ASTManager extends ApplicationComponent, SettingsOwner {
Val<Node> compilationUnitProperty();


Val<ClassLoader> classLoaderProperty();
Val<List<File>> classpathProperty();


Val<ParseAbortedException> currentExceptionProperty();
Expand Down
27 changes: 17 additions & 10 deletions src/main/java/net/sourceforge/pmd/util/fxdesigner/app/services/ASTManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,19 @@

package net.sourceforge.pmd.util.fxdesigner.app.services;

import static java.util.Collections.emptyList;
import static net.sourceforge.pmd.util.fxdesigner.util.reactfx.ReactfxUtil.latestValue;
import static net.sourceforge.pmd.util.fxdesigner.util.reactfx.VetoableEventStream.vetoableNull;

import java.io.File;
import java.time.Duration;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;

import org.apache.commons.lang3.StringUtils;
import org.checkerframework.checker.nullness.qual.NonNull;
Expand Down Expand Up @@ -55,7 +59,7 @@ public class ASTManagerImpl implements ASTManager {

private final DesignerRoot designerRoot;

private final Var<ClassLoader> auxclasspathClassLoader = Var.newSimpleVar(null);
private final Var<List<File>> auxclasspathFiles = Var.newSimpleVar(emptyList());

/**
* Most up-to-date compilation unit. Is null if the current source cannot be parsed.
Expand All @@ -81,18 +85,18 @@ public ASTManagerImpl(DesignerRoot owner) {

// Refresh the AST anytime the text, classloader, or language version changes
sourceCode.values()
.or(classLoaderProperty().values())
.or(classpathProperty().values())
.or(languageVersionProperty().values())
.subscribe(tick -> {
// note: if either of these values would be null (e.g. classloader _is_ null at some point)
// note: if either of these values would be null
// the optional is empty.
Optional<ClassLoader> changedClassLoader = tick.asLeft().filter(Either::isRight).map(Either::getRight);
Optional<List<File>> changedClasspath = tick.asLeft().filter(Either::isRight).map(Either::getRight);
Optional<LanguageVersion> changedLanguageVersion = Optional.of(tick).filter(Either::isRight).map(Either::getRight);

Node updated;
try {
updated = refreshAST(this, getSourceCode(), getLanguageVersion(),
refreshRegistry(changedLanguageVersion.isPresent(), changedClassLoader.isPresent())).orElse(null);
refreshRegistry(changedLanguageVersion.isPresent(), changedClasspath.isPresent())).orElse(null);
currentException.setValue(null);
} catch (ParseAbortedException e) {
updated = null;
Expand Down Expand Up @@ -144,8 +148,8 @@ public void setSourceCode(String sourceCode) {
}

@Override
public Var<ClassLoader> classLoaderProperty() {
return auxclasspathClassLoader;
public Var<List<File>> classpathProperty() {
return auxclasspathFiles;
}

@Override
Expand Down Expand Up @@ -199,12 +203,15 @@ public Var<ParseAbortedException> currentExceptionProperty() {
return currentException;
}

private LanguageProcessorRegistry createNewRegistry(LanguageVersion version, ClassLoader classLoader) {
private LanguageProcessorRegistry createNewRegistry(LanguageVersion version, List<File> classpath) {
Map<Language, LanguagePropertyBundle> langProperties = new HashMap<>();
LanguagePropertyBundle bundle = version.getLanguage().newPropertyBundle();
bundle.setLanguageVersion(version.getVersion());
if (bundle instanceof JvmLanguagePropertyBundle) {
((JvmLanguagePropertyBundle) bundle).setClassLoader(classLoader);
bundle.setProperty(JvmLanguagePropertyBundle.AUX_CLASSPATH,
classpath.stream()
.map(File::getAbsolutePath)
.collect(Collectors.joining(File.pathSeparator)));
}

langProperties.put(version.getLanguage(), bundle);
Expand All @@ -230,7 +237,7 @@ private LanguageProcessorRegistry refreshRegistry(boolean changedLanguageVersion
current.close();
}

LanguageProcessorRegistry newRegistry = createNewRegistry(getLanguageVersion(), classLoaderProperty().getValue());
LanguageProcessorRegistry newRegistry = createNewRegistry(getLanguageVersion(), classpathProperty().getValue());
lpRegistry.setValue(newRegistry);
return newRegistry;
}
Expand Down