fix: fix no-disconnected rule Closes #88

[david-d-le]

I think this will solve Issue #88

One of the connections is enough for events. Other elements need both incoming and outgoing present

[CLAassistant]

All committers have signed the CLA.

[nikku]

@david-d-le Thanks for your contribution.

Looking at #88, specifically https://github.com/bpmn-io/bpmnlint/issues/88#issuecomment-1223633347 I'm wondering why you check this in no-disconnected? For me the proper place would be end-event-required.

[david-d-le]

I thought that end-event-required rule is working fine. It checks that Process itself has end event.

So having a rule that will check that every Task has incoming and outgoing flow somewhere (doesn't need to be an end or start event at all) will also solve this.

[nikku]

The end-event-required rule explicitly states:

Explicitly modeling it [the end event] improves the understandability of drawn process diagrams.

I'd argue that https://github.com/bpmn-io/bpmnlint/issues/88 is a violation of that rule rather than the no-disconnected rule.

With end-event-required I enforce explicit modeling of end events, and no implicit ends.

[david-d-le]

Yes end-event-required rule checks explicit ends but only for Process and Subprocesses:

Ensures that every process and sub-process has an end event. ...

So, when there is an end event in Process then end-event-required evaluates to true which is fine and correct.

However in #88 the user wanted to ensure that activities have end events attached. That means that outgoing flow must be present, because that flow will flow into end event or other activity which will then flow into end event.

I was looking at your picture in your comment which is a way how I solved this problem. There it is ensured that task choose recipe and empty task below it has end event attached.

Or am I understanding this wrong?

[nikku]

So, when there is an end event in Process then end-event-required evaluates to true which is fine and correct.

The end-event-required rule documentation explicitly states:

Explicitly modeling it [the end event] improves the understandability of drawn process diagrams.

So the attached scenario is a violation of that end-event-required rule. As the end event behind the task is missing, when it should be present ("no implicit ends").

It feels redundant to add an additional rule (no-implicit-ends) if end-event-required is about the same thing. It feels wrong to trigger no-disconnected, as the task is clearly connected.

As we design and maintain rules we want to ensure that they are cohesive. A user may not care about end events being present (disable end-event-required) but still care about the fact that disconnected elements shall not be present.

[david-d-le]

So do you think that solution should be inside end-event-required rule?

I think that it will have my solutions logic but it will be inside end-event-required which will also be triggered (probably with different error message) for tasks without end?

[nikku]

So do you think that solution should be inside end-event-required rule?

Yes, this is what I'm thinking. I've reached out to other maintainers for their input on this matter.

[barmac]

Agreed. The problem we try to solve is that an element stops the execution flow before the token approaches an end event which more specific than "Element does not have outgoing connections".

This is still invalid according to the rule even though the task has an outgoing connection:

[david-d-le]

Agreed. The problem we try to solve is that an element stops the execution flow before the token approaches an end event which more specific than "Element does not have outgoing connections".

This is still invalid according to the rule even though the task has an outgoing connection:

Yes, but in this diagram it is Data output association and not a Sequence Flow which is considered as a connection by the rule.

[nikku]

@david-d-le This PR stalled and I'm not sure why. Do you plan to follow up?

My suggestion was to move the validation to end-event-required (https://github.com/bpmn-io/bpmnlint/pull/100#issuecomment-1473682818).

[david-d-le]

I didn't know that decision was final. I will move the logic into end-event-required rule as soon as I have time for it.

[nikku]

Closing this issue as it stalled. We'll fix it ourselves.