Skip to content

Commit

Permalink
dsldevkit#51: Autocorrect to add check to configuration does not put …
Browse files Browse the repository at this point in the history
…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: dsldevkit#51
  • Loading branch information
GrahamPearsonAvaloq committed Feb 12, 2018
1 parent b8d3463 commit 2261b4b
Show file tree
Hide file tree
Showing 21 changed files with 688 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -26,4 +28,3 @@ protected void addDefaultTemplateVariables() {
}

}

2 changes: 1 addition & 1 deletion com.avaloq.tools.ddk.check.ui/templates/templates.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
/**
* @todo Document catalog.
*/
catalog ${catalogName:ResourceName('catalog')}
catalog ${catalogName:ResourceName('file')}
for grammar ${grammarName:CrossReference(CheckCatalog.grammar)} {
${cursor}
}</template>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;


Expand All @@ -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}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;


Expand All @@ -23,6 +24,7 @@ public class CheckCfgTemplateContextType extends XtextTemplateContextType {
@Override
protected void addDefaultTemplateVariables() {
super.addDefaultTemplateVariables();
addResolver(new ResourceNameTemplateVariableResolver());
addResolver(new SimpleEnumTemplateVariableResolver());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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$
}

}
Expand Down Expand Up @@ -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$
Expand Down
2 changes: 1 addition & 1 deletion com.avaloq.tools.ddk.checkcfg.ui/templates/templates.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}</template>
<template
Expand Down
12 changes: 7 additions & 5 deletions com.avaloq.tools.ddk.xtext.ui.test/.classpath
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?><classpath>
<classpathentry kind="src" path="src"/>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8"/>
<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
<classpathentry kind="output" path="bin"/>
<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry kind="src" path="src"/>
<classpathentry kind="src" path="xtend-gen"/>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8"/>
<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
<classpathentry kind="output" path="bin"/>
</classpath>
1 change: 1 addition & 0 deletions com.avaloq.tools.ddk.xtext.ui.test/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Bundle-Version: 1.1.0.qualifier
Bundle-Vendor: Avaloq Evolution AG
Bundle-RequiredExecutionEnvironment: JavaSE-1.8
Require-Bundle: org.eclipse.xtext.ui,
org.eclipse.xtext.xbase.lib,
org.junit,
org.mockito,
com.avaloq.tools.ddk.xtext.ui,
Expand Down
3 changes: 2 additions & 1 deletion com.avaloq.tools.ddk.xtext.ui.test/build.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
source.. = src/
source.. = src/,\
xtend-gen/
output.. = bin/
bin.includes = META-INF/,\
.
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/*******************************************************************************
* 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.xtext.ui.templates;

import org.eclipse.core.resources.IFile;
import org.eclipse.jface.text.templates.TemplateException;
import org.eclipse.jface.text.templates.TemplateVariable;
import org.eclipse.xtext.XtextRuntimeModule;
import org.eclipse.xtext.ui.editor.model.IXtextDocument;
import org.eclipse.xtext.ui.editor.templates.XtextTemplateContext;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
import org.mockito.Mockito;

import com.google.common.collect.Iterables;
import com.google.inject.Guice;


public class ResourceNameTemplateVariableResolverTest {

private static final Object[] FILE = new Object[] {"file"}; //$NON-NLS-1$
private static final String FILENAME = "filename"; //$NON-NLS-1$

private static XtextTemplateContext mockContext;
private static IXtextDocument mockDocument;
private static IFile mockFile;

private static TemplateVariableResolverTestHelper helper;

private static ResourceNameTemplateVariableResolver resolver;

@BeforeClass
public static void beforeClass() {
mockContext = Mockito.mock(XtextTemplateContext.class);
mockDocument = Mockito.mock(IXtextDocument.class);
mockFile = Mockito.mock(IFile.class);

helper = Guice.createInjector(new XtextRuntimeModule()).getInstance(TemplateVariableResolverTestHelper.class);

resolver = new ResourceNameTemplateVariableResolver();

Mockito.when(mockContext.getDocument()).thenReturn(mockDocument);
Mockito.when(mockDocument.getAdapter(IFile.class)).thenReturn(mockFile);
}

@AfterClass
public static void afterClass() {
mockContext = null;
mockDocument = null;
mockFile = null;

helper = null;

resolver = null;
}

@Test(expected = NullPointerException.class)
public void testResolveValuesWithNullVariable() {
resolver.resolveValues(null, mockContext);
}

@Test(expected = NullPointerException.class)
public void testResolveValuesWithNullContext() {
resolver.resolveValues(Mockito.mock(TemplateVariable.class), null);
}

@Test
public void testResolveValuesWithFileWithoutExtension() throws TemplateException {
final String filename = "filenamewithnoextension"; //$NON-NLS-1$
testResolveValues(FILE, filename, filename);
}

@Test
public void testResolveValuesWithFileWithExtension() throws TemplateException {
testResolveValues(FILE, "filename.with.extension", "filename.with"); //$NON-NLS-1$//$NON-NLS-2$
}

@Test
public void testResolveValuesWithExtraParams() throws TemplateException {
testResolveValues(new Object[] {FILE[0], "other", "random", "values"}, FILENAME, FILENAME); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
}

@Test
public void testResolveValuesWithUnknownParam() throws TemplateException {
testResolveValues(new Object[] {"This is not the parameter you are looking for"}, FILENAME); //$NON-NLS-1$
}

@Test
public void testResolveValuesWithWrongTypeOfParam() throws TemplateException {
testResolveValues(new Object[] {42}, FILENAME);
}

/**
* Test resolveValues().
*
* @param values
* values to resolve
* @param filename
* filename to return from the mock {@link IFile}
* @param expectedResolvedValues
* expected return value
*/
public void testResolveValues(final Object[] values, final String filename, final String... expectedResolvedValues) throws TemplateException {
// ARRANGE
final TemplateVariable variable = helper.createTemplateVariable(resolver, "name", values); //$NON-NLS-1$
Mockito.when(mockFile.getName()).thenReturn(filename);

// ACT
final String[] actualResolvedValues = Iterables.toArray(resolver.resolveValues(variable, mockContext), String.class);

// ASSERT
Assert.assertArrayEquals(expectedResolvedValues, actualResolvedValues);
}

}
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.xtext.ui.templates;

import static org.junit.Assert.assertArrayEquals;
import static org.mockito.Mockito.mock;

import java.util.List;

import org.eclipse.jface.text.templates.TemplateException;
import org.eclipse.jface.text.templates.TemplateVariable;
import org.eclipse.xtext.XtextRuntimeModule;
import org.eclipse.xtext.ui.editor.templates.XtextTemplateContext;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;

import com.google.inject.Guice;


public class SimpleEnumTemplateVariableResolverTest {

private static XtextTemplateContext mockContext;
private static TemplateVariableResolverTestHelper helper;
private static SimpleEnumTemplateVariableResolver resolver;

@BeforeClass
public static void beforeClass() {
mockContext = mock(XtextTemplateContext.class);
helper = Guice.createInjector(new XtextRuntimeModule()).getInstance(TemplateVariableResolverTestHelper.class);
resolver = new SimpleEnumTemplateVariableResolver();
}

@AfterClass
public static void afterClass() {
mockContext = null;
helper = null;
resolver = null;
}

@Test(expected = NullPointerException.class)
public void testResolveValuesWithNullVariable() {
resolver.resolveValues(null, mockContext);
}

public void testResolveValuesWithNoParams() throws TemplateException {
testResolveValues();
}

public void testResolveValuesWithOneParam() throws TemplateException {
testResolveValues("Value"); //$NON-NLS-1$
}

public void testResolveValuesWithMultipleParams() throws TemplateException {
testResolveValues("Value 1", "Value 2", "Value 3"); //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$
}

private void testResolveValues(final Object... values) throws TemplateException {
// ARRANGE
final TemplateVariable variable = helper.createTemplateVariable(resolver, "name", values); //$NON-NLS-1$

// ACT
final List<String> resolvedValues = resolver.resolveValues(variable, mockContext);

// ASSERT
assertArrayEquals(values, resolvedValues.toArray(new String[resolvedValues.size()]));
}

}
Loading

0 comments on commit 2261b4b

Please sign in to comment.