From 915a8d99c46887bca80e9609b2d02bc31dd7d57b Mon Sep 17 00:00:00 2001 From: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com> Date: Wed, 11 Dec 2024 10:51:22 +0100 Subject: [PATCH 1/2] refactor: make class members readonly when possible Make the members readonly when they are not reassigned outside the constructor. This improves type safety and promotes immutability. This has been detected by SonarCloud. Use the related typescript-eslint rule to prevent this from appearing again. The eslint errors are auto-fixable, so this change will be transparent for developers. --- .eslintrc.cjs | 2 ++ dev/ts/component/DropFileUserInterface.ts | 12 +++++------ dev/ts/component/SvgExporter.ts | 4 ++-- src/component/mxgraph/BpmnCellRenderer.ts | 2 +- .../mxgraph/config/StyleConfigurator.ts | 2 +- .../mxgraph/shape/activity-shapes.ts | 2 +- src/component/mxgraph/shape/event-shapes.ts | 2 +- .../mxgraph/shape/render/BpmnCanvas.ts | 2 +- src/component/mxgraph/style/style-updater.ts | 2 +- .../json/converter/CategoryConverter.ts | 2 +- .../json/converter/CollaborationConverter.ts | 4 ++-- .../parser/json/converter/DiagramConverter.ts | 4 ++-- .../converter/EventDefinitionConverter.ts | 2 +- .../json/converter/GlobalTaskConverter.ts | 2 +- .../parser/json/converter/ProcessConverter.ts | 12 +++++------ src/component/parser/json/converter/utils.ts | 20 +++++++++---------- src/component/parser/parsing-messages.ts | 2 +- src/component/parser/xml/BpmnXmlParser.ts | 4 ++-- .../registry/bpmn-elements-registry.ts | 4 ++-- src/component/registry/bpmn-model-registry.ts | 2 +- src/component/registry/css-registry.ts | 2 +- test/integration/helpers/html-utils.ts | 4 ++-- test/shared/visu/bpmn-page-utils.ts | 8 ++++---- 23 files changed, 52 insertions(+), 50 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 0c3f6984ce..f39d7f5d1c 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -128,6 +128,8 @@ module.exports = { }, ], + '@typescript-eslint/prefer-readonly': 'error', + // The following lines are commented, because they show errors on files other than the demo: // '@typescript-eslint/no-base-to-string': 'error', // '@typescript-eslint/no-unsafe-assignment': 'error', diff --git a/dev/ts/component/DropFileUserInterface.ts b/dev/ts/component/DropFileUserInterface.ts index f5e39d7658..f37c5f11c4 100644 --- a/dev/ts/component/DropFileUserInterface.ts +++ b/dev/ts/component/DropFileUserInterface.ts @@ -17,14 +17,14 @@ limitations under the License. import { logErrorAndOpenAlert } from '../shared/internal-helpers'; export class DropFileUserInterface { - private document: Document; - private head: Element; + private readonly document: Document; + private readonly head: Element; constructor( - private window: Window, - private outerContainerId: string, - private containerToFade: HTMLElement, - private dropCallback: (file: File) => void, + private readonly window: Window, + private readonly outerContainerId: string, + private readonly containerToFade: HTMLElement, + private readonly dropCallback: (file: File) => void, ) { this.document = window.document; this.head = document.head; diff --git a/dev/ts/component/SvgExporter.ts b/dev/ts/component/SvgExporter.ts index 826ba4224f..2a27695b4e 100644 --- a/dev/ts/component/SvgExporter.ts +++ b/dev/ts/component/SvgExporter.ts @@ -31,7 +31,7 @@ interface SvgExportOptions { // https://github.com/jgraph/drawio/blob/v14.7.7/src/main/webapp/js/diagramly/Editor.js#L5932 // https://github.com/jgraph/drawio/blob/v14.8.0/src/main/webapp/js/grapheditor/Graph.js#L9007 export class SvgExporter { - constructor(private graph: mxGraph) {} + constructor(private readonly graph: mxGraph) {} exportSvg(): string { return this.doSvgExport(true); @@ -111,7 +111,7 @@ ${svgAsString} class CanvasForExport extends mxSvgCanvas2D { // Convert HTML entities - private htmlConverter = document.createElement('div'); + private readonly htmlConverter = document.createElement('div'); constructor(node: Element) { super(node); diff --git a/src/component/mxgraph/BpmnCellRenderer.ts b/src/component/mxgraph/BpmnCellRenderer.ts index d2d27a3313..ceb87eb335 100644 --- a/src/component/mxgraph/BpmnCellRenderer.ts +++ b/src/component/mxgraph/BpmnCellRenderer.ts @@ -23,7 +23,7 @@ import { OverlayBadgeShape } from './overlay/shapes'; import { overrideCreateSvgCanvas } from './shape/utils'; export class BpmnCellRenderer extends mxgraph.mxCellRenderer { - constructor(private iconPainter: IconPainter) { + constructor(private readonly iconPainter: IconPainter) { super(); } diff --git a/src/component/mxgraph/config/StyleConfigurator.ts b/src/component/mxgraph/config/StyleConfigurator.ts index 91d49da1c7..372bc35f9b 100644 --- a/src/component/mxgraph/config/StyleConfigurator.ts +++ b/src/component/mxgraph/config/StyleConfigurator.ts @@ -116,7 +116,7 @@ const specificAssociationFlowStyles = new MapWithDefault void>([ + private readonly markerPainterFunctions = new Map void>([ [ShapeBpmnMarkerKind.EXPAND, (paintParameter: PaintParameter) => this.iconPainter.paintExpandIcon(paintParameter)], [ShapeBpmnMarkerKind.LOOP, (paintParameter: PaintParameter) => this.iconPainter.paintLoopIcon(paintParameter)], [ShapeBpmnMarkerKind.MULTI_INSTANCE_PARALLEL, (paintParameter: PaintParameter) => this.iconPainter.paintParallelMultiInstanceIcon(paintParameter)], diff --git a/src/component/mxgraph/shape/event-shapes.ts b/src/component/mxgraph/shape/event-shapes.ts index 63be5f2064..7a1994c2fb 100644 --- a/src/component/mxgraph/shape/event-shapes.ts +++ b/src/component/mxgraph/shape/event-shapes.ts @@ -31,7 +31,7 @@ export class EventShape extends mxgraph.mxEllipse { protected iconPainter: IconPainter = undefined; // refactor: when all/more event types will be supported, we could move to a Record/MappedType - private iconPainters = new Map void>([ + private readonly iconPainters = new Map void>([ [ShapeBpmnEventDefinitionKind.MESSAGE, (paintParameter: PaintParameter) => this.iconPainter.paintEnvelopeIcon({ ...paintParameter, ratioFromParent: 0.5 })], [ShapeBpmnEventDefinitionKind.TERMINATE, (paintParameter: PaintParameter) => this.iconPainter.paintCircleIcon({ ...paintParameter, ratioFromParent: 0.6 })], [ diff --git a/src/component/mxgraph/shape/render/BpmnCanvas.ts b/src/component/mxgraph/shape/render/BpmnCanvas.ts index 5e6a1fc2ee..920b5a747a 100644 --- a/src/component/mxgraph/shape/render/BpmnCanvas.ts +++ b/src/component/mxgraph/shape/render/BpmnCanvas.ts @@ -80,7 +80,7 @@ export function computeScaledIconSize(initialIconSize: Size, iconStyleConfigurat * @experimental */ export class BpmnCanvas { - private canvas: mxAbstractCanvas2D; + private readonly canvas: mxAbstractCanvas2D; private readonly iconOriginalSize: Size; private readonly scaleX: number; diff --git a/src/component/mxgraph/style/style-updater.ts b/src/component/mxgraph/style/style-updater.ts index 9671660e8a..5a56f9cfba 100644 --- a/src/component/mxgraph/style/style-updater.ts +++ b/src/component/mxgraph/style/style-updater.ts @@ -97,7 +97,7 @@ export class StyleUpdater { const cssClassesStyleIdentifier = BpmnStyleIdentifier.EXTRA_CSS_CLASSES; class StyleManager { - private stylesCache = new Map(); + private readonly stylesCache = new Map(); constructor(private readonly model: mxGraphModel) {} diff --git a/src/component/parser/json/converter/CategoryConverter.ts b/src/component/parser/json/converter/CategoryConverter.ts index aba8b1abd4..1c4bdfb586 100644 --- a/src/component/parser/json/converter/CategoryConverter.ts +++ b/src/component/parser/json/converter/CategoryConverter.ts @@ -24,7 +24,7 @@ import { ensureIsArray } from '../../../helpers/array-utils'; * @internal */ export default class CategoryConverter { - constructor(private convertedElements: ConvertedElements) {} + constructor(private readonly convertedElements: ConvertedElements) {} deserialize(definitions: TDefinitions): void { const categoryValues = ensureIsArray(definitions.category).flatMap(category => ensureIsArray(category.categoryValue)); diff --git a/src/component/parser/json/converter/CollaborationConverter.ts b/src/component/parser/json/converter/CollaborationConverter.ts index 1938d3269e..2f1c6458ab 100644 --- a/src/component/parser/json/converter/CollaborationConverter.ts +++ b/src/component/parser/json/converter/CollaborationConverter.ts @@ -33,8 +33,8 @@ import { buildShapeBpmnGroup, convertAndRegisterAssociationFlows } from './utils */ export default class CollaborationConverter { constructor( - private convertedElements: ConvertedElements, - private parsingMessageCollector: ParsingMessageCollector, + private readonly convertedElements: ConvertedElements, + private readonly parsingMessageCollector: ParsingMessageCollector, ) {} deserialize(collaborations: string | TCollaboration | (string | TCollaboration)[]): void { diff --git a/src/component/parser/json/converter/DiagramConverter.ts b/src/component/parser/json/converter/DiagramConverter.ts index e1d1c2e7da..d85f9c878c 100644 --- a/src/component/parser/json/converter/DiagramConverter.ts +++ b/src/component/parser/json/converter/DiagramConverter.ts @@ -36,8 +36,8 @@ import { EdgeUnknownBpmnElementWarning, LabelStyleMissingFontWarning, ShapeUnkno */ export default class DiagramConverter { constructor( - private convertedElements: ConvertedElements, - private parsingMessageCollector: ParsingMessageCollector, + private readonly convertedElements: ConvertedElements, + private readonly parsingMessageCollector: ParsingMessageCollector, ) {} private convertedFonts = new Map(); diff --git a/src/component/parser/json/converter/EventDefinitionConverter.ts b/src/component/parser/json/converter/EventDefinitionConverter.ts index f66bf1b583..379d5de287 100644 --- a/src/component/parser/json/converter/EventDefinitionConverter.ts +++ b/src/component/parser/json/converter/EventDefinitionConverter.ts @@ -28,7 +28,7 @@ type EventDefinitions = string | TEventDefinition | (string | TEventDefinition)[ * @internal */ export default class EventDefinitionConverter { - constructor(private convertedElements: ConvertedElements) {} + constructor(private readonly convertedElements: ConvertedElements) {} deserialize(definitions: TDefinitions): void { for (const eventDefinitionKind of eventDefinitionKinds) { diff --git a/src/component/parser/json/converter/GlobalTaskConverter.ts b/src/component/parser/json/converter/GlobalTaskConverter.ts index 90f40eab29..6e64a6a999 100644 --- a/src/component/parser/json/converter/GlobalTaskConverter.ts +++ b/src/component/parser/json/converter/GlobalTaskConverter.ts @@ -26,7 +26,7 @@ import { ensureIsArray } from '../../../helpers/array-utils'; * @internal */ export default class GlobalTaskConverter { - constructor(private convertedElements: ConvertedElements) {} + constructor(private readonly convertedElements: ConvertedElements) {} deserialize(definitions: TDefinitions): void { this.parseGlobalTasks(definitions.globalTask, ShapeBpmnElementKind.GLOBAL_TASK); diff --git a/src/component/parser/json/converter/ProcessConverter.ts b/src/component/parser/json/converter/ProcessConverter.ts index 8e57af0e27..4650e41e09 100644 --- a/src/component/parser/json/converter/ProcessConverter.ts +++ b/src/component/parser/json/converter/ProcessConverter.ts @@ -90,14 +90,14 @@ function getShapeBpmnElementKind(bpmnSemanticType: BpmnSemanticType): ShapeBpmnE * @internal */ export default class ProcessConverter { - private defaultSequenceFlowIds: string[] = []; - private elementsWithoutParentByProcessId = new Map(); - private callActivitiesCallingProcess = new Map(); - private eventsByLinkEventDefinition = new Map(); + private readonly defaultSequenceFlowIds: string[] = []; + private readonly elementsWithoutParentByProcessId = new Map(); + private readonly callActivitiesCallingProcess = new Map(); + private readonly eventsByLinkEventDefinition = new Map(); constructor( - private convertedElements: ConvertedElements, - private parsingMessageCollector: ParsingMessageCollector, + private readonly convertedElements: ConvertedElements, + private readonly parsingMessageCollector: ParsingMessageCollector, ) {} deserialize(processes: string | TProcess | (string | TProcess)[]): void { diff --git a/src/component/parser/json/converter/utils.ts b/src/component/parser/json/converter/utils.ts index eb37dab1c8..dda3fe10bd 100644 --- a/src/component/parser/json/converter/utils.ts +++ b/src/component/parser/json/converter/utils.ts @@ -38,11 +38,11 @@ export class ConvertedElements { return [...this.messageFlows.values(), ...this.sequenceFlows.values(), ...this.associationFlows.values()]; } - private poolsById = new Map(); + private readonly poolsById = new Map(); findPoolById(id: string): ShapeBpmnElement { return this.poolsById.get(id); } - private poolsByProcessRef = new Map(); + private readonly poolsByProcessRef = new Map(); findPoolByProcessRef(processReference: string): ShapeBpmnElement { return this.poolsByProcessRef.get(processReference); } @@ -53,7 +53,7 @@ export class ConvertedElements { } } - private messageFlows = new Map(); + private readonly messageFlows = new Map(); findMessageFlow(id: string): MessageFlow { return this.messageFlows.get(id); } @@ -61,7 +61,7 @@ export class ConvertedElements { this.messageFlows.set(messageFlow.id, messageFlow); } - private flowNodes = new Map(); + private readonly flowNodes = new Map(); findFlowNode(id: string): ShapeBpmnElement { return this.flowNodes.get(id); } @@ -69,7 +69,7 @@ export class ConvertedElements { this.flowNodes.set(flowNode.id, flowNode); } - private lanes = new Map(); + private readonly lanes = new Map(); findLane(id: string): ShapeBpmnElement { return this.lanes.get(id); } @@ -77,7 +77,7 @@ export class ConvertedElements { this.lanes.set(lane.id, lane); } - private sequenceFlows = new Map(); + private readonly sequenceFlows = new Map(); findSequenceFlow(id: string): SequenceFlow { return this.sequenceFlows.get(id); } @@ -85,7 +85,7 @@ export class ConvertedElements { this.sequenceFlows.set(sequenceFlow.id, sequenceFlow); } - private associationFlows = new Map(); + private readonly associationFlows = new Map(); findAssociationFlow(id: string): AssociationFlow { return this.associationFlows.get(id); } @@ -93,7 +93,7 @@ export class ConvertedElements { this.associationFlows.set(associationFlow.id, associationFlow); } - private eventDefinitionsOfDefinitions = new Map(); + private readonly eventDefinitionsOfDefinitions = new Map(); findEventDefinitionOfDefinition(id: string): RegisteredEventDefinition { return this.eventDefinitionsOfDefinitions.get(id); } @@ -101,7 +101,7 @@ export class ConvertedElements { this.eventDefinitionsOfDefinitions.set(id, eventDefinition); } - private globalTasks = new Map(); + private readonly globalTasks = new Map(); findGlobalTask(id: string): GlobalTaskKind { return this.globalTasks.get(id); } @@ -109,7 +109,7 @@ export class ConvertedElements { this.globalTasks.set(id, kind); } - private categoryValues = new Map(); + private readonly categoryValues = new Map(); findCategoryValue(categoryValue: string): CategoryValueData { return this.categoryValues.get(categoryValue); } diff --git a/src/component/parser/parsing-messages.ts b/src/component/parser/parsing-messages.ts index e2fab6a7dd..605e350407 100644 --- a/src/component/parser/parsing-messages.ts +++ b/src/component/parser/parsing-messages.ts @@ -28,7 +28,7 @@ export abstract class JsonParsingWarning { export type ParsingMessageCollectorOptions = Pick; export class ParsingMessageCollector { - constructor(private options?: ParsingMessageCollectorOptions) {} + constructor(private readonly options?: ParsingMessageCollectorOptions) {} warning(warning: JsonParsingWarning): void { if (this.options?.disableConsoleLog) { diff --git a/src/component/parser/xml/BpmnXmlParser.ts b/src/component/parser/xml/BpmnXmlParser.ts index de97d04c51..e2af0dc28b 100644 --- a/src/component/parser/xml/BpmnXmlParser.ts +++ b/src/component/parser/xml/BpmnXmlParser.ts @@ -80,9 +80,9 @@ export default class BpmnXmlParser { return this.processAttribute(value); }, }; - private xmlParser: XMLParser = new XMLParser(this.x2jOptions); + private readonly xmlParser: XMLParser = new XMLParser(this.x2jOptions); - constructor(private options?: XmlParserOptions) {} + constructor(private readonly options?: XmlParserOptions) {} parse(xml: string): BpmnJsonModel { let model: BpmnJsonModel; diff --git a/src/component/registry/bpmn-elements-registry.ts b/src/component/registry/bpmn-elements-registry.ts index 1fbb13fca3..7eaa50da3d 100644 --- a/src/component/registry/bpmn-elements-registry.ts +++ b/src/component/registry/bpmn-elements-registry.ts @@ -140,8 +140,8 @@ export class BpmnElementsRegistry implements CssClassesRegistry, ElementsRegistr class HtmlElementRegistry { constructor( - private container: HTMLElement, - private querySelectors: BpmnQuerySelectors, + private readonly container: HTMLElement, + private readonly querySelectors: BpmnQuerySelectors, ) {} /** diff --git a/src/component/registry/bpmn-model-registry.ts b/src/component/registry/bpmn-model-registry.ts index a825cdcf67..6094e2b870 100644 --- a/src/component/registry/bpmn-model-registry.ts +++ b/src/component/registry/bpmn-model-registry.ts @@ -126,7 +126,7 @@ export interface RenderedModel { } class SearchableModel { - private elements = new Map(); + private readonly elements = new Map(); constructor(bpmnModel: BpmnModel) { for (const shapeOrEdge of [...bpmnModel.pools, ...bpmnModel.lanes, ...bpmnModel.flowNodes, ...bpmnModel.edges]) { diff --git a/src/component/registry/css-registry.ts b/src/component/registry/css-registry.ts index 51ad0ab850..da82f35662 100644 --- a/src/component/registry/css-registry.ts +++ b/src/component/registry/css-registry.ts @@ -76,7 +76,7 @@ export class CssClassesRegistryImpl implements CssClassesRegistry { * @internal */ export class CssClassesCache { - private classNamesByBpmnId = new Map>(); + private readonly classNamesByBpmnId = new Map>(); /** * Clear all classes that were registered. diff --git a/test/integration/helpers/html-utils.ts b/test/integration/helpers/html-utils.ts index cfc23dde0d..23995946a6 100644 --- a/test/integration/helpers/html-utils.ts +++ b/test/integration/helpers/html-utils.ts @@ -32,9 +32,9 @@ export interface MessageFlowRequestedChecks extends RequestedChecks { } export class HtmlElementLookup { - private bpmnQuerySelectors = new BpmnQuerySelectorsForTests(); + private readonly bpmnQuerySelectors = new BpmnQuerySelectorsForTests(); - constructor(private bpmnVisualization: BpmnVisualization) {} + constructor(private readonly bpmnVisualization: BpmnVisualization) {} expectNoElement(bpmnId: string): void { const svgGroupElement = this.findSvgElement(bpmnId); diff --git a/test/shared/visu/bpmn-page-utils.ts b/test/shared/visu/bpmn-page-utils.ts index 1f02effd9c..6b36f9334f 100644 --- a/test/shared/visu/bpmn-page-utils.ts +++ b/test/shared/visu/bpmn-page-utils.ts @@ -38,11 +38,11 @@ import { BpmnQuerySelectorsForTests } from '@test/shared/query-selectors'; const pageCheckLog = debugLogger('bv:test:page-check'); class BpmnPage { - private bpmnQuerySelectors = new BpmnQuerySelectorsForTests(); + private readonly bpmnQuerySelectors = new BpmnQuerySelectorsForTests(); constructor( - private bpmnContainerId: string, - private page: Page, + private readonly bpmnContainerId: string, + private readonly page: Page, ) {} async expectAvailableBpmnContainer(options?: PageWaitForSelectorOptions): Promise { @@ -298,7 +298,7 @@ export class PageTester { } export class BpmnPageSvgTester extends PageTester { - private bpmnQuerySelectors = new BpmnQuerySelectorsForTests(); + private readonly bpmnQuerySelectors = new BpmnQuerySelectorsForTests(); override async gotoPageAndLoadBpmnDiagram(bpmnDiagramName?: string): Promise { await super.gotoPageAndLoadBpmnDiagram(bpmnDiagramName ?? 'not-used-dedicated-diagram-loaded-by-the-page', { From 4f51a034a43e9bf4b24dc89a874dbb7214ea19f0 Mon Sep 17 00:00:00 2001 From: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com> Date: Wed, 11 Dec 2024 12:40:21 +0100 Subject: [PATCH 2/2] EXTRA DiagramConverter.ts prevent extra init and mark member as readonly --- src/component/parser/json/converter/DiagramConverter.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/component/parser/json/converter/DiagramConverter.ts b/src/component/parser/json/converter/DiagramConverter.ts index d85f9c878c..87bfd905f8 100644 --- a/src/component/parser/json/converter/DiagramConverter.ts +++ b/src/component/parser/json/converter/DiagramConverter.ts @@ -40,7 +40,7 @@ export default class DiagramConverter { private readonly parsingMessageCollector: ParsingMessageCollector, ) {} - private convertedFonts = new Map(); + private readonly convertedFonts = new Map(); deserialize(bpmnDiagrams: BPMNDiagram[] | BPMNDiagram): BpmnModel { const flowNodes: Shape[] = []; @@ -66,8 +66,6 @@ export default class DiagramConverter { } private deserializeFonts(bpmnLabelStyle: BPMNLabelStyle[] | BPMNLabelStyle): void { - this.convertedFonts = new Map(); - for (const labelStyle of ensureIsArray(bpmnLabelStyle)) for (const font of ensureIsArray(labelStyle.Font)) this.convertedFonts.set(labelStyle.id, new Font(font.name, font.size, font.isBold, font.isItalic, font.isUnderline, font.isStrikeThrough));