diff --git a/magik-checks/src/main/java/nl/ramsolutions/sw/magik/checks/MagikChecksConfiguration.java b/magik-checks/src/main/java/nl/ramsolutions/sw/magik/checks/MagikChecksConfiguration.java new file mode 100644 index 00000000..bf1aada2 --- /dev/null +++ b/magik-checks/src/main/java/nl/ramsolutions/sw/magik/checks/MagikChecksConfiguration.java @@ -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 getIgnores() { + return this.properties.getPropertyList(KEY_IGNORED_PATHS); + } + + /** + * Get {@link MagikCheck}s, each contained by a {@link MagikCheckHolder}. + * @return + */ + public List getAllChecks() { + final List holders = new ArrayList<>(); + + final List disableds = this.properties.getPropertyList(KEY_DISABLED_CHECKS); + final List 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 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) 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(" ", "-"); + } + +} diff --git a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/MagikTextDocumentService.java b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/MagikTextDocumentService.java index 6db3c720..5178feba 100644 --- a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/MagikTextDocumentService.java +++ b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/MagikTextDocumentService.java @@ -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; @@ -250,7 +250,7 @@ private void publishDiagnostics(final MagikTypedFile magikFile) { private List 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) { @@ -261,7 +261,7 @@ private List getDiagnosticsLinter(final MagikTypedFile magikFile) { } private List getDiagnosticsTyping(final MagikTypedFile magikFile) { - final MagikTypeDiagnosticsProvider typeProvider = new MagikTypeDiagnosticsProvider(); + final MagikTypedChecksDiagnosticsProvider typeProvider = new MagikTypedChecksDiagnosticsProvider(); try { return typeProvider.getDiagnostics(magikFile); } catch (final IOException exception) { diff --git a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/CodeActionProvider.java b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/CodeActionProvider.java index e3964766..6a2aca6a 100644 --- a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/CodeActionProvider.java +++ b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/CodeActionProvider.java @@ -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; @@ -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 provideCodeActions( final MagikTypedFile magikFile, @@ -48,7 +50,7 @@ public List provideCodeActions( this.typedChecksCodeActionProvider.provideCodeActions(magikFile, range).stream() ) .collect(Collectors.toList()); - } catch (final ReflectiveOperationException exception) { + } catch (final IOException | ReflectiveOperationException exception) { LOGGER.error(exception.getMessage(), exception); } diff --git a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/MagikChecksCodeActionProvider.java b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/MagikChecksCodeActionProvider.java index 24b3b2b2..673d0561 100644 --- a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/MagikChecksCodeActionProvider.java +++ b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/MagikChecksCodeActionProvider.java @@ -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; @@ -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. @@ -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 provideCodeActions( final MagikTypedFile magikFile, - final Range range) throws ReflectiveOperationException { + final Range range) throws ReflectiveOperationException, IOException { final List codeActions = new ArrayList<>(); for (final Entry, List>> entry : CheckList.getFixers().entrySet()) { final Class checkClass = entry.getKey(); @@ -50,15 +51,15 @@ public List 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 allChecks = MagikLint.getAllChecks(config); + final MagikChecksConfiguration config = configPath != null + ? new MagikChecksConfiguration(configPath) + : new MagikChecksConfiguration(); + final List allChecks = config.getAllChecks(); for (final MagikCheckHolder checkHolder : allChecks) { if (checkHolder.getCheckClass().equals(checkClass)) { return checkHolder.isEnabled(); diff --git a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/MagikTypedChecksCodeActionProvider.java b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/MagikTypedChecksCodeActionProvider.java index ced80da0..3884121a 100644 --- a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/MagikTypedChecksCodeActionProvider.java +++ b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/codeactions/MagikTypedChecksCodeActionProvider.java @@ -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; @@ -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; @@ -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 provideCodeActions( final MagikTypedFile magikFile, - final Range range) throws ReflectiveOperationException { + final Range range) throws ReflectiveOperationException, IOException { final List codeActions = new ArrayList<>(); for (final Entry, List>> entry : CheckList.getFixers().entrySet()) { final Class checkClass = entry.getKey(); @@ -50,15 +51,15 @@ public List 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 allChecks = MagikLint.getAllChecks(config); + final MagikChecksConfiguration config = configPath != null + ? new MagikChecksConfiguration(configPath) + : new MagikChecksConfiguration(); + final List allChecks = config.getAllChecks(); for (final MagikCheckHolder checkHolder : allChecks) { if (checkHolder.getCheckClass().equals(checkClass)) { return checkHolder.isEnabled(); diff --git a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/definitions/DefinitionsProvider.java b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/definitions/DefinitionsProvider.java index 5230f95d..1245ac9e 100644 --- a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/definitions/DefinitionsProvider.java +++ b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/definitions/DefinitionsProvider.java @@ -134,7 +134,7 @@ private List 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); diff --git a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikLintDiagnosticsProvider.java b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikChecksDiagnosticsProvider.java similarity index 87% rename from magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikLintDiagnosticsProvider.java rename to magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikChecksDiagnosticsProvider.java index ede38850..2293f944 100644 --- a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikLintDiagnosticsProvider.java +++ b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikChecksDiagnosticsProvider.java @@ -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; @@ -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 LINT_SEVERITY_MAPPING = Map.of( "Critical", DiagnosticSeverity.Error, @@ -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; } @@ -57,9 +57,9 @@ public List 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); diff --git a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikTypeDiagnosticsProvider.java b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikTypedChecksDiagnosticsProvider.java similarity index 91% rename from magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikTypeDiagnosticsProvider.java rename to magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikTypedChecksDiagnosticsProvider.java index ef14fdfb..6de62e7e 100644 --- a/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikTypeDiagnosticsProvider.java +++ b/magik-language-server/src/main/java/nl/ramsolutions/sw/magik/languageserver/diagnostics/MagikTypedChecksDiagnosticsProvider.java @@ -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.. @@ -61,9 +61,7 @@ private Set createTypedChecks() { final Set parameters = Collections.emptySet(); final MagikCheckHolder holder = new MagikCheckHolder((Class) 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); diff --git a/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/Configuration.java b/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/Configuration.java deleted file mode 100644 index c38010d5..00000000 --- a/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/Configuration.java +++ /dev/null @@ -1,162 +0,0 @@ -package nl.ramsolutions.sw.magik.lint; - -import java.io.File; -import java.io.FileInputStream; -import java.io.FileNotFoundException; -import java.io.IOException; -import java.io.InputStream; -import java.nio.file.Path; -import java.util.Collections; -import java.util.List; -import java.util.Objects; -import java.util.Properties; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import java.util.stream.Collectors; -import javax.annotation.CheckForNull; -import nl.ramsolutions.sw.magik.checks.CheckList; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.sonar.check.Rule; - -/** - * Configuration. - */ -public class Configuration { - - private static final Logger LOGGER = LoggerFactory.getLogger(Configuration.class); - - private final Properties properties = new Properties(); - - /** - * Constructor. - */ - public Configuration() { - this.setDisabledChecks(); - } - - /** - * Constructor which reads properties from {@code path}. - * @param path {@link Path} to read properties from. - */ - public Configuration(final Path path) { - LOGGER.debug("Reading configuration from: {}", path.toAbsolutePath()); - this.setDisabledChecks(); - this.readFileProperties(path); - } - - private void setDisabledChecks() { - final String disabled = this.getDisabledCheckNames(); - this.properties.put("disabled", disabled); - } - - private String getDisabledCheckNames() { - return CheckList.getDisabledByDefaultChecks().stream() - .map(checkClass -> checkClass.getAnnotation(org.sonar.check.Rule.class)) - .filter(Objects::nonNull) - .map(Rule::key) - .map(Configuration::toKebabCase) - .collect(Collectors.joining(",")); - } - - private void readFileProperties(final Path path) { - final File file = path.toFile(); - try (InputStream inputStream = new FileInputStream(file)) { - this.properties.load(inputStream); - } catch (FileNotFoundException exception) { - LOGGER.warn("Configuration not found at: {}", path); - } catch (IOException exception) { - LOGGER.error(exception.getMessage(), exception); - } - } - - /** - * Get a property. - * @param key Key of property. - * @return Values of property. - */ - @CheckForNull - public String getPropertyString(final String key) { - return this.properties.getProperty(key); - } - - /** - * Get property as integer. - * - * @param key Key of property. - * @return Value of property, as integer. - */ - @CheckForNull - public Integer getPropertyInt(final String key) { - final String value = this.getPropertyString(key); - if (value == null) { - return null; - } - - return Integer.valueOf(value); - } - - /** - * Get property as boolean. - * - * @param key Key of property. - * @return Value of property, as boolean. - */ - @CheckForNull - public Boolean getPropertyBoolean(final String key) { - final String value = this.getPropertyString(key); - if (value == null) { - return null; - } - - return Boolean.valueOf(value); - } - - /** - * Test if property with key exists. - * - * @param key Key of property. - * @return True if property exists, false if not. - */ - public boolean hasProperty(final String key) { - return this.properties.getProperty(key) != null; - } - - /** - * Set a property. - * @param key Key of property. - * @param value Value of property. - */ - public void setProperty(final String key, final String value) { - this.properties.setProperty(key, value); - } - - /** - * Log the configuration. - */ - public void logProperties() { - LOGGER.debug("Configuration:"); - final List propertyNames = Collections.list(this.properties.propertyNames()); - for (final Object propertyName : propertyNames) { - final String name = (String) propertyName; - final Object propertyValue = this.properties.get(name); - LOGGER.debug(" {}: {}", name, propertyValue); - } - } - - /** - * Utility method to convert camel case to kebab case. - * @param string String in camel case. - * @return String in kebab case. - */ - public static String toKebabCase(final String string) { - final Pattern pattern = Pattern.compile("(?=[A-Z][a-z])"); - final Matcher matcher = pattern.matcher(string); - final String stringKebab = matcher.replaceAll("-").toLowerCase(); - if (stringKebab.startsWith("-")) { - return stringKebab.substring(1); - } - return stringKebab; - } - -} diff --git a/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/MagikLint.java b/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/MagikLint.java index 55e612b1..2f3dde9e 100644 --- a/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/MagikLint.java +++ b/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/MagikLint.java @@ -10,33 +10,26 @@ import java.nio.file.Path; import java.nio.file.PathMatcher; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.Comparator; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import java.util.stream.Collectors; import javax.annotation.Nullable; import nl.ramsolutions.sw.FileCharsetDeterminer; import nl.ramsolutions.sw.Utils; import nl.ramsolutions.sw.magik.Location; import nl.ramsolutions.sw.magik.MagikFile; -import nl.ramsolutions.sw.magik.analysis.scope.GlobalScope; import nl.ramsolutions.sw.magik.analysis.scope.Scope; -import nl.ramsolutions.sw.magik.checks.CheckList; import nl.ramsolutions.sw.magik.checks.MagikCheck; import nl.ramsolutions.sw.magik.checks.MagikCheckHolder; +import nl.ramsolutions.sw.magik.checks.MagikChecksConfiguration; import nl.ramsolutions.sw.magik.checks.MagikIssue; import nl.ramsolutions.sw.magik.lint.output.Reporter; import nl.ramsolutions.sw.magik.parser.CommentInstructionReader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.sonar.check.Rule; -import org.sonar.check.RuleProperty; /** * Magik Lint main class. @@ -45,12 +38,13 @@ public class MagikLint { private static final Logger LOGGER = LoggerFactory.getLogger(MagikLint.class); - private static final CommentInstructionReader.InstructionType MLINT_INSTRUCTION = - CommentInstructionReader.InstructionType.createInstructionType("mlint"); + private static final String KEY_DISABLE = "disable"; + private static final CommentInstructionReader.InstructionType MLINT_LINE_INSTRUCTION = + CommentInstructionReader.InstructionType.createStatementInstructionType("mlint"); private static final CommentInstructionReader.InstructionType MLINT_SCOPE_INSTRUCTION = CommentInstructionReader.InstructionType.createScopeInstructionType("mlint"); - private final Configuration config; + private final MagikLintConfiguration config; private final Reporter reporter; /** @@ -59,7 +53,7 @@ public class MagikLint { * @param configuration Configuration. * @param reporter Reporter. */ - public MagikLint(final Configuration configuration, final Reporter reporter) { + public MagikLint(final MagikLintConfiguration configuration, final Reporter reporter) { this.config = configuration; this.reporter = reporter; } @@ -114,7 +108,11 @@ private List runCheckOnFile(final MagikFile magikFile, final MagikCh * @throws IOException - */ void showChecks(final Writer writer, final boolean showDisabled) throws ReflectiveOperationException, IOException { - final Iterable holders = MagikLint.getAllChecks(this.config); + final Path configPath = this.config.getPath(); + final MagikChecksConfiguration checksConfig = configPath != null + ? new MagikChecksConfiguration(configPath) + : new MagikChecksConfiguration(); + final Iterable holders = checksConfig.getAllChecks(); for (final MagikCheckHolder holder : holders) { if (!showDisabled && holder.isEnabled() || showDisabled && !holder.isEnabled()) { writer.write("- " + holder.getSqKey() + " (" + holder.getTitle() + ")\n"); @@ -161,21 +159,19 @@ void showDisabledChecks(final Writer writer) throws ReflectiveOperationException * @param instructionsHandler Instruction handler to use. * @return true if issue is disabled at line. */ - private boolean isMagikIssueDisabled( - final MagikFile magikFile, - final MagikIssue magikIssue, - final CommentInstructionReader instructionReader) { + private boolean isMagikIssueDisabled(final MagikFile magikFile, final MagikIssue magikIssue) { final MagikCheckHolder holder = magikIssue.check().getHolder(); Objects.requireNonNull(holder); final Integer line = magikIssue.startLine(); - final String checkKey = holder.getCheckKeyKebabCase(); - + final Scope scope = magikFile.getGlobalScope().getScopeForLineColumn(line, Integer.MAX_VALUE); final Map scopeInstructions = - MagikLint.getScopeInstructions(magikFile, instructionReader, line); - final Map lineInstructions = MagikLint.getLineInstructions(instructionReader, line); - final String[] scopeDisableds = scopeInstructions.getOrDefault("disable", "").split(","); - final String[] lineDisableds = lineInstructions.getOrDefault("disable", "").split(","); + magikFile.getScopeInstructions(MLINT_SCOPE_INSTRUCTION).get(scope); + final Map lineInstructions = + magikFile.getLineInstructions(MLINT_LINE_INSTRUCTION).get(line); + final String[] scopeDisableds = scopeInstructions.getOrDefault(MagikLint.KEY_DISABLE, "").split(","); + final String[] lineDisableds = lineInstructions.getOrDefault(MagikLint.KEY_DISABLE, "").split(","); + final String checkKey = holder.getCheckKeyKebabCase(); return List.of(scopeDisableds).contains(checkKey) || List.of(lineDisableds).contains(checkKey); } @@ -189,8 +185,6 @@ private boolean isMagikIssueDisabled( private List runChecksOnFile(final MagikFile magikFile, final Iterable holders) { LOGGER.trace("Thread: {}, checking file: {}", Thread.currentThread().getName(), magikFile); - final CommentInstructionReader instructionReader = new CommentInstructionReader( - magikFile, Set.of(MLINT_INSTRUCTION, MLINT_SCOPE_INSTRUCTION)); final List magikIssues = new ArrayList<>(); // run checks on files @@ -201,14 +195,14 @@ private List runChecksOnFile(final MagikFile magikFile, final Iterab try { final List issues = this.runCheckOnFile(magikFile, holder).stream() - .filter(magikIssue -> !this.isMagikIssueDisabled(magikFile, magikIssue, instructionReader)) + .filter(magikIssue -> !this.isMagikIssueDisabled(magikFile, magikIssue)) .collect(Collectors.toList()); magikIssues.addAll(issues); } catch (ReflectiveOperationException exception) { LOGGER.error(exception.getMessage(), exception); } - } + return magikIssues; } @@ -219,39 +213,22 @@ private List runChecksOnFile(final MagikFile magikFile, final Iterab * @throws ReflectiveOperationException - */ public void run(final Collection paths) throws IOException, ReflectiveOperationException { - // Gather ignore matchers. - final List ignoreMatchers; - if (this.config.hasProperty("ignore")) { - @SuppressWarnings("java:S2259") - final String[] ignores = this.config.getPropertyString("ignore").split(","); - final FileSystem fs = FileSystems.getDefault(); - ignoreMatchers = Arrays.stream(ignores) - .map(fs::getPathMatcher) - .collect(Collectors.toList()); - } else { - ignoreMatchers = new ArrayList<>(); - } + final Path configPath = this.config.getPath(); + final MagikChecksConfiguration checksConfig = configPath != null + ? new MagikChecksConfiguration(configPath) + : new MagikChecksConfiguration(); - final Integer untabify; - if (this.config.hasProperty("untabify")) { - untabify = Integer.parseInt(this.config.getPropertyString("untabify")); - if (untabify < 1) { - throw new NumberFormatException("Must be a positive integer."); - } - } else { - untabify = null; - } - - final long maxInfractions; - if (this.config.hasProperty("max-infractions")) { - maxInfractions = Long.parseLong(this.config.getPropertyString("max-infractions")); - } else { - maxInfractions = Long.MAX_VALUE; - } + // Gather ignore matchers. + final FileSystem fs = FileSystems.getDefault(); + final List ignoreMatchers = checksConfig.getIgnores().stream() + .map(fs::getPathMatcher) + .collect(Collectors.toList()); + final Integer untabify = this.config.getUntabify(); + final long maxInfractions = this.config.getMaxInfractions(); // Sorting. final Location.LocationRangeComparator locationCompare = new Location.LocationRangeComparator(); - final Iterable holders = MagikLint.getAllChecks(this.config); + final Iterable holders = checksConfig.getAllChecks(); paths.stream() .parallel() .filter(path -> { @@ -274,12 +251,17 @@ public void run(final Collection paths) throws IOException, ReflectiveOper } /** - * Run the linter on {@code magikFile}. + * Run the linter on {@link MagikFile}. * @param magikFile File to run on. * @throws ReflectiveOperationException - + * @throws IOException - */ - public void run(final MagikFile magikFile) throws ReflectiveOperationException { - final Iterable holders = MagikLint.getAllChecks(this.config); + public void run(final MagikFile magikFile) throws ReflectiveOperationException, IOException { + final URI uri = magikFile.getUri(); + final Path magikFilePath = Path.of(uri); + final Path configPath = ConfigurationLocator.locateConfiguration(magikFilePath); + final MagikChecksConfiguration checksConfig = new MagikChecksConfiguration(configPath); + final Iterable holders = checksConfig.getAllChecks(); final Comparator byLine = Comparator.comparing(MagikIssue::startLine); final Comparator byColumn = Comparator.comparing(MagikIssue::startColumn); this.runChecksOnFile(magikFile, holders).stream() @@ -287,130 +269,4 @@ public void run(final MagikFile magikFile) throws ReflectiveOperationException { .forEach(this.reporter::reportIssue); } - /** - * Get all checks, enabled in the given configuration. - * - * @param config Configuration to use - * @return Collection of {@link MagikCheckHolder}s. - */ - @SuppressWarnings("unchecked") - public static List getAllChecks(final Configuration config) { - final List holders = new ArrayList<>(); - - final String disabledProperty = config.getPropertyString("disabled"); - final String disabled = disabledProperty != null - ? disabledProperty - : ""; - final List disableds = Arrays.stream(disabled.split(",")) - .map(String::trim) - .collect(Collectors.toList()); - final String enabledProperty = config.getPropertyString("enabled"); - final String enabled = enabledProperty != null - ? enabledProperty - : ""; - final List enableds = Arrays.stream(enabled.split(",")) - .map(String::trim) - .collect(Collectors.toList()); - - for (final Class checkClass : CheckList.getChecks()) { - final Rule annotation = checkClass.getAnnotation(Rule.class); - final String checkKey = annotation.key(); - final String checkKeyKebabCase = MagikCheckHolder.toKebabCase(checkKey); - final boolean checkEnabled = - enableds.contains(checkKeyKebabCase) - || !disableds.contains(checkKeyKebabCase) && !disableds.contains("all"); - - // Gather parameters from MagikCheck, value from config. - final String name = MagikCheckHolder.toKebabCase(checkClass.getAnnotation(Rule.class).key()); - final Set parameters = Arrays.stream(checkClass.getFields()) - .map(field -> field.getAnnotation(RuleProperty.class)) - .filter(Objects::nonNull) - .filter(ruleProperty -> config.hasProperty(name + "." + ruleProperty.key().replace(" ", "-"))) - .map(ruleProperty -> { - final String key = ruleProperty.key().replace(" ", "-"); - final String configKey = name + "." + key; - - // Store parameter. - final String description = ruleProperty.description(); - final MagikCheckHolder.Parameter parameter; - if (ruleProperty.type().equals("INTEGER")) { - final Integer configValue = config.getPropertyInt(configKey); - parameter = new MagikCheckHolder.Parameter(key, description, configValue); - } else if (ruleProperty.type().equals("STRING")) { - final String configValue = config.getPropertyString(configKey); - parameter = new MagikCheckHolder.Parameter(key, description, configValue); - } else if (ruleProperty.type().equals("BOOLEAN")) { - final Boolean configValue = config.getPropertyBoolean(configKey); - parameter = new MagikCheckHolder.Parameter(key, description, configValue); - } else { - throw new IllegalStateException("Unknown type for property: " + ruleProperty.type()); - } - return parameter; - }) - .collect(Collectors.toSet()); - - final MagikCheckHolder holder = - new MagikCheckHolder((Class) checkClass, parameters, checkEnabled); - holders.add(holder); - } - return holders; - } - - /** - * Get scope instructions at line. - * @param magikFile Magik file. - * @param instructionReader Instruction reader to use. - * @param line Line of scope. - * @return Instructions in scope and ancestor scopes. - */ - public static Map getScopeInstructions( - final MagikFile magikFile, - final CommentInstructionReader instructionReader, - final int line) { - final Map instructions = new HashMap<>(); - final GlobalScope globalScope = magikFile.getGlobalScope(); - if (globalScope == null) { - return instructions; - } - - // Ensure we can find a Scope. - final String[] sourceLines = magikFile.getSourceLines(); - final int column = sourceLines[line - 1].length() - 1; - final Scope fromScope = globalScope.getScopeForLineColumn(line, column); - if (fromScope == null) { - return instructions; - } - - // Iterate over all ancestor scopes, see if the check is disabled in any scope. - final List scopes = fromScope.getSelfAndAncestorScopes(); - // Reverse such that if a narrower scope overrides a broader scope instruction. - Collections.reverse(scopes); - for (final Scope scope : scopes) { - final Set scopeInstructions = - instructionReader.getScopeInstructions(scope, MLINT_SCOPE_INSTRUCTION); - final Map parsedScopeInstructions = scopeInstructions.stream() - .map(CommentInstructionReader::parseInstructions) - .reduce( - instructions, - (acc, elem) -> { - acc.putAll(elem); - return acc; - }); - instructions.putAll(parsedScopeInstructions); - } - - return instructions; - } - - private static Map getLineInstructions( - final CommentInstructionReader instructionReader, - final int line) { - final String str = instructionReader.getInstructionsAtLine(line, MLINT_INSTRUCTION); - if (str == null) { - return Collections.emptyMap(); - } - - return CommentInstructionReader.parseInstructions(str); - } - } diff --git a/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/MagikLintConfiguration.java b/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/MagikLintConfiguration.java new file mode 100644 index 00000000..5f1f05dc --- /dev/null +++ b/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/MagikLintConfiguration.java @@ -0,0 +1,106 @@ +package nl.ramsolutions.sw.magik.lint; + +import java.io.IOException; +import java.nio.file.Path; +import javax.annotation.CheckForNull; +import nl.ramsolutions.sw.MagikToolsProperties; + +/** + * {@link MagikLint} specific configuration. + */ +public class MagikLintConfiguration { + + private static final String KEY_UNTABIFY = "untabify"; + private static final String KEY_MAX_INFRACTIONS = "max-infractions"; + private static final String KEY_COLUMN_OFFSET = "column-offset"; + private static final String KEY_MSG_TEMPLATE = "msg-template"; + + private final Path path; + private final MagikToolsProperties properties; + + /** + * Constructor. + * @throws IOException + */ + public MagikLintConfiguration() throws IOException { + this.path = null; + this.properties = new MagikToolsProperties(); + } + + /** + * Constructor which reads properties from {@code path}. + * @param path {@link Path} to read properties from. + * @throws IOException + */ + public MagikLintConfiguration(final Path path) throws IOException { + this.path = path; + this.properties = new MagikToolsProperties(path); + } + + @CheckForNull + public Path getPath() { + return this.path; + } + + /** + * Get untabify. + * @return Untabify. + */ + @CheckForNull + public Integer getUntabify() { + final String untabifyStr = this.properties.getPropertyString(KEY_UNTABIFY); + if (untabifyStr == null) { + return null; + } + + return Integer.parseInt(untabifyStr); + } + + public void setUntabify(final Integer untabify) { + this.properties.setProperty(KEY_UNTABIFY, untabify); + } + + /** + * Get max infractions. + * @return Max infractions. + */ + public long getMaxInfractions() { + final Long maxInfractions = this.properties.getPropertyLong(KEY_MAX_INFRACTIONS); + if (maxInfractions == null) { + return Long.MAX_VALUE; + } + + return maxInfractions; + } + + public void setMaxInfractions(final Long maxInfractions) { + this.properties.setProperty(KEY_UNTABIFY, maxInfractions); + } + + /** + * Get column offset. + * @return Column offset. + */ + public Long getColumnOffset() { + final Long columnOffset = this.properties.getPropertyLong(KEY_COLUMN_OFFSET); + if (columnOffset == null) { + return 0L; + } + + return columnOffset; + } + + public void setColumnOffset(final Long columnOffset) { + this.properties.setProperty(KEY_COLUMN_OFFSET, columnOffset); + } + + @CheckForNull + public String getReporterFormat() { + return this.properties.getPropertyString(KEY_MSG_TEMPLATE); + } + + public void setReporterFormat(final String reporterFormat) { + this.properties.setProperty(KEY_MSG_TEMPLATE, reporterFormat); + } + +} diff --git a/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/Main.java b/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/Main.java index 47f1d31d..dacc1259 100644 --- a/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/Main.java +++ b/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/Main.java @@ -64,16 +64,6 @@ public final class Main { .hasArg() .type(PatternOptionBuilder.NUMBER_VALUE) .build(); - private static final Option OPTION_DISABLED = Option.builder() - .longOpt("disabled") - .desc("Disable checks, use 'all' to disable all checks") - .hasArg() - .build(); - private static final Option OPTION_ENABLED = Option.builder() - .longOpt("enabled") - .desc("Enable checks") - .hasArg() - .build(); private static final Option OPTION_DEBUG = Option.builder() .longOpt("debug") .desc("Enable showing of debug information") @@ -96,8 +86,6 @@ public final class Main { OPTIONS.addOption(OPTION_UNTABIFY); OPTIONS.addOption(OPTION_COLUMN_OFFSET); OPTIONS.addOption(OPTION_MAX_INFRACTIONS); - OPTIONS.addOption(OPTION_DISABLED); - OPTIONS.addOption(OPTION_ENABLED); OPTIONS.addOption(OPTION_DEBUG); OPTIONS.addOption(OPTION_VERSION); } @@ -144,18 +132,13 @@ private static void initDebugLogger() throws IOException { * @param configuration Configuration. * @return Reporter. */ - private static Reporter createReporter(final Configuration configuration) { - final String msgTemplateOptName = OPTION_MSG_TEMPLATE.getLongOpt(); - final String template = configuration.getPropertyString(msgTemplateOptName); - final String format = configuration.hasProperty(msgTemplateOptName) && template != null - ? template + private static Reporter createReporter(final MagikLintConfiguration configuration) { + final String configReporterFormat = configuration.getReporterFormat(); + final String format = configReporterFormat != null + ? configReporterFormat : MessageFormatReporter.DEFAULT_FORMAT; - final String columnOffsetOptName = OPTION_COLUMN_OFFSET.getLongOpt(); - final String columnOffsetStr = configuration.getPropertyString(columnOffsetOptName); - final Long columnOffset = configuration.hasProperty(columnOffsetOptName) - ? Long.parseLong(columnOffsetStr) - : null; + final Long columnOffset = configuration.getColumnOffset(); final PrintStream outStream = Main.getOutStream(); return new MessageFormatReporter(outStream, format, columnOffset); @@ -193,7 +176,7 @@ public static void main(final String[] args) throws ParseException, IOException, } // Read configuration. - final Configuration config; + final MagikLintConfiguration config; if (commandLine.hasOption(OPTION_RCFILE)) { final File rcfile = (File) commandLine.getParsedOptionValue(OPTION_RCFILE); final Path path = rcfile.toPath(); @@ -203,22 +186,17 @@ public static void main(final String[] args) throws ParseException, IOException, System.exit(1); } - config = new Configuration(path); + config = new MagikLintConfiguration(path); } else { final Path currentWorkingPath = Path.of(""); final Path path = ConfigurationLocator.locateConfiguration(currentWorkingPath); config = path != null - ? new Configuration(path) - : new Configuration(); + ? new MagikLintConfiguration(path) + : new MagikLintConfiguration(); } - // Fill configuration from command line. - Main.copyOptionToConfig(commandLine, config, OPTION_UNTABIFY); - Main.copyOptionToConfig(commandLine, config, OPTION_MAX_INFRACTIONS); - Main.copyOptionToConfig(commandLine, config, OPTION_COLUMN_OFFSET); - Main.copyOptionToConfig(commandLine, config, OPTION_MSG_TEMPLATE); - Main.copyOptionToConfig(commandLine, config, OPTION_DISABLED); - Main.copyOptionToConfig(commandLine, config, OPTION_ENABLED); + // Copy configuration from command line. + Main.copyOptionsToConfig(commandLine, config); // Show checks. if (commandLine.hasOption(OPTION_SHOW_CHECKS)) { @@ -254,14 +232,28 @@ public static void main(final String[] args) throws ParseException, IOException, System.exit(exitCode); } - private static void copyOptionToConfig( - final CommandLine commandLine, - final Configuration config, - final Option option) { - final String key = option.getLongOpt(); - if (commandLine.hasOption(key)) { - final String value = commandLine.getOptionValue(key); - config.setProperty(key, value); + private static void copyOptionsToConfig(final CommandLine commandLine, final MagikLintConfiguration config) { + if (commandLine.hasOption(OPTION_UNTABIFY)) { + final String value = commandLine.getOptionValue(OPTION_UNTABIFY); + final Integer untabify = Integer.parseInt(value); + config.setUntabify(untabify); + } + + if (commandLine.hasOption(OPTION_MAX_INFRACTIONS)) { + final String value = commandLine.getOptionValue(OPTION_MAX_INFRACTIONS); + final Long maxInfractions = Long.parseLong(value); + config.setMaxInfractions(maxInfractions); + } + + if (commandLine.hasOption(OPTION_COLUMN_OFFSET)) { + final String value = commandLine.getOptionValue(OPTION_COLUMN_OFFSET); + final Long maxInfractions = Long.parseLong(value); + config.setColumnOffset(maxInfractions); + } + + if (commandLine.hasOption(OPTION_MSG_TEMPLATE)) { + final String value = commandLine.getOptionValue(OPTION_MSG_TEMPLATE); + config.setReporterFormat(value); } } diff --git a/magik-squid/src/main/java/nl/ramsolutions/sw/MagikToolsProperties.java b/magik-squid/src/main/java/nl/ramsolutions/sw/MagikToolsProperties.java new file mode 100644 index 00000000..177befb9 --- /dev/null +++ b/magik-squid/src/main/java/nl/ramsolutions/sw/MagikToolsProperties.java @@ -0,0 +1,160 @@ +package nl.ramsolutions.sw; + +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Properties; +import java.util.stream.Collectors; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Magik-tools properties. + */ +public class MagikToolsProperties { + + private static final Logger LOGGER = LoggerFactory.getLogger(MagikToolsProperties.class); + private static final String DEFAULT_PROPERTIES_FILENAME = "magik-tools-defaults.properties"; + private static final String LIST_SEPARATOR = ","; + + private final Properties properties = new Properties(); + + public MagikToolsProperties() throws IOException { + this(MagikToolsProperties.class.getClassLoader().getResourceAsStream(DEFAULT_PROPERTIES_FILENAME)); + LOGGER.debug("Read default configuration from: {}", DEFAULT_PROPERTIES_FILENAME); + } + + public MagikToolsProperties(final Path path) throws FileNotFoundException, IOException { + this(new FileInputStream(path.toFile())); + LOGGER.debug("Read configuration from: {}", path.toAbsolutePath()); + } + + private MagikToolsProperties(final InputStream stream) throws IOException { + this.properties.load(stream); + } + + /** + * Set property. + * @param key Key of property. + * @param value Value of property. + */ + public void setProperty(final String key, @Nullable final String value) { + if (value == null) { + this.properties.remove(key); + } + + this.properties.setProperty(key, value); + } + + /** + * Set property. + * @param key Key of property. + * @param value Value of property. + */ + public void setProperty(final String key, @Nullable final Integer value) { + if (value == null) { + this.properties.remove(key); + } else { + final String valueStr = Integer.toString(value); + this.properties.setProperty(key, valueStr); + } + } + + /** + * Set property. + * @param key Key of property. + * @param value Value of property. + */ + public void setProperty(final String key, @Nullable final Long value) { + if (value == null) { + this.properties.remove(key); + } else { + final String valueStr = Long.toString(value); + this.properties.setProperty(key, valueStr); + } + } + + /** + * Get property. + * @param key Key of property. + * @return Value of property. + */ + @CheckForNull + public String getPropertyString(final String key) { + return this.properties.getProperty(key); + } + + /** + * Get property, converted to a {@link Boolean}. + * @param key Key of property. + * @return Value of property. + */ + @CheckForNull + public Boolean getPropertyBoolean(final String key) { + final String value = this.getPropertyString(key); + if (value == null) { + return null; + } + + return Boolean.valueOf(value); + } + + /** + * Get property, converted to an {@link Integer}. + * @param key Key of property. + * @return Value of property. + */ + @CheckForNull + public Integer getPropertyInteger(final String key) { + final String value = this.getPropertyString(key); + if (value == null) { + return null; + } + + return Integer.valueOf(value); + } + + /** + * Get property, converted to a {@link Long}. + * @param key Key of property. + * @return Value of property. + */ + @CheckForNull + public Long getPropertyLong(final String key) { + final String value = this.getPropertyString(key); + if (value == null) { + return null; + } + + return Long.valueOf(value); + } + + public boolean hasProperty(final String key) { + return this.getPropertyString(key) != null; + } + + /** + * Get a property value as a list. Items are separated by {@link MagikToolsProperties.LIST_SEPARATOR}. + * @param key Key of the property. + * @return List of values. + */ + public List getPropertyList(final String key) { + final String value = this.getPropertyString(key); + if (value == null) { + return Collections.emptyList(); + } + + final String[] values = value.split(MagikToolsProperties.LIST_SEPARATOR); + return Arrays.stream(values) + .map(String::trim) + .collect(Collectors.toList()); + } + +} diff --git a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/MagikFile.java b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/MagikFile.java index adaf9254..e9b3d316 100644 --- a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/MagikFile.java +++ b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/MagikFile.java @@ -7,12 +7,21 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import nl.ramsolutions.sw.FileCharsetDeterminer; import nl.ramsolutions.sw.magik.analysis.definitions.Definition; import nl.ramsolutions.sw.magik.analysis.definitions.DefinitionReader; import nl.ramsolutions.sw.magik.analysis.scope.GlobalScope; +import nl.ramsolutions.sw.magik.analysis.scope.Scope; import nl.ramsolutions.sw.magik.analysis.scope.ScopeBuilderVisitor; +import nl.ramsolutions.sw.magik.parser.CommentInstructionReader; +import nl.ramsolutions.sw.magik.parser.CommentInstructionReader.InstructionType; import nl.ramsolutions.sw.magik.parser.MagikParser; /** @@ -25,6 +34,10 @@ public class MagikFile { private AstNode astNode; private GlobalScope globalScope; private List definitions; + private final Map>> lineInstructions + = new HashMap<>(); + private final Map>> scopeInstructions + = new HashMap<>(); /** * Constructor. @@ -88,10 +101,7 @@ public synchronized AstNode getTopNode() { if (this.astNode == null) { final MagikParser parser = new MagikParser(); final String magikSource = this.getSource(); - this.astNode = parser.parseSafe(magikSource); - - // Update URI. - MagikParser.updateUri(this.astNode, this.uri); + this.astNode = parser.parseSafe(magikSource, this.uri); } return this.astNode; @@ -127,6 +137,74 @@ public synchronized List getDefinitions() { return Collections.unmodifiableList(this.definitions); } + /** + * Get all the line instructions for {@link CommentInstructionReader.InstructionType}. + * @param instructionType Instruction type to get. + * @return Map with all instructions, keyed by line number. + */ + public synchronized Map> getLineInstructions( + final CommentInstructionReader.InstructionType instructionType) { + if (!this.lineInstructions.containsKey(instructionType)) { + final Map> instructions = this.readLineInstructions(instructionType); + this.lineInstructions.put(instructionType, instructions); + } + + return Collections.unmodifiableMap(this.lineInstructions.get(instructionType)); + } + + private Map> readLineInstructions(final InstructionType instructionType) { + final CommentInstructionReader instructionReader = new CommentInstructionReader(this, Set.of(instructionType)); + + final String[] sourceLines = this.getSourceLines(); + return IntStream.range(0, sourceLines.length) + .mapToObj(line -> { + final String instrAtLine = instructionReader.getInstructionsAtLine(line, instructionType); + final String lineInstrsStr = Objects.requireNonNullElse(instrAtLine, ""); + final Map lineInstrs = CommentInstructionReader.parseInstructions(lineInstrsStr); + return Map.entry(line, lineInstrs); + }) + .collect(Collectors.toMap( + Map.Entry::getKey, + Map.Entry::getValue)); + } + + /** + * Get all the Scope instructions for {@link CommentInstructionReader.InstructionType}. + * @param instructionType Instruction type to get. + * @return Map with all instructions, keyed by {@link Scope}. + */ + public synchronized Map> getScopeInstructions( + final CommentInstructionReader.InstructionType instructionType) { + if (!this.scopeInstructions.containsKey(instructionType)) { + final Map> instructions = this.readScopeInstructions(instructionType); + this.scopeInstructions.put(instructionType, instructions); + } + + return Collections.unmodifiableMap(this.scopeInstructions.get(instructionType)); + } + + private Map> readScopeInstructions(final InstructionType instructionType) { + final Map> instructions = new HashMap<>(); + final GlobalScope glblScope = this.getGlobalScope(); + Objects.requireNonNull(glblScope); + + final CommentInstructionReader instructionReader = new CommentInstructionReader(this, Set.of(instructionType)); + glblScope.getSelfAndDescendantScopes().stream() + .forEach(scope -> { + final Map scopeInstrs = + instructionReader.getScopeInstructions(scope, instructionType).stream() + .map(instr -> Objects.requireNonNullElse(instr, "")) + .map(CommentInstructionReader::parseInstructions) + .flatMap(instrs -> instrs.entrySet().stream()) + .collect(Collectors.toMap( + Map.Entry::getKey, + Map.Entry::getValue)); + instructions.put(scope, scopeInstrs); + }); + + return instructions; + } + @Override public String toString() { return String.format( diff --git a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/scope/Scope.java b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/scope/Scope.java index 7a8654d9..2e3694d9 100644 --- a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/scope/Scope.java +++ b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/scope/Scope.java @@ -4,6 +4,7 @@ import com.sonar.sslr.api.Token; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -47,13 +48,21 @@ protected Scope() { } /** - * Add a child scope to self. + * Add a child {@link Scope} to self. * @param childScope Child scope to add. */ void addChildScope(final Scope childScope) { this.childScopes.add(childScope); } + /** + * Get child {@link Scope}s. + * @return Children. + */ + public List getChildScopes() { + return Collections.unmodifiableList(this.childScopes); + } + /** * Get {@link AstNode} where scope begins. * @return {@link AstNode} where scope begins diff --git a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/typing/LocalTypeReasoner.java b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/typing/LocalTypeReasoner.java index c31e77ed..94898bd5 100644 --- a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/typing/LocalTypeReasoner.java +++ b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/typing/LocalTypeReasoner.java @@ -105,9 +105,9 @@ public class LocalTypeReasoner extends AstWalker { private static final long BIGNUM_START = 1 << 29; private static final String DEFAULT_PACKAGE = "user"; private static final CommentInstructionReader.InstructionType TYPE_INSTRUCTION = - CommentInstructionReader.InstructionType.createInstructionType("type"); + CommentInstructionReader.InstructionType.createStatementInstructionType("type"); private static final CommentInstructionReader.InstructionType ITER_TYPE_INSTRUCTION = - CommentInstructionReader.InstructionType.createInstructionType("iter-type"); + CommentInstructionReader.InstructionType.createStatementInstructionType("iter-type"); private final AstNode topNode; private final ITypeKeeper typeKeeper; diff --git a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/typing/indexer/MagikIndexer.java b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/typing/indexer/MagikIndexer.java index 6d022296..e3ea207b 100644 --- a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/typing/indexer/MagikIndexer.java +++ b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/typing/indexer/MagikIndexer.java @@ -89,7 +89,7 @@ public class MagikIndexer { MethodDefinition.Modifier.PRIVATE, Method.Modifier.PRIVATE); private static final CommentInstructionReader.InstructionType TYPE_INSTRUCTION = - CommentInstructionReader.InstructionType.createInstructionType("type"); + CommentInstructionReader.InstructionType.createStatementInstructionType("type"); private final ITypeKeeper typeKeeper; private final Map> indexedPackages = new HashMap<>(); diff --git a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/parser/CommentInstructionReader.java b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/parser/CommentInstructionReader.java index 96358cb4..09fdad58 100644 --- a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/parser/CommentInstructionReader.java +++ b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/parser/CommentInstructionReader.java @@ -32,8 +32,8 @@ private InstructionType(final String type, final boolean isScopeInstruction) { this.type = type; this.isScopeInstruction = isScopeInstruction; this.pattern = isScopeInstruction - ? Pattern.compile("^\\s*#[ \t]*" + Pattern.quote(type) + ": ?(.*)") - : Pattern.compile(".*#[ \t]*" + Pattern.quote(type) + ": ?(.*)"); + ? Pattern.compile("^\\s*#\\s*" + Pattern.quote(type) + ":\\s*(.*)$") + : Pattern.compile("^\\s*[^\\s].*#\\s*" + Pattern.quote(type) + ":\\s*(.*)$"); } public String getType() { @@ -53,7 +53,7 @@ public Pattern getPattern() { * @param type Name of type. * @return New `InstructionType`. */ - public static InstructionType createInstructionType(final String type) { + public static InstructionType createStatementInstructionType(final String type) { return new InstructionType(type, false); } @@ -67,6 +67,30 @@ public static InstructionType createScopeInstructionType(final String type) { return new InstructionType(type, true); } + @Override + public int hashCode() { + return Objects.hash(this.type, this.isScopeInstruction, this.pattern); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + + if (obj == null) { + return false; + } + + if (this.getClass() != obj.getClass()) { + return false; + } + + final InstructionType other = (InstructionType) obj; + return Objects.equals(this.type, other.type) + && Objects.equals(this.isScopeInstruction, other.isScopeInstruction) + && Objects.equals(this.pattern, other.pattern); + } } private final MagikFile magikFile; @@ -119,7 +143,7 @@ public String getInstructionsAtLine(final int lineNo, final InstructionType inst } /** - * Get instructions for this scope. + * Get instructions for this scope. This scope only and not any of its child scopes. * @param scope Scope to get instructions from. * @param instructionType Instruction type. * @return Instructions in scope. @@ -132,6 +156,11 @@ public Set getScopeInstructions(final Scope scope, final InstructionType final int fromLine = scope.getStartLine(); final int toLine = scope.getEndLine(); return IntStream.range(fromLine, toLine) + .filter(line -> + // Filter any lines where a child scope lives. + scope.getChildScopes().stream() + .anyMatch(childScope -> + childScope.getStartLine() >= line && line < childScope.getEndLine())) .mapToObj(line -> this.getInstructionsAtLine(line, instructionType)) .filter(Objects::nonNull) .collect(Collectors.toSet()); diff --git a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/parser/MagikParser.java b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/parser/MagikParser.java index f08d5588..dced87c4 100644 --- a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/parser/MagikParser.java +++ b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/parser/MagikParser.java @@ -32,6 +32,7 @@ public class MagikParser { private static final Logger LOGGER = LoggerFactory.getLogger(MagikParser.class); + private static final URI DEFAULT_URI = URI.create("file:///"); private static final Map RULE_MAPPING = new EnumMap<>(MagikGrammar.class); @@ -69,18 +70,29 @@ public MagikParser() { } /** - * Parse a string and return the AstNode. - * IOExceptions are caught, not handled. - * - * @param source Source to parse - * @return Tree + * Parse safe and set {@link MagikParser.DEFAULT_URI}. + * @param source Source to parse. + * @return Parsed source. */ public AstNode parseSafe(final String source) { + return this.parseSafe(source, MagikParser.DEFAULT_URI); + } + + /** + * Parse safe and set {@link URI}. + * @param source Source to parse. + * @param uri URI to set. + * @return Parsed source. + */ + public AstNode parseSafe(final String source, final URI uri) { try { - return this.parse(source); + final AstNode node = this.parse(source); + this.updateUri(node, uri); + return node; } catch (final IOException exception) { LOGGER.error(exception.getMessage(), exception); } + return null; } @@ -172,7 +184,7 @@ private void applyRuleMapping(final AstNode node) { * @param node Node to start at. * @param newUri New URI to set. */ - public static void updateUri(final AstNode node, final URI newUri) { + private void updateUri(final AstNode node, final URI newUri) { final Token token = node.getToken(); if (token != null) { try { @@ -184,7 +196,7 @@ public static void updateUri(final AstNode node, final URI newUri) { } } - node.getChildren().forEach(childNode -> MagikParser.updateUri(childNode, newUri)); + node.getChildren().forEach(childNode -> this.updateUri(childNode, newUri)); } /** diff --git a/magik-squid/src/main/resources/magik-tools-defaults.properties b/magik-squid/src/main/resources/magik-tools-defaults.properties new file mode 100644 index 00000000..d59f4911 --- /dev/null +++ b/magik-squid/src/main/resources/magik-tools-defaults.properties @@ -0,0 +1 @@ +; empty file diff --git a/magik-squid/src/test/java/nl/ramsolutions/sw/magik/parser/CommentInstructionReaderReaderTest.java b/magik-squid/src/test/java/nl/ramsolutions/sw/magik/parser/CommentInstructionReaderReaderTest.java index 7433e525..818f118d 100644 --- a/magik-squid/src/test/java/nl/ramsolutions/sw/magik/parser/CommentInstructionReaderReaderTest.java +++ b/magik-squid/src/test/java/nl/ramsolutions/sw/magik/parser/CommentInstructionReaderReaderTest.java @@ -13,13 +13,13 @@ */ class CommentInstructionReaderReaderTest { - private static final CommentInstructionReader.InstructionType STATEMENT_INSTRUCTION_TYPE = - CommentInstructionReader.InstructionType.createInstructionType("mlint"); - private static final CommentInstructionReader.InstructionType SCOPE_INSTRUCTION_TYPE = + private static final CommentInstructionReader.InstructionType MLINT_LINE_INSTRUCTION = + CommentInstructionReader.InstructionType.createStatementInstructionType("mlint"); + private static final CommentInstructionReader.InstructionType MLINT_SCOPE_INSTRUCTION = CommentInstructionReader.InstructionType.createScopeInstructionType("mlint"); @Test - void testReadInstruction() { + void testReadStatementInstruction() { final String code = "" + "_proc()\n" + " print(10) # mlint: disable=forbidden-call\n" @@ -27,16 +27,16 @@ void testReadInstruction() { final MagikFile magikFile = new MagikFile("tests://unittest", code); final CommentInstructionReader instructionReader = - new CommentInstructionReader(magikFile, Set.of(STATEMENT_INSTRUCTION_TYPE)); + new CommentInstructionReader(magikFile, Set.of(MLINT_LINE_INSTRUCTION)); final String instructionAtLine = - instructionReader.getInstructionsAtLine(2, STATEMENT_INSTRUCTION_TYPE); + instructionReader.getInstructionsAtLine(2, MLINT_LINE_INSTRUCTION); assertThat(instructionAtLine).isEqualTo("disable=forbidden-call"); // Cannot read instruction via scope. final GlobalScope globalScope = magikFile.getGlobalScope(); - final Scope scope = globalScope.getScopeForLineColumn(1, 99); - final Set scopeInstructions = instructionReader.getScopeInstructions(scope, SCOPE_INSTRUCTION_TYPE); + final Scope scope = globalScope.getChildScopes().get(0); + final Set scopeInstructions = instructionReader.getScopeInstructions(scope, MLINT_SCOPE_INSTRUCTION); assertThat(scopeInstructions).isEmpty(); } @@ -45,26 +45,49 @@ void testReadScopeInstruction() { final String code = "" + "_proc()\n" + " # mlint: disable=no-self-use\n" - + " print(10) # mlint: disable=forbidden-call\n" + + " print(10, 20) # mlint: disable=forbidden-call\n" + + " _block\n" + + " show(:a, :b, :c)\n" + + " _endblock\n" + "_endproc"; final MagikFile magikFile = new MagikFile("tests://unittest", code); final CommentInstructionReader instructionReader = - new CommentInstructionReader(magikFile, Set.of(SCOPE_INSTRUCTION_TYPE)); - - // Can read a scope/single line instruction at the specific line. - final String instructionAtLine2 = instructionReader.getInstructionsAtLine(2, SCOPE_INSTRUCTION_TYPE); - assertThat(instructionAtLine2).isEqualTo("disable=no-self-use"); + new CommentInstructionReader(magikFile, Set.of(MLINT_LINE_INSTRUCTION, MLINT_SCOPE_INSTRUCTION)); - // Can read instruction via scope. + // Read no instruction in global scope. final GlobalScope globalScope = magikFile.getGlobalScope(); - final Scope scope = globalScope.getScopeForLineColumn(1, 99); - final Set scopeInstructions = instructionReader.getScopeInstructions(scope, SCOPE_INSTRUCTION_TYPE); - assertThat(scopeInstructions).containsOnly("disable=no-self-use"); + final Set globalScopeInstructions = + instructionReader.getScopeInstructions(globalScope, MLINT_SCOPE_INSTRUCTION); + assertThat(globalScopeInstructions).isEmpty(); + + // Read the scope instruction in proc scope. + final Scope procScope = globalScope.getChildScopes().get(0); + final Set procScopeInstructions = + instructionReader.getScopeInstructions(procScope, MLINT_SCOPE_INSTRUCTION); + assertThat(procScopeInstructions).containsOnly("disable=no-self-use"); + + // Read the scope instruction in block scope. + final Scope blockScope = procScope.getChildScopes().get(0); + final Set blockScopeInstructions = + instructionReader.getScopeInstructions(blockScope, MLINT_SCOPE_INSTRUCTION); + assertThat(blockScopeInstructions).isEmpty(); + + // No line instruction. + final String instructionAtLine1 = instructionReader.getInstructionsAtLine(1, MLINT_LINE_INSTRUCTION); + assertThat(instructionAtLine1).isNull(); + + // No line instruction; Line instruction does not match the scope instruction. + final String instructionAtLine2 = instructionReader.getInstructionsAtLine(2, MLINT_LINE_INSTRUCTION); + assertThat(instructionAtLine2).isNull(); + + // Read line instruction. + final String instructionAtLine3 = instructionReader.getInstructionsAtLine(3, MLINT_LINE_INSTRUCTION); + assertThat(instructionAtLine3).isEqualTo("disable=forbidden-call"); - // Does not match the statement instruction. - final String instructionAtLine3 = instructionReader.getInstructionsAtLine(3, STATEMENT_INSTRUCTION_TYPE); - assertThat(instructionAtLine3).isNull(); + // No line instruction. + final String instructionAtLine4 = instructionReader.getInstructionsAtLine(4, MLINT_LINE_INSTRUCTION); + assertThat(instructionAtLine4).isNull(); } } diff --git a/magik-typed-checks/src/main/java/nl/ramsolutions/sw/magik/typedchecks/fixers/TypeDocReturnTypeFixer.java b/magik-typed-checks/src/main/java/nl/ramsolutions/sw/magik/typedchecks/fixers/TypeDocReturnTypeFixer.java index 6a0c9df5..eb3216c4 100644 --- a/magik-typed-checks/src/main/java/nl/ramsolutions/sw/magik/typedchecks/fixers/TypeDocReturnTypeFixer.java +++ b/magik-typed-checks/src/main/java/nl/ramsolutions/sw/magik/typedchecks/fixers/TypeDocReturnTypeFixer.java @@ -79,6 +79,10 @@ private List extractReturnTypeCodeActions( return this.createRemoveReturnCodeAction(typeValueNode); } + if (typeDocEntry == null) { // Keep checker happy. + return null; + } + final TypeString typeDocTypeString = typeDocEntry.getValue(); if (methodTypeString != null // && typeDocTypeString != null && !methodTypeString.containsUndefined() diff --git a/pom.xml b/pom.xml index 8a864798..a2708647 100644 --- a/pom.xml +++ b/pom.xml @@ -400,6 +400,7 @@ nl.ramsolutions.sw.magik.languageserver.symbol.SymbolProvider.getSymbols(java.lang.String) nl.ramsolutions.sw.magik.lint.ConfigurationLocator.locateConfiguration() nl.ramsolutions.sw.magik.parser.MagikWhitespaceTriviaAdder.createWhitespaceTokens(com.sonar.sslr.api.Token, com.sonar.sslr.api.Token) + nl.ramsolutions.sw.magik.typedchecks.fixers.TypeDocReturnTypeFixer.lambda$extractReturnTypeCodeActions$2(nl.ramsolutions.sw.magik.analysis.definitions.MethodDefinition, java.util.Map.Entry)