From 2261b4b669a798e24375ea2f922704fa3ba03de7 Mon Sep 17 00:00:00 2001 From: Graham Pearson Date: Wed, 7 Feb 2018 12:01:33 +0000 Subject: [PATCH] #51: Autocorrect to add check to configuration does not put string params in quotes * Extract the code for creating a literal-value template pattern from CheckCfgTemplateProposalProvider into a new helper, TemplateProposalProviderHelper, where it can be unit tested. * Avoid getting a JvmType representation of String just to check the type of a Check parameter. * CheckConfiguration template now gets the check configuration name from the filename instead of using hardwired value "name". * Move ResourceNameTemplateVariableResolver from ddk.check.ui to ddk.xtext.ui so CheckCfgTemplateProposalProvider can use it. Changed its 'catalog' parameter value to 'file' to better reflect its meaning. * Add unit tests for ** ResourceNameTemplateVariableResolver (only when resolving a filename; resolving a package name looks much less testable and hasn't been touched) ** SimpleEnumTemplateVariableResolver ** TemplateProposalProviderHelper Issue-Id: #51 --- .../templates/CheckTemplateContextType.java | 3 +- .../templates/templates.xml | 2 +- .../ddk/checkcfg/ui/CheckCfgUiModule.java | 3 +- .../CheckCfgTemplateContextType.java | 2 + .../CheckCfgTemplateProposalProvider.java | 24 +- .../templates/templates.xml | 2 +- com.avaloq.tools.ddk.xtext.ui.test/.classpath | 12 +- .../META-INF/MANIFEST.MF | 1 + .../build.properties | 3 +- ...ourceNameTemplateVariableResolverTest.java | 126 +++++++++ ...impleEnumTemplateVariableResolverTest.java | 78 ++++++ .../TemplateProposalProviderHelperTest.xtend | 260 ++++++++++++++++++ .../TemplateVariableResolverTestHelper.java | 57 ++++ .../ddk/xtext/ui/test/XtextUiTestSuite.java | 8 +- .../xtend-gen/.gitignore | 0 com.avaloq.tools.ddk.xtext.ui/.classpath | 1 + .../META-INF/MANIFEST.MF | 65 ++--- .../build.properties | 3 +- .../ResourceNameTemplateVariableResolver.java | 29 +- .../TemplateProposalProviderHelper.xtend | 80 ++++++ .../xtend-gen/.gitignore | 0 21 files changed, 688 insertions(+), 71 deletions(-) create mode 100644 com.avaloq.tools.ddk.xtext.ui.test/src/com/avaloq/tools/ddk/xtext/ui/templates/ResourceNameTemplateVariableResolverTest.java create mode 100644 com.avaloq.tools.ddk.xtext.ui.test/src/com/avaloq/tools/ddk/xtext/ui/templates/SimpleEnumTemplateVariableResolverTest.java create mode 100644 com.avaloq.tools.ddk.xtext.ui.test/src/com/avaloq/tools/ddk/xtext/ui/templates/TemplateProposalProviderHelperTest.xtend create mode 100644 com.avaloq.tools.ddk.xtext.ui.test/src/com/avaloq/tools/ddk/xtext/ui/templates/TemplateVariableResolverTestHelper.java create mode 100644 com.avaloq.tools.ddk.xtext.ui.test/xtend-gen/.gitignore rename {com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check => com.avaloq.tools.ddk.xtext.ui/src/com/avaloq/tools/ddk/xtext}/ui/templates/ResourceNameTemplateVariableResolver.java (72%) create mode 100644 com.avaloq.tools.ddk.xtext.ui/src/com/avaloq/tools/ddk/xtext/ui/templates/TemplateProposalProviderHelper.xtend create mode 100644 com.avaloq.tools.ddk.xtext.ui/xtend-gen/.gitignore diff --git a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/templates/CheckTemplateContextType.java b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/templates/CheckTemplateContextType.java index 8dac516ee..8bc672e59 100644 --- a/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/templates/CheckTemplateContextType.java +++ b/com.avaloq.tools.ddk.check.ui/src/com/avaloq/tools/ddk/check/ui/templates/CheckTemplateContextType.java @@ -12,6 +12,8 @@ import org.eclipse.xtext.ui.editor.templates.XtextTemplateContextType; +import com.avaloq.tools.ddk.xtext.ui.templates.ResourceNameTemplateVariableResolver; + /** * Used for adding custom template variable resolvers. @@ -26,4 +28,3 @@ protected void addDefaultTemplateVariables() { } } - diff --git a/com.avaloq.tools.ddk.check.ui/templates/templates.xml b/com.avaloq.tools.ddk.check.ui/templates/templates.xml index 43f82071c..872c0cb62 100644 --- a/com.avaloq.tools.ddk.check.ui/templates/templates.xml +++ b/com.avaloq.tools.ddk.check.ui/templates/templates.xml @@ -10,7 +10,7 @@ /** * @todo Document catalog. */ -catalog ${catalogName:ResourceName('catalog')} +catalog ${catalogName:ResourceName('file')} for grammar ${grammarName:CrossReference(CheckCatalog.grammar)} { ${cursor} } diff --git a/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/CheckCfgUiModule.java b/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/CheckCfgUiModule.java index b3721f481..bf05f0e3a 100644 --- a/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/CheckCfgUiModule.java +++ b/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/CheckCfgUiModule.java @@ -23,6 +23,7 @@ import com.avaloq.tools.ddk.checkcfg.ui.templates.CheckCfgTemplateProposalProvider; import com.avaloq.tools.ddk.xtext.ui.editor.FixedDirtyStateEditorSupport; import com.avaloq.tools.ddk.xtext.ui.templates.KeywordAwareCrossReferenceTemplateVariableResolver; +import com.avaloq.tools.ddk.xtext.ui.templates.ResourceNameTemplateVariableResolver; import com.avaloq.tools.ddk.xtext.ui.templates.SimpleEnumTemplateVariableResolver; @@ -35,7 +36,7 @@ public CheckCfgUiModule(final AbstractUIPlugin plugin) { } /** - * Binds a {@link XtextTemplateContextType} which adds {@link SimpleEnumTemplateVariableResolver}. + * Binds a {@link XtextTemplateContextType} which adds {@link ResourceNameTemplateVariableResolver} and {@link SimpleEnumTemplateVariableResolver}. * * @return {@link CheckCfgTemplateContextType} */ diff --git a/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/templates/CheckCfgTemplateContextType.java b/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/templates/CheckCfgTemplateContextType.java index 4663e2203..ec118ce02 100644 --- a/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/templates/CheckCfgTemplateContextType.java +++ b/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/templates/CheckCfgTemplateContextType.java @@ -12,6 +12,7 @@ import org.eclipse.xtext.ui.editor.templates.XtextTemplateContextType; +import com.avaloq.tools.ddk.xtext.ui.templates.ResourceNameTemplateVariableResolver; import com.avaloq.tools.ddk.xtext.ui.templates.SimpleEnumTemplateVariableResolver; @@ -23,6 +24,7 @@ public class CheckCfgTemplateContextType extends XtextTemplateContextType { @Override protected void addDefaultTemplateVariables() { super.addDefaultTemplateVariables(); + addResolver(new ResourceNameTemplateVariableResolver()); addResolver(new SimpleEnumTemplateVariableResolver()); } diff --git a/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/templates/CheckCfgTemplateProposalProvider.java b/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/templates/CheckCfgTemplateProposalProvider.java index 9c852071a..e0cc78c81 100644 --- a/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/templates/CheckCfgTemplateProposalProvider.java +++ b/com.avaloq.tools.ddk.checkcfg.ui/src/com/avaloq/tools/ddk/checkcfg/ui/templates/CheckCfgTemplateProposalProvider.java @@ -12,7 +12,6 @@ import java.util.List; import java.util.NoSuchElementException; -import java.util.Objects; import java.util.StringJoiner; import org.eclipse.emf.ecore.util.EcoreUtil; @@ -24,7 +23,6 @@ import org.eclipse.jface.text.templates.persistence.TemplateStore; import org.eclipse.swt.graphics.Image; import org.eclipse.xtext.EcoreUtil2; -import org.eclipse.xtext.common.types.JvmType; import org.eclipse.xtext.conversion.impl.QualifiedNameValueConverter; import org.eclipse.xtext.resource.IEObjectDescription; import org.eclipse.xtext.scoping.IScopeProvider; @@ -34,7 +32,6 @@ import org.eclipse.xtext.ui.editor.templates.DefaultTemplateProposalProvider; import org.eclipse.xtext.util.Strings; import org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter; -import org.eclipse.xtext.xbase.jvmmodel.JvmTypeReferenceBuilder; import com.avaloq.tools.ddk.check.check.Check; import com.avaloq.tools.ddk.check.check.CheckCatalog; @@ -44,6 +41,7 @@ import com.avaloq.tools.ddk.checkcfg.checkcfg.ConfiguredCatalog; import com.avaloq.tools.ddk.checkcfg.checkcfg.ConfiguredCheck; import com.avaloq.tools.ddk.checkcfg.ui.labeling.CheckCfgImages; +import com.avaloq.tools.ddk.xtext.ui.templates.TemplateProposalProviderHelper; import com.google.common.base.Function; import com.google.common.base.Predicate; import com.google.common.base.Predicates; @@ -72,10 +70,10 @@ public class CheckCfgTemplateProposalProvider extends DefaultTemplateProposalPro @Inject private QualifiedNameValueConverter qualifiedNameValueConverter; - private final TemplateStore templateStore; - @Inject - private JvmTypeReferenceBuilder.Factory typeRefBuilderFactory; + private TemplateProposalProviderHelper helper; + + private final TemplateStore templateStore; @Inject public CheckCfgTemplateProposalProvider(final TemplateStore templateStore, final ContextTypeRegistry registry, final ContextTypeIdHelper helper) { @@ -153,8 +151,7 @@ private void addCatalogConfigurations(final TemplateContext templateContext, fin builder.append(Strings.newLine()); } final String catalogName = qualifiedNameValueConverter.toString(description.getQualifiedName().toString()); - builder.append("catalog ").append(catalogName).append(" {").append(Strings.newLine()); //$NON-NLS-1$ //$NON-NLS-2$ - builder.append('}').append(Strings.newLine()); + builder.append("catalog ").append(catalogName).append(" {}").append(Strings.newLine()); //$NON-NLS-1$ //$NON-NLS-2$ } } @@ -218,23 +215,20 @@ public String apply(final ConfiguredCheck from) { } }), Predicates.notNull()); final CheckCatalog catalog = configuredCatalog.getCatalog(); - final JvmType stringType = typeRefBuilderFactory.create(context.getResource().getResourceSet()).typeRef(String.class).getType(); for (final Check check : catalog.getAllChecks()) { // create a template on the fly final String checkName = check.getName(); if (!Iterables.contains(alreadyConfiguredCheckNames, checkName)) { // check if referenced check has configurable parameters - final StringJoiner paramsJoiner = new StringJoiner(", ", " ( ", ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + final StringJoiner paramsJoiner = new StringJoiner(", ", " (", ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ paramsJoiner.setEmptyValue(""); //$NON-NLS-1$ for (final FormalParameter param : check.getFormalParameters()) { - final JvmType paramType = param.getType().getType(); final String paramName = param.getName(); - final String defaultValue = String.valueOf(interpreter.evaluate(param.getRight()).getResult()); + final Object defaultValue = interpreter.evaluate(param.getRight()).getResult(); - // Use SimpleEnumTemplateVariableResolver with strings because otherwise TemplateTranslator can't cope with whitespace - paramsJoiner.add(paramName + " = " + (Objects.equals(stringType, paramType) ? "\"${defaultValue:SimpleEnum('" + defaultValue + "')}\"" //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ - : "${" + defaultValue + '}')); //$NON-NLS-1$ + final String valuePlaceholder = helper.createLiteralValuePattern(paramName, defaultValue); + paramsJoiner.add(paramName + " = " + valuePlaceholder); //$NON-NLS-1$ } final String severity = (catalog.isFinal() || check.isFinal()) ? "default " : "${default:Enum('SeverityKind')} "; //$NON-NLS-1$ //$NON-NLS-2$ diff --git a/com.avaloq.tools.ddk.checkcfg.ui/templates/templates.xml b/com.avaloq.tools.ddk.checkcfg.ui/templates/templates.xml index be22b7771..07b55026c 100644 --- a/com.avaloq.tools.ddk.checkcfg.ui/templates/templates.xml +++ b/com.avaloq.tools.ddk.checkcfg.ui/templates/templates.xml @@ -10,7 +10,7 @@ deleted="false" description="Creates a new empty check configuration" enabled="true" - name="CheckConfiguration">check configuration ${name} { + name="CheckConfiguration">check configuration ${name:ResourceName('file')} { ${cursor} }