Eclipse Community Forums: Epsilon » Bug in org.eclipse.epsilon.evl.emf.validation

Help

Home

Home » Modeling » Epsilon » Bug in org.eclipse.epsilon.evl.emf.validation

Show: Today's Messages :: Show Polls :: Message Navigator

Bug in org.eclipse.epsilon.evl.emf.validation [message #834757]

Mon, 02 April 2012 11:04

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

Report message to a moderator

Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835531 is a reply to message #834757]

Tue, 03 April 2012 11:06

Antonio Garcia-Dominguez

Messages: 594
Registered: January 2010
Location: Birmingham, UK

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

Report message to a moderator

Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835542 is a reply to message #835531]

Tue, 03 April 2012 11:25

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

Report message to a moderator

Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835617 is a reply to message #835542]

Tue, 03 April 2012 13:12

Antonio Garcia-Dominguez

Messages: 594
Registered: January 2010
Location: Birmingham, UK

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

Report message to a moderator

Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835730 is a reply to message #835617]

Tue, 03 April 2012 15:53

Antonio Garcia-Dominguez

Messages: 594
Registered: January 2010
Location: Birmingham, UK

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

Report message to a moderator

Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835741 is a reply to message #835730]

Tue, 03 April 2012 16:01

Ed Merks

Messages: 33140
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, 
 
I'm not sure the full context, but EMF's generated validators
generally take this kind of approach: 
<blockquote> public boolean validateEClass(EClass eClass,
DiagnosticChain diagnostics, Map<Object, Object> context) 
{ 
 if (!validate_NoCircularContainment(eClass, diagnostics,
context)) return false; 
 boolean result = validate_EveryMultiplicityConforms(eClass,
diagnostics, context); 
 if (result || diagnostics != null) result &=
validate_EveryDataValueConforms(eClass, diagnostics, context); 
 if (result || diagnostics != null) result &=
validate_EveryReferenceIsContained(eClass, diagnostics,
context); 
 if (result || diagnostics != null) result &=
validate_EveryBidirectionalReferenceIsPaired(eClass,
diagnostics, context); 
 if (result || diagnostics != null) result &=
validate_EveryProxyResolves(eClass, diagnostics, context); 
 if (result || diagnostics != null) result &=
validate_UniqueID(eClass, diagnostics, context); 
 if (result || diagnostics != null) result &=
validate_EveryKeyUnique(eClass, diagnostics, context); 
 if (result || diagnostics != null) result &=
validate_EveryMapEntryUnique(eClass, diagnostics, context); 
 if (result || diagnostics != null) result &=
validateENamedElement_WellFormedName(eClass, diagnostics,
context); 
 if (result || diagnostics != null) result &=
validateEClassifier_WellFormedInstanceTypeName(eClass,
diagnostics, context); 
 if (result || diagnostics != null) result &=
validateEClassifier_UniqueTypeParameterNames(eClass,
diagnostics, context); 
 if (result || diagnostics != null) result &=
validateEClass_InterfaceIsAbstract(eClass, diagnostics,
context); 
 if (result || diagnostics != null) result &=
validateEClass_AtMostOneID(eClass, diagnostics, context); 
 if (result || diagnostics != null) result &=
validateEClass_UniqueFeatureNames(eClass, diagnostics, context); 
 if (result || diagnostics != null) result &=
validateEClass_UniqueOperationSignatures(eClass, diagnostics,
context); 
 if (result || diagnostics != null) result &=
validateEClass_NoCircularSuperTypes(eClass, diagnostics,
context); 
 if (result || diagnostics != null) result &=
validateEClass_WellFormedMapEntryClass(eClass, diagnostics,
context); 
 if (result || diagnostics != null) result &=
validateEClass_ConsistentSuperTypes(eClass, diagnostics,
context); 
 if (result || diagnostics != null) result &=
validateEClass_DisjointFeatureAndOperationSignatures(eClass,
diagnostics, context); 
 return result; 
} 
</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... 
 
 
On 03/04/2012 8:53 AM, Antonio Garcia-Dominguez wrote:
<blockquote cite="mid:jlf6e8$q7$1@xxxxxxxxe.org" type="cite">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
 
</blockquote>
</body>
</html>

Ed Merks
Professional Support: https://www.macromodeling.com/

Report message to a moderator

Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835793 is a reply to message #835730]

Tue, 03 April 2012 17:22

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

Report message to a moderator

Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835844 is a reply to message #835793]

Tue, 03 April 2012 18:47

Ed Merks

Messages: 33140
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

Ed Merks
Professional Support: https://www.macromodeling.com/

Report message to a moderator

Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #835889 is a reply to message #835844]

Tue, 03 April 2012 20:14

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

Report message to a moderator

Re: Bug in org.eclipse.epsilon.evl.emf.validation [message #836257 is a reply to message #835889]

Wed, 04 April 2012 09:30

Antonio Garcia-Dominguez

Messages: 594
Registered: January 2010
Location: Birmingham, UK

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.

Report message to a moderator