Be Able to Specify Path When Reporting Issue Through Reporter#report

[philippfromme]

When using Reporter#report one can only provide the element ID and the error message. There is no way to provide additional properties e.g. the path of the property that has caused the error to be reported. I'd image something along the lines of:

Reporter.report('ServiceTask_1', '<zeebe:TaskDefinition> must have <zeebe:type> property', {
  path: [ 'extensionElements', 'values', 0, 'zeebe:type' ]
});

Adding an optional third argument wouldn't introduce any breaking changes.

---

Related to https://github.com/bpmn-io/internal-docs/issues/430

[nikku]

To consider: Does a user need to construct the path herself? Or does bpmnlint take care of it?

[nikku]

The fundamental BPMNLint feature we're talking about here is: Be able to address sub-paths within an element.

[nikku]

Another way to think about it: We could also turn this thing upside down.

No path is introduced, instead elements are reported using the standard way:

Reporter.report(zeebeTaskDefinition, 'must have <zeebe:type> property');

// or

Reporter.report(buildPath(serviceTask, zebeeTaskDefinition), 'must have <zeebe:type> property');

[philippfromme]

@nikku That's a good question. We could introduce this as a general feature, yes.

[philippfromme]

The utility for getting the path would have to be available in the respective plugins (e.g. bpmnlint-plugin-camunda-compat). I'm not sure what Reporter.report(buildPath(serviceTask, zebeeTaskDefinition), 'must have <zeebe:type> property'); would look like? Reporter.report([ 'ServiceTask_1', 'extensionElements', 0 'type' ], 'must have <zeebe:type> property');? That would introduce a breaking change and also require us to parse the path assuming the first key is the element's ID. I'm not a huge fan of that idea. My current approach looks like this:

class Reporter {
  constructor({ moddleRoot, rule }) {
    this.rule = rule;
    this.moddleRoot = moddleRoot;
    this.messages = [];
    this.report = this.report.bind(this);
  }

  report(id, message, options) {
    let report = {
      id,
      message
    };

    if (options) {
      report = {
        ...report,
        options // optional
      };
    }

    this.messages.push(report);
  }
}

The rule itself would look something like this:

// ...

return {
  message: 'Element of type <zeebe:TaskDefinition> must have <zeebe:type> property',
  path: [ ...getPath(taskDefinition, node), 'zeebe:type' ] // get the path of `taskDefinition` relative to `node`
};

As @smbea is going to start working on camunda/bpmnlint-plugin-camunda-compat#7 we need to come to an agreement regarding the solution for this issue.

@nikku What are your thoughts? Shall we have a chat about this?

[nikku]

How would the CLI output look like in your idea?

[nikku]

We have to think this end-to-end, and if that means a major breaking change, I'd be fine with it.

[philippfromme]

I guess you'd expect something similar to:

> bpmnlint invoice.bpmn

/Projects/process-application/resources/invoice.bpmn
  ServiceTask_1#extensionElements.values.0    error    Element of type <zeebe:TaskDefinition> must have <zeebe:type> property    camunda-cloud-1-0

✖ 1 problem (1 error, 0 warnings)

In that case we'd probably make the path concept known to bpmnlint. 🤔

[philippfromme]

The API could still look like Reporter#report(id, message, path) with path being the only optional parameter. The CLI output could be formatted accordingly.

[nikku]

Had a short conversation with @philippfromme on the topic:

• It makes sense to build path awareness into the core:
  • Reporter#report(id, message, path: [string|number][])
  • Report path in a sensible way via the CLI, too
• We cannot rely on $parent for traversal (bpmnlint, bpmn-js-properties-panel)
  • Our usage of moddle does not guarantee that it is being maintained properly
  • If we decide to rely on it, our usage shall be fail-safe

Created bpmn-io/moddle#41 as a follow-up. I don't think we'll be able to tackle it in due time.

---

Sketch:

// getPath will be required not only by bpmnlint plugins (e.g. bpmnlint-plugin-camunda-compat)
// but also by bpmn-js-properties-panel
import { getPath } from '@bpmn-io/moddle-helpers';

// ...

reporter.report('ServiceTask_1', 'Task definition missing');
reporter.report('ServiceTask_1', 'Task definition type missing', [ 'extensionElements', 'values', 0, 'type' ]); // create path manually
reporter.report('ServiceTask_1', 'Task definition retries missing', [ ...getPath(moddleElement, parentModdleElement), 'retries' ]); // create path using utility

console.log(reporter.messages);

// [
//   {
//     id: 'ServiceTask_1',
//     message: 'Task definition missing'
//   },
//   {
//     id: 'ServiceTask_1',
//     message: 'Task definition type missing',
//     path: [ 'extensionElements', 'values', 0, 'type' ]
//   },
//   {
//     id: 'ServiceTask_1',
//     message: 'Task definition retries missing',
//     path: [ 'extensionElements', 'values', 0, 'retries' ]
//   }
// ]

CLI output:

> bpmnlint invoice.bpmn

/Projects/process-application/resources/invoice.bpmn
 ServiceTask_1    error    Task definition missing    camunda-cloud-1-0
 ServiceTask_1#extensionElements.values.0.type    error    Task definition type missing    camunda-cloud-1-0
 ServiceTask_1#extensionElements.values.0.retries    error    Task definition retries missing    camunda-cloud-1-0

✖ 3 problems (3 errors, 0 warnings)