From 3e33e68a22bae5951b0dea50fa8594b3165658c5 Mon Sep 17 00:00:00 2001 From: Rene Schneider Date: Wed, 8 Apr 2015 09:12:01 +0200 Subject: [PATCH 1/3] fixed #84: Autocomplete suggestions do not prioritize enum values over variables/constants --- .../META-INF/MANIFEST.MF | 1 + .../de/gebit/integrity/ui/DSLUiModule.java | 10 +++++ .../DSLContentProposalPriorities.java | 41 +++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/DSLContentProposalPriorities.java diff --git a/de.gebit.integrity.dsl.ui/META-INF/MANIFEST.MF b/de.gebit.integrity.dsl.ui/META-INF/MANIFEST.MF index f793a588b..1b13cfcfd 100644 --- a/de.gebit.integrity.dsl.ui/META-INF/MANIFEST.MF +++ b/de.gebit.integrity.dsl.ui/META-INF/MANIFEST.MF @@ -28,6 +28,7 @@ Import-Package: org.apache.log4j, org.eclipse.jdt.core.util, org.eclipse.jdt.internal.core, org.eclipse.jdt.launching, + org.eclipse.jdt.ui.text.java, org.eclipse.jface.text.contentassist, org.eclipse.jface.viewers, org.eclipse.swt.graphics, diff --git a/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/DSLUiModule.java b/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/DSLUiModule.java index 1f3dfe1b4..2225ea848 100644 --- a/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/DSLUiModule.java +++ b/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/DSLUiModule.java @@ -14,6 +14,7 @@ import org.eclipse.xtext.documentation.IEObjectDocumentationProvider; import org.eclipse.xtext.formatting.IWhitespaceInformationProvider; import org.eclipse.xtext.ui.editor.XtextEditor; +import org.eclipse.xtext.ui.editor.contentassist.IContentProposalPriorities; import org.eclipse.xtext.ui.editor.contentassist.PrefixMatcher; import org.eclipse.xtext.ui.editor.hover.IEObjectHoverProvider; import org.eclipse.xtext.ui.editor.preferences.LanguageRootPreferencePage; @@ -21,6 +22,7 @@ import org.eclipse.xtext.ui.editor.syntaxcoloring.IHighlightingConfiguration; import org.eclipse.xtext.ui.editor.syntaxcoloring.ISemanticHighlightingCalculator; +import de.gebit.integrity.ui.contentassist.DSLContentProposalPriorities; import de.gebit.integrity.ui.contentassist.IntegrityPrefixMatcher; import de.gebit.integrity.ui.documentation.IntegrityEObjectDocumentationProvider; import de.gebit.integrity.ui.documentation.IntegrityEObjectHoverProvider; @@ -145,4 +147,12 @@ public Class bindIWhitespaceInformatio return DSLWhitespaceInformationProvider.class; } + /** + * Defines the {@link IContentProposalPriorities} implementation. + * + * @return + */ + public Class bindContentProposalPriorities() { + return DSLContentProposalPriorities.class; + } } diff --git a/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/DSLContentProposalPriorities.java b/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/DSLContentProposalPriorities.java new file mode 100644 index 000000000..45c841ec8 --- /dev/null +++ b/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/DSLContentProposalPriorities.java @@ -0,0 +1,41 @@ +/******************************************************************************* + * Copyright (c) 2013 Rene Schneider, GEBIT Solutions GmbH 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 + *******************************************************************************/ +package de.gebit.integrity.ui.contentassist; + +import org.eclipse.jface.text.contentassist.ICompletionProposal; +import org.eclipse.xtext.common.types.JvmEnumerationLiteral; +import org.eclipse.xtext.ui.editor.contentassist.ContentProposalPriorities; + +/** + * A custom proposal priority implementation in order to adjust proposal priorities. + * + * @author Rene Schneider - initial API and implementation + * + */ +public class DSLContentProposalPriorities extends ContentProposalPriorities { + + /** + * Priority for enumeration literals. + */ + private static final int ENUM_LITERAL_PRIORITY = 600; + + @Override + protected void adjustPriority(ICompletionProposal aProposal, String aPrefix, int aPriority) { + super.adjustPriority(aProposal, aPrefix, aPriority); + + if (aProposal instanceof IntegrityConfigurableCompletionProposal) { + IntegrityConfigurableCompletionProposal tempProposal = (IntegrityConfigurableCompletionProposal) aProposal; + + Object tempAdditionalInfo = tempProposal.getAdditionalProposalInfo(null); + if (tempAdditionalInfo instanceof JvmEnumerationLiteral) { + tempProposal.setPriority(ENUM_LITERAL_PRIORITY); + } + } + } + +} From 71b201f5c4bfa8b1749c68af260ac6c71a2e4f42 Mon Sep 17 00:00:00 2001 From: Rene Schneider Date: Wed, 8 Apr 2015 09:12:18 +0200 Subject: [PATCH 2/3] added test fixtures for enums --- .../experiments/fixtures/AdditionFixture.java | 10 ++++++++++ .../src/folder/importtest.integrity | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/de.gebit.integrity.dsl.experimentation/src/de/gebit/integrity/experiments/fixtures/AdditionFixture.java b/de.gebit.integrity.dsl.experimentation/src/de/gebit/integrity/experiments/fixtures/AdditionFixture.java index 7b6ea1299..0d4ee6afc 100644 --- a/de.gebit.integrity.dsl.experimentation/src/de/gebit/integrity/experiments/fixtures/AdditionFixture.java +++ b/de.gebit.integrity.dsl.experimentation/src/de/gebit/integrity/experiments/fixtures/AdditionFixture.java @@ -127,6 +127,16 @@ public NamedResultContainer multiResultFixture(@FixtureParameter(name = "param") return new NamedResultContainer(aParam, 100); } + @FixtureMethod() + public TestEnum returnFixedEnum() { + return TestEnum.VALUE_ONE; + } + + @FixtureMethod() + public TestEnum returnEnum(@FixtureParameter(name = "enum") TestEnum anEnum) { + return anEnum; + } + private void pause() { try { Thread.sleep(1000); diff --git a/de.gebit.integrity.dsl.experimentation/src/folder/importtest.integrity b/de.gebit.integrity.dsl.experimentation/src/folder/importtest.integrity index b23af9008..17c58765c 100644 --- a/de.gebit.integrity.dsl.experimentation/src/folder/importtest.integrity +++ b/de.gebit.integrity.dsl.experimentation/src/folder/importtest.integrity @@ -55,6 +55,13 @@ packagedef mypackage with calldef extendedResult uses de.gebit.integrity.experiments.fixtures.ExtendedResultTestFixture#returnExtendedStuff testdef extendedResultTest uses de.gebit.integrity.experiments.fixtures.ExtendedResultTestFixture#returnExtendedStuff + calldef returnEnumCall uses de.gebit.integrity.experiments.fixtures.AdditionFixture#returnEnum + testdef returnEnumTest uses de.gebit.integrity.experiments.fixtures.AdditionFixture#returnEnum + + calldef returnFixedEnumCall uses de.gebit.integrity.experiments.fixtures.AdditionFixture#returnFixedEnum + testdef returnFixedEnumTest uses de.gebit.integrity.experiments.fixtures.AdditionFixture#returnFixedEnum + + variable dummy initially 2 packageend From e521040ff1416e4f004d0f16aae35cc611907b10 Mon Sep 17 00:00:00 2001 From: Rene Schneider Date: Wed, 8 Apr 2015 10:06:26 +0200 Subject: [PATCH 3/3] fixed #85: Autocomplete suggestions should prioritize cross-references based on distance --- .../DSLContentProposalPriorities.java | 49 +++++++++- ...tegrityConfigurableCompletionProposal.java | 92 +++++++++++++++++++ 2 files changed, 139 insertions(+), 2 deletions(-) diff --git a/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/DSLContentProposalPriorities.java b/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/DSLContentProposalPriorities.java index 45c841ec8..d8406532a 100644 --- a/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/DSLContentProposalPriorities.java +++ b/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/DSLContentProposalPriorities.java @@ -7,10 +7,14 @@ *******************************************************************************/ package de.gebit.integrity.ui.contentassist; +import org.eclipse.emf.ecore.EObject; import org.eclipse.jface.text.contentassist.ICompletionProposal; import org.eclipse.xtext.common.types.JvmEnumerationLiteral; +import org.eclipse.xtext.common.types.JvmIdentifiableElement; import org.eclipse.xtext.ui.editor.contentassist.ContentProposalPriorities; +import com.google.common.base.CharMatcher; + /** * A custom proposal priority implementation in order to adjust proposal priorities. * @@ -24,6 +28,24 @@ public class DSLContentProposalPriorities extends ContentProposalPriorities { */ private static final int ENUM_LITERAL_PRIORITY = 600; + /** + * Maximum score bonus awarded because of "distance". Distance is measured in dots required to reference the element + * in the shortest way - that is, after considering package imports. So if you need to write + * "anotherpackage.element" to access an element, it has a distance of one, while "element" would have a distance of + * zero and "outerpackage.anotherpackage.element" would be two. + */ + private static final int CROSS_REFERENCE_DISTANCE_BONUS_MAX = 10; + + /** + * Bonus awarded to the distance score if the element is in the local suite. + */ + private static final int CROSS_REFERENCE_IN_LOCAL_SUITE_BONUS = 50; + + /** + * Bonus awarded to the distance score if the element is in the local package. + */ + private static final int CROSS_REFERENCE_IN_LOCAL_PACKAGE_BONUS = 25; + @Override protected void adjustPriority(ICompletionProposal aProposal, String aPrefix, int aPriority) { super.adjustPriority(aProposal, aPrefix, aPriority); @@ -31,11 +53,34 @@ protected void adjustPriority(ICompletionProposal aProposal, String aPrefix, int if (aProposal instanceof IntegrityConfigurableCompletionProposal) { IntegrityConfigurableCompletionProposal tempProposal = (IntegrityConfigurableCompletionProposal) aProposal; - Object tempAdditionalInfo = tempProposal.getAdditionalProposalInfo(null); + EObject tempAdditionalInfo = tempProposal.getAdditionalProposalInfoObject(); + + // issue #84: enum literals should be prioritized if (tempAdditionalInfo instanceof JvmEnumerationLiteral) { tempProposal.setPriority(ENUM_LITERAL_PRIORITY); } + + // Only do the following if we have an element of our DSL model. Don't know any other way to find this out + // other than checking whether we NOT have a JVM element reference. + if (!(tempAdditionalInfo instanceof JvmIdentifiableElement)) { + // Prioritize primarily by "distance" to the local position. This is done by calculating a distance + // score (higher is better) and adding it to the priority. + int tempCrossReferenceDistance = CharMatcher.is('.').countIn(tempProposal.getReplacementString()); + int tempCrossReferenceDistanceScore = CROSS_REFERENCE_DISTANCE_BONUS_MAX - tempCrossReferenceDistance; + if (tempCrossReferenceDistanceScore < 0) { + tempCrossReferenceDistanceScore = 0; + } + + if (tempProposal.isReferencingObjectInLocalPackage()) { + if (tempProposal.isReferencingObjectInLocalSuite()) { + tempCrossReferenceDistanceScore += CROSS_REFERENCE_IN_LOCAL_SUITE_BONUS; + } else { + tempCrossReferenceDistanceScore += CROSS_REFERENCE_IN_LOCAL_PACKAGE_BONUS; + } + } + + tempProposal.setPriority(tempProposal.getPriority() + tempCrossReferenceDistanceScore); + } } } - } diff --git a/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/IntegrityConfigurableCompletionProposal.java b/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/IntegrityConfigurableCompletionProposal.java index f1dd92f6e..5e2c5a63a 100644 --- a/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/IntegrityConfigurableCompletionProposal.java +++ b/de.gebit.integrity.dsl.ui/src/de/gebit/integrity/ui/contentassist/IntegrityConfigurableCompletionProposal.java @@ -8,6 +8,7 @@ package de.gebit.integrity.ui.contentassist; import org.eclipse.emf.ecore.EObject; +import org.eclipse.emf.ecore.resource.Resource; import org.eclipse.emf.ecore.util.EcoreUtil; import org.eclipse.jface.internal.text.html.BrowserInformationControl; import org.eclipse.jface.resource.JFaceResources; @@ -25,9 +26,11 @@ import com.google.inject.Provider; import de.gebit.integrity.dsl.CallDefinition; +import de.gebit.integrity.dsl.PackageDefinition; import de.gebit.integrity.dsl.SuiteDefinition; import de.gebit.integrity.dsl.TestDefinition; import de.gebit.integrity.dsl.VariableOrConstantEntity; +import de.gebit.integrity.utils.IntegrityDSLUtil; /** * A context-aware configurable completion proposal. This proposal knows its content assist context and uses this in @@ -59,6 +62,21 @@ public class IntegrityConfigurableCompletionProposal extends ConfigurableComplet */ private boolean useHtmlAdditionalProposalInfo; + /** + * The additional proposal info object. Stored here because the superclass one is private. + */ + private Object additionalProposalInfoObject; + + /** + * The resolved {@link #additionalProposalInfoObject} is cached here. + */ + private EObject resolvedAdditionalProposalInfoObject; + + /** + * The context resource. Stored here because the superclass one is private. + */ + private Resource proposalContextResource; + public void setUseHtmlAdditionalProposalInfo(boolean aUseHtmlAdditionalProposalInfoFlag) { this.useHtmlAdditionalProposalInfo = aUseHtmlAdditionalProposalInfoFlag; } @@ -111,9 +129,45 @@ public EObject get() { setReplacementString(tempReplacementStringParts[tempReplacementStringParts.length - 1]); } + additionalProposalInfoObject = tempAdditionalProposalInfo; super.setAdditionalProposalInfo(tempAdditionalProposalInfo); } + @Override + public void setProposalContextResource(Resource aContextResource) { + proposalContextResource = aContextResource; + super.setProposalContextResource(aContextResource); + } + + /** + * Returns the additional proposal info object, if possible. This only returns the plain object (but attempts to + * resolve it, if necessary). + * + * @return the object or null + */ + public EObject getAdditionalProposalInfoObject() { + if (resolvedAdditionalProposalInfoObject == null) { + EObject tempResult = null; + if (additionalProposalInfoObject instanceof EObject) { + tempResult = (EObject) additionalProposalInfoObject; + } else { + if (additionalProposalInfoObject instanceof Provider) { + Object tempObject = ((Provider) additionalProposalInfoObject).get(); + if (tempObject instanceof EObject) { + tempResult = (EObject) tempObject; + } + } + } + if (tempResult != null && tempResult.eIsProxy()) { + tempResult = EcoreUtil.resolve(tempResult, proposalContextResource); + } + + resolvedAdditionalProposalInfoObject = tempResult; + } + + return resolvedAdditionalProposalInfoObject; + } + private boolean requiresResolvingForContentAssist(EObject anObject) { return (anObject instanceof TestDefinition || anObject instanceof CallDefinition); } @@ -141,4 +195,42 @@ public IInformationControl createInformationControl(Shell aParent) { return super.getInformationControlCreator(); } } + + /** + * Checks whether the proposed element is in the "local" suite. + * + * @return + */ + public boolean isReferencingObjectInLocalSuite() { + if (getAdditionalProposalInfoObject() != null) { + SuiteDefinition tempContainingSuite = IntegrityDSLUtil.findUpstreamContainer(SuiteDefinition.class, + (EObject) getAdditionalProposalInfoObject()); + if (tempContainingSuite != null) { + SuiteDefinition tempCurrentSuite = IntegrityDSLUtil.findUpstreamContainer(SuiteDefinition.class, + context.getCurrentModel()); + return tempCurrentSuite == tempContainingSuite; + } + } + + return false; + } + + /** + * Checks whether the proposed element is in the "local" package. + * + * @return + */ + public boolean isReferencingObjectInLocalPackage() { + if (getAdditionalProposalInfoObject() != null) { + PackageDefinition tempContainingPackage = IntegrityDSLUtil.findUpstreamContainer(PackageDefinition.class, + (EObject) getAdditionalProposalInfoObject()); + if (tempContainingPackage != null) { + PackageDefinition tempCurrentPackage = IntegrityDSLUtil.findUpstreamContainer(PackageDefinition.class, + context.getCurrentModel()); + return tempCurrentPackage == tempContainingPackage; + } + } + + return false; + } }