Skip to content

Commit

Permalink
Refactor Configuration into linter configuration and checks configura…
Browse files Browse the repository at this point in the history
…tion
  • Loading branch information
StevenLoomanEnexis committed Sep 14, 2023
1 parent 22e0456 commit 617751f
Show file tree
Hide file tree
Showing 23 changed files with 688 additions and 464 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package nl.ramsolutions.sw.magik.checks;

import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import nl.ramsolutions.sw.MagikToolsProperties;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;

/**
* Configuration.
*/
public class MagikChecksConfiguration {

private static final String KEY_DISABLED_CHECKS = "disabled";
private static final String KEY_ENABLED_CHECKS = "enabled";
private static final String KEY_IGNORED_PATHS = "ignore";

private final MagikToolsProperties properties;

/**
* Constructor.
* @throws IOException
*/
public MagikChecksConfiguration() throws IOException {
this.properties = new MagikToolsProperties();
}

/**
* Constructor which reads properties from {@code path}.
* @param path {@link Path} to read properties from.
* @throws IOException
*/
public MagikChecksConfiguration(final Path path) throws IOException {
this.properties = new MagikToolsProperties(path);
}

public List<String> getIgnores() {
return this.properties.getPropertyList(KEY_IGNORED_PATHS);
}

/**
* Get {@link MagikCheck}s, each contained by a {@link MagikCheckHolder}.
* @return
*/
public List<MagikCheckHolder> getAllChecks() {
final List<MagikCheckHolder> holders = new ArrayList<>();

final List<String> disableds = this.properties.getPropertyList(KEY_DISABLED_CHECKS);
final List<String> enableds = this.properties.getPropertyList(KEY_ENABLED_CHECKS);

for (final Class<?> checkClass : CheckList.getChecks()) {
final String checkKey = MagikChecksConfiguration.checkKey(checkClass);
final boolean checkEnabled =
enableds.contains(checkKey)
|| !disableds.contains(checkKey) && !disableds.contains("all");

// Gather parameters from MagikCheck, value from config.
final Set<MagikCheckHolder.Parameter> parameters = Arrays.stream(checkClass.getFields())
.map(field -> field.getAnnotation(RuleProperty.class))
.filter(Objects::nonNull)
.map(ruleProperty -> {
final String propertyKey = MagikChecksConfiguration.propertyKey(ruleProperty);
final String configKey = checkKey + "." + propertyKey;
if (!this.properties.hasProperty(configKey)) {
return null;
}

// Store parameter.
final String description = ruleProperty.description();
final MagikCheckHolder.Parameter parameter;
if (ruleProperty.type().equals("INTEGER")) {
final Integer configValue = this.properties.getPropertyInteger(configKey);
parameter = new MagikCheckHolder.Parameter(configKey, description, configValue);
} else if (ruleProperty.type().equals("STRING")) {
final String configValue = this.properties.getPropertyString(configKey);
parameter = new MagikCheckHolder.Parameter(configKey, description, configValue);
} else if (ruleProperty.type().equals("BOOLEAN")) {
final Boolean configValue = this.properties.getPropertyBoolean(configKey);
parameter = new MagikCheckHolder.Parameter(configKey, description, configValue);
} else {
throw new IllegalStateException("Unknown type for property: " + ruleProperty.type());
}

return parameter;
})
.filter(Objects::nonNull)
.collect(Collectors.toSet());

@SuppressWarnings("unchecked")
final MagikCheckHolder holder =
new MagikCheckHolder((Class<MagikCheck>) checkClass, parameters, checkEnabled);
holders.add(holder);
}
return holders;
}

private static String checkKey(final Class<?> checkClass) {
final Rule annotation = checkClass.getAnnotation(Rule.class);
final String checkKey = annotation.key();
return MagikCheckHolder.toKebabCase(checkKey);
}

private static String propertyKey(final RuleProperty ruleProperty) {
return ruleProperty.key().replace(" ", "-");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
import nl.ramsolutions.sw.magik.languageserver.codeactions.CodeActionProvider;
import nl.ramsolutions.sw.magik.languageserver.completion.CompletionProvider;
import nl.ramsolutions.sw.magik.languageserver.definitions.DefinitionsProvider;
import nl.ramsolutions.sw.magik.languageserver.diagnostics.MagikLintDiagnosticsProvider;
import nl.ramsolutions.sw.magik.languageserver.diagnostics.MagikTypeDiagnosticsProvider;
import nl.ramsolutions.sw.magik.languageserver.diagnostics.MagikChecksDiagnosticsProvider;
import nl.ramsolutions.sw.magik.languageserver.diagnostics.MagikTypedChecksDiagnosticsProvider;
import nl.ramsolutions.sw.magik.languageserver.documentsymbols.DocumentSymbolProvider;
import nl.ramsolutions.sw.magik.languageserver.folding.FoldingRangeProvider;
import nl.ramsolutions.sw.magik.languageserver.formatting.FormattingProvider;
Expand Down Expand Up @@ -250,7 +250,7 @@ private void publishDiagnostics(final MagikTypedFile magikFile) {
private List<Diagnostic> getDiagnosticsLinter(final MagikTypedFile magikFile) {
final Path overrideSettingsPath = MagikSettings.INSTANCE.getChecksOverrideSettingsPath();

final MagikLintDiagnosticsProvider lintProvider = new MagikLintDiagnosticsProvider(overrideSettingsPath);
final MagikChecksDiagnosticsProvider lintProvider = new MagikChecksDiagnosticsProvider(overrideSettingsPath);
try {
return lintProvider.getDiagnostics(magikFile);
} catch (final IOException exception) {
Expand All @@ -261,7 +261,7 @@ private List<Diagnostic> getDiagnosticsLinter(final MagikTypedFile magikFile) {
}

private List<Diagnostic> getDiagnosticsTyping(final MagikTypedFile magikFile) {
final MagikTypeDiagnosticsProvider typeProvider = new MagikTypeDiagnosticsProvider();
final MagikTypedChecksDiagnosticsProvider typeProvider = new MagikTypedChecksDiagnosticsProvider();
try {
return typeProvider.getDiagnostics(magikFile);
} catch (final IOException exception) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package nl.ramsolutions.sw.magik.languageserver.codeactions;

import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -37,6 +38,7 @@ public void setCapabilities(final ServerCapabilities capabilities) {
* @param range Range to provide code actions for.
* @param context Code action context.
* @return List of code actions.
* @throws IOException -
*/
public List<CodeAction> provideCodeActions(
final MagikTypedFile magikFile,
Expand All @@ -48,7 +50,7 @@ public List<CodeAction> provideCodeActions(
this.typedChecksCodeActionProvider.provideCodeActions(magikFile, range).stream()
)
.collect(Collectors.toList());
} catch (final ReflectiveOperationException exception) {
} catch (final IOException | ReflectiveOperationException exception) {
LOGGER.error(exception.getMessage(), exception);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package nl.ramsolutions.sw.magik.languageserver.codeactions;

import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
Expand All @@ -12,10 +13,9 @@
import nl.ramsolutions.sw.magik.checks.MagikCheck;
import nl.ramsolutions.sw.magik.checks.MagikCheckFixer;
import nl.ramsolutions.sw.magik.checks.MagikCheckHolder;
import nl.ramsolutions.sw.magik.checks.MagikChecksConfiguration;
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 @@ -27,11 +27,12 @@ public class MagikChecksCodeActionProvider {
* @param magikFile {@link MagikTypedFile{} to check on.
* @param range {@link Range} to get {@link CodeAction}s for.
* @return List of {@link CodeAction}s.
* @throws ReflectiveOperationException
* @throws ReflectiveOperationException -
* @throws IOException -
*/
public List<CodeAction> provideCodeActions(
final MagikTypedFile magikFile,
final Range range) throws ReflectiveOperationException {
final Range range) throws ReflectiveOperationException, IOException {
final List<CodeAction> codeActions = new ArrayList<>();
for (final Entry<Class<?>, List<Class<?>>> entry : CheckList.getFixers().entrySet()) {
final Class<?> checkClass = entry.getKey();
Expand All @@ -50,15 +51,15 @@ public List<CodeAction> provideCodeActions(
return codeActions;
}

private boolean isCheckEnabled(final MagikFile magikFile, final Class<?> checkClass) {
private boolean isCheckEnabled(final MagikFile magikFile, final Class<?> checkClass) throws IOException {
final Path searchPath = Path.of(magikFile.getUri()).getParent();
final Path configPath = MagikSettings.INSTANCE.getChecksOverrideSettingsPath() != null
? MagikSettings.INSTANCE.getChecksOverrideSettingsPath()
: ConfigurationLocator.locateConfiguration(searchPath);
final Configuration config = configPath != null
? new Configuration(configPath)
: new Configuration();
final List<MagikCheckHolder> allChecks = MagikLint.getAllChecks(config);
final MagikChecksConfiguration config = configPath != null
? new MagikChecksConfiguration(configPath)
: new MagikChecksConfiguration();
final List<MagikCheckHolder> allChecks = config.getAllChecks();
for (final MagikCheckHolder checkHolder : allChecks) {
if (checkHolder.getCheckClass().equals(checkClass)) {
return checkHolder.isEnabled();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package nl.ramsolutions.sw.magik.languageserver.codeactions;

import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
Expand All @@ -9,10 +10,9 @@
import nl.ramsolutions.sw.magik.MagikTypedFile;
import nl.ramsolutions.sw.magik.Range;
import nl.ramsolutions.sw.magik.checks.MagikCheckHolder;
import nl.ramsolutions.sw.magik.checks.MagikChecksConfiguration;
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;
Expand All @@ -28,10 +28,11 @@ public class MagikTypedChecksCodeActionProvider {
* @param range {@link Range} to get {@link CodeAction}s for.
* @return List of {@link CodeAction}s.
* @throws ReflectiveOperationException
* @throws IOException -
*/
public List<CodeAction> provideCodeActions(
final MagikTypedFile magikFile,
final Range range) throws ReflectiveOperationException {
final Range range) throws ReflectiveOperationException, IOException {
final List<CodeAction> codeActions = new ArrayList<>();
for (final Entry<Class<?>, List<Class<?>>> entry : CheckList.getFixers().entrySet()) {
final Class<?> checkClass = entry.getKey();
Expand All @@ -50,15 +51,15 @@ public List<CodeAction> provideCodeActions(
return codeActions;
}

private boolean isCheckEnabled(final MagikFile magikFile, final Class<?> checkClass) {
private boolean isCheckEnabled(final MagikFile magikFile, final Class<?> checkClass) throws IOException {
final Path searchPath = Path.of(magikFile.getUri()).getParent();
final Path configPath = MagikSettings.INSTANCE.getChecksOverrideSettingsPath() != null
? MagikSettings.INSTANCE.getChecksOverrideSettingsPath()
: ConfigurationLocator.locateConfiguration(searchPath);
final Configuration config = configPath != null
? new Configuration(configPath)
: new Configuration();
final List<MagikCheckHolder> allChecks = MagikLint.getAllChecks(config);
final MagikChecksConfiguration config = configPath != null
? new MagikChecksConfiguration(configPath)
: new MagikChecksConfiguration();
final List<MagikCheckHolder> allChecks = config.getAllChecks();
for (final MagikCheckHolder checkHolder : allChecks) {
if (checkHolder.getCheckClass().equals(checkClass)) {
return checkHolder.isEnabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ private List<Location> definitionsForMethodInvocation(final MagikTypedFile magik
final MethodInvocationNodeHelper helper = new MethodInvocationNodeHelper(methodInvocationNode);
final String methodName = helper.getMethodName();

final AstNode previousSiblingNode = wantedNode.getPreviousSibling();
final AstNode previousSiblingNode = methodInvocationNode.getPreviousSibling();
final LocalTypeReasoner reasoner = magikFile.getTypeReasoner();
final ExpressionResult result = reasoner.getNodeType(previousSiblingNode);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
import nl.ramsolutions.sw.magik.MagikFile;
import nl.ramsolutions.sw.magik.checks.MagikCheckHolder;
import nl.ramsolutions.sw.magik.languageserver.Lsp4jConversion;
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.lint.MagikLintConfiguration;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.DiagnosticSeverity;
import org.eclipse.lsp4j.Location;
Expand All @@ -24,9 +24,9 @@
/**
* MagikLint diagnostics provider.
*/
public class MagikLintDiagnosticsProvider {
public class MagikChecksDiagnosticsProvider {

static final Logger LOGGER = LoggerFactory.getLogger(MagikLintDiagnosticsProvider.class);
static final Logger LOGGER = LoggerFactory.getLogger(MagikChecksDiagnosticsProvider.class);

private static final Map<String, DiagnosticSeverity> LINT_SEVERITY_MAPPING = Map.of(
"Critical", DiagnosticSeverity.Error,
Expand All @@ -38,7 +38,7 @@ public class MagikLintDiagnosticsProvider {
/**
* Constructor. Locates configuration to be used.
*/
public MagikLintDiagnosticsProvider(final @Nullable Path overrideConfigurationPath) {
public MagikChecksDiagnosticsProvider(final @Nullable Path overrideConfigurationPath) {
this.overrideConfigurationPath = overrideConfigurationPath;
}

Expand All @@ -57,9 +57,9 @@ public List<Diagnostic> getDiagnostics(final MagikFile magikFile) throws IOExcep
final Path configurationPath = this.overrideConfigurationPath != null
? this.overrideConfigurationPath
: ConfigurationLocator.locateConfiguration(searchPath);
final Configuration configuration = configurationPath != null
? new Configuration(configurationPath)
: new Configuration(); // Default configuration.
final MagikLintConfiguration configuration = configurationPath != null
? new MagikLintConfiguration(configurationPath)
: new MagikLintConfiguration(); // Default configuration.
final MagikLintDiagnosticsReporter reporter = new MagikLintDiagnosticsReporter();
final MagikLint magikLint = new MagikLint(configuration, reporter);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
/**
* MagikType diagnostics provider.
*/
public class MagikTypeDiagnosticsProvider {
public class MagikTypedChecksDiagnosticsProvider {

static final Logger LOGGER = LoggerFactory.getLogger(MagikTypeDiagnosticsProvider.class);
static final Logger LOGGER = LoggerFactory.getLogger(MagikTypedChecksDiagnosticsProvider.class);

/**
* Get {@link Diagnostic}s for typing errors..
Expand Down Expand Up @@ -61,9 +61,7 @@ private Set<MagikTypedCheck> createTypedChecks() {
final Set<Parameter> parameters = Collections.emptySet();
final MagikCheckHolder holder =
new MagikCheckHolder((Class<MagikCheck>) checkClass, parameters, true);
final MagikTypedCheck magikCheck =
(MagikTypedCheck) checkClass.getDeclaredConstructor().newInstance();
magikCheck.setHolder(holder);
final MagikTypedCheck magikCheck = (MagikTypedCheck) holder.createCheck();
return magikCheck;
} catch (final ReflectiveOperationException exception) {
LOGGER.error(exception.getMessage(), exception);
Expand Down
Loading

0 comments on commit 617751f

Please sign in to comment.