[serializer/perf] produce diagnostics only on error

Don't always run the old ConcreteSyntaxValidator before serialization.
Instead, create diagnostics in the serializer only when serialization
fails.

see https://github.com/eclipse/xtext-core/issues/48

Signed-off-by: Moritz Eysholdt <moritz.eysholdt@typefox.io>

org.eclipse.xtext.tests/src-gen/org/eclipse/xtext/serializer/tests/SequencerTestLanguageInjectorProvider.java

@@ -0,0 +1,66 @@
+/*
+ * generated by Xtext
+ */
+package org.eclipse.xtext.serializer.tests;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import org.eclipse.xtext.serializer.SequencerTestLanguageRuntimeModule;
+import org.eclipse.xtext.serializer.SequencerTestLanguageStandaloneSetup;
+import org.eclipse.xtext.testing.GlobalRegistries;
+import org.eclipse.xtext.testing.GlobalRegistries.GlobalStateMemento;
+import org.eclipse.xtext.testing.IInjectorProvider;
+import org.eclipse.xtext.testing.IRegistryConfigurator;
+
+public class SequencerTestLanguageInjectorProvider implements IInjectorProvider, IRegistryConfigurator {
+
+	protected GlobalStateMemento stateBeforeInjectorCreation;
+	protected GlobalStateMemento stateAfterInjectorCreation;
+	protected Injector injector;
+
+	static {
+		GlobalRegistries.initializeDefaults();
+	}
+
+	@Override
+	public Injector getInjector() {
+		if (injector == null) {
+			stateBeforeInjectorCreation = GlobalRegistries.makeCopyOfGlobalState();
+			this.injector = internalCreateInjector();
+			stateAfterInjectorCreation = GlobalRegistries.makeCopyOfGlobalState();
+		}
+		return injector;
+	}
+
+	protected Injector internalCreateInjector() {
+		return new SequencerTestLanguageStandaloneSetup() {
+			@Override
+			public Injector createInjector() {
+				return Guice.createInjector(createRuntimeModule());
+			}
+		}.createInjectorAndDoEMFRegistration();
+	}
+
+	protected SequencerTestLanguageRuntimeModule createRuntimeModule() {
+		// make it work also with Maven/Tycho and OSGI
+		// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=493672
+		return new SequencerTestLanguageRuntimeModule() {
+			@Override
+			public ClassLoader bindClassLoaderToInstance() {
+				return SequencerTestLanguageInjectorProvider.class
+						.getClassLoader();
+			}
+		};
+	}
+
+	@Override
+	public void restoreRegistry() {
+		stateBeforeInjectorCreation.restoreGlobalState();
+	}
+
+	@Override
+	public void setupRegistry() {
+		getInjector();
+		stateAfterInjectorCreation.restoreGlobalState();
+	}
+}

org.eclipse.xtext.tests/src/org/eclipse/xtext/GenerateAllTestLanguages.mwe2

@@ -467,6 +467,9 @@ Workflow {
 		language = { 
 			grammarUri = "classpath:/org/eclipse/xtext/serializer/SequencerTestLanguage.xtext"
 			fragment = @TestLanguagesFragments {}
+			fragment = junit.Junit4Fragment2 {
+				generateStub = false
+			}
 		}
 		language = {
 			grammarUri = "classpath:/org/eclipse/xtext/serializer/SyntacticSequencerTestLanguage.xtext"

org.eclipse.xtext.tests/src/org/eclipse/xtext/serializer/SerializerValidationDiagnosticsTest.xtend

@@ -0,0 +1,112 @@
+/*******************************************************************************
+ * Copyright (c) 2016 itemis AG (http://www.itemis.eu) 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 org.eclipse.xtext.serializer
+
+import com.google.inject.Inject
+import org.eclipse.emf.ecore.EObject
+import org.eclipse.xtext.serializer.sequencertest.Model
+import org.eclipse.xtext.serializer.sequencertest.MultiKeywordsOrID
+import org.eclipse.xtext.serializer.tests.SequencerTestLanguageInjectorProvider
+import org.eclipse.xtext.testing.InjectWith
+import org.eclipse.xtext.testing.XtextRunner
+import org.eclipse.xtext.testing.util.ParseHelper
+import org.junit.Assert
+import org.junit.Test
+import org.junit.runner.RunWith
+
+/**
+ * @author Moritz Eysholdt - Initial contribution and API
+ */
+@RunWith(XtextRunner)
+@InjectWith(SequencerTestLanguageInjectorProvider)
+class SerializerValidationDiagnosticsTest {
+
+	@Inject extension ParseHelper<Model>
+	@Inject extension ISerializer
+
+	@Test def void testSingleValueMandatoryGenerated() {
+		val model = '''
+			#1 foo bar
+		'''.parse
+		model.x1.val1 = null
+		model.assertSerializationError('''
+			A value for feature 'val1' is missing but required.
+			Semantic Object: Model.x1->SimpleGroup
+			URI: __synthetic0.sequencertestlanguage
+		''')
+	}
+
+	@Test def void testSingleValueMandatoryBacktracking() {
+		val model = '''
+			#3 foo kw1 kw2 bar kw3
+		'''.parse
+		model.x3.val1 = null
+		model.assertSerializationError('''
+			Could not serialize SimpleMultiplicities:
+			SimpleMultiplicities.val1 is required to have a value, but it does not.
+			Semantic Object: Model.x3->SimpleMultiplicities
+			URI: __synthetic0.sequencertestlanguage
+			Context: SimpleMultiplicities returns SimpleMultiplicities
+		''')
+	}
+	
+	@Test def void testMultiValueUpperBoundBacktracking() {
+		val model = '''
+			#17 foo
+		'''.parse
+		val mt = model.x11 as MultiKeywordsOrID
+		mt.^val += "bar"
+		model.assertSerializationError('''
+			Could not serialize MultiKeywordsOrID:
+			MultiKeywordsOrID.val violates the upper bound: It holds 2 values, but only 1 are allowed.
+			Semantic Object: Model.x11->MultiKeywordsOrID
+			URI: __synthetic0.sequencertestlanguage
+			Context: MultiKeywordsOrID returns MultiKeywordsOrID
+		''')
+	}
+	
+	@Test def void testMultiValueLowerBoundBacktracking() {
+		val model = '''
+			#17 foo
+		'''.parse
+		val mt = model.x11 as MultiKeywordsOrID
+		mt.^val.clear
+		model.assertSerializationError('''
+			Could not serialize MultiKeywordsOrID:
+			MultiKeywordsOrID.val violates the lower bound: It holds 0 values, but at least 1 are required.
+			Semantic Object: Model.x11->MultiKeywordsOrID
+			URI: __synthetic0.sequencertestlanguage
+			Context: MultiKeywordsOrID returns MultiKeywordsOrID
+		''')
+	}
+	
+	@Test def void testBacktracking() {
+		val model = '''
+			#8 foo bar
+		'''.parse
+		model.x8.val3 = "baz"
+		model.assertSerializationError('''
+			Could not serialize AltList1 via backtracking.
+			Constraint: AltList1_AltList1 returns AltList1: ((val1=ID val2=ID) | (val1=ID val3=ID) | (val1=ID val4=ID?));
+			Values: val1(1), val2(1), val3(1)
+			Semantic Object: Model.x8->AltList1
+			URI: __synthetic0.sequencertestlanguage
+			Context: AltList1 returns AltList1
+		''')
+	}
+
+	def private assertSerializationError(EObject obj, String expected) {
+		try {
+			obj.serialize
+			Assert.fail("Serialization should not succeed.")
+		} catch (Throwable t) {
+			Assert.assertEquals(expected.toString.trim, t.message)
+		}
+	}
+
+}

org.eclipse.xtext.tests/xtend-gen/org/eclipse/xtext/serializer/SerializerValidationDiagnosticsTest.java

@@ -0,0 +1,193 @@
+/**
+ * Copyright (c) 2016 itemis AG (http://www.itemis.eu) 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 org.eclipse.xtext.serializer;
+
+import com.google.inject.Inject;
+import org.eclipse.emf.common.util.EList;
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.xtend2.lib.StringConcatenation;
+import org.eclipse.xtext.serializer.ISerializer;
+import org.eclipse.xtext.serializer.sequencertest.AltList1;
+import org.eclipse.xtext.serializer.sequencertest.Model;
+import org.eclipse.xtext.serializer.sequencertest.MultiKeywordsOrID;
+import org.eclipse.xtext.serializer.sequencertest.SimpleGroup;
+import org.eclipse.xtext.serializer.sequencertest.SimpleMultiplicities;
+import org.eclipse.xtext.serializer.tests.SequencerTestLanguageInjectorProvider;
+import org.eclipse.xtext.testing.InjectWith;
+import org.eclipse.xtext.testing.XtextRunner;
+import org.eclipse.xtext.testing.util.ParseHelper;
+import org.eclipse.xtext.xbase.lib.Exceptions;
+import org.eclipse.xtext.xbase.lib.Extension;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/**
+ * @author Moritz Eysholdt - Initial contribution and API
+ */
+@RunWith(XtextRunner.class)
+@InjectWith(SequencerTestLanguageInjectorProvider.class)
+@SuppressWarnings("all")
+public class SerializerValidationDiagnosticsTest {
+  @Inject
+  @Extension
+  private ParseHelper<Model> _parseHelper;
+  
+  @Inject
+  @Extension
+  private ISerializer _iSerializer;
+  
+  @Test
+  public void testSingleValueMandatoryGenerated() {
+    try {
+      StringConcatenation _builder = new StringConcatenation();
+      _builder.append("#1 foo bar");
+      _builder.newLine();
+      final Model model = this._parseHelper.parse(_builder);
+      SimpleGroup _x1 = model.getX1();
+      _x1.setVal1(null);
+      StringConcatenation _builder_1 = new StringConcatenation();
+      _builder_1.append("A value for feature \'val1\' is missing but required.");
+      _builder_1.newLine();
+      _builder_1.append("Semantic Object: Model.x1->SimpleGroup");
+      _builder_1.newLine();
+      _builder_1.append("URI: __synthetic0.sequencertestlanguage");
+      _builder_1.newLine();
+      this.assertSerializationError(model, _builder_1.toString());
+    } catch (Throwable _e) {
+      throw Exceptions.sneakyThrow(_e);
+    }
+  }
+  
+  @Test
+  public void testSingleValueMandatoryBacktracking() {
+    try {
+      StringConcatenation _builder = new StringConcatenation();
+      _builder.append("#3 foo kw1 kw2 bar kw3");
+      _builder.newLine();
+      final Model model = this._parseHelper.parse(_builder);
+      SimpleMultiplicities _x3 = model.getX3();
+      _x3.setVal1(null);
+      StringConcatenation _builder_1 = new StringConcatenation();
+      _builder_1.append("Could not serialize SimpleMultiplicities:");
+      _builder_1.newLine();
+      _builder_1.append("SimpleMultiplicities.val1 is required to have a value, but it does not.");
+      _builder_1.newLine();
+      _builder_1.append("Semantic Object: Model.x3->SimpleMultiplicities");
+      _builder_1.newLine();
+      _builder_1.append("URI: __synthetic0.sequencertestlanguage");
+      _builder_1.newLine();
+      _builder_1.append("Context: SimpleMultiplicities returns SimpleMultiplicities");
+      _builder_1.newLine();
+      this.assertSerializationError(model, _builder_1.toString());
+    } catch (Throwable _e) {
+      throw Exceptions.sneakyThrow(_e);
+    }
+  }
+  
+  @Test
+  public void testMultiValueUpperBoundBacktracking() {
+    try {
+      StringConcatenation _builder = new StringConcatenation();
+      _builder.append("#17 foo");
+      _builder.newLine();
+      final Model model = this._parseHelper.parse(_builder);
+      EObject _x11 = model.getX11();
+      final MultiKeywordsOrID mt = ((MultiKeywordsOrID) _x11);
+      EList<String> _val = mt.getVal();
+      _val.add("bar");
+      StringConcatenation _builder_1 = new StringConcatenation();
+      _builder_1.append("Could not serialize MultiKeywordsOrID:");
+      _builder_1.newLine();
+      _builder_1.append("MultiKeywordsOrID.val violates the upper bound: It holds 2 values, but only 1 are allowed.");
+      _builder_1.newLine();
+      _builder_1.append("Semantic Object: Model.x11->MultiKeywordsOrID");
+      _builder_1.newLine();
+      _builder_1.append("URI: __synthetic0.sequencertestlanguage");
+      _builder_1.newLine();
+      _builder_1.append("Context: MultiKeywordsOrID returns MultiKeywordsOrID");
+      _builder_1.newLine();
+      this.assertSerializationError(model, _builder_1.toString());
+    } catch (Throwable _e) {
+      throw Exceptions.sneakyThrow(_e);
+    }
+  }
+  
+  @Test
+  public void testMultiValueLowerBoundBacktracking() {
+    try {
+      StringConcatenation _builder = new StringConcatenation();
+      _builder.append("#17 foo");
+      _builder.newLine();
+      final Model model = this._parseHelper.parse(_builder);
+      EObject _x11 = model.getX11();
+      final MultiKeywordsOrID mt = ((MultiKeywordsOrID) _x11);
+      EList<String> _val = mt.getVal();
+      _val.clear();
+      StringConcatenation _builder_1 = new StringConcatenation();
+      _builder_1.append("Could not serialize MultiKeywordsOrID:");
+      _builder_1.newLine();
+      _builder_1.append("MultiKeywordsOrID.val violates the lower bound: It holds 0 values, but at least 1 are required.");
+      _builder_1.newLine();
+      _builder_1.append("Semantic Object: Model.x11->MultiKeywordsOrID");
+      _builder_1.newLine();
+      _builder_1.append("URI: __synthetic0.sequencertestlanguage");
+      _builder_1.newLine();
+      _builder_1.append("Context: MultiKeywordsOrID returns MultiKeywordsOrID");
+      _builder_1.newLine();
+      this.assertSerializationError(model, _builder_1.toString());
+    } catch (Throwable _e) {
+      throw Exceptions.sneakyThrow(_e);
+    }
+  }
+  
+  @Test
+  public void testBacktracking() {
+    try {
+      StringConcatenation _builder = new StringConcatenation();
+      _builder.append("#8 foo bar");
+      _builder.newLine();
+      final Model model = this._parseHelper.parse(_builder);
+      AltList1 _x8 = model.getX8();
+      _x8.setVal3("baz");
+      StringConcatenation _builder_1 = new StringConcatenation();
+      _builder_1.append("Could not serialize AltList1 via backtracking.");
+      _builder_1.newLine();
+      _builder_1.append("Constraint: AltList1_AltList1 returns AltList1: ((val1=ID val2=ID) | (val1=ID val3=ID) | (val1=ID val4=ID?));");
+      _builder_1.newLine();
+      _builder_1.append("Values: val1(1), val2(1), val3(1)");
+      _builder_1.newLine();
+      _builder_1.append("Semantic Object: Model.x8->AltList1");
+      _builder_1.newLine();
+      _builder_1.append("URI: __synthetic0.sequencertestlanguage");
+      _builder_1.newLine();
+      _builder_1.append("Context: AltList1 returns AltList1");
+      _builder_1.newLine();
+      this.assertSerializationError(model, _builder_1.toString());
+    } catch (Throwable _e) {
+      throw Exceptions.sneakyThrow(_e);
+    }
+  }
+  
+  private void assertSerializationError(final EObject obj, final String expected) {
+    try {
+      this._iSerializer.serialize(obj);
+      Assert.fail("Serialization should not succeed.");
+    } catch (final Throwable _t) {
+      if (_t instanceof Throwable) {
+        final Throwable t = (Throwable)_t;
+        String _string = expected.toString();
+        String _trim = _string.trim();
+        String _message = t.getMessage();
+        Assert.assertEquals(_trim, _message);
+      } else {
+        throw Exceptions.sneakyThrow(_t);
+      }
+    }
+  }
+}

org.eclipse.xtext/src/org/eclipse/xtext/resource/SaveOptions.java

@@ -27,6 +27,8 @@ public class SaveOptions {
 	protected static final String KEY = SaveOptions.class.getName();
 	
 	private final boolean formatting;
+	
+	@Deprecated // see https://github.com/eclipse/xtext-core/issues/48
 	private final boolean validating;
 
 	protected SaveOptions(boolean formatting, boolean validating) {
@@ -76,6 +78,7 @@ public class SaveOptions {
 		return formatting;
 	}
 
+	@Deprecated // see https://github.com/eclipse/xtext-core/issues/48
 	public boolean isValidating() {
 		return validating;
 	}
@@ -141,6 +144,7 @@ public class SaveOptions {
 			return this;
 		}
 		
+		@Deprecated // see https://github.com/eclipse/xtext-core/issues/48
 		public Builder noValidation() {
 			this.validating = false;
 			return this;

org.eclipse.xtext/src/org/eclipse/xtext/serializer/diagnostic/SequencerDiagnosticProvider.java

@@ -18,6 +18,7 @@ import org.eclipse.xtext.IGrammarAccess;
 import org.eclipse.xtext.serializer.ISerializationContext;
 import org.eclipse.xtext.serializer.analysis.IGrammarConstraintProvider;
 import org.eclipse.xtext.serializer.analysis.IGrammarConstraintProvider.IConstraint;
+import org.eclipse.xtext.serializer.analysis.IGrammarConstraintProvider.IFeatureInfo;
 import org.eclipse.xtext.serializer.analysis.ISemanticSequencerNfaProvider.ISemState;
 import org.eclipse.xtext.serializer.analysis.SerializationContext;
 import org.eclipse.xtext.serializer.analysis.SerializationContextMap;
@@ -65,13 +66,45 @@ public class SequencerDiagnosticProvider implements ISemanticSequencerDiagnostic
 	}
 
 	@Override
-	public ISerializationDiagnostic createBacktrackingFailedDiagnostic(SerializableObject sem, ISerializationContext ctx,
-			IConstraint constraint) {
-		StringBuilder msg = new StringBuilder();
-		msg.append("Could not serialize EObject via backtracking.\n");
-		msg.append("Constraint: " + constraint + "\n");
-		msg.append(sem.getValuesString());
-		return new SerializationDiagnostic(BACKTRACKING_FAILED, sem.getEObject(), ctx, grammarAccess.getGrammar(), msg.toString());
+	public ISerializationDiagnostic createBacktrackingFailedDiagnostic(SerializableObject sem,
+			ISerializationContext ctx, IConstraint constraint) {
+		EClass type = constraint.getType();
+		List<String> hints = Lists.newArrayList();
+		for (IFeatureInfo feature : constraint.getFeatures()) {
+			int featureID = type.getFeatureID(feature.getFeature());
+			int count = sem.getValueCount(featureID);
+			String name = feature.getFeature().getEContainingClass().getName() + "." + feature.getFeature().getName();
+			if (!sem.isOptional(featureID)) {
+				int upperBound = feature.getUpperBound();
+				if (count > upperBound) {
+					if (feature.getFeature().isMany()) {
+						hints.add(name + " violates the upper bound: It holds " + count + " values, but only " + upperBound + " are allowed.");
+					} else {
+						hints.add(name + " is not allowed to have a value, but it does.");
+					}
+				}
+			}
+			int lowerBound = feature.getLowerBound();
+			if (count < lowerBound) {
+				if (feature.getFeature().isMany()) {
+					hints.add(name + " violates the lower bound: It holds " + count + " values, but at least " + lowerBound + " are required.");
+				} else {
+					hints.add(name + " is required to have a value, but it does not.");
+				}
+			}
+		}
+		if (!hints.isEmpty()) {
+			StringBuilder msg = new StringBuilder();
+			msg.append("Could not serialize " + type.getName() + ":\n");
+			msg.append(Joiner.on("\n").join(hints));
+			return new SerializationDiagnostic(BACKTRACKING_FAILED, sem.getEObject(), ctx, grammarAccess.getGrammar(), msg.toString());
+		} else {
+			StringBuilder msg = new StringBuilder();
+			msg.append("Could not serialize " + type.getName() + " via backtracking.\n");
+			msg.append("Constraint: " + constraint + "\n");
+			msg.append(sem.getValuesString());
+			return new SerializationDiagnostic(BACKTRACKING_FAILED, sem.getEObject(), ctx, grammarAccess.getGrammar(), msg.toString());
+		}
 	}
 
 	@Override

org.eclipse.xtext/src/org/eclipse/xtext/serializer/impl/Serializer.java

@@ -9,12 +9,9 @@ package org.eclipse.xtext.serializer.impl;
 
 import java.io.IOException;
 import java.io.Writer;
-import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 
-import org.eclipse.emf.common.util.Diagnostic;
 import org.eclipse.emf.ecore.EObject;
 import org.eclipse.xtext.IGrammarAccess;
 import org.eclipse.xtext.formatting.IFormatter;
@@ -119,17 +116,6 @@ public class Serializer implements ISerializer {
 	}
 
 	protected void serialize(EObject obj, ITokenStream tokenStream, SaveOptions options) throws IOException {
-
-		// use the CSV as long as there are cases where is provides better messages than the serializer itself.
-		if (options.isValidating()) {
-			List<Diagnostic> diagnostics = new ArrayList<Diagnostic>();
-			validator.validateRecursive(obj, new IConcreteSyntaxValidator.DiagnosticListAcceptor(diagnostics),
-					new HashMap<Object, Object>());
-			if (!diagnostics.isEmpty())
-				throw new IConcreteSyntaxValidator.InvalidConcreteSyntaxException(
-						"These errors need to be fixed before the model can be serialized.", diagnostics);
-		}
-
 		ISerializationDiagnostic.Acceptor errors = ISerializationDiagnostic.EXCEPTION_THROWING_ACCEPTOR;
 		ITokenStream formatterTokenStream;
 		if (formatter instanceof IFormatterExtension)