Skip to content

Commit

Permalink
Code action providers for MagikChecks/MagikTypedChecks are only activ…
Browse files Browse the repository at this point in the history
…e if check is enabled in configuration
  • Loading branch information
StevenLooman committed Sep 3, 2023
1 parent 66d3e4e commit d3ec3b0
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 55 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
- Add type hierarchy provider to magik-language-server.
- Add inlay hint provider to magik-language-server.
- Add code actions for @parameter and @return type-doc parts.
- Code action providers for MagikChecks/MagikTypedChecks are only active if check is enabled in configuration.
- MethodReturnMatchesDocCheck points to the actual type-doc part.
- Checks now mark the complete violating part, instead of the first token.
- Show source-check for magik-typed checks in magik-language-server.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,14 @@ public boolean isEnabled() {
return this.enabled;
}

/**
* Get the {@link MagikCheck} class.
* @return {@link MagikCheck} class.
*/
public Class<MagikCheck> getCheckClass() {
return this.checkClass;
}

/**
* Get all parameters.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package nl.ramsolutions.sw.magik.languageserver;

import com.google.gson.JsonObject;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -35,7 +34,6 @@ public class MagikLanguageServer implements LanguageServer, LanguageClientAware
private final MagikWorkspaceService magikWorkspaceService;
private final MagikNotebookDocumentService magikNotebookDocumentService;
private LanguageClient languageClient;
private MagikSettings settings = MagikSettings.DEFAULT;

/**
* Constructor.
Expand Down Expand Up @@ -151,23 +149,4 @@ public List<WorkspaceFolder> getWorkspaceFolders() {
return Collections.unmodifiableList(this.workspaceFolders);
}

/**
* Set settings for workspace.
* @param settings Settings object.
*/
public void setSettings(final JsonObject settings) {
this.settings = new MagikSettings(settings);

// Log configuration.
LOGGER.info("Settings: {}", settings);
}

/**
* Get settings from workspace.
* @return Settings object.
*/
public MagikSettings getMagikSettings() {
return this.settings;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,28 @@
/**
* Magik settings.
*/
public class MagikSettings {
public final class MagikSettings {

/**
* Default MagikSettings, to prevent NPEs.
* Singleton instance.
*/
public static final MagikSettings DEFAULT = new MagikSettings(new JsonObject());
public static final MagikSettings INSTANCE = new MagikSettings();

private static final String TOP_LEVEL = "magik";

private final JsonObject settings;
private JsonObject settings = new JsonObject();

/**
* Constructor.
* @param settings Settings from client.
* Private constructor.
*/
public MagikSettings(final JsonObject settings) {
private MagikSettings() {
}

/**
* Set new settings.
* @param settings
*/
public void setSettings(final JsonObject settings) {
this.settings = settings;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,7 @@ private void publishDiagnostics(final MagikTypedFile magikFile) {
diagnostics.addAll(diagnosticsLinter);

// Typing diagnostics.
final MagikSettings magikSettings = this.languageServer.getMagikSettings();
final Boolean typingEnableChecks = magikSettings.getTypingEnableChecks();
final Boolean typingEnableChecks = MagikSettings.INSTANCE.getTypingEnableChecks();
if (Boolean.TRUE.equals(typingEnableChecks)) {
final List<Diagnostic> diagnosticsTyping = this.getDiagnosticsTyping(magikFile);
diagnostics.addAll(diagnosticsTyping);
Expand All @@ -238,8 +237,7 @@ private void publishDiagnostics(final MagikTypedFile magikFile) {
}

private List<Diagnostic> getDiagnosticsLinter(final MagikTypedFile magikFile) {
final MagikSettings magikSettings = this.languageServer.getMagikSettings();
final Path overrideSettingsPath = magikSettings.getChecksOverrideSettingsPath();
final Path overrideSettingsPath = MagikSettings.INSTANCE.getChecksOverrideSettingsPath();

final MagikLintDiagnosticsProvider lintProvider = new MagikLintDiagnosticsProvider(overrideSettingsPath);
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import nl.ramsolutions.sw.magik.analysis.typing.ClassInfoTypeKeeperReader;
import nl.ramsolutions.sw.magik.analysis.typing.ITypeKeeper;
import nl.ramsolutions.sw.magik.analysis.typing.ReadOnlyTypeKeeperAdapter;
import nl.ramsolutions.sw.magik.analysis.typing.TypeKeeper;
import nl.ramsolutions.sw.magik.analysis.typing.indexer.MagikIndexer;
import nl.ramsolutions.sw.magik.analysis.typing.io.JsonTypeKeeperReader;
import nl.ramsolutions.sw.magik.languageserver.munit.MUnitTestItem;
Expand Down Expand Up @@ -84,7 +85,9 @@ public void didChangeConfiguration(final DidChangeConfigurationParams params) {
LOGGER.trace("didChangeConfiguration");

final JsonObject settings = (JsonObject) params.getSettings();
this.languageServer.setSettings(settings);

LOGGER.debug("New settings: {}", settings);
MagikSettings.INSTANCE.setSettings(settings);

this.runIndexersInBackground();
}
Expand Down Expand Up @@ -283,12 +286,11 @@ private void runIndexers() {
LOGGER.trace("Run indexers");

// Read types db.
final MagikSettings magikSettings = this.languageServer.getMagikSettings();
final List<String> typesDbPaths = magikSettings.getTypingTypeDatabasePaths();
final List<String> typesDbPaths = MagikSettings.INSTANCE.getTypingTypeDatabasePaths();
this.readTypesDbs(typesDbPaths);

// Read method docs.
final String smallworldGisDir = magikSettings.getSmallworldGis();
final String smallworldGisDir = MagikSettings.INSTANCE.getSmallworldGis();
if (smallworldGisDir != null) {
this.readLibsDocs(smallworldGisDir);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ public List<CodeAction> provideCodeActions(
final CodeActionContext context) {
try {
return Stream.concat(
this.checksCodeActionProvider.provideCodeActions(magikFile, range, context).stream(),
this.typedChecksCodeActionProvider.provideCodeActions(magikFile, range, context).stream()
this.checksCodeActionProvider.provideCodeActions(magikFile, range).stream(),
this.typedChecksCodeActionProvider.provideCodeActions(magikFile, range).stream()
)
.collect(Collectors.toList());
} catch (final ReflectiveOperationException exception) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
package nl.ramsolutions.sw.magik.languageserver.codeactions;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Map.Entry;
import nl.ramsolutions.sw.magik.CodeAction;
import nl.ramsolutions.sw.magik.MagikFile;
import nl.ramsolutions.sw.magik.MagikTypedFile;
import nl.ramsolutions.sw.magik.Range;
import nl.ramsolutions.sw.magik.checks.CheckList;
import nl.ramsolutions.sw.magik.checks.MagikCheck;
import nl.ramsolutions.sw.magik.checks.MagikCheckFixer;
import org.eclipse.lsp4j.CodeActionContext;
import nl.ramsolutions.sw.magik.checks.MagikCheckHolder;
import nl.ramsolutions.sw.magik.languageserver.MagikSettings;
import nl.ramsolutions.sw.magik.lint.Configuration;
import nl.ramsolutions.sw.magik.lint.ConfigurationLocator;
import nl.ramsolutions.sw.magik.lint.MagikLint;

/**
* Provide {@link CodeAction}s for {@link MagikCheck}s.
Expand All @@ -20,20 +26,18 @@ public class MagikChecksCodeActionProvider {
* Provide {@link CodeAction} for {@link MagikCheck} checks.
* @param magikFile {@link MagikTypedFile{} to check on.
* @param range {@link Range} to get {@link CodeAction}s for.
* @param context Context, not used currently.
* @return List of {@link CodeAction}s.
* @throws ReflectiveOperationException
*/
public List<CodeAction> provideCodeActions(
final MagikTypedFile magikFile,
final Range range,
final CodeActionContext context) throws ReflectiveOperationException {
final Range range) throws ReflectiveOperationException {
final List<CodeAction> codeActions = new ArrayList<>();
for (final Entry<Class<?>, List<Class<?>>> entry : CheckList.getFixers().entrySet()) {
final Class<?> checkClass = entry.getKey();
final List<Class<?>> fixerClassses = entry.getValue();
for (final Class<?> fixerClass : fixerClassses) {
if (!this.isCheckEnabled(checkClass)) {
if (!this.isCheckEnabled(magikFile, checkClass)) {
continue;
}

Expand All @@ -46,9 +50,21 @@ public List<CodeAction> provideCodeActions(
return codeActions;
}

private boolean isCheckEnabled(final Class<?> checkClass) {
// TODO: Implement this.
return true;
private boolean isCheckEnabled(final MagikFile magikFile, final Class<?> checkClass) {
final Path searchPath = Path.of(magikFile.getUri()).getParent();
final Path configPath = MagikSettings.INSTANCE.getChecksOverrideSettingsPath() != null
? MagikSettings.INSTANCE.getChecksOverrideSettingsPath()
: ConfigurationLocator.locateConfiguration(searchPath);
final Configuration config = new Configuration(configPath);
final List<MagikCheckHolder> allChecks = MagikLint.getAllChecks(config);
for (final MagikCheckHolder checkHolder : allChecks) {
if (checkHolder.getCheckClass().equals(checkClass)) {
return checkHolder.isEnabled();
}
}

// Check not found, so not enabled.
return false;
}

}
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
package nl.ramsolutions.sw.magik.languageserver.codeactions;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Map.Entry;
import nl.ramsolutions.sw.magik.CodeAction;
import nl.ramsolutions.sw.magik.MagikFile;
import nl.ramsolutions.sw.magik.MagikTypedFile;
import nl.ramsolutions.sw.magik.Range;
import nl.ramsolutions.sw.magik.checks.MagikCheckHolder;
import nl.ramsolutions.sw.magik.languageserver.MagikSettings;
import nl.ramsolutions.sw.magik.lint.Configuration;
import nl.ramsolutions.sw.magik.lint.ConfigurationLocator;
import nl.ramsolutions.sw.magik.lint.MagikLint;
import nl.ramsolutions.sw.magik.typedchecks.CheckList;
import nl.ramsolutions.sw.magik.typedchecks.MagikTypedCheck;
import nl.ramsolutions.sw.magik.typedchecks.MagikTypedCheckFixer;
import org.eclipse.lsp4j.CodeActionContext;

/**
* Provide {@link CodeAction}s for {@link MagikTypedCheck}s.
Expand All @@ -20,35 +26,45 @@ public class MagikTypedChecksCodeActionProvider {
* Provide {@link CodeAction} for {@link MagikTypedCheck} checks.
* @param magikFile {@link MagikTypedFile{} to check on.
* @param range {@link Range} to get {@link CodeAction}s for.
* @param context Context, not used currently.
* @return List of {@link CodeAction}s.
* @throws ReflectiveOperationException
*/
public List<CodeAction> provideCodeActions(
final MagikTypedFile magikFile,
final Range range,
final CodeActionContext context) throws ReflectiveOperationException {
final Range range) throws ReflectiveOperationException {
final List<CodeAction> codeActions = new ArrayList<>();
for (final Entry<Class<?>, List<Class<?>>> entry : CheckList.getFixers().entrySet()) {
final Class<?> checkClass = entry.getKey();
final List<Class<?>> fixerClassses = entry.getValue();
for (final Class<?> fixerClass : fixerClassses) {
if (!this.isCheckEnabled(checkClass)) {
if (!this.isCheckEnabled(magikFile, checkClass)) {
continue;
}

final MagikTypedCheckFixer fixer =
(MagikTypedCheckFixer) fixerClass.getDeclaredConstructor().newInstance();
List<CodeAction> fixerCodeActions = fixer.provideCodeActions(magikFile, range);
final List<CodeAction> fixerCodeActions = fixer.provideCodeActions(magikFile, range);
codeActions.addAll(fixerCodeActions);
}
}
return codeActions;
}

private boolean isCheckEnabled(final Class<?> checkClass) {
// TODO: Implement this.
return true;
private boolean isCheckEnabled(final MagikFile magikFile, final Class<?> checkClass) {
final Path searchPath = Path.of(magikFile.getUri()).getParent();
final Path configPath = MagikSettings.INSTANCE.getChecksOverrideSettingsPath() != null
? MagikSettings.INSTANCE.getChecksOverrideSettingsPath()
: ConfigurationLocator.locateConfiguration(searchPath);
final Configuration config = new Configuration(configPath);
final List<MagikCheckHolder> allChecks = MagikLint.getAllChecks(config);
for (final MagikCheckHolder checkHolder : allChecks) {
if (checkHolder.getCheckClass().equals(checkClass)) {
return checkHolder.isEnabled();
}
}

// Check not found, so not enabled.
return false;
}

}

0 comments on commit d3ec3b0

Please sign in to comment.