From 68d32b6bdd3b8c5c65e8d39bce9286536a5cb848 Mon Sep 17 00:00:00 2001 From: Ruben Halman Date: Wed, 12 Feb 2025 12:42:50 +0100 Subject: [PATCH 01/11] move naming logic from Flow Model to ParseFlows, and add metadata type preprocessing on Flow Model(description, apiversion, etc) --- src/main/libs/ParseFlows.ts | 25 +++++++++++++------------ src/main/models/Flow.ts | 36 +++++++++++++++--------------------- tests/models/Flow.test.ts | 6 +++--- 3 files changed, 31 insertions(+), 36 deletions(-) diff --git a/src/main/libs/ParseFlows.ts b/src/main/libs/ParseFlows.ts index 8671219..bdb1bf2 100644 --- a/src/main/libs/ParseFlows.ts +++ b/src/main/libs/ParseFlows.ts @@ -1,25 +1,26 @@ -import * as p from "path"; +import p from "path"; import { Flow } from "../models/Flow"; -import fs from "fs"; +import * as fs from "fs"; import { convert } from "xmlbuilder2"; import { ParsedFlow } from "../models/ParsedFlow"; export async function ParseFlows(selectedUris: string[]): Promise { - const parseResults: ParsedFlow[] = []; - for (const uri of selectedUris) { + let parseResults: ParsedFlow[] = []; + for (let uri of selectedUris) { try { - const normalizedURI = p.normalize(uri); - const content = await fs.readFileSync(normalizedURI); - const xmlString = content.toString(); + let normalizedURI = p.normalize(uri); + let fsPath = p.resolve(normalizedURI); + let flowName = p.basename(p.basename(fsPath), p.extname(fsPath)); + if (flowName.includes(".")) { + flowName = flowName.split(".")[0]; + } + let content = await fs.readFileSync(normalizedURI); + let xmlString = content.toString(); const flowObj = convert(xmlString, { format: "object" }); - parseResults.push(new ParsedFlow(uri, new Flow(uri, flowObj))); + parseResults.push(new ParsedFlow(uri, new Flow(flowName, flowObj))); } catch (e) { parseResults.push(new ParsedFlow(uri, undefined, e.errorMessage)); } } return parseResults; } - -export function parse(selectedUris: string[]): Promise { - return ParseFlows(selectedUris); -} diff --git a/src/main/models/Flow.ts b/src/main/models/Flow.ts index 119e823..bd029f1 100644 --- a/src/main/models/Flow.ts +++ b/src/main/models/Flow.ts @@ -2,15 +2,12 @@ import { FlowNode } from "./FlowNode"; import { FlowMetadata } from "./FlowMetadata"; import { FlowElement } from "./FlowElement"; import { FlowVariable } from "./FlowVariable"; -import * as p from "path"; import { FlowResource } from "./FlowResource"; -import { XMLSerializedAsObject } from "xmlbuilder2/lib/interfaces"; -import { create } from "xmlbuilder2"; export class Flow { public label: string; public xmldata; - public name?: string; + public name: string; public interviewLabel?: string; public processType?; public processMetadataValues?; @@ -18,11 +15,13 @@ export class Flow { public start?; public startElementReference?; public status?; - public fsPath; - public root?; public elements?: FlowElement[]; public startReference; public triggerOrder?: number; + public decisions; + public loops; + public description; + public apiVersion; private flowVariables = ["choices", "constants", "dynamicChoiceSets", "formulas", "variables"]; private flowResources = ["textTemplates", "stages"]; @@ -65,20 +64,11 @@ export class Flow { "waits", ]; - constructor(path?: string, data?: unknown); - constructor(path: string, data?: unknown) { - if (path) { - this.fsPath = p.resolve(path); - let flowName = p.basename(p.basename(this.fsPath), p.extname(this.fsPath)); - if (flowName.includes(".")) { - flowName = flowName.split(".")[0]; - } - this.name = flowName; - } - if (data) { - const hasFlowElement = !!data && typeof data === "object" && "Flow" in data; - if (hasFlowElement) { - this.xmldata = (data as XMLSerializedAsObject).Flow; + constructor(flowName: string, data?: any) { + this.name = flowName; + if(data){ + if (data.Flow) { + this.xmldata = data.Flow; } else this.xmldata = data; this.preProcessNodes(); } @@ -93,6 +83,8 @@ export class Flow { this.start = this.xmldata.start; this.status = this.xmldata.status; this.type = this.xmldata.processType; + this.description = this.xmldata.description; + this.apiVersion = this.xmldata.apiVersion; this.triggerOrder = this.xmldata.triggerOrder; const allNodes: (FlowVariable | FlowNode | FlowMetadata)[] = []; for (const nodeType in this.xmldata) { @@ -136,6 +128,8 @@ export class Flow { } } this.elements = allNodes; + this.decisions = this.elements.filter((node) => node.subtype === "decisions"); + this.loops = this.elements.filter(node => node.subtype === 'loops'); this.startReference = this.findStart(); } @@ -151,7 +145,7 @@ export class Flow { return n.subtype === "start"; }) ) { - const startElement = flowElements.find((n) => { + let startElement = flowElements.find((n) => { return n.subtype === "start"; }); start = startElement.connectors[0]["reference"]; diff --git a/tests/models/Flow.test.ts b/tests/models/Flow.test.ts index da0a74a..4764727 100644 --- a/tests/models/Flow.test.ts +++ b/tests/models/Flow.test.ts @@ -10,7 +10,7 @@ describe("Flow Model", () => { }); it("should print as xml when correct parameters", () => { - const sut: Flow = new Flow(); + const sut: Flow = new Flow('flow A'); sut.xmldata = { "@xmlns": "http://soap.sforce.com/2006/04/metadata", "@xmlns:xsi": "http://www.w3.org/2001/XMLSchema-instance", @@ -34,7 +34,7 @@ describe("Flow Model", () => { }); it("should never throw an exception for toXMLString", () => { - const sut: Flow = new Flow(); + const sut: Flow = new Flow('flow B'); sut.xmldata = { test: "test" }; jest.spyOn(xmlbuilder, "create").mockReturnValue({ root: () => ({ @@ -62,7 +62,7 @@ describe("Flow Model", () => { }; it("should throw an exception for bad document", async () => { - const sut: Flow = new Flow(); + const sut: Flow = new Flow('flow C'); const errors = getError(sut["generateDoc"]); expect(errors).toBeTruthy(); expect(errors).not.toBeInstanceOf(NoErrorThrownError); From 99f8bbafd8f77fbe0071e0e0f2085152bb0b627b Mon Sep 17 00:00:00 2001 From: Ruben Halman Date: Wed, 12 Feb 2025 13:04:10 +0100 Subject: [PATCH 02/11] cyclomatic complexity --- src/main/libs/ParseFlows.ts | 14 +- src/main/rules/CyclomaticComplexity.ts | 50 ++ tests/CyclomaticComplexity.test.ts | 48 ++ tests/UnconnectedElement.test.ts | 6 +- tests/UnsafeRunningContext.test.ts | 8 +- .../Cyclomatic_Complexity.flow-meta.xml | 653 ++++++++++++++++++ 6 files changed, 765 insertions(+), 14 deletions(-) create mode 100644 src/main/rules/CyclomaticComplexity.ts create mode 100644 tests/CyclomaticComplexity.test.ts create mode 100644 tests/xmlfiles/Cyclomatic_Complexity.flow-meta.xml diff --git a/src/main/libs/ParseFlows.ts b/src/main/libs/ParseFlows.ts index bdb1bf2..dd18669 100644 --- a/src/main/libs/ParseFlows.ts +++ b/src/main/libs/ParseFlows.ts @@ -4,18 +4,18 @@ import * as fs from "fs"; import { convert } from "xmlbuilder2"; import { ParsedFlow } from "../models/ParsedFlow"; -export async function ParseFlows(selectedUris: string[]): Promise { - let parseResults: ParsedFlow[] = []; - for (let uri of selectedUris) { +export async function parse(selectedUris: string[]): Promise { + const parseResults: ParsedFlow[] = []; + for (const uri of selectedUris) { try { - let normalizedURI = p.normalize(uri); - let fsPath = p.resolve(normalizedURI); + const normalizedURI = p.normalize(uri); + const fsPath = p.resolve(normalizedURI); let flowName = p.basename(p.basename(fsPath), p.extname(fsPath)); if (flowName.includes(".")) { flowName = flowName.split(".")[0]; } - let content = await fs.readFileSync(normalizedURI); - let xmlString = content.toString(); + const content = await fs.readFileSync(normalizedURI); + const xmlString = content.toString(); const flowObj = convert(xmlString, { format: "object" }); parseResults.push(new ParsedFlow(uri, new Flow(flowName, flowObj))); } catch (e) { diff --git a/src/main/rules/CyclomaticComplexity.ts b/src/main/rules/CyclomaticComplexity.ts new file mode 100644 index 0000000..29b79e6 --- /dev/null +++ b/src/main/rules/CyclomaticComplexity.ts @@ -0,0 +1,50 @@ +import { RuleCommon } from "../models/RuleCommon"; +import * as core from "../internals/internals"; + +export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefinition { + constructor() { + super({ + name: "CyclomaticComplexity", + label: "Cyclomatic Complexity", + description: + "The number of loops and decision rules, plus the number of decisions. Use subflows to reduce the cyclomatic complexity within a single flow, ensuring maintainability and simplicity.", + supportedTypes: core.FlowType.backEndTypes, + docRefs: [], + isConfigurable: true, + autoFixable: false, + }); + } + + public execute(flow: core.Flow, options?: { threshold: string }): core.RuleResult { + // Set Threshold + let threshold = 0; + + if (options && options.threshold) { + threshold = +options.threshold; + } else { + threshold = 25; + } + + // Calculate Cyclomatic Complexity based on the number of decision rules and loops, adding the number of decisions plus 1. + let cyclomaticComplexity = 1; + for (const decision of flow.decisions) { + const rules = decision.element["rules"]; + if (Array.isArray(rules)) { + cyclomaticComplexity += rules.length + 1; + } else { + cyclomaticComplexity += 1; + } + } + cyclomaticComplexity += flow.loops.length; + + const results: core.ResultDetails[] = []; + if (cyclomaticComplexity > threshold) { + results.push( + new core.ResultDetails( + new core.FlowAttribute("" + cyclomaticComplexity, "CyclomaticComplexity", ">" + threshold) + ) + ); + } + return new core.RuleResult(this, results); + } +} diff --git a/tests/CyclomaticComplexity.test.ts b/tests/CyclomaticComplexity.test.ts new file mode 100644 index 0000000..0afe805 --- /dev/null +++ b/tests/CyclomaticComplexity.test.ts @@ -0,0 +1,48 @@ +import * as core from "../src"; +import * as path from "path"; + +import { describe, it, expect } from "@jest/globals"; + +describe("CyclomaticComplexity ", () => { + const example_uri = path.join(__dirname, "./xmlfiles/Cyclomatic_Complexity.flow-meta.xml"); + const other_uri = path.join(__dirname, "./xmlfiles/SOQL_Query_In_A_Loop.flow-meta.xml"); + + it("should have a result when there are more than 25 decision options", async () => { + const flows = await core.parse([example_uri]); + const results: core.ScanResult[] = core.scan(flows); + const occurringResults = results[0].ruleResults.filter((rule) => rule.occurs); + expect(occurringResults).toHaveLength(1); + expect(occurringResults[0].ruleName).toBe("CyclomaticComplexity"); + }); + + it("should have no result when value is below threshold", async () => { + const flows = await core.parse([other_uri]); + const ruleConfig = { + rules: { + CyclomaticComplexity: { + severity: "error", + }, + }, + }; + + const results: core.ScanResult[] = core.scan(flows, ruleConfig); + const occurringResults = results[0].ruleResults.filter((rule) => rule.occurs); + expect(occurringResults).toHaveLength(0); + }); + + it("should have a result when value surpasses a configured threshold", async () => { + const flows = await core.parse([other_uri]); + const ruleConfig = { + rules: { + CyclomaticComplexity: { + threshold: 1, + severity: "error", + }, + }, + }; + + const results: core.ScanResult[] = core.scan(flows, ruleConfig); + const occurringResults = results[0].ruleResults.filter((rule) => rule.occurs); + expect(occurringResults).toHaveLength(1); + }); +}); diff --git a/tests/UnconnectedElement.test.ts b/tests/UnconnectedElement.test.ts index 78bdd01..1cef55b 100644 --- a/tests/UnconnectedElement.test.ts +++ b/tests/UnconnectedElement.test.ts @@ -1,7 +1,7 @@ import * as core from "../src"; import * as path from "path"; -import { ParseFlows } from "../src/main/libs/ParseFlows"; +import { parse } from "../src/main/libs/ParseFlows"; import { ParsedFlow } from "../src/main/models/ParsedFlow"; import { UnconnectedElement } from "../src/main/rules/UnconnectedElement"; @@ -16,7 +16,7 @@ describe("UnconnectedElement", () => { __dirname, "./xmlfiles/Unconnected_Element.flow-meta.xml" ); - const parsed: ParsedFlow = (await ParseFlows([connectedElementTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await parse([connectedElementTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unconnectedElementRule.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(true); expect(ruleResult.details).not.toHaveLength(0); @@ -30,7 +30,7 @@ describe("UnconnectedElement", () => { __dirname, "./xmlfiles/Unconnected_Element_Async.flow-meta.xml" ); - const parsed: ParsedFlow = (await ParseFlows([connectedElementTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await parse([connectedElementTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unconnectedElementRule.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(true); ruleResult.details.forEach((ruleDetail) => { diff --git a/tests/UnsafeRunningContext.test.ts b/tests/UnsafeRunningContext.test.ts index bac0bee..3ffe4d2 100644 --- a/tests/UnsafeRunningContext.test.ts +++ b/tests/UnsafeRunningContext.test.ts @@ -1,7 +1,7 @@ import * as core from "../src"; import * as path from "path"; -import { ParseFlows } from "../src/main/libs/ParseFlows"; +import { parse } from "../src/main/libs/ParseFlows"; import { ParsedFlow } from "../src/main/models/ParsedFlow"; import { describe, it, expect } from "@jest/globals"; @@ -15,7 +15,7 @@ describe("UnsafeRunningContext", () => { __dirname, "./xmlfiles/Unsafe_Running_Context.flow-meta.xml" ); - const parsed: ParsedFlow = (await ParseFlows([unsafeContextTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await parse([unsafeContextTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unsafeRunningContext.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(true); expect(ruleResult.details).not.toHaveLength(0); @@ -27,7 +27,7 @@ describe("UnsafeRunningContext", () => { __dirname, "./xmlfiles/Unsafe_Running_Context_WithSharing.flow-meta.xml" ); - const parsed: ParsedFlow = (await ParseFlows([unsafeContextTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await parse([unsafeContextTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unsafeRunningContext.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(false); expect(ruleResult.details).toHaveLength(0); @@ -38,7 +38,7 @@ describe("UnsafeRunningContext", () => { __dirname, "./xmlfiles/Unsafe_Running_Context_Default.flow-meta.xml" ); - const parsed: ParsedFlow = (await ParseFlows([unsafeContextTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await parse([unsafeContextTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unsafeRunningContext.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(false); expect(ruleResult.details).toHaveLength(0); diff --git a/tests/xmlfiles/Cyclomatic_Complexity.flow-meta.xml b/tests/xmlfiles/Cyclomatic_Complexity.flow-meta.xml new file mode 100644 index 0000000..f42363a --- /dev/null +++ b/tests/xmlfiles/Cyclomatic_Complexity.flow-meta.xml @@ -0,0 +1,653 @@ + + + + createtesttask + + 2822 + 434 + FeedItem.NewTaskFromFeedItem + quickAction + CurrentTransaction + + contextId + + $Flow.CurrentRecord + + + FeedItem.NewTaskFromFeedItem + 1 + + 60.0 + + assignvalue1 + + 50 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + assignvalue10 + + 2426 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + assignvalue11 + + 2690 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + assignvalue12 + + 5594 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + assignvalue2 + + 314 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + assignvalue3 + + 578 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + assignvalue4 + + 842 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + assignvalue5 + + 1106 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + assignvalue6 + + 1370 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + assignvalue7 + + 1634 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + assignvalue8 + + 1898 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + assignvalue9 + + 2162 + 242 + + testvar + Assign + + 1.0 + + + + createtesttask + + + + Example decision above default threshold + DecideOutcome + + 2822 + 134 + + assignvalue12 + + Default Outcome + + outcome1 + and + + $Flow.CurrentRecord + Contains + + a + + + + assignvalue1 + + + + + outcome2 + and + + $Flow.CurrentRecord + Contains + + b + + + + assignvalue2 + + + + + outcome3 + and + + $Flow.CurrentRecord + Contains + + c + + + + assignvalue3 + + + + + outcome4 + and + + $Flow.CurrentRecord + Contains + + d + + + + assignvalue4 + + + + + outcome5 + and + + $Flow.CurrentRecord + Contains + + e + + + + assignvalue5 + + + + + outcome6 + and + + $Flow.CurrentRecord + Contains + + f + + + + assignvalue6 + + + + + outcome7 + and + + $Flow.CurrentRecord + Contains + + g + + + + assignvalue7 + + + + + outcome8 + and + + $Flow.CurrentRecord + Contains + + h + + + + assignvalue8 + + + + + outcome9 + and + + $Flow.CurrentRecord + Contains + + i + + + + assignvalue9 + + + + + outcome10 + and + + $Flow.CurrentRecord + Contains + + j + + + + assignvalue10 + + + + + outcome11 + and + + $Flow.CurrentRecord + Contains + + k + + + + assignvalue11 + + + + + outcome12 + and + + testvar + EqualTo + + 12.0 + + + + createtesttask + + + + + outcome13 + and + + testvar + EqualTo + + 13.0 + + + + createtesttask + + + + + outcome14 + and + + testvar + EqualTo + + 14.0 + + + + createtesttask + + + + + outcome15 + and + + testvar + EqualTo + + 15.0 + + + + createtesttask + + + + + outcome16 + and + + testvar + EqualTo + + 16.0 + + + + createtesttask + + + + + outcome17 + and + + testvar + EqualTo + + 17.0 + + + + createtesttask + + + + + outcome18 + and + + testvar + EqualTo + + 18.0 + + + + createtesttask + + + + + outcome19 + and + + testvar + EqualTo + + 19.0 + + + + createtesttask + + + + + outcome20 + and + + testvar + EqualTo + + 20.0 + + + + createtesttask + + + + + outcome21 + and + + testvar + EqualTo + + 21.0 + + + + createtesttask + + + + + outcome22 + and + + testvar + EqualTo + + 22.0 + + + + createtesttask + + + + + outcome23 + and + + testvar + EqualTo + + 23.0 + + + + createtesttask + + + + + outcome24 + and + + testvar + EqualTo + + 24.0 + + + + createtesttask + + + + + outcome25 + and + + testvar + EqualTo + + 25.0 + + + + createtesttask + + + + + outcome26 + and + + testvar + EqualTo + + 26.0 + + + + createtesttask + + + + + This flow demonstrates a violation of the rule 'CyclomaticComplexity' + Default + Cyclomatic {!$Flow.CurrentDateTime} + + + BuilderType + + LightningFlowBuilder + + + + CanvasMode + + AUTO_LAYOUT_CANVAS + + + + OriginBuilderType + + LightningFlowBuilder + + + AutoLaunchedFlow + + 2696 + 0 + + DecideOutcome + + + Active + + testvar + Number + false + false + false + 2 + + From e6d7d20b527dc170af3ecdd052f13fc2adc00a19 Mon Sep 17 00:00:00 2001 From: Ruben Halman Date: Wed, 12 Feb 2025 13:09:07 +0100 Subject: [PATCH 03/11] cyclomatic complexity --- tests/CyclomaticComplexity.test.ts | 4 ++-- tests/models/Flow.test.ts | 15 ++++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/CyclomaticComplexity.test.ts b/tests/CyclomaticComplexity.test.ts index 0afe805..2ec4037 100644 --- a/tests/CyclomaticComplexity.test.ts +++ b/tests/CyclomaticComplexity.test.ts @@ -11,8 +11,8 @@ describe("CyclomaticComplexity ", () => { const flows = await core.parse([example_uri]); const results: core.ScanResult[] = core.scan(flows); const occurringResults = results[0].ruleResults.filter((rule) => rule.occurs); - expect(occurringResults).toHaveLength(1); - expect(occurringResults[0].ruleName).toBe("CyclomaticComplexity"); + expect(occurringResults.length).toBeGreaterThanOrEqual(1); + // expect(occurringResults[0].ruleName).toBe("CyclomaticComplexity"); }); it("should have no result when value is below threshold", async () => { diff --git a/tests/models/Flow.test.ts b/tests/models/Flow.test.ts index 4764727..f9d9830 100644 --- a/tests/models/Flow.test.ts +++ b/tests/models/Flow.test.ts @@ -10,7 +10,7 @@ describe("Flow Model", () => { }); it("should print as xml when correct parameters", () => { - const sut: Flow = new Flow('flow A'); + const sut: Flow = new Flow("flow A"); sut.xmldata = { "@xmlns": "http://soap.sforce.com/2006/04/metadata", "@xmlns:xsi": "http://www.w3.org/2001/XMLSchema-instance", @@ -27,14 +27,15 @@ describe("Flow Model", () => { picklistObject: "Task", }, }; - const out = sut.toXMLString(); - expect(out).toBeTruthy(); - expect(out).toMatch(''); - expect(out).toMatch(''); + // todo fix test ! + // const out = sut.toXMLString(); + // expect(out).toBeTruthy(); + // expect(out).toMatch(''); + // expect(out).toMatch(''); }); it("should never throw an exception for toXMLString", () => { - const sut: Flow = new Flow('flow B'); + const sut: Flow = new Flow("flow B"); sut.xmldata = { test: "test" }; jest.spyOn(xmlbuilder, "create").mockReturnValue({ root: () => ({ @@ -62,7 +63,7 @@ describe("Flow Model", () => { }; it("should throw an exception for bad document", async () => { - const sut: Flow = new Flow('flow C'); + const sut: Flow = new Flow("flow C"); const errors = getError(sut["generateDoc"]); expect(errors).toBeTruthy(); expect(errors).not.toBeInstanceOf(NoErrorThrownError); From 517581457ae83f86419557bea690a39681dfbda2 Mon Sep 17 00:00:00 2001 From: Ruben Halman Date: Wed, 12 Feb 2025 14:58:43 +0100 Subject: [PATCH 04/11] fix tests and add new rule to default rules --- src/main/rules/CyclomaticComplexity.ts | 4 +++- src/main/store/DefaultRuleStore.ts | 2 ++ tests/CyclomaticComplexity.test.ts | 14 +++++++++++--- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/main/rules/CyclomaticComplexity.ts b/src/main/rules/CyclomaticComplexity.ts index 29b79e6..8677591 100644 --- a/src/main/rules/CyclomaticComplexity.ts +++ b/src/main/rules/CyclomaticComplexity.ts @@ -35,7 +35,9 @@ export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefini cyclomaticComplexity += 1; } } - cyclomaticComplexity += flow.loops.length; + if (flow.loops && flow.loops.length > 0) { + cyclomaticComplexity += flow.loops.length; + } const results: core.ResultDetails[] = []; if (cyclomaticComplexity > threshold) { diff --git a/src/main/store/DefaultRuleStore.ts b/src/main/store/DefaultRuleStore.ts index 5043d59..d4126f6 100644 --- a/src/main/store/DefaultRuleStore.ts +++ b/src/main/store/DefaultRuleStore.ts @@ -1,6 +1,7 @@ import { APIVersion } from "../rules/APIVersion"; import { AutoLayout } from "../rules/AutoLayout"; import { CopyAPIName } from "../rules/CopyAPIName"; +import { CyclomaticComplexity } from "../rules/CyclomaticComplexity"; import { DMLStatementInLoop } from "../rules/DMLStatementInLoop"; import { DuplicateDMLOperation } from "../rules/DuplicateDMLOperation"; import { FlowDescription } from "../rules/FlowDescription"; @@ -20,6 +21,7 @@ export const DefaultRuleStore: object = { APIVersion, AutoLayout, CopyAPIName, + CyclomaticComplexity, DMLStatementInLoop, DuplicateDMLOperation, FlowDescription, diff --git a/tests/CyclomaticComplexity.test.ts b/tests/CyclomaticComplexity.test.ts index 2ec4037..d8b159a 100644 --- a/tests/CyclomaticComplexity.test.ts +++ b/tests/CyclomaticComplexity.test.ts @@ -6,13 +6,20 @@ import { describe, it, expect } from "@jest/globals"; describe("CyclomaticComplexity ", () => { const example_uri = path.join(__dirname, "./xmlfiles/Cyclomatic_Complexity.flow-meta.xml"); const other_uri = path.join(__dirname, "./xmlfiles/SOQL_Query_In_A_Loop.flow-meta.xml"); + const defaultConfig = { + rules: { + CyclomaticComplexity: { + severity: "error", + }, + }, + }; it("should have a result when there are more than 25 decision options", async () => { const flows = await core.parse([example_uri]); - const results: core.ScanResult[] = core.scan(flows); + const results: core.ScanResult[] = core.scan(flows, defaultConfig); const occurringResults = results[0].ruleResults.filter((rule) => rule.occurs); - expect(occurringResults.length).toBeGreaterThanOrEqual(1); - // expect(occurringResults[0].ruleName).toBe("CyclomaticComplexity"); + expect(occurringResults).toHaveLength(1); + expect(occurringResults[0].ruleName).toBe("CyclomaticComplexity"); }); it("should have no result when value is below threshold", async () => { @@ -44,5 +51,6 @@ describe("CyclomaticComplexity ", () => { const results: core.ScanResult[] = core.scan(flows, ruleConfig); const occurringResults = results[0].ruleResults.filter((rule) => rule.occurs); expect(occurringResults).toHaveLength(1); + expect(occurringResults[0].ruleName).toBe("CyclomaticComplexity"); }); }); From f049f3ee2432168a5b4455f1eb24a4cb290b5ebf Mon Sep 17 00:00:00 2001 From: Ruben Halman Date: Thu, 13 Feb 2025 00:59:24 +0100 Subject: [PATCH 05/11] use parse as part of core --- tests/UnconnectedElement.test.ts | 5 ++--- tests/UnsafeRunningContext.test.ts | 7 +++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/UnconnectedElement.test.ts b/tests/UnconnectedElement.test.ts index 1cef55b..51846fb 100644 --- a/tests/UnconnectedElement.test.ts +++ b/tests/UnconnectedElement.test.ts @@ -1,7 +1,6 @@ import * as core from "../src"; import * as path from "path"; -import { parse } from "../src/main/libs/ParseFlows"; import { ParsedFlow } from "../src/main/models/ParsedFlow"; import { UnconnectedElement } from "../src/main/rules/UnconnectedElement"; @@ -16,7 +15,7 @@ describe("UnconnectedElement", () => { __dirname, "./xmlfiles/Unconnected_Element.flow-meta.xml" ); - const parsed: ParsedFlow = (await parse([connectedElementTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await core.parse([connectedElementTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unconnectedElementRule.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(true); expect(ruleResult.details).not.toHaveLength(0); @@ -30,7 +29,7 @@ describe("UnconnectedElement", () => { __dirname, "./xmlfiles/Unconnected_Element_Async.flow-meta.xml" ); - const parsed: ParsedFlow = (await parse([connectedElementTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await core.parse([connectedElementTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unconnectedElementRule.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(true); ruleResult.details.forEach((ruleDetail) => { diff --git a/tests/UnsafeRunningContext.test.ts b/tests/UnsafeRunningContext.test.ts index 3ffe4d2..48e55bb 100644 --- a/tests/UnsafeRunningContext.test.ts +++ b/tests/UnsafeRunningContext.test.ts @@ -1,7 +1,6 @@ import * as core from "../src"; import * as path from "path"; -import { parse } from "../src/main/libs/ParseFlows"; import { ParsedFlow } from "../src/main/models/ParsedFlow"; import { describe, it, expect } from "@jest/globals"; @@ -15,7 +14,7 @@ describe("UnsafeRunningContext", () => { __dirname, "./xmlfiles/Unsafe_Running_Context.flow-meta.xml" ); - const parsed: ParsedFlow = (await parse([unsafeContextTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await core.parse([unsafeContextTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unsafeRunningContext.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(true); expect(ruleResult.details).not.toHaveLength(0); @@ -27,7 +26,7 @@ describe("UnsafeRunningContext", () => { __dirname, "./xmlfiles/Unsafe_Running_Context_WithSharing.flow-meta.xml" ); - const parsed: ParsedFlow = (await parse([unsafeContextTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await core.parse([unsafeContextTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unsafeRunningContext.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(false); expect(ruleResult.details).toHaveLength(0); @@ -38,7 +37,7 @@ describe("UnsafeRunningContext", () => { __dirname, "./xmlfiles/Unsafe_Running_Context_Default.flow-meta.xml" ); - const parsed: ParsedFlow = (await parse([unsafeContextTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await core.parse([unsafeContextTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unsafeRunningContext.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(false); expect(ruleResult.details).toHaveLength(0); From bc44216764a1f5659cbd47dc042097450874c125 Mon Sep 17 00:00:00 2001 From: Ruben Halman Date: Thu, 13 Feb 2025 01:17:42 +0100 Subject: [PATCH 06/11] remove flow model changes and apply loop count to rule --- src/main/models/Flow.ts | 14 +++----------- src/main/rules/CyclomaticComplexity.ts | 10 +++++++--- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/main/models/Flow.ts b/src/main/models/Flow.ts index bd029f1..ee17c16 100644 --- a/src/main/models/Flow.ts +++ b/src/main/models/Flow.ts @@ -18,10 +18,6 @@ export class Flow { public elements?: FlowElement[]; public startReference; public triggerOrder?: number; - public decisions; - public loops; - public description; - public apiVersion; private flowVariables = ["choices", "constants", "dynamicChoiceSets", "formulas", "variables"]; private flowResources = ["textTemplates", "stages"]; @@ -64,9 +60,9 @@ export class Flow { "waits", ]; - constructor(flowName: string, data?: any) { + constructor(flowName: string, data?: { Flow: Flow }) { this.name = flowName; - if(data){ + if (data) { if (data.Flow) { this.xmldata = data.Flow; } else this.xmldata = data; @@ -83,8 +79,6 @@ export class Flow { this.start = this.xmldata.start; this.status = this.xmldata.status; this.type = this.xmldata.processType; - this.description = this.xmldata.description; - this.apiVersion = this.xmldata.apiVersion; this.triggerOrder = this.xmldata.triggerOrder; const allNodes: (FlowVariable | FlowNode | FlowMetadata)[] = []; for (const nodeType in this.xmldata) { @@ -128,8 +122,6 @@ export class Flow { } } this.elements = allNodes; - this.decisions = this.elements.filter((node) => node.subtype === "decisions"); - this.loops = this.elements.filter(node => node.subtype === 'loops'); this.startReference = this.findStart(); } @@ -145,7 +137,7 @@ export class Flow { return n.subtype === "start"; }) ) { - let startElement = flowElements.find((n) => { + const startElement = flowElements.find((n) => { return n.subtype === "start"; }); start = startElement.connectors[0]["reference"]; diff --git a/src/main/rules/CyclomaticComplexity.ts b/src/main/rules/CyclomaticComplexity.ts index 8677591..7808ad4 100644 --- a/src/main/rules/CyclomaticComplexity.ts +++ b/src/main/rules/CyclomaticComplexity.ts @@ -27,7 +27,11 @@ export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefini // Calculate Cyclomatic Complexity based on the number of decision rules and loops, adding the number of decisions plus 1. let cyclomaticComplexity = 1; - for (const decision of flow.decisions) { + + const flowDecisions = flow.elements.filter((node) => node.subtype === "decisions"); + const flowLoops = flow.elements.filter((node) => node.subtype === "loops"); + + for (const decision of flowDecisions) { const rules = decision.element["rules"]; if (Array.isArray(rules)) { cyclomaticComplexity += rules.length + 1; @@ -35,8 +39,8 @@ export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefini cyclomaticComplexity += 1; } } - if (flow.loops && flow.loops.length > 0) { - cyclomaticComplexity += flow.loops.length; + if (flowLoops && flowLoops.length > 0) { + cyclomaticComplexity += flowLoops.length; } const results: core.ResultDetails[] = []; From b28e238e5693b1d553ac70f6372c7fae9ea4b1a8 Mon Sep 17 00:00:00 2001 From: Ruben Halman Date: Thu, 13 Feb 2025 01:20:31 +0100 Subject: [PATCH 07/11] revert unnessecary stylistic changes --- src/main/libs/ParseFlows.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/libs/ParseFlows.ts b/src/main/libs/ParseFlows.ts index dd18669..9ef4a45 100644 --- a/src/main/libs/ParseFlows.ts +++ b/src/main/libs/ParseFlows.ts @@ -1,6 +1,6 @@ -import p from "path"; +import * as p from "path"; import { Flow } from "../models/Flow"; -import * as fs from "fs"; +import fs from "fs"; import { convert } from "xmlbuilder2"; import { ParsedFlow } from "../models/ParsedFlow"; From c74ef13ebb91be527ffdc7e36a727429da2ed3ac Mon Sep 17 00:00:00 2001 From: junners Date: Wed, 12 Feb 2025 19:34:54 -0800 Subject: [PATCH 08/11] fix: prefer flow parametic polymorphism on constructor --- src/main/libs/ParseFlows.ts | 7 +------ src/main/models/Flow.ts | 24 +++++++++++++++++++----- src/main/rules/CyclomaticComplexity.ts | 20 +++++++++----------- tests/UnconnectedElement.test.ts | 5 +++-- tests/UnsafeRunningContext.test.ts | 7 ++++--- tests/models/Flow.test.ts | 15 +++++++-------- 6 files changed, 43 insertions(+), 35 deletions(-) diff --git a/src/main/libs/ParseFlows.ts b/src/main/libs/ParseFlows.ts index 9ef4a45..02950a0 100644 --- a/src/main/libs/ParseFlows.ts +++ b/src/main/libs/ParseFlows.ts @@ -9,15 +9,10 @@ export async function parse(selectedUris: string[]): Promise { for (const uri of selectedUris) { try { const normalizedURI = p.normalize(uri); - const fsPath = p.resolve(normalizedURI); - let flowName = p.basename(p.basename(fsPath), p.extname(fsPath)); - if (flowName.includes(".")) { - flowName = flowName.split(".")[0]; - } const content = await fs.readFileSync(normalizedURI); const xmlString = content.toString(); const flowObj = convert(xmlString, { format: "object" }); - parseResults.push(new ParsedFlow(uri, new Flow(flowName, flowObj))); + parseResults.push(new ParsedFlow(uri, new Flow(uri, flowObj))); } catch (e) { parseResults.push(new ParsedFlow(uri, undefined, e.errorMessage)); } diff --git a/src/main/models/Flow.ts b/src/main/models/Flow.ts index ee17c16..119e823 100644 --- a/src/main/models/Flow.ts +++ b/src/main/models/Flow.ts @@ -2,12 +2,15 @@ import { FlowNode } from "./FlowNode"; import { FlowMetadata } from "./FlowMetadata"; import { FlowElement } from "./FlowElement"; import { FlowVariable } from "./FlowVariable"; +import * as p from "path"; import { FlowResource } from "./FlowResource"; +import { XMLSerializedAsObject } from "xmlbuilder2/lib/interfaces"; +import { create } from "xmlbuilder2"; export class Flow { public label: string; public xmldata; - public name: string; + public name?: string; public interviewLabel?: string; public processType?; public processMetadataValues?; @@ -15,6 +18,8 @@ export class Flow { public start?; public startElementReference?; public status?; + public fsPath; + public root?; public elements?: FlowElement[]; public startReference; public triggerOrder?: number; @@ -60,11 +65,20 @@ export class Flow { "waits", ]; - constructor(flowName: string, data?: { Flow: Flow }) { - this.name = flowName; + constructor(path?: string, data?: unknown); + constructor(path: string, data?: unknown) { + if (path) { + this.fsPath = p.resolve(path); + let flowName = p.basename(p.basename(this.fsPath), p.extname(this.fsPath)); + if (flowName.includes(".")) { + flowName = flowName.split(".")[0]; + } + this.name = flowName; + } if (data) { - if (data.Flow) { - this.xmldata = data.Flow; + const hasFlowElement = !!data && typeof data === "object" && "Flow" in data; + if (hasFlowElement) { + this.xmldata = (data as XMLSerializedAsObject).Flow; } else this.xmldata = data; this.preProcessNodes(); } diff --git a/src/main/rules/CyclomaticComplexity.ts b/src/main/rules/CyclomaticComplexity.ts index 7808ad4..bc0c584 100644 --- a/src/main/rules/CyclomaticComplexity.ts +++ b/src/main/rules/CyclomaticComplexity.ts @@ -15,21 +15,19 @@ export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefini }); } - public execute(flow: core.Flow, options?: { threshold: string }): core.RuleResult { - // Set Threshold - let threshold = 0; + private defaultThreshold: number = 25; - if (options && options.threshold) { - threshold = +options.threshold; - } else { - threshold = 25; - } + public execute(flow: core.Flow, options?: { threshold: number }): core.RuleResult { + // Set Threshold + const threshold = options?.threshold ?? this.defaultThreshold; // Calculate Cyclomatic Complexity based on the number of decision rules and loops, adding the number of decisions plus 1. let cyclomaticComplexity = 1; - const flowDecisions = flow.elements.filter((node) => node.subtype === "decisions"); - const flowLoops = flow.elements.filter((node) => node.subtype === "loops"); + const flowDecisions = flow?.elements?.filter( + (node) => node.subtype === "decisions" + ) as core.FlowElement[]; + const flowLoops = flow?.elements?.filter((node) => node.subtype === "loops"); for (const decision of flowDecisions) { const rules = decision.element["rules"]; @@ -47,7 +45,7 @@ export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefini if (cyclomaticComplexity > threshold) { results.push( new core.ResultDetails( - new core.FlowAttribute("" + cyclomaticComplexity, "CyclomaticComplexity", ">" + threshold) + new core.FlowAttribute(`${cyclomaticComplexity}`, "CyclomaticComplexity", `>${threshold}`) ) ); } diff --git a/tests/UnconnectedElement.test.ts b/tests/UnconnectedElement.test.ts index 51846fb..1cef55b 100644 --- a/tests/UnconnectedElement.test.ts +++ b/tests/UnconnectedElement.test.ts @@ -1,6 +1,7 @@ import * as core from "../src"; import * as path from "path"; +import { parse } from "../src/main/libs/ParseFlows"; import { ParsedFlow } from "../src/main/models/ParsedFlow"; import { UnconnectedElement } from "../src/main/rules/UnconnectedElement"; @@ -15,7 +16,7 @@ describe("UnconnectedElement", () => { __dirname, "./xmlfiles/Unconnected_Element.flow-meta.xml" ); - const parsed: ParsedFlow = (await core.parse([connectedElementTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await parse([connectedElementTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unconnectedElementRule.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(true); expect(ruleResult.details).not.toHaveLength(0); @@ -29,7 +30,7 @@ describe("UnconnectedElement", () => { __dirname, "./xmlfiles/Unconnected_Element_Async.flow-meta.xml" ); - const parsed: ParsedFlow = (await core.parse([connectedElementTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await parse([connectedElementTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unconnectedElementRule.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(true); ruleResult.details.forEach((ruleDetail) => { diff --git a/tests/UnsafeRunningContext.test.ts b/tests/UnsafeRunningContext.test.ts index 48e55bb..3ffe4d2 100644 --- a/tests/UnsafeRunningContext.test.ts +++ b/tests/UnsafeRunningContext.test.ts @@ -1,6 +1,7 @@ import * as core from "../src"; import * as path from "path"; +import { parse } from "../src/main/libs/ParseFlows"; import { ParsedFlow } from "../src/main/models/ParsedFlow"; import { describe, it, expect } from "@jest/globals"; @@ -14,7 +15,7 @@ describe("UnsafeRunningContext", () => { __dirname, "./xmlfiles/Unsafe_Running_Context.flow-meta.xml" ); - const parsed: ParsedFlow = (await core.parse([unsafeContextTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await parse([unsafeContextTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unsafeRunningContext.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(true); expect(ruleResult.details).not.toHaveLength(0); @@ -26,7 +27,7 @@ describe("UnsafeRunningContext", () => { __dirname, "./xmlfiles/Unsafe_Running_Context_WithSharing.flow-meta.xml" ); - const parsed: ParsedFlow = (await core.parse([unsafeContextTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await parse([unsafeContextTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unsafeRunningContext.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(false); expect(ruleResult.details).toHaveLength(0); @@ -37,7 +38,7 @@ describe("UnsafeRunningContext", () => { __dirname, "./xmlfiles/Unsafe_Running_Context_Default.flow-meta.xml" ); - const parsed: ParsedFlow = (await core.parse([unsafeContextTestFile])).pop() as ParsedFlow; + const parsed: ParsedFlow = (await parse([unsafeContextTestFile])).pop() as ParsedFlow; const ruleResult: core.RuleResult = unsafeRunningContext.execute(parsed.flow as core.Flow); expect(ruleResult.occurs).toBe(false); expect(ruleResult.details).toHaveLength(0); diff --git a/tests/models/Flow.test.ts b/tests/models/Flow.test.ts index f9d9830..da0a74a 100644 --- a/tests/models/Flow.test.ts +++ b/tests/models/Flow.test.ts @@ -10,7 +10,7 @@ describe("Flow Model", () => { }); it("should print as xml when correct parameters", () => { - const sut: Flow = new Flow("flow A"); + const sut: Flow = new Flow(); sut.xmldata = { "@xmlns": "http://soap.sforce.com/2006/04/metadata", "@xmlns:xsi": "http://www.w3.org/2001/XMLSchema-instance", @@ -27,15 +27,14 @@ describe("Flow Model", () => { picklistObject: "Task", }, }; - // todo fix test ! - // const out = sut.toXMLString(); - // expect(out).toBeTruthy(); - // expect(out).toMatch(''); - // expect(out).toMatch(''); + const out = sut.toXMLString(); + expect(out).toBeTruthy(); + expect(out).toMatch(''); + expect(out).toMatch(''); }); it("should never throw an exception for toXMLString", () => { - const sut: Flow = new Flow("flow B"); + const sut: Flow = new Flow(); sut.xmldata = { test: "test" }; jest.spyOn(xmlbuilder, "create").mockReturnValue({ root: () => ({ @@ -63,7 +62,7 @@ describe("Flow Model", () => { }; it("should throw an exception for bad document", async () => { - const sut: Flow = new Flow("flow C"); + const sut: Flow = new Flow(); const errors = getError(sut["generateDoc"]); expect(errors).toBeTruthy(); expect(errors).not.toBeInstanceOf(NoErrorThrownError); From 828d2b61e6365926c2b526a94867313b712fc879 Mon Sep 17 00:00:00 2001 From: junners Date: Wed, 12 Feb 2025 20:35:01 -0800 Subject: [PATCH 09/11] test: add unit tests for cyclomatic complexity --- jest.config.ts | 24 +------ src/main/models/FlowElement.ts | 2 +- src/main/rules/CyclomaticComplexity.ts | 10 +-- tests/CyclomaticComplexity.test.ts | 87 +++++++++++++++++++++++--- 4 files changed, 85 insertions(+), 38 deletions(-) diff --git a/jest.config.ts b/jest.config.ts index 0818614..7dcd13f 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -172,30 +172,8 @@ const config: Config = { "^.+\\.(t|j)sx?$": [ "@swc/jest", { - $schema: "https://swc.rs/schema.json", sourceMaps: "inline", - module: { - type: "es6", - strictMode: true, - noInterop: false, - resolveFully: false, - }, - jsc: { - externalHelpers: false, - target: "es2015", - parser: { - syntax: "typescript", - tsx: true, - decorators: true, - dynamicImport: true, - }, - transform: { - legacyDecorator: true, - decoratorMetadata: false, - }, - keepClassNames: true, - baseUrl: ".", - }, + minify: false, }, ], }, diff --git a/src/main/models/FlowElement.ts b/src/main/models/FlowElement.ts index 13cf07f..7436cea 100644 --- a/src/main/models/FlowElement.ts +++ b/src/main/models/FlowElement.ts @@ -1,7 +1,7 @@ export class FlowElement { public subtype: string; public metaType: string; - public element: string | object = {}; + public element: string | object[] | object = {}; public connectors?: object[]; public name?: string; public locationX?: string; diff --git a/src/main/rules/CyclomaticComplexity.ts b/src/main/rules/CyclomaticComplexity.ts index bc0c584..19f21a2 100644 --- a/src/main/rules/CyclomaticComplexity.ts +++ b/src/main/rules/CyclomaticComplexity.ts @@ -17,6 +17,8 @@ export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefini private defaultThreshold: number = 25; + private cyclomaticComplexityUnit: number = 0; + public execute(flow: core.Flow, options?: { threshold: number }): core.RuleResult { // Set Threshold const threshold = options?.threshold ?? this.defaultThreshold; @@ -29,7 +31,7 @@ export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefini ) as core.FlowElement[]; const flowLoops = flow?.elements?.filter((node) => node.subtype === "loops"); - for (const decision of flowDecisions) { + for (const decision of flowDecisions || []) { const rules = decision.element["rules"]; if (Array.isArray(rules)) { cyclomaticComplexity += rules.length + 1; @@ -37,9 +39,9 @@ export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefini cyclomaticComplexity += 1; } } - if (flowLoops && flowLoops.length > 0) { - cyclomaticComplexity += flowLoops.length; - } + cyclomaticComplexity += flowLoops?.length ?? 0; + + this.cyclomaticComplexityUnit = cyclomaticComplexity; // for unit testing const results: core.ResultDetails[] = []; if (cyclomaticComplexity > threshold) { diff --git a/tests/CyclomaticComplexity.test.ts b/tests/CyclomaticComplexity.test.ts index d8b159a..f0fdcaf 100644 --- a/tests/CyclomaticComplexity.test.ts +++ b/tests/CyclomaticComplexity.test.ts @@ -1,8 +1,9 @@ -import * as core from "../src"; import * as path from "path"; - import { describe, it, expect } from "@jest/globals"; +import * as core from "../src"; +import { CyclomaticComplexity } from "../src/main/rules/CyclomaticComplexity"; + describe("CyclomaticComplexity ", () => { const example_uri = path.join(__dirname, "./xmlfiles/Cyclomatic_Complexity.flow-meta.xml"); const other_uri = path.join(__dirname, "./xmlfiles/SOQL_Query_In_A_Loop.flow-meta.xml"); @@ -16,6 +17,7 @@ describe("CyclomaticComplexity ", () => { it("should have a result when there are more than 25 decision options", async () => { const flows = await core.parse([example_uri]); + debugger; const results: core.ScanResult[] = core.scan(flows, defaultConfig); const occurringResults = results[0].ruleResults.filter((rule) => rule.occurs); expect(occurringResults).toHaveLength(1); @@ -24,15 +26,8 @@ describe("CyclomaticComplexity ", () => { it("should have no result when value is below threshold", async () => { const flows = await core.parse([other_uri]); - const ruleConfig = { - rules: { - CyclomaticComplexity: { - severity: "error", - }, - }, - }; - const results: core.ScanResult[] = core.scan(flows, ruleConfig); + const results: core.ScanResult[] = core.scan(flows, defaultConfig); const occurringResults = results[0].ruleResults.filter((rule) => rule.occurs); expect(occurringResults).toHaveLength(0); }); @@ -53,4 +48,76 @@ describe("CyclomaticComplexity ", () => { expect(occurringResults).toHaveLength(1); expect(occurringResults[0].ruleName).toBe("CyclomaticComplexity"); }); + + it("should correctly count the number of decisions and underlying rules one level", () => { + const sut = new CyclomaticComplexity(); + const raw = { + elements: [ + { + subtype: "decisions", + element: { + rules: [{}, {}, {}], + }, + }, + ], + } as Partial; + const given = raw as core.Flow; + sut.execute(given); + expect(sut["cyclomaticComplexityUnit"]).toBe(5); + }); + + it("should correctly count the number of decisions and underlying rules multi level", () => { + const sut = new CyclomaticComplexity(); + const raw = { + elements: [ + { + subtype: "decisions", + element: { + rules: [{}, {}, {}], + }, + }, + { subtype: "decisions", element: { rules: [{}] } }, + ], + } as Partial; + const given = raw as core.Flow; + sut.execute(given); + expect(sut["cyclomaticComplexityUnit"]).toBe(7); + }); + + it("should not throw an exception when theres no elements at all", () => { + const sut = new CyclomaticComplexity(); + const raw = { + elements: [], + } as Partial; + const given = raw as core.Flow; + expect(() => { + sut.execute(given); + }).not.toThrow(); + }); + + it("should not throw an exception when element isn't present", () => { + const sut = new CyclomaticComplexity(); + const raw = {} as Partial; + const given = raw as core.Flow; + expect(() => { + sut.execute(given); + }).not.toThrow(); + }); + + it("should correctly count the number of loops", () => { + const sut = new CyclomaticComplexity(); + const raw = { + elements: [ + { + subtype: "loops", + }, + { + subtype: "loops", + }, + ], + } as Partial; + const given = raw as core.Flow; + sut.execute(given); + expect(sut["cyclomaticComplexityUnit"]).toBe(3); + }); }); From 2a99a21427c36eb9c620198ebc32a74a16405046 Mon Sep 17 00:00:00 2001 From: junners Date: Wed, 12 Feb 2025 21:15:16 -0800 Subject: [PATCH 10/11] docs: add cyclomatic complexity to default docs --- readme.md | 1 + src/main/rules/CyclomaticComplexity.ts | 32 +++++++++++++++++--------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/readme.md b/readme.md index 77f1ddd..81657ca 100644 --- a/readme.md +++ b/readme.md @@ -32,6 +32,7 @@ _An Extensible Rule Engine capable of conducting static analysis on the metadata | **Unsafe Running Context** ([`UnsafeRunningContext`](https://github.com/Lightning-Flow-Scanner/lightning-flow-scanner-core/tree/master/src/main/rules/UnsafeRunningContext.ts)) | This flow is configured to run in System Mode without Sharing. This system context grants all running users the permission to view and edit all data in your org. Running a flow in System Mode without Sharing can lead to unsafe data access. | | **Same Record Field Updates** ([`SameRecordFieldUpdates`](https://github.com/Lightning-Flow-Scanner/lightning-flow-scanner-core/tree/master/src/main/rules/SameRecordFieldUpdates.ts)) | Much like triggers, before contexts can update the same record by accessing the trigger variables `$Record` without needing to invoke a DML. | | **Trigger Order** ([`TriggerOrder`](https://github.com/Lightning-Flow-Scanner/lightning-flow-scanner-core/tree/master/src/main/rules/TriggerOrder.ts)) | Guarantee your flow execution order with the Trigger Order property introduced in Spring '22 | +| **Cyclomatic Complexity** ([`CyclomaticComplexity`](https://github.com/Lightning-Flow-Scanner/lightning-flow-scanner-core/tree/master/src/main/rules/TriggerOrder.ts)) | The number of loops and decision rules, plus the number of decisions. Use subflows to reduce the cyclomatic complexity within a single flow, ensuring maintainability and simplicity. | ## Core Functions diff --git a/src/main/rules/CyclomaticComplexity.ts b/src/main/rules/CyclomaticComplexity.ts index 19f21a2..9d573b6 100644 --- a/src/main/rules/CyclomaticComplexity.ts +++ b/src/main/rules/CyclomaticComplexity.ts @@ -3,16 +3,26 @@ import * as core from "../internals/internals"; export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefinition { constructor() { - super({ - name: "CyclomaticComplexity", - label: "Cyclomatic Complexity", - description: - "The number of loops and decision rules, plus the number of decisions. Use subflows to reduce the cyclomatic complexity within a single flow, ensuring maintainability and simplicity.", - supportedTypes: core.FlowType.backEndTypes, - docRefs: [], - isConfigurable: true, - autoFixable: false, - }); + super( + { + name: "CyclomaticComplexity", + label: "Cyclomatic Complexity", + description: `The number of loops and decision rules, plus the number of decisions. + Use a combination of 1) subflows and 2) breakdown flows into multiple trigger ordered flows, + to reduce the cyclomatic complexity within a single flow, ensuring maintainability and simplicity.`, + supportedTypes: core.FlowType.backEndTypes, + docRefs: [ + { + label: `Cyclomatic complexity is a software metric used to indicate the complexity of a program. + It is a quantitative measure of the number of linearly independent paths through a program's source code.`, + path: "https://en.wikipedia.org/wiki/Cyclomatic_complexity", + }, + ], + isConfigurable: true, + autoFixable: false, + }, + { severity: "note" } + ); } private defaultThreshold: number = 25; @@ -21,7 +31,7 @@ export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefini public execute(flow: core.Flow, options?: { threshold: number }): core.RuleResult { // Set Threshold - const threshold = options?.threshold ?? this.defaultThreshold; + const threshold = options?.threshold || this.defaultThreshold; // Calculate Cyclomatic Complexity based on the number of decision rules and loops, adding the number of decisions plus 1. let cyclomaticComplexity = 1; From c4377da7b482bdc9e5c9c6a11e7ca7eca27fa25c Mon Sep 17 00:00:00 2001 From: junners Date: Wed, 12 Feb 2025 21:17:53 -0800 Subject: [PATCH 11/11] docs: change verbiage on cyclomatic complexity --- readme.md | 2 +- src/main/rules/CyclomaticComplexity.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 81657ca..2b2a99a 100644 --- a/readme.md +++ b/readme.md @@ -32,7 +32,7 @@ _An Extensible Rule Engine capable of conducting static analysis on the metadata | **Unsafe Running Context** ([`UnsafeRunningContext`](https://github.com/Lightning-Flow-Scanner/lightning-flow-scanner-core/tree/master/src/main/rules/UnsafeRunningContext.ts)) | This flow is configured to run in System Mode without Sharing. This system context grants all running users the permission to view and edit all data in your org. Running a flow in System Mode without Sharing can lead to unsafe data access. | | **Same Record Field Updates** ([`SameRecordFieldUpdates`](https://github.com/Lightning-Flow-Scanner/lightning-flow-scanner-core/tree/master/src/main/rules/SameRecordFieldUpdates.ts)) | Much like triggers, before contexts can update the same record by accessing the trigger variables `$Record` without needing to invoke a DML. | | **Trigger Order** ([`TriggerOrder`](https://github.com/Lightning-Flow-Scanner/lightning-flow-scanner-core/tree/master/src/main/rules/TriggerOrder.ts)) | Guarantee your flow execution order with the Trigger Order property introduced in Spring '22 | -| **Cyclomatic Complexity** ([`CyclomaticComplexity`](https://github.com/Lightning-Flow-Scanner/lightning-flow-scanner-core/tree/master/src/main/rules/TriggerOrder.ts)) | The number of loops and decision rules, plus the number of decisions. Use subflows to reduce the cyclomatic complexity within a single flow, ensuring maintainability and simplicity. | +| **Cyclomatic Complexity** ([`CyclomaticComplexity`](https://github.com/Lightning-Flow-Scanner/lightning-flow-scanner-core/tree/master/src/main/rules/TriggerOrder.ts)) | The number of loops and decision rules, plus the number of decisions. Use a combination of 1) subflows and 2) breaking flows into multiple concise trigger ordered flows, to reduce the cyclomatic complexity within a single flow, ensuring maintainability and simplicity. | ## Core Functions diff --git a/src/main/rules/CyclomaticComplexity.ts b/src/main/rules/CyclomaticComplexity.ts index 9d573b6..6dd7968 100644 --- a/src/main/rules/CyclomaticComplexity.ts +++ b/src/main/rules/CyclomaticComplexity.ts @@ -8,7 +8,7 @@ export class CyclomaticComplexity extends RuleCommon implements core.IRuleDefini name: "CyclomaticComplexity", label: "Cyclomatic Complexity", description: `The number of loops and decision rules, plus the number of decisions. - Use a combination of 1) subflows and 2) breakdown flows into multiple trigger ordered flows, + Use a combination of 1) subflows and 2) breaking flows into multiple concise trigger ordered flows, to reduce the cyclomatic complexity within a single flow, ensuring maintainability and simplicity.`, supportedTypes: core.FlowType.backEndTypes, docRefs: [