Skip to content

Commit

Permalink
refactor: make class members readonly when possible (#3227)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tbouffard authored Dec 11, 2024
1 parent f31b004 commit ff0b4c5
Show file tree
Hide file tree
Showing 23 changed files with 53 additions and 53 deletions.
2 changes: 2 additions & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
12 changes: 6 additions & 6 deletions dev/ts/component/DropFileUserInterface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions dev/ts/component/SvgExporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/component/mxgraph/BpmnCellRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
2 changes: 1 addition & 1 deletion src/component/mxgraph/config/StyleConfigurator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ const specificAssociationFlowStyles = new MapWithDefault<AssociationDirectionKin
* @experimental
*/
export class StyleConfigurator {
constructor(private graph: BpmnGraph) {}
constructor(private readonly graph: BpmnGraph) {}

configureStyles(): void {
this.configureDefaultVertexStyle();
Expand Down
2 changes: 1 addition & 1 deletion src/component/mxgraph/shape/activity-shapes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export abstract class BaseActivityShape extends mxRectangleShape {
// The actual value is injected at runtime by BpmnCellRenderer
protected iconPainter: IconPainter = undefined;

private markerPainterFunctions = new Map<ShapeBpmnMarkerKind, (paintParameter: PaintParameter) => void>([
private readonly markerPainterFunctions = new Map<ShapeBpmnMarkerKind, (paintParameter: PaintParameter) => 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)],
Expand Down
2 changes: 1 addition & 1 deletion src/component/mxgraph/shape/event-shapes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ShapeBpmnEventDefinitionKind, (paintParameter: PaintParameter) => void>([
private readonly iconPainters = new Map<ShapeBpmnEventDefinitionKind, (paintParameter: PaintParameter) => void>([
[ShapeBpmnEventDefinitionKind.MESSAGE, (paintParameter: PaintParameter) => this.iconPainter.paintEnvelopeIcon({ ...paintParameter, ratioFromParent: 0.5 })],
[ShapeBpmnEventDefinitionKind.TERMINATE, (paintParameter: PaintParameter) => this.iconPainter.paintCircleIcon({ ...paintParameter, ratioFromParent: 0.6 })],
[
Expand Down
2 changes: 1 addition & 1 deletion src/component/mxgraph/shape/render/BpmnCanvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/component/mxgraph/style/style-updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class StyleUpdater {

const cssClassesStyleIdentifier = BpmnStyleIdentifier.EXTRA_CSS_CLASSES;
class StyleManager {
private stylesCache = new Map<string, string>();
private readonly stylesCache = new Map<string, string>();

constructor(private readonly model: mxGraphModel) {}

Expand Down
2 changes: 1 addition & 1 deletion src/component/parser/json/converter/CategoryConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<TCategory>(definitions.category).flatMap(category => ensureIsArray(category.categoryValue));
Expand Down
4 changes: 2 additions & 2 deletions src/component/parser/json/converter/CollaborationConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 3 additions & 5 deletions src/component/parser/json/converter/DiagramConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ 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<string, Font>();
private readonly convertedFonts = new Map<string, Font>();

deserialize(bpmnDiagrams: BPMNDiagram[] | BPMNDiagram): BpmnModel {
const flowNodes: Shape[] = [];
Expand All @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/component/parser/json/converter/GlobalTaskConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions src/component/parser/json/converter/ProcessConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,14 @@ function getShapeBpmnElementKind(bpmnSemanticType: BpmnSemanticType): ShapeBpmnE
* @internal
*/
export default class ProcessConverter {
private defaultSequenceFlowIds: string[] = [];
private elementsWithoutParentByProcessId = new Map<string, ShapeBpmnElement[]>();
private callActivitiesCallingProcess = new Map<string, ShapeBpmnElement>();
private eventsByLinkEventDefinition = new Map<RegisteredEventDefinition, ShapeBpmnIntermediateThrowEvent | ShapeBpmnIntermediateCatchEvent>();
private readonly defaultSequenceFlowIds: string[] = [];
private readonly elementsWithoutParentByProcessId = new Map<string, ShapeBpmnElement[]>();
private readonly callActivitiesCallingProcess = new Map<string, ShapeBpmnElement>();
private readonly eventsByLinkEventDefinition = new Map<RegisteredEventDefinition, ShapeBpmnIntermediateThrowEvent | ShapeBpmnIntermediateCatchEvent>();

constructor(
private convertedElements: ConvertedElements,
private parsingMessageCollector: ParsingMessageCollector,
private readonly convertedElements: ConvertedElements,
private readonly parsingMessageCollector: ParsingMessageCollector,
) {}

deserialize(processes: string | TProcess | (string | TProcess)[]): void {
Expand Down
20 changes: 10 additions & 10 deletions src/component/parser/json/converter/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ export class ConvertedElements {
return [...this.messageFlows.values(), ...this.sequenceFlows.values(), ...this.associationFlows.values()];
}

private poolsById = new Map<string, ShapeBpmnElement>();
private readonly poolsById = new Map<string, ShapeBpmnElement>();
findPoolById(id: string): ShapeBpmnElement {
return this.poolsById.get(id);
}
private poolsByProcessRef = new Map<string, ShapeBpmnElement>();
private readonly poolsByProcessRef = new Map<string, ShapeBpmnElement>();
findPoolByProcessRef(processReference: string): ShapeBpmnElement {
return this.poolsByProcessRef.get(processReference);
}
Expand All @@ -53,63 +53,63 @@ export class ConvertedElements {
}
}

private messageFlows = new Map<string, MessageFlow>();
private readonly messageFlows = new Map<string, MessageFlow>();
findMessageFlow(id: string): MessageFlow {
return this.messageFlows.get(id);
}
registerMessageFlow(messageFlow: MessageFlow): void {
this.messageFlows.set(messageFlow.id, messageFlow);
}

private flowNodes = new Map<string, ShapeBpmnElement>();
private readonly flowNodes = new Map<string, ShapeBpmnElement>();
findFlowNode(id: string): ShapeBpmnElement {
return this.flowNodes.get(id);
}
registerFlowNode(flowNode: ShapeBpmnElement): void {
this.flowNodes.set(flowNode.id, flowNode);
}

private lanes = new Map<string, ShapeBpmnElement>();
private readonly lanes = new Map<string, ShapeBpmnElement>();
findLane(id: string): ShapeBpmnElement {
return this.lanes.get(id);
}
registerLane(lane: ShapeBpmnElement): void {
this.lanes.set(lane.id, lane);
}

private sequenceFlows = new Map<string, SequenceFlow>();
private readonly sequenceFlows = new Map<string, SequenceFlow>();
findSequenceFlow(id: string): SequenceFlow {
return this.sequenceFlows.get(id);
}
registerSequenceFlow(sequenceFlow: SequenceFlow): void {
this.sequenceFlows.set(sequenceFlow.id, sequenceFlow);
}

private associationFlows = new Map<string, AssociationFlow>();
private readonly associationFlows = new Map<string, AssociationFlow>();
findAssociationFlow(id: string): AssociationFlow {
return this.associationFlows.get(id);
}
registerAssociationFlow(associationFlow: AssociationFlow): void {
this.associationFlows.set(associationFlow.id, associationFlow);
}

private eventDefinitionsOfDefinitions = new Map<string, RegisteredEventDefinition>();
private readonly eventDefinitionsOfDefinitions = new Map<string, RegisteredEventDefinition>();
findEventDefinitionOfDefinition(id: string): RegisteredEventDefinition {
return this.eventDefinitionsOfDefinitions.get(id);
}
registerEventDefinitionsOfDefinition(id: string, eventDefinition: RegisteredEventDefinition): void {
this.eventDefinitionsOfDefinitions.set(id, eventDefinition);
}

private globalTasks = new Map<string, GlobalTaskKind>();
private readonly globalTasks = new Map<string, GlobalTaskKind>();
findGlobalTask(id: string): GlobalTaskKind {
return this.globalTasks.get(id);
}
registerGlobalTask(id: string, kind: GlobalTaskKind): void {
this.globalTasks.set(id, kind);
}

private categoryValues = new Map<string, CategoryValueData>();
private readonly categoryValues = new Map<string, CategoryValueData>();
findCategoryValue(categoryValue: string): CategoryValueData {
return this.categoryValues.get(categoryValue);
}
Expand Down
2 changes: 1 addition & 1 deletion src/component/parser/parsing-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export abstract class JsonParsingWarning {
export type ParsingMessageCollectorOptions = Pick<ParserOptions, 'disableConsoleLog'>;

export class ParsingMessageCollector {
constructor(private options?: ParsingMessageCollectorOptions) {}
constructor(private readonly options?: ParsingMessageCollectorOptions) {}

warning(warning: JsonParsingWarning): void {
if (this.options?.disableConsoleLog) {
Expand Down
4 changes: 2 additions & 2 deletions src/component/parser/xml/BpmnXmlParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/component/registry/bpmn-elements-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
) {}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/component/registry/bpmn-model-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export interface RenderedModel {
}

class SearchableModel {
private elements = new Map<string, Shape | Edge>();
private readonly elements = new Map<string, Shape | Edge>();

constructor(bpmnModel: BpmnModel) {
for (const shapeOrEdge of [...bpmnModel.pools, ...bpmnModel.lanes, ...bpmnModel.flowNodes, ...bpmnModel.edges]) {
Expand Down
2 changes: 1 addition & 1 deletion src/component/registry/css-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export class CssClassesRegistryImpl implements CssClassesRegistry {
* @internal
*/
export class CssClassesCache {
private classNamesByBpmnId = new Map<string, Set<string>>();
private readonly classNamesByBpmnId = new Map<string, Set<string>>();

/**
* Clear all classes that were registered.
Expand Down
4 changes: 2 additions & 2 deletions test/integration/helpers/html-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions test/shared/visu/bpmn-page-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
Expand Down Expand Up @@ -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<void> {
await super.gotoPageAndLoadBpmnDiagram(bpmnDiagramName ?? 'not-used-dedicated-diagram-loaded-by-the-page', {
Expand Down

0 comments on commit ff0b4c5

Please sign in to comment.