Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Modeling » Epsilon » Bug in org.eclipse.epsilon.evl.emf.validation
Bug in org.eclipse.epsilon.evl.emf.validation [message #834757] Mon, 02 April 2012 11:04 Go to next message
Andrey Skorikov is currently offline Andrey Skorikov
Messages: 8
Registered: September 2011
Junior Member
Hi guys,

there is a bug in org.eclipse.epsilon.evl.emf.validation.CompositeEValidator.java at line 44:

if one of the validators fails none of the remaining validators are invoked because the corresponding expression is not evaluated anymore. This causes a NullPointerException in org.eclipse.epsilon.evl.emf.validation.EvlValidator.java at line 160 when validating the children since the validation results are not initialized.

Just replace the "short-circuit" AND operator (&&) with the normal one (&) to fix it.

Best regards,
Andrey Skorikov
Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835531 is a reply to message #834757] Tue, 03 April 2012 11:06 Go to previous messageGo to next message
Antonio Garcia-Dominguez is currently offline Antonio Garcia-Dominguez
Messages: 309
Registered: January 2010
Senior Member
Hi Andrey,

Thanks for your tip. However, I don't think that's the cause. This is the code you mention, right?

public boolean validate(EClass class1, EObject object,
  DiagnosticChain diagnostics, Map<Object, Object> context)
{
  boolean validate = true;
  for (EValidator validator : delegates) {
    validate = validate && validator.validate(class1, object, diagnostics, context);
  }
  return validate;
}


That's a regular "foreach" Java 6 loop. Even if validate becomes false, we will loop through all the delegates anyway. Switching from && to & would have no effect in this case.

Are you having problems with some EVL script in particular?

Cheers,
Antonio
Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835542 is a reply to message #835531] Tue, 03 April 2012 11:25 Go to previous messageGo to next message
Andrey Skorikov is currently offline Andrey Skorikov
Messages: 8
Registered: September 2011
Junior Member
Hi Antonio,

thanks for the fast reply! Surely, the following expression is evaluated for every delegate in the collection:
validate = validate && validator.validate(class1, object, diagnostics, context)


However, it does not hold for the following subexpression:
validator.validate(class1, object, diagnostics, context)

because when "validate" becomes false, the whole expression must yield false, so there is no need to evaluate the remaining subexpression. That's the idea behind the "short-circuit" AND operator. Unfortunately, the expression has to be evaluated in any case because of its side effects, otherwise we'll get exceptions later.

Cheers,
Andrey
Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835617 is a reply to message #835542] Tue, 03 April 2012 13:12 Go to previous messageGo to next message
Antonio Garcia-Dominguez is currently offline Antonio Garcia-Dominguez
Messages: 309
Registered: January 2010
Senior Member
Hi Andrey,

Oh! Sorry, it seems I wasn't fully awake yet at the time Smile. Yes, you are right.

I'll have another look at the code in a while, then.

Cheers,
Antonio
Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835730 is a reply to message #835617] Tue, 03 April 2012 15:53 Go to previous messageGo to next message
Antonio Garcia-Dominguez is currently offline Antonio Garcia-Dominguez
Messages: 309
Registered: January 2010
Senior Member
Hi again,

I don't think that the short-circuit logic is wrong here. A model element is not valid as soon as it doesn't meet some constraint, so we shouldn't keep on evaluating the rest. The bug here is in the EvlValidator class itself: it shouldn't produce a NullPointerException in that case.

I've just committed a fix for EvlValidator that ensures that no NPEs will be produced if validate(...) has not been called before. Could you try it out?

Cheers,
Antonio
Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835741 is a reply to message #835730] Tue, 03 April 2012 16:01 Go to previous messageGo to next message
Ed Merks is currently offline Ed Merks
Messages: 26150
Registered: July 2009
Senior Member
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Antonio,<br>
<br>
I'm not sure the full context, but EMF's generated validators
generally take this kind of approach:<br>
<blockquote><small>  public boolean validateEClass(EClass eClass,
DiagnosticChain diagnostics, Map&lt;Object, Object&gt; context)<br>
  {<br>
    if (!validate_NoCircularContainment(eClass, diagnostics,
context)) return false;<br>
    boolean result = validate_EveryMultiplicityConforms(eClass,
diagnostics, context);<br>
    if (result || diagnostics != null) result &amp;=
validate_EveryDataValueConforms(eClass, diagnostics, context);<br>
    if (result || diagnostics != null) result &amp;=
validate_EveryReferenceIsContained(eClass, diagnostics,
context);<br>
    if (result || diagnostics != null) result &amp;=
validate_EveryBidirectionalReferenceIsPaired(eClass,
diagnostics, context);<br>
    if (result || diagnostics != null) result &amp;=
validate_EveryProxyResolves(eClass, diagnostics, context);<br>
    if (result || diagnostics != null) result &amp;=
validate_UniqueID(eClass, diagnostics, context);<br>
    if (result || diagnostics != null) result &amp;=
validate_EveryKeyUnique(eClass, diagnostics, context);<br>
    if (result || diagnostics != null) result &amp;=
validate_EveryMapEntryUnique(eClass, diagnostics, context);<br>
    if (result || diagnostics != null) result &amp;=
validateENamedElement_WellFormedName(eClass, diagnostics,
context);<br>
    if (result || diagnostics != null) result &amp;=
validateEClassifier_WellFormedInstanceTypeName(eClass,
diagnostics, context);<br>
    if (result || diagnostics != null) result &amp;=
validateEClassifier_UniqueTypeParameterNames(eClass,
diagnostics, context);<br>
    if (result || diagnostics != null) result &amp;=
validateEClass_InterfaceIsAbstract(eClass, diagnostics,
context);<br>
    if (result || diagnostics != null) result &amp;=
validateEClass_AtMostOneID(eClass, diagnostics, context);<br>
    if (result || diagnostics != null) result &amp;=
validateEClass_UniqueFeatureNames(eClass, diagnostics, context);<br>
    if (result || diagnostics != null) result &amp;=
validateEClass_UniqueOperationSignatures(eClass, diagnostics,
context);<br>
    if (result || diagnostics != null) result &amp;=
validateEClass_NoCircularSuperTypes(eClass, diagnostics,
context);<br>
    if (result || diagnostics != null) result &amp;=
validateEClass_WellFormedMapEntryClass(eClass, diagnostics,
context);<br>
    if (result || diagnostics != null) result &amp;=
validateEClass_ConsistentSuperTypes(eClass, diagnostics,
context);<br>
    if (result || diagnostics != null) result &amp;=
validateEClass_DisjointFeatureAndOperationSignatures(eClass,
diagnostics, context);<br>
    return result;<br>
  }</small><br>
</blockquote>
I.e., if diagnostics is null, then only the boolean result in
interesting, so as soon as it's false, no more constraints are
checked.  But if the client wants diagnostics (too), not just a
boolean result, then all the constraints are checked and all the
reasons for being invalid are collected...<br>
<br>
<br>
On 03/04/2012 8:53 AM, Antonio Garcia-Dominguez wrote:
<blockquote cite="mid:jlf6e8$q7$1@xxxxxxxxe.org" type="cite">Hi
again,
<br>
<br>
I don't think that the short-circuit logic is wrong here. A model
element is not valid as soon as it doesn't meet some constraint,
so we shouldn't keep on evaluating the rest. The bug here is in
the EvlValidator class itself: it shouldn't produce a
NullPointerException in that case.
<br>
<br>
I've just committed a fix for EvlValidator that ensures that no
NPEs will be produced if validate(...) has not been called before.
Could you try it out?
<br>
<br>
Cheers,
<br>
Antonio
<br>
</blockquote>
</body>
</html>
Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835793 is a reply to message #835730] Tue, 03 April 2012 17:22 Go to previous messageGo to next message
Andrey Skorikov is currently offline Andrey Skorikov
Messages: 8
Registered: September 2011
Junior Member
Hi Antonio,

thank you for the fix! I checked out the new version and the NPE is gone. However, I still think that skipping the remaining validators in case one of them fails is not the right thing to do. We want to see the results of running all the validators, such that fixing all the warnings/errors in the model should result in a completely valid model.

Cheers,
Andrey
Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835844 is a reply to message #835793] Tue, 03 April 2012 18:47 Go to previous messageGo to next message
Ed Merks is currently offline Ed Merks
Messages: 26150
Registered: July 2009
Senior Member
Andrey,

Yes, that's the basis for my comment about, in the case that diagnostics
are being collected, being sure to collect them all. It would be
annoying to fix a problem, only to have another one show up, and doing
that several times. Better to know about all the problem; after all,
one might cause another...


On 03/04/2012 10:22 AM, Andrey Skorikov wrote:
> Hi Antonio,
>
> thank you for the fix! I checked out the new version and the NPE is
> gone. However, I still think that skipping the remaining validators in
> case one of them fails is not the right thing to do. We want to see
> the results of running all the validators, such that fixing all the
> warnings/errors in the model should result in a completely valid model.
>
> Cheers,
> Andrey
Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835889 is a reply to message #835844] Tue, 03 April 2012 20:14 Go to previous messageGo to next message
Andrey Skorikov is currently offline Andrey Skorikov
Messages: 8
Registered: September 2011
Junior Member
Ed Merks wrote on Tue, 03 April 2012 14:47
It would be annoying to fix a problem, only to have another one show up, and doing that several times.


Exactly. Thank you. Smile
Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #836257 is a reply to message #835889] Wed, 04 April 2012 09:30 Go to previous messageGo to next message
Antonio Garcia-Dominguez is currently offline Antonio Garcia-Dominguez
Messages: 309
Registered: January 2010
Senior Member
Ed Merks: thank you for the insight! Running all validators when diagnostics are requested does seem like the most sensible option.

Andrey Skorikov: I've committed another version that mimics the approach shown by Ed Merks. If diagnostics is not null, it will always run all validators. Otherwise, it will stop at the first failed validation.
Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #836270 is a reply to message #836257] Wed, 04 April 2012 09:51 Go to previous message
Andrey Skorikov is currently offline Andrey Skorikov
Messages: 8
Registered: September 2011
Junior Member
Thanks Antonio! I tried it out. Everything works well. Smile
Previous Topic:Call Java methods from GMF Editor (generated with eugenia)
Next Topic:[Eugenia] Phantom nodes
Goto Forum:
  


Current Time: Thu Oct 30 16:13:43 GMT 2014

Powered by FUDForum. Page generated in 0.01727 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software