search

eMoflon / emoflon-ibex-ui

Independent and reusable GUI components for graph transformation rules and Triple Graph Grammars
GNU General Public License v3.0
5 stars 2 forks source link

TGG editor: Insufficient validation for node types for refined rules #73

Open patrickrobrecht opened 6 years ago

patrickrobrecht commented 6 years ago

Example: Super rules: Declared node p: Person and p: Female Refined rules: p: Male does not show an error in the editor (but transformations fail which is written on the console).

anthonyanjorin commented 5 years ago

Any progress? Have you been able to reproduce with a concrete example?

patrickrobrecht commented 5 years ago

For the GT part a check for this has been implemented:

LegionaryCohort commented 5 years ago

I was unable to reproduce the problem. I've tested this using the BenchmarxFamiliesToPersons TGG test project: The abstract rule FamilyMember2Person contains a node definition ++ p : Person, the refined rule FatherToMale overwrites that to ++ p : Male. Changing the definition in the abstract rule to ++ p : Female causes no errors when running MODELGEN. Src, trg and corr model instances are produced containing the affected nodes. It appears that the node definition in the refined rule completely overwrites the definition in the abstract rule.

As far as I understand it, this refinement shouldn't be valid, but it appears to work fine regardless. Am I misunderstanding the issue?

LegionaryCohort commented 5 years ago

I should note that I had to add some max-rule-counts to the MODELGENStopCriterion to ensure that it doesn't simply create large heaps of empty families. This shouldn't have any effect on this issue though, should it..?

anthonyanjorin commented 5 years ago

I should note that I had to add some max-rule-counts to the MODELGENStopCriterion to ensure that it doesn't simply create large heaps of empty families. This shouldn't have any effect on this issue though, should it..?

No not at all.

anthonyanjorin commented 5 years ago

I was unable to reproduce the problem. I've tested this using the BenchmarxFamiliesToPersons TGG test project: The abstract rule FamilyMember2Person contains a node definition ++ p : Person, the refined rule FatherToMale overwrites that to ++ p : Male. Changing the definition in the abstract rule to ++ p : Female causes no errors when running MODELGEN. Src, trg and corr model instances are produced containing the affected nodes. It appears that the node definition in the refined rule completely overwrites the definition in the abstract rule.

As far as I understand it, this refinement shouldn't be valid, but it appears to work fine regardless. Am I misunderstanding the issue?

Hmm... this is a difficult question/decision. I think the easiest answer is that Male should not be able to override Female in a refinement, as it is not a subclass. The argument for this is that arbitrary overriding would require checking all possible links attached to the node. Overriding only with subclasses is automatically safe in this regard. Allowing arbitrary overriding would be flexible, but also a bit "dirty": for example, it would work for some time, but suddenly stop working as soon as a forbidden link is added in the rule.

So I would vote for forbidding this (even if it limits specification freedom a bit).

What do you think?

LegionaryCohort commented 5 years ago

I agree that only subclasses should be allowed, especially since this conforms to the intuitive understanding of inheritance/refinement.

This aside, the description of this issue mentions that transformations fail with a corresponding error output to the console. This was not the case for me, which leaves me somewhat uncertain as to whether I'm understanding correctly what the issue exactly is.

anthonyanjorin commented 5 years ago

Apparently some recent changes have made things more robust... so all that is needed here is a validation rule in the xtext editor with a useful error message. I’m sure enough that we understand the issue correctly.

LegionaryCohort commented 5 years ago

If I'm seeing this right, it should be sufficient to check each rule's source and target patterns against the patterns of the rule's supertypes (recursively, not just against the direct supertype). Is there any other place where the type-check should be done? The correspondence types defined in the schema cannot be overwritten, right?

anthonyanjorin commented 5 years ago

@LegionaryCohort Have you seen the tips provided by @patrickrobrecht above? You might be able to just adapt the code to the case of TGGs...

anthonyanjorin commented 5 years ago

Please also check if this issue is not very much related to: https://github.com/eMoflon/emoflon-ibex/issues/369

LegionaryCohort commented 5 years ago

I've already looked at the code in the GTValidator. I'm expecting the code for TGGs to end up looking very similar, but the GT code uses some utility methods that don't exist for TGGs as far as I can tell.

As for the other issue, as far as I can tell they are certainly related, but still seperate issues. If I'm understanding the other Issue correctly, it's dealing with a diamond problem caused by being able to refine multiple rules. This Issue on the other hand is a much more basic type-checking problem.

anthonyanjorin commented 5 years ago

As far as I can tell the issues are one and the same - our refinement flattening Code first combines all super rules with the current rule, and then picks out a clear subtype from all candidates (with the same name). If this candidate is not unique, we should complain and list all conflicting candidates. I think the other issue would be solved by this as well.

But just start somewhere and yell if you can’t make any progress.

LegionaryCohort commented 5 years ago

All right, given the additional knowledge about how the refinements are flattened, you've convinced me that they are indeed pretty much the same.

As discussed in Slack, I'll put this issue on hold for now.