feat(rules): implement global rule
#137
Conversation
rules/event-reference.js
Outdated
| reporter.report(node.id, 'Event reference is missing name'); | ||
| } | ||
|
|
||
| const rootElement = getRootElement(node); |
There was a problem hiding this comment.
I'd rather implement this on bpmn:Definitions and search for globals; this ensures we don't rely on the hidden $parent property to discover the elements; it also will be computationally less expensive, as we have to compute bpmnElements only once, rather than n times for n global elements.
There was a problem hiding this comment.
You can then return false to stop further drill-down.
There was a problem hiding this comment.
I'd rather implement this on
bpmn:Definitionsand search for globals; this ensures we don't rely on the hidden$parentproperty to discover the elements; it also will be computationally less expensive, as we have to computebpmnElementsonly once, rather thanntimes fornglobal elements.
I hope I understood you correctly. I am now traversing from bpmn:Definitions downwards, collecting events and event definitions on the way and do the validation. See aad2302
There was a problem hiding this comment.
You did understand me correctly. 🙂
config/recommended.js
Outdated
| @@ -2,6 +2,7 @@ module.exports = { | |||
| rules: { | |||
| 'conditional-flows': 'error', | |||
| 'end-event-required': 'error', | |||
| 'event-reference': 'warn', | |||
There was a problem hiding this comment.
I'll internally discuss which name we want to give this rule.
|
Found one bug related to event definitions without refs, cf. 16c1774.
|
|
Ensured we stop traversing early (e239e32). |
Good catch, did not think of that. |
I assume it should also apply for |
Yes, it should also apply to |
|
Decision: We'll call this rule |
|
Executed via c1aadc8. |
A rule that verifies that global elements are properly used. Currently recognized global elements are: * `bpmn:Error` * `bpmn:Escalation` * `bpmn:Signal` * `bpmn:Message` For each of these elements proper usage implies: * element must have a name * element is used (referenced) from event definitions * there exists only a single element per type with a given name Related to camunda/camunda-modeler#4339
|
Squashed via f090f86. Thanks! |
nikku
changed the title
event-referenceglobal rule
Adds `global` rule to validate correct usage of global elements (Message, Error, Signal, ...). Cf. bpmn-io/bpmnlint#137
|
Thanks for helping me getting this merged 👍🏼 |
Adds `global` rule to validate correct usage of global elements (Message, Error, Signal, ...). Cf. bpmn-io/bpmnlint#137
Hey @nikku,
after our short discussion from yesterday I had a go at implementing a rule to validate event references:
I have provided test cases for the rules that demonstrate they actually work as intented.
I did not yet provide a documentation because I wanted to make sure that the implementation is correct before documenting it.
I was struggeling a bit with validating the usage of a reference and I'm certain that my approach could be improved (see https://github.com/strangelookingnerd/bpmnlint/blob/4e3515167c8d201088cef3a1304bc19be19ec2bb/rules/event-reference.js#L52-L103)
Further im not 100% sure that these checks only apply to
Error,EscalationandMessagebut maybe also other elements.This PR sure still is a little rough on the edges, but I'd be happy to further improve it in order to be merged.