Combining multiple expressions in RuleConfig creates false results

pujson6767 commented 7 months ago

I was used .flowScanner.json file the same mentioned in example

{
  "rules": {
    "APIVersion":
    {
        "severity": "error",
        "expression": "===58"
    },
    "FlowName":
    {
        "severity": "error",
        "expression": "[A-Za-z0-9]+_[A-Za-z0-9]+"
    }
  }
}

And files from example repository: Outdated_API_Version.flow-meta.xml, here I added only apiVersion tag compare to your example

<?xml version="1.0" encoding="UTF-8"?>
<Flow xmlns="http://soap.sforce.com/2006/04/metadata">
    <apiVersion>50.0</apiVersion>
    <description>This flow demonstrates a violation of the rule &quot;Outdated API Version&quot;.</description>
    <environments>Default</environments>
    <interviewLabel>Outdated API Version {!$Flow.CurrentDateTime}</interviewLabel>
    <label>Outdated API Version</label>
    <processMetadataValues>
        <name>BuilderType</name>
        <value>
            <stringValue>LightningFlowBuilder</stringValue>
        </value>
    </processMetadataValues>
    <processMetadataValues>
        <name>CanvasMode</name>
        <value>
            <stringValue>AUTO_LAYOUT_CANVAS</stringValue>
        </value>
    </processMetadataValues>
    <processMetadataValues>
        <name>OriginBuilderType</name>
        <value>
            <stringValue>LightningFlowBuilder</stringValue>
        </value>
    </processMetadataValues>
    <processType>Flow</processType>
    <screens>
        <name>mockscreen</name>
        <label>mockscreen</label>
        <locationX>176</locationX>
        <locationY>134</locationY>
        <allowBack>true</allowBack>
        <allowFinish>true</allowFinish>
        <allowPause>true</allowPause>
        <showFooter>true</showFooter>
        <showHeader>true</showHeader>
    </screens>
    <start>
        <locationX>50</locationX>
        <locationY>0</locationY>
        <connector>
            <targetReference>mockscreen</targetReference>
        </connector>
    </start>
    <status>Active</status>
</Flow>

FlowNamingConvention.flow-meta.xml

<?xml version="1.0" encoding="UTF-8"?>
<Flow xmlns="http://soap.sforce.com/2006/04/metadata">
    <apiVersion>58.0</apiVersion>
    <decisions>
        <name>isAccountsProvided</name>
        <label>isAccountsProvided</label>
        <locationX>176</locationX>
        <locationY>134</locationY>
        <defaultConnectorLabel>Default Outcome</defaultConnectorLabel>
        <rules>
            <name>NotNull</name>
            <conditionLogic>and</conditionLogic>
            <conditions>
                <leftValueReference>Accounts</leftValueReference>
                <operator>IsNull</operator>
                <rightValue>
                    <booleanValue>false</booleanValue>
                </rightValue>
            </conditions>
            <label>NotNull</label>
        </rules>
    </decisions>
    <description>This flow demonstrates a violation of the rule &quot;Flow Naming Convention&quot;.</description>
    <environments>Default</environments>
    <interviewLabel>Flow Naming Convention {!$Flow.CurrentDateTime}</interviewLabel>
    <label>Flow Naming Convention</label>
    <processMetadataValues>
        <name>BuilderType</name>
        <value>
            <stringValue>LightningFlowBuilder</stringValue>
        </value>
    </processMetadataValues>
    <processMetadataValues>
        <name>CanvasMode</name>
        <value>
            <stringValue>AUTO_LAYOUT_CANVAS</stringValue>
        </value>
    </processMetadataValues>
    <processMetadataValues>
        <name>OriginBuilderType</name>
        <value>
            <stringValue>LightningFlowBuilder</stringValue>
        </value>
    </processMetadataValues>
    <processType>AutoLaunchedFlow</processType>
    <start>
        <locationX>50</locationX>
        <locationY>0</locationY>
        <connector>
            <targetReference>isAccountsProvided</targetReference>
        </connector>
    </start>
    <status>Active</status>
    <variables>
        <name>Accounts</name>
        <dataType>SObject</dataType>
        <isCollection>true</isCollection>
        <isInput>true</isInput>
        <isOutput>false</isOutput>
        <objectType>Account</objectType>
    </variables>
</Flow>

Command what I used sfdx flow:scan --directory force-app/main/default/flows --json --config ./.flow-scanner.json

Result:

{
  "status": 1,
  "result": {
    "summary": {
      "flowsNumber": 2,
      "results": 3,
      "message": "A total of 3 results have been found in 2 flows.",
      "errorLevelsDetails": {}
    },
    "status": 1,
    "results": [
      {
        "violation": {
          "metaType": "attribute",
          "name": "58",
          "subtype": "apiVersion",
          "expression": "<58"
        },
        "name": "58",
        "metaType": "attribute",
        "type": "apiVersion",
        "details": {
          "expression": "<58"
        },
        "ruleDescription": "Introducing newer API components may lead to unexpected issues with older versions of Flows, as they might not align with the underlying mechanics. Starting from API version 50.0, the 'Api Version' attribute has been readily available on the Flow Object. To ensure smooth operation and reduce discrepancies between API versions, it is strongly advised to regularly update and maintain them.",
        "rule": "Outdated API Version",
        "flowName": "Flow Naming Convention",
        "flowType": "AutoLaunchedFlow",
        "severity": "error"
      },
      {
        "violation": {
          "metaType": "attribute",
          "name": "FlowNamingConvention",
          "subtype": "name",
          "expression": "<58"
        },
        "name": "FlowNamingConvention",
        "metaType": "attribute",
        "type": "name",
        "details": {
          "expression": "<58"
        },
        "ruleDescription": "The readability of a flow is of utmost importance. Establishing a naming convention for the Flow Name significantly enhances findability, searchability, and maintains overall consistency. It is advisable to include at least a domain and a brief description of the actions carried out in the flow, for instance, 'Service_OrderFulfillment'.",
        "rule": "Flow Naming Convention",
        "flowName": "Flow Naming Convention",
        "flowType": "AutoLaunchedFlow",
        "severity": "error"
      },
      {
        "violation": {
          "metaType": "attribute",
          "name": "Outdated_API_Version",
          "subtype": "name",
          "expression": "<58"
        },
        "name": "Outdated_API_Version",
        "metaType": "attribute",
        "type": "name",
        "details": {
          "expression": "<58"
        },
        "ruleDescription": "The readability of a flow is of utmost importance. Establishing a naming convention for the Flow Name significantly enhances findability, searchability, and maintains overall consistency. It is advisable to include at least a domain and a brief description of the actions carried out in the flow, for instance, 'Service_OrderFulfillment'.",
        "rule": "Flow Naming Convention",
        "flowName": "Outdated API Version",
        "flowType": "Flow",
        "severity": "error"
      }
    ]
  }
}

As we can see, there should be only 2 violations found, there are 3, also in details.expression there are always expression from first rule. I was tried to debug rules/APIVersion.js and rules/FlowName.js files from your npm package, and it's looking like always first expression "<58" is taken to APIVersion.js file but also for FlowName.js

Also, when config file not contain expression for FlowName the same wrong result appear.

{
  "rules": {
    "APIVersion":
    {
        "severity": "error",
        "expression": "<58"
    },
    "FlowName":
    {
        "severity": "error"
    }
  }
}

I would be grateful for an answer if maybe it is not a problem with the wrong creation of flowScanner.json, although I used the same as in the example. I hope the errors can be fixed

RubenHalman commented 7 months ago

@pujson6767 Sorry for my slow response. First of all, my compliments in your efforts to debug, I know its not easy! It sees your configuration is correct so I am not sure what is going on. I will try to reproduce this in the coming days. Do you use the latest version?

pujson6767 commented 7 months ago

I was use lightning-flow-scanner 2.18.0 (which is using lightning-flow-scanner-core 2.28.0)

RubenHalman commented 6 months ago

@pujson6767 I managed to find the issue and included a fix with the latest version of the core. I will soon publish this in both the sfdx and vsce solutions.

pujson6767 commented 6 months ago

Thank You! By the way, while I was using lightning-flow-scanner, I noticed that these properties in the JSON output can be useful:

"line" / "lineNumber": line number of file where violation occurred. I know it's hard to define exactly which line an issue belongs to, but it would be useful for adding comments on violation PRs indicating on which line the issue occurs.
"flowFileName": There is already "flowName" which is a Label of flow. In Salesforce, it is possible to use the same labels for flows (although it is known that this is not recommended). In large SF orgs, it sometimes happens that there are the same flow labels within the organization. After running the flow-scanner, from the output, it is not clear which "flowName" belongs to which flow.

Lightning-Flow-Scanner / lightning-flow-scanner-core

Combining multiple expressions in RuleConfig creates false results #68