Skip to content

Commit

Permalink
\dsldevkit#103: CHECKCFG allows duplicate languages
Browse files Browse the repository at this point in the history
 - Add validation against duplicate language configurations in a CHECKCFG
   file
 - Add quickfix to remove duplicates by merging their contents
  • Loading branch information
Robbie Henderson authored and Robbie Henderson committed May 23, 2018
1 parent 626cdb7 commit 42ac756
Show file tree
Hide file tree
Showing 9 changed files with 156 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.avaloq.tools.ddk.checkcfg.scoping.CheckCfgScopeProviderTest;
import com.avaloq.tools.ddk.checkcfg.syntax.CheckCfgSyntaxTest;
import com.avaloq.tools.ddk.checkcfg.validation.CheckCfgConfiguredParameterValidationsTest;
import com.avaloq.tools.ddk.checkcfg.validation.CheckCfgDuplicateChecksTest;
import com.avaloq.tools.ddk.checkcfg.validation.CheckCfgTest;
import com.avaloq.tools.ddk.checkcfg.validation.CheckCfgValidationTest;

Expand All @@ -32,7 +33,8 @@
CheckCfgScopeProviderTest.class,
CheckCfgSyntaxTest.class,
CheckCfgTest.class,
CheckCfgValidationTest.class
CheckCfgValidationTest.class,
CheckCfgDuplicateChecksTest.class
// @Format-On
})

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*******************************************************************************
* Copyright (c) 2016 Avaloq Evolution AG and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Avaloq Evolution AG - initial API and implementation
*******************************************************************************/

package com.avaloq.tools.ddk.checkcfg.validation

import org.junit.Test
import com.google.inject.Inject
import org.eclipse.xtext.junit4.validation.ValidationTestHelper
import org.eclipse.xtext.junit4.util.ParseHelper
import com.avaloq.tools.ddk.checkcfg.checkcfg.CheckConfiguration
import org.eclipse.xtext.junit4.InjectWith
import org.junit.runner.RunWith
import com.avaloq.tools.ddk.checkcfg.CheckCfgUiInjectorProvider
import org.eclipse.xtext.junit4.XtextRunner
import com.avaloq.tools.ddk.checkcfg.checkcfg.CheckcfgPackage

@InjectWith(CheckCfgUiInjectorProvider)
@RunWith(XtextRunner)
class CheckCfgDuplicateChecksTest{

@Inject
private ValidationTestHelper helper;

@Inject
private ParseHelper<CheckConfiguration> parser;

/**
* Test for validation of duplicate language configurations.
*/
@Test
def testDuplicateLanguageError() {
val model = parser.parse('''
check configuration DuplicateLanguages
for com.avaloq.tools.ddk.^check.TestLanguage {}
for com.avaloq.tools.ddk.^check.TestLanguage {}
}'''
)
helper.assertError(model, CheckcfgPackage::Literals::CONFIGURED_LANGUAGE_VALIDATOR, IssueCodes::DUPLICATE_LANGUAGE_CONFIGURATION)
}

/**
* Test for validation of duplicate catalog configurations.
*/
@Test
def testDuplicateCatalogError() {
val model = parser.parse('''
check configuration DuplicateCatalogs {
catalog com.avaloq.tools.ddk.^check.validation.ExecutionEnvironment {}
catalog com.avaloq.tools.ddk.^check.validation.ExecutionEnvironment {}
}'''
)
helper.assertError(model, CheckcfgPackage::Literals::CONFIGURED_CATALOG, IssueCodes::DUPLICATE_CATALOG_CONFIGURATION)
}

/**
* Test for validation of duplicate check configurations.
*/
@Test
def testDuplicateCheckError() {
val model = parser.parse('''
check configuration DuplicateChecks {
catalog com.avaloq.tools.ddk.^check.validation.ExecutionEnvironment {
default GreetingNameLength
default GreetingNameLength
}
}'''
)
helper.assertError(model, CheckcfgPackage::Literals::CONFIGURED_CHECK, IssueCodes::DUPLICATE_CHECK_CONFIGURATION)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.Multimaps;
import com.google.common.collect.Sets;
import com.google.inject.Inject;


Expand Down Expand Up @@ -209,7 +210,7 @@ public boolean apply(final FormalParameter input) {
info(NLS.bind(Messages.CheckCfgJavaValidator_CONFIGURED_PARAM_EQUALS_DEFAULT, param.getName()), configParam, CheckcfgPackage.Literals.CONFIGURED_PARAMETER__NEW_VALUE, ValidationMessageAcceptor.INSIGNIFICANT_INDEX, IssueCodes.CONFIGURED_PARAM_EQUALS_DEFAULT);
}
} catch (NoSuchElementException e) {
LOGGER.debug("Could not find referenced formal parameter");
LOGGER.debug("Could not find referenced formal parameter"); //$NON-NLS-1$
}
}
}
Expand Down Expand Up @@ -306,10 +307,25 @@ public String apply(final ConfiguredCheck from) {
@Check
public void checkConfiguredLanguageExists(final ConfiguredLanguageValidator validator) {
if (!checkCfgUtil.getAllLanguages().contains(validator.getLanguage())) {
error("Unknown language", validator, CheckcfgPackage.Literals.CONFIGURED_LANGUAGE_VALIDATOR__LANGUAGE, ValidationMessageAcceptor.INSIGNIFICANT_INDEX, IssueCodes.UNKNOWN_LANGUAGE);
error("Unknown language", validator, CheckcfgPackage.Literals.CONFIGURED_LANGUAGE_VALIDATOR__LANGUAGE, ValidationMessageAcceptor.INSIGNIFICANT_INDEX, IssueCodes.UNKNOWN_LANGUAGE); //$NON-NLS-1$
}
}

/**
* Checks that language referenced in validator configuration is unique.
*
* @param checkCfg
* the check configuration containing {@link ConfiguredLanguageValidator}'s
*/
@Check
public void checkConfiguredLanguageUnique(final CheckConfiguration checkCfg) {
Set<String> existingLanguages = Sets.newHashSet();
checkCfg.getLanguageValidatorConfigurations() //
.stream() //
.filter(lang -> !existingLanguages.add(lang.getLanguage())) // Don't do the mapping first - it's easier having the EObject
.forEach(lang -> error(Messages.CheckCfgJavaValidator_DUPLICATE_LANGUAGE_CONFIGURATION, lang, CheckcfgPackage.Literals.CONFIGURED_LANGUAGE_VALIDATOR__LANGUAGE, IssueCodes.DUPLICATE_LANGUAGE_CONFIGURATION));
}

/**
* Checks that a Configured Check has unique Configured Parameters.
*
Expand All @@ -321,18 +337,9 @@ public void checkConfiguredParameterUnique(final ConfiguredCheck configuredCheck
if (configuredCheck.getParameterConfigurations().size() < 2) {
return;
}
Predicate<ConfiguredParameter> predicate = new Predicate<ConfiguredParameter>() {
@Override
public boolean apply(final ConfiguredParameter configuredParameter) {
return configuredParameter.getParameter() != null && !configuredParameter.getParameter().eIsProxy();
}
};
Function<ConfiguredParameter, String> function = new Function<ConfiguredParameter, String>() {
@Override
public String apply(final ConfiguredParameter from) {
return from.getParameter().getName();
}
};
Predicate<ConfiguredParameter> predicate = (final ConfiguredParameter configuredParameter) -> configuredParameter.getParameter() != null
&& !configuredParameter.getParameter().eIsProxy();
Function<ConfiguredParameter, String> function = (final ConfiguredParameter from) -> from.getParameter().getName();
for (final ConfiguredParameter p : getDuplicates(predicate, function, configuredCheck.getParameterConfigurations())) {
error(Messages.CheckCfgJavaValidator_DUPLICATE_PARAMETER_CONFIGURATION, p, CheckcfgPackage.Literals.CONFIGURED_PARAMETER__PARAMETER, ValidationMessageAcceptor.INSIGNIFICANT_INDEX, IssueCodes.DUPLICATE_PARAMETER_CONFIGURATION);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
/**
* Issue codes used for the Check Configuration Java validator.
*/
@SuppressWarnings("nls") // Issue codes only.
public final class IssueCodes {
private static final String ISSUE_CODE_PREFIX = "com.avaloq.tools.ddk.checkcfg.validation.IssueCodes.";

Expand All @@ -24,6 +25,7 @@ public final class IssueCodes {
public static final String CONFIGURED_PARAM_EQUALS_DEFAULT = ISSUE_CODE_PREFIX + "configured_param_equals_default";
public static final String DUPLICATE_CATALOG_CONFIGURATION = ISSUE_CODE_PREFIX + "duplicate_catalog_configuration";
public static final String DUPLICATE_CHECK_CONFIGURATION = ISSUE_CODE_PREFIX + "duplicate_check_configuration";
public static final String DUPLICATE_LANGUAGE_CONFIGURATION = ISSUE_CODE_PREFIX + "duplicate_language_configuration";
public static final String UNKNOWN_LANGUAGE = ISSUE_CODE_PREFIX + "unknown_language";
public static final String DUPLICATE_PARAMETER_CONFIGURATION = ISSUE_CODE_PREFIX + "duplicate_parameter_configuration";
public static final String SEVERITY_NOT_ALLOWED = ISSUE_CODE_PREFIX + "severity_not_allowed";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public class Messages extends NLS {
public static String CheckCfgJavaValidator_CONFIGURED_PARAM_EQUALS_DEFAULT;
public static String CheckCfgJavaValidator_DUPLICATE_CATALOG_CONFIGURATION;
public static String CheckCfgJavaValidator_DUPLICATE_CHECK_CONFIGURATION;
public static String CheckCfgJavaValidator_DUPLICATE_LANGUAGE_CONFIGURATION;
public static String CheckCfgJavaValidator_DUPLICATE_PARAMETER_CONFIGURATION;
public static String CheckCfgJavaValidator_SEVERITY_NOT_ALLOWED;

Expand All @@ -37,4 +38,3 @@ private Messages() {
super();
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ CheckCfgJavaValidator_FINAL_CHECK_NOT_CONFIGURABLE=Final checks may not be confi
CheckCfgJavaValidator_CONFIGURED_PARAM_EQUALS_DEFAULT=Configured value for ''{0}'' equals default
CheckCfgJavaValidator_DUPLICATE_CATALOG_CONFIGURATION=Duplicate catalog configuration
CheckCfgJavaValidator_DUPLICATE_CHECK_CONFIGURATION=Duplicate check configuration
CheckCfgJavaValidator_DUPLICATE_LANGUAGE_CONFIGURATION=Duplicate language configuration
CheckCfgJavaValidator_DUPLICATE_PARAMETER_CONFIGURATION=Duplicate parameter configuration
CheckCfgJavaValidator_SEVERITY_NOT_ALLOWED=Configured severity is not allowed
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
*******************************************************************************/
package com.avaloq.tools.ddk.checkcfg.ui.quickfix;

import java.util.List;
import java.util.stream.Collectors;

import org.eclipse.emf.ecore.EObject;
import org.eclipse.jface.text.BadLocationException;
import org.eclipse.osgi.util.NLS;
Expand All @@ -22,10 +25,12 @@
import org.eclipse.xtext.ui.editor.quickfix.Fix;
import org.eclipse.xtext.ui.editor.quickfix.IssueResolutionAcceptor;
import org.eclipse.xtext.validation.Issue;
import org.eclipse.xtext.xbase.lib.IterableExtensions;

import com.avaloq.tools.ddk.checkcfg.checkcfg.CheckConfiguration;
import com.avaloq.tools.ddk.checkcfg.checkcfg.ConfiguredCatalog;
import com.avaloq.tools.ddk.checkcfg.checkcfg.ConfiguredCheck;
import com.avaloq.tools.ddk.checkcfg.checkcfg.ConfiguredLanguageValidator;
import com.avaloq.tools.ddk.checkcfg.checkcfg.SeverityKind;
import com.avaloq.tools.ddk.checkcfg.validation.IssueCodes;

Expand All @@ -39,7 +44,7 @@ public class CheckCfgQuickfixProvider extends DefaultQuickfixProvider {

/**
* Removes a duplicate catalog configuration.
*
*
* @param issue
* the issue
* @param acceptor
Expand All @@ -48,6 +53,7 @@ public class CheckCfgQuickfixProvider extends DefaultQuickfixProvider {
@Fix(IssueCodes.DUPLICATE_CATALOG_CONFIGURATION)
public void removeDuplicateCatalogConfiguration(final Issue issue, final IssueResolutionAcceptor acceptor) {
acceptor.accept(issue, Messages.CheckCfgQuickfixProvider_REMOVE_DUPLICATE_CATALOG_LABEL, Messages.CheckCfgQuickfixProvider_REMOVE_DUPLICATE_CATALOG_DESCN, null, new ISemanticModification() {
@Override
public void apply(final EObject element, final IModificationContext context) {
CheckConfiguration configuration = EcoreUtil2.getContainerOfType(element, CheckConfiguration.class);
configuration.getLegacyCatalogConfigurations().remove(element);
Expand All @@ -58,7 +64,7 @@ public void apply(final EObject element, final IModificationContext context) {
/**
* Fix severity by setting it to a legal value as is defined by severity range of referenced check. Legal
* severities are passed as issue data (org.eclipse.xtext.validation.Issue#getData()).
*
*
* @param issue
* the issue
* @param acceptor
Expand All @@ -72,6 +78,7 @@ public void fixSeverityToMaxSeverity(final Issue issue, final IssueResolutionAcc
final String descn = NLS.bind(Messages.CheckCfgQuickfixProvider_CORRECT_SEVERITY_DESCN, severityProposal);

acceptor.accept(issue, label, descn, NO_IMAGE, new IModification() {
@Override
public void apply(final IModificationContext context) throws BadLocationException {
IXtextDocument xtextDocument = context.getXtextDocument();
xtextDocument.replace(issue.getOffset(), issue.getLength(), severityProposal);
Expand All @@ -83,7 +90,7 @@ public void apply(final IModificationContext context) throws BadLocationExceptio

/**
* Removes a duplicate check configuration.
*
*
* @param issue
* the issue
* @param acceptor
Expand All @@ -92,6 +99,7 @@ public void apply(final IModificationContext context) throws BadLocationExceptio
@Fix(IssueCodes.DUPLICATE_CHECK_CONFIGURATION)
public void removeDuplicateCheckConfiguration(final Issue issue, final IssueResolutionAcceptor acceptor) {
acceptor.accept(issue, Messages.CheckCfgQuickfixProvider_REMOVE_DUPLICATE_CHECK_LABEL, Messages.CheckCfgQuickfixProvider_REMOVE_DUPLICATE_CHECK_DESCN, null, new ISemanticModification() {
@Override
public void apply(final EObject element, final IModificationContext context) {
ConfiguredCatalog catalog = EcoreUtil2.getContainerOfType(element, ConfiguredCatalog.class);
catalog.getCheckConfigurations().remove(element);
Expand All @@ -101,7 +109,7 @@ public void apply(final EObject element, final IModificationContext context) {

/**
* Removes a duplicate parameter configuration.
*
*
* @param issue
* the issue
* @param acceptor
Expand All @@ -110,16 +118,45 @@ public void apply(final EObject element, final IModificationContext context) {
@Fix(IssueCodes.DUPLICATE_PARAMETER_CONFIGURATION)
public void removeDuplicateParameterConfiguration(final Issue issue, final IssueResolutionAcceptor acceptor) {
acceptor.accept(issue, Messages.CheckCfgQuickfixProvider_REMOVE_DUPLICATE_PARAM_LABEL, Messages.CheckCfgQuickfixProvider_REMOVE_DUPLICATE_PARAM_DESCN, null, new ISemanticModification() {
@Override
public void apply(final EObject element, final IModificationContext context) {
ConfiguredCheck check = EcoreUtil2.getContainerOfType(element, ConfiguredCheck.class);
check.getParameterConfigurations().remove(element);
}
});
}

/**
* Removes a duplicate language configuration.
*
* @param issue
* the issue
* @param acceptor
* the acceptor
*/
@Fix(IssueCodes.DUPLICATE_LANGUAGE_CONFIGURATION)
public void removeDuplicateLanguageConfiguration(final Issue issue, final IssueResolutionAcceptor acceptor) {
acceptor.accept(issue, Messages.CheckCfgQuickfixProvider_REMOVE_DUPLICATE_LANG_CONFIG_LABEL, Messages.CheckCfgQuickfixProvider_REMOVE_DUPLICATE_LANG_CONFIG_DESC, null, (element, context) -> {
final CheckConfiguration check = EcoreUtil2.getContainerOfType(element, CheckConfiguration.class);
final String languageName = ((ConfiguredLanguageValidator) element).getLanguage();
if (check != null && languageName != null) {
final List<ConfiguredLanguageValidator> allMatchingLanguages = check.getLanguageValidatorConfigurations().stream().filter(langName -> languageName.equals(langName.getLanguage())).collect(Collectors.toList());
if (allMatchingLanguages.size() <= 1) {
return; // Something went wrong. Validation guarantees 2.
}
for (ConfiguredLanguageValidator duplicate : IterableExtensions.tail(allMatchingLanguages)) {
// For each matching language after the first, merge contents into first and remove the language
IterableExtensions.head(allMatchingLanguages).getParameterConfigurations().addAll(duplicate.getParameterConfigurations());
IterableExtensions.head(allMatchingLanguages).getCatalogConfigurations().addAll(duplicate.getCatalogConfigurations());
check.getLanguageValidatorConfigurations().remove(duplicate);
}
}
});
}

/**
* Removes the configured values of a disabled check.
*
*
* @param issue
* the issue
* @param acceptor
Expand All @@ -128,6 +165,7 @@ public void apply(final EObject element, final IModificationContext context) {
@Fix(IssueCodes.DISABLED_CHECK_NOT_CONFIGURED)
public void removeConfiguredParamsOfDisabledCheck(final Issue issue, final IssueResolutionAcceptor acceptor) {
acceptor.accept(issue, Messages.CheckCfgQuickfixProvider_REMOVE_CONFIGURED_PARAM_LABEL, Messages.CheckCfgQuickfixProvider_REMOVE_CONFIGURED_PARAM_DESCN, null, new ISemanticModification() {
@Override
public void apply(final EObject element, final IModificationContext context) {
ConfiguredCheck check = EcoreUtil2.getContainerOfType(element, ConfiguredCheck.class);
check.getParameterConfigurations().removeAll(check.getParameterConfigurations());
Expand All @@ -137,7 +175,7 @@ public void apply(final EObject element, final IModificationContext context) {

/**
* Reset the severity of a configured check which is final to {@code default}.
*
*
* @param issue
* the issue
* @param acceptor
Expand All @@ -146,11 +184,11 @@ public void apply(final EObject element, final IModificationContext context) {
@Fix(IssueCodes.FINAL_CHECK_NOT_CONFIGURABLE)
public void resetSeverityOfFinalCheck(final Issue issue, final IssueResolutionAcceptor acceptor) {
acceptor.accept(issue, Messages.CheckCfgQuickfixProvider_CORRECT_SEVERITY_OF_FINAL_CHECK_LABEL, Messages.CheckCfgQuickfixProvider_CORRECT_SEVERITY_OF_FINAL_CHECK_DESCN, null, new ISemanticModification() {
@Override
public void apply(final EObject element, final IModificationContext context) {
ConfiguredCheck check = EcoreUtil2.getContainerOfType(element, ConfiguredCheck.class);
check.setSeverity(SeverityKind.DEFAULT);
}
});
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ public class Messages extends NLS {
public static String CheckCfgQuickfixProvider_REMOVE_DUPLICATE_CHECK_LABEL;
public static String CheckCfgQuickfixProvider_REMOVE_DUPLICATE_PARAM_DESCN;
public static String CheckCfgQuickfixProvider_REMOVE_DUPLICATE_PARAM_LABEL;
public static String CheckCfgQuickfixProvider_REMOVE_DUPLICATE_LANG_CONFIG_LABEL;
public static String CheckCfgQuickfixProvider_REMOVE_DUPLICATE_LANG_CONFIG_DESC;

static {
// initialize resource bundle
Expand All @@ -40,4 +42,3 @@ private Messages() {
super();
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,6 @@ CheckCfgQuickfixProvider_REMOVE_DUPLICATE_CATALOG_LABEL=Remove duplicate catalog
CheckCfgQuickfixProvider_REMOVE_DUPLICATE_CHECK_DESCN=Remove the duplicate check configuration.
CheckCfgQuickfixProvider_REMOVE_DUPLICATE_CHECK_LABEL=Remove duplicate check
CheckCfgQuickfixProvider_REMOVE_DUPLICATE_PARAM_DESCN=Remove the duplicate parameter configuration.
CheckCfgQuickfixProvider_REMOVE_DUPLICATE_PARAM_LABEL=Remove duplicate parameter
CheckCfgQuickfixProvider_REMOVE_DUPLICATE_PARAM_LABEL=Remove duplicate parameter
CheckCfgQuickfixProvider_REMOVE_DUPLICATE_LANG_CONFIG_LABEL=Remove duplicate language(s)
CheckCfgQuickfixProvider_REMOVE_DUPLICATE_LANG_CONFIG_DESC=Remove duplicate language configuration(s).

0 comments on commit 42ac756

Please sign in to comment.