From 023d19f60add22deb4c08700e490b3d913b03c92 Mon Sep 17 00:00:00 2001 From: jkohnlein Date: Tue, 30 Sep 2008 15:50:35 +0000 Subject: [PATCH] Next patch from Heiko: Validation of output see https://bugs.eclipse.org/bugs/attachment.cgi?id=113883 --- .../src/org/eclipse/xtext/EcoreUtil2.java | 31 ++++++++-- .../resource/metamodel/ErrorAcceptor.java | 2 +- .../metamodel/TypeHierarchyHelper.java | 42 ++++++++++++- .../metamodel/Xtext2EcoreTransformer.java | 14 ++++- .../org/eclipse/xtext/EcoreUtil2Tests.java | 15 +++++ .../metamodel/TypeHierarchyHelperTests.java | 62 ++++++++++++++++++- .../Xtext2EcoreTransformerTests.java | 34 +++++++++- 7 files changed, 183 insertions(+), 17 deletions(-) diff --git a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/EcoreUtil2.java b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/EcoreUtil2.java index 1b89e7875..1164c2697 100644 --- a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/EcoreUtil2.java +++ b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/EcoreUtil2.java @@ -12,7 +12,9 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.apache.log4j.Logger; import org.eclipse.emf.common.util.EList; @@ -180,7 +182,7 @@ public class EcoreUtil2 extends EcoreUtil { ca.add(eClass); return ca; } - + public static boolean isFeatureSemanticallyEqualApartFromType(EStructuralFeature f1, EStructuralFeature f2) { boolean result = f1.getName().equals(f1.getName()); result &= f1.getLowerBound() == f2.getLowerBound(); @@ -199,15 +201,15 @@ public class EcoreUtil2 extends EcoreUtil { public enum FindResult { FeatureDoesNotExist, FeatureExists, DifferentFeatureWithSameNameExists } - + public static EStructuralFeature findFeatureByName(Collection features, String name) { for (EStructuralFeature feature : features) - if(feature.getName().equals(name)) + if (feature.getName().equals(name)) return feature; - + return null; } - + public static FindResult containsSemanticallyEqualFeature(EClass eClass, EStructuralFeature feature) { return containsSemanticallyEqualFeature(eClass.getEAllStructuralFeatures(), feature); } @@ -244,4 +246,23 @@ public class EcoreUtil2 extends EcoreUtil { return result; } + private static void collectAllSuperTypes(Set collectedTypes, EClass eClass) { + for (EClass superType : eClass.getESuperTypes()) + if (!collectedTypes.contains(superType)) { + collectedTypes.add(superType); + collectAllSuperTypes(collectedTypes, superType); + } + } + + /** + * In addition to + * org.eclipse.xtext.resource.metamodel.EClassifierInfos.getAllEClassInfos() + * this implementation can deal with cycles in type hierarchy + */ + public static Collection getAllSuperTypes(EClass eClass) { + Set allSuperTypes = new HashSet(); + collectAllSuperTypes(allSuperTypes, eClass); + return Collections.unmodifiableSet(allSuperTypes); + } + } diff --git a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/ErrorAcceptor.java b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/ErrorAcceptor.java index fc8f1f281..2126dacf4 100644 --- a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/ErrorAcceptor.java +++ b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/ErrorAcceptor.java @@ -16,7 +16,7 @@ import org.eclipse.emf.ecore.EObject; */ public interface ErrorAcceptor { public enum ErrorCode { - NoSuchTypeAvailable, MoreThanOneTypeChangeInOneRule, CannotLoadMetamodel, CannotCreateTypeInSealedMetamodel, FeatureWithDifferentConfigurationAlreadyExists, NoCompatibleFeatureTypeAvailable, AliasForMetamodelAlreadyExists, NoSuchRuleAvailable + NoSuchTypeAvailable, MoreThanOneTypeChangeInOneRule, CannotLoadMetamodel, CannotCreateTypeInSealedMetamodel, FeatureWithDifferentConfigurationAlreadyExists, NoCompatibleFeatureTypeAvailable, AliasForMetamodelAlreadyExists, NoSuchRuleAvailable, TypeWithCycleInHierarchy, MoreThanOneFeatureWithSameName } public void acceptError(ErrorCode errorCode, String message, EObject element); diff --git a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/TypeHierarchyHelper.java b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/TypeHierarchyHelper.java index a47bf2467..405722f6d 100644 --- a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/TypeHierarchyHelper.java +++ b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/TypeHierarchyHelper.java @@ -8,6 +8,7 @@ *******************************************************************************/ package org.eclipse.xtext.resource.metamodel; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -20,6 +21,7 @@ import org.eclipse.emf.ecore.EStructuralFeature; import org.eclipse.xtext.EcoreUtil2; import org.eclipse.xtext.EcoreUtil2.FindResult; import org.eclipse.xtext.resource.metamodel.EClassifierInfo.EClassInfo; +import org.eclipse.xtext.resource.metamodel.ErrorAcceptor.ErrorCode; /** * @author Heiko Behrens - Initial contribution and API @@ -30,10 +32,12 @@ public class TypeHierarchyHelper { private Map> subTypesMap = new HashMap>(); private Set rootInfos = new HashSet(); private Set traversedTypes = new HashSet(); + private ErrorAcceptor errorAcceptor; - public TypeHierarchyHelper(EClassifierInfos infos) { + public TypeHierarchyHelper(EClassifierInfos infos, ErrorAcceptor errorAcceptor) { super(); this.infos = infos; + this.errorAcceptor = errorAcceptor; collectTypeData(); } @@ -100,7 +104,7 @@ public class TypeHierarchyHelper { for (Iterator iterator = featuresToBeModified.iterator(); iterator.hasNext();) if (EcoreUtil2.containsSemanticallyEqualFeature(features, iterator.next()) == FindResult.FeatureExists) iterator.remove(); - + } private Collection joinFeaturesInto(Collection commonFeatures, @@ -166,7 +170,7 @@ public class TypeHierarchyHelper { Collection features = classInfo.getEClass().getEStructuralFeatures(); for (Iterator iterator = features.iterator(); iterator.hasNext();) - if(anySuperTypeContainsSemanticallyEqualFeature(classInfo.getEClass(), iterator.next())) + if (anySuperTypeContainsSemanticallyEqualFeature(classInfo.getEClass(), iterator.next())) iterator.remove(); } @@ -178,4 +182,36 @@ public class TypeHierarchyHelper { return EcoreUtil2.containsSemanticallyEqualFeature(allSupertypesFeatures, feature) == FindResult.FeatureExists; } + public void detectEClassesWithCyclesInTypeHierachy() { + for (EClassInfo info : infos.getAllEClassInfos()) { + EClass eClass = info.getEClass(); + Collection allSuperTypes = EcoreUtil2.getAllSuperTypes(eClass); + if (allSuperTypes.contains(eClass)) + reportError(ErrorCode.TypeWithCycleInHierarchy, "Type with cycle in hierarchy: " + eClass.getName()); + } + } + + private void reportError(ErrorCode errorCode, String message) { + errorAcceptor.acceptError(errorCode, message, null); + } + + public void detectDuplicatedFeatures() { + for (EClassInfo info : infos.getAllEClassInfos()) { + detectDuplicatedFeature(info); + } + } + + private void detectDuplicatedFeature(EClassInfo info) { + EClass eClass = info.getEClass(); + Collection directFeatures = eClass.getEStructuralFeatures(); + Collection allFeatures = new ArrayList(eClass + .getEAllStructuralFeatures()); + for (EStructuralFeature feature : directFeatures) { + allFeatures.remove(feature); + if (EcoreUtil2.findFeatureByName(allFeatures, feature.getName()) != null) + reportError(ErrorCode.MoreThanOneFeatureWithSameName, "Feature " + feature.getName() + + " exists more than once in type hierarchy of " + eClass.getName()); + } + } + } diff --git a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/Xtext2EcoreTransformer.java b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/Xtext2EcoreTransformer.java index 0b576354c..a06a38f0e 100644 --- a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/Xtext2EcoreTransformer.java +++ b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/resource/metamodel/Xtext2EcoreTransformer.java @@ -98,7 +98,7 @@ public class Xtext2EcoreTransformer { deriveTypes(); deriveFeatures(); - normalizeGeneratedPackages(); + normalizeAndValidateGeneratedPackages(); return getGeneratedPackagesSortedByName(); } @@ -216,10 +216,18 @@ public class Xtext2EcoreTransformer { return result; } - private void normalizeGeneratedPackages() { - TypeHierarchyHelper helper = new TypeHierarchyHelper(this.eClassifierInfos); + private void normalizeAndValidateGeneratedPackages() { + TypeHierarchyHelper helper = new TypeHierarchyHelper(this.eClassifierInfos, this.errorAcceptor); helper.liftUpFeaturesRecursively(); helper.removeDuplicateDerivedFeatures(); + helper.detectEClassesWithCyclesInTypeHierachy(); + + // duplicated features can occur in rare cases when alternatives produce + // different types of a feature + // If the internal structure (Set) of the underlying algorithm + // produces the features for the subtype first the implementation of EClassInfo + // wont find a conflict + helper.detectDuplicatedFeatures(); } private void deriveTypesAndHierarchy(EClassifierInfo ruleReturnType, AbstractElement element) diff --git a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/EcoreUtil2Tests.java b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/EcoreUtil2Tests.java index 88fed92a6..546c4dd2d 100644 --- a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/EcoreUtil2Tests.java +++ b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/EcoreUtil2Tests.java @@ -68,4 +68,19 @@ public class EcoreUtil2Tests extends TestCase { assertSame(b, EcoreUtil2.getCompatibleType(b, d)); assertSame(a, EcoreUtil2.getCompatibleType(d, e)); } + + public void testGetAllSuperTypesWithCycle() { + EClass a = createEClass("a"); + EClass b = createEClass("b"); + b.getESuperTypes().add(a); + a.getESuperTypes().add(b); + + // inconsistent and quasi-unpredictable in complex scenarios due to caching + assertTrue(a.getEAllSuperTypes().contains(a)); + assertFalse(b.getEAllSuperTypes().contains(b)); + + // always stable + assertTrue(EcoreUtil2.getAllSuperTypes(a).contains(a)); + assertTrue(EcoreUtil2.getAllSuperTypes(b).contains(b)); + } } diff --git a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/resource/metamodel/TypeHierarchyHelperTests.java b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/resource/metamodel/TypeHierarchyHelperTests.java index ff04a67ef..5e4be42f8 100644 --- a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/resource/metamodel/TypeHierarchyHelperTests.java +++ b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/resource/metamodel/TypeHierarchyHelperTests.java @@ -8,14 +8,20 @@ *******************************************************************************/ package org.eclipse.xtext.resource.metamodel; +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.same; import junit.framework.TestCase; +import org.easymock.EasyMock; import org.eclipse.emf.ecore.EAttribute; import org.eclipse.emf.ecore.EClass; import org.eclipse.emf.ecore.EDataType; +import org.eclipse.emf.ecore.EObject; import org.eclipse.emf.ecore.EReference; import org.eclipse.emf.ecore.EcoreFactory; import org.eclipse.xtext.resource.metamodel.EClassifierInfo.EClassInfo; +import org.eclipse.xtext.resource.metamodel.ErrorAcceptor.ErrorCode; /** * @author Heiko Behrens - Initial contribution and API @@ -27,10 +33,23 @@ public class TypeHierarchyHelperTests extends TestCase { private EClassifierInfos infos = new EClassifierInfos(); private EDataType INT = EcoreFactory.eINSTANCE.createEDataType(); private EDataType STRING = EcoreFactory.eINSTANCE.createEDataType(); + private ErrorAcceptor errorAcceptorMock; + @Override + protected void setUp() throws Exception { + super.setUp(); + errorAcceptorMock = createMock(ErrorAcceptor.class); + } + private void liftUpFeatures() throws Exception { - helper = new TypeHierarchyHelper(infos); + initializeHelper(); helper.liftUpFeaturesRecursively(); + EasyMock.verify(errorAcceptorMock); + } + + private void initializeHelper() { + EasyMock.replay(errorAcceptorMock); + helper = new TypeHierarchyHelper(infos, errorAcceptorMock); } private EClassInfo addClass(String name, boolean isGenerated) { @@ -204,7 +223,7 @@ public class TypeHierarchyHelperTests extends TestCase { public void testDublicateDerivedFeature() throws Exception { EClassInfo a = addClass("a"); EClassInfo b = addClass("b"); - EClassInfo c = addClass("b"); + EClassInfo c = addClass("c"); b.addSupertype(a); c.addSupertype(b); addAttribute(a, INT, "f"); @@ -214,12 +233,49 @@ public class TypeHierarchyHelperTests extends TestCase { assertEquals(0, b.getEClass().getEStructuralFeatures().size()); assertEquals(1, c.getEClass().getEStructuralFeatures().size()); - helper = new TypeHierarchyHelper(infos); + initializeHelper(); helper.removeDuplicateDerivedFeatures(); assertEquals(1, a.getEClass().getEStructuralFeatures().size()); assertEquals(0, b.getEClass().getEStructuralFeatures().size()); assertEquals(0, c.getEClass().getEStructuralFeatures().size()); } + + public void testCylceInTypeHierarchy() throws Exception { + EClassInfo a = addClass("a"); + EClassInfo b = addClass("b"); + EClassInfo c = addClass("c"); + EClassInfo d = addClass("d"); + a.addSupertype(c); + b.addSupertype(a); + c.addSupertype(b); + d.addSupertype(a); + + errorAcceptorMock.acceptError(same(ErrorCode.TypeWithCycleInHierarchy), (String) anyObject(), + (EObject) anyObject()); + EasyMock.expectLastCall().times(3); + + initializeHelper(); + helper.detectEClassesWithCyclesInTypeHierachy(); + EasyMock.verify(errorAcceptorMock); + } + + public void testDuplicateFeatures01() throws Exception { + EClassInfo a = addClass("a"); + EClassInfo b = addClass("b"); + + b.addSupertype(a); + addAttribute(a, INT, "f1"); + addAttribute(a, STRING, "f2"); + addAttribute(b, INT, "f2"); + + errorAcceptorMock.acceptError(same(ErrorCode.MoreThanOneFeatureWithSameName), (String) anyObject(), + (EObject) anyObject()); + + + initializeHelper(); + helper.detectDuplicatedFeatures(); + EasyMock.verify(errorAcceptorMock); + } } diff --git a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/resource/metamodel/Xtext2EcoreTransformerTests.java b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/resource/metamodel/Xtext2EcoreTransformerTests.java index 2f4fe8e1e..b9c3a3216 100644 --- a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/resource/metamodel/Xtext2EcoreTransformerTests.java +++ b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/resource/metamodel/Xtext2EcoreTransformerTests.java @@ -15,6 +15,7 @@ import static org.easymock.EasyMock.verify; import java.util.List; +import org.easymock.EasyMock; import org.eclipse.emf.common.util.EList; import org.eclipse.emf.ecore.EAttribute; import org.eclipse.emf.ecore.EClass; @@ -377,7 +378,6 @@ public class Xtext2EcoreTransformerTests extends AbstractGeneratorTest { assertSame(type, item.getESuperTypes().get(0)); assertEquals(1, thing.getESuperTypes().size()); assertSame(type, thing.getESuperTypes().get(0)); - } public void testAssignedRuleCall() throws Exception { @@ -442,7 +442,6 @@ public class Xtext2EcoreTransformerTests extends AbstractGeneratorTest { assertEquals(4, ruleA.getEReferences().size()); assertReferenceConfiguration(ruleA, 0, "refA1", "TypeB", true, 0, 1); - // TODO should be common compatible type according to #248430 assertReferenceConfiguration(ruleA, 1, "refA2", "TypeB", true, 0, 1); assertReferenceConfiguration(ruleA, 2, "refA3", "TypeB", true, 0, -1); assertReferenceConfiguration(ruleA, 3, "refA4", "TypeB", true, 0, 1); @@ -656,6 +655,37 @@ public class Xtext2EcoreTransformerTests extends AbstractGeneratorTest { assertNotNull(ruleA); assertEquals(1, ruleA.getEAttributes().size()); assertAttributeConfiguration(ruleA, 0, "featureA", "EString"); + } + + public void testCycleInTypeHierarchy() throws Exception { + String grammar = "language test generate test 'http://test'"; + grammar += " RuleA: RuleB;"; + grammar += " RuleB: RuleC;"; + grammar += " RuleC: RuleA;"; + grammar += " RuleD: RuleA;"; + errorAcceptorMock.acceptError(same(ErrorCode.TypeWithCycleInHierarchy), (String) anyObject(), + (EObject) anyObject()); + EasyMock.expectLastCall().times(3); + + EPackage ePackage = getEPackageFromGrammar(grammar); + assertEquals(4, ePackage.getEClassifiers().size()); + EClass ruleA = (EClass) ePackage.getEClassifier("RuleA"); + assertNotNull(ruleA); + EClass ruleB = (EClass) ePackage.getEClassifier("RuleB"); + assertNotNull(ruleB); + EClass ruleC = (EClass) ePackage.getEClassifier("RuleC"); + assertNotNull(ruleC); + EClass ruleD = (EClass) ePackage.getEClassifier("RuleD"); + assertNotNull(ruleD); + + assertEquals(2, ruleA.getESuperTypes().size()); + assertSame(ruleC, ruleA.getESuperTypes().get(0)); + assertSame(ruleD, ruleA.getESuperTypes().get(1)); + assertEquals(1, ruleB.getESuperTypes().size()); + assertSame(ruleA, ruleB.getESuperTypes().get(0)); + assertEquals(1, ruleC.getESuperTypes().size()); + assertSame(ruleB, ruleC.getESuperTypes().get(0)); + assertEquals(0, ruleD.getESuperTypes().size()); } }