refactor(frontend): replace for...in usages and forbid via lint rule

Replaces 33 for...in loops across 18 files with for...of + Object.keys/entries
or indexed for loops. for...in iterates enumerable string keys including
inherited ones, which is especially risky on reactive arrays (tasks, labels,
assignees, etc.) where polyfilled properties may appear.

Loops that mutate via splice during iteration now iterate backwards to avoid
index-shift bugs. Adds a no-restricted-syntax ESLint rule forbidding
ForInStatement to prevent regressions.

Closes #513

frontend/eslint.config.js

@@ -112,6 +112,11 @@ export default [
 
 			'depend/ban-dependencies': 'warn',
 
+			'no-restricted-syntax': ['error', {
+				selector: 'ForInStatement',
+				message: 'Use for...of with Object.keys/entries, or .forEach, instead of for...in. See https://github.com/go-vikunja/vikunja/issues/513',
+			}],
+
 			'@typescript-eslint/no-unused-vars': [
 				'error',
 				{

frontend/src/components/input/Multiselect.vue

@@ -448,7 +448,7 @@ function createOrSelectOnEnter() {
 }
 
 function remove(item: T) {
-	for (const ind in internalValue.value) {
+	for (let ind = 0; ind < internalValue.value.length; ind++) {
 		if (internalValue.value[ind] === item) {
 			internalValue.value.splice(ind, 1)
 			break

frontend/src/components/project/views/ProjectList.vue

@@ -235,7 +235,7 @@ function updateTasks(updatedTask: ITask) {
 		return
 	}
 
-	for (const t in tasks.value) {
+	for (let t = 0; t < tasks.value.length; t++) {
 		if (tasks.value[t].id === updatedTask.id) {
 			tasks.value[t] = updatedTask
 			break

frontend/src/components/sharing/UserTeam.vue

@@ -291,7 +291,7 @@ async function deleteSharable() {
 
 	await stuffService.delete(stuffModel)
 	showDeleteModal.value = false
-	for (const i in sharables.value) {
+	for (let i = sharables.value.length - 1; i >= 0; i--) {
 		if (
 			(sharables.value[i].username === stuffModel.username && props.shareType === 'user') ||
 			(sharables.value[i].id === stuffModel.teamId && props.shareType === 'team')
@@ -344,15 +344,15 @@ async function toggleType(sharable) {
 	}
 
 	const r = await stuffService.update(stuffModel)
-	for (const i in sharables.value) {
+	for (const sharableEntry of sharables.value) {
 		if (
-			(sharables.value[i].username ===
+			(sharableEntry.username ===
 				stuffModel.username &&
 				props.shareType === 'user') ||
-			(sharables.value[i].id === stuffModel.teamId &&
+			(sharableEntry.id === stuffModel.teamId &&
 				props.shareType === 'team')
 		) {
-			sharables.value[i].permission = r.permission
+			sharableEntry.permission = r.permission
 		}
 	}
 	success({message: t('project.share.userTeam.updatedSuccess', {type: shareTypeName.value})})

frontend/src/components/tasks/partials/Comments.vue

@@ -478,7 +478,7 @@ async function editComment() {
 	commentEdit.taskId = props.taskId
 	try {
 		const comment = await taskCommentService.update(commentEdit)
-		for (const c in comments.value) {
+		for (let c = 0; c < comments.value.length; c++) {
 			if (comments.value[c].id === commentEdit.id) {
 				comments.value[c] = comment
 			}

frontend/src/components/tasks/partials/EditAssignees.vue

@@ -99,7 +99,7 @@ async function removeAssignee(user: IUser) {
 	await taskStore.removeAssignee({user: user, taskId: props.taskId})
 
 	// Remove the assignee from the project
-	for (const a in assignees.value) {
+	for (let a = assignees.value.length - 1; a >= 0; a--) {
 		if (assignees.value[a].id === user.id) {
 			assignees.value.splice(a, 1)
 		}

frontend/src/components/tasks/partials/EditLabels.vue

@@ -124,9 +124,9 @@ async function removeLabel(label: ILabel) {
 		await taskStore.removeLabel({label, taskId: props.taskId})
 	}
 
-	for (const l in labels.value) {
+	for (let l = labels.value.length - 1; l >= 0; l--) {
 		if (labels.value[l].id === label.id) {
-			labels.value.splice(l, 1) // FIXME: l should be index
+			labels.value.splice(l, 1)
 		}
 	}
 	emit('update:modelValue', labels.value)

frontend/src/helpers/case.ts

@@ -13,7 +13,7 @@ export function objectToCamelCase(object: Record<string, any>) {
 
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any
 	const parsedObject: Record<string, any> = {}
-	for (const m in object) {
+	for (const m of Object.keys(object)) {
 		parsedObject[camelCase(m)] = object[m]
 
 		// Recursive processing
@@ -51,7 +51,7 @@ export function objectToSnakeCase(object: Record<string, any>) {
 
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any
 	const parsedObject: Record<string, any> = {}
-	for (const m in object) {
+	for (const m of Object.keys(object)) {
 		parsedObject[snakeCase(m)] = object[m]
 
 		// Recursive processing

frontend/src/helpers/filters.test.ts

@@ -19,11 +19,11 @@ describe('Filter Transformation', () => {
 	}
 
 	describe('For API', () => {
-		for (const c in fieldCases) {
+		for (const [c, snakeCase] of Object.entries(fieldCases)) {
 			it('should transform all filter params for ' + c + ' to snake_case', () => {
 				const transformed = transformFilterStringForApi(c + ' = ipsum', nullTitleToIdResolver, nullTitleToIdResolver)
 
-				expect(transformed).toBe(fieldCases[c] + ' = ipsum')
+				expect(transformed).toBe(snakeCase + ' = ipsum')
 			})
 		}
 
@@ -416,9 +416,9 @@ describe('Filter Transformation', () => {
 	})
 
 	describe('To API', () => {
-		for (const c in fieldCases) {
+		for (const [c, snakeCase] of Object.entries(fieldCases)) {
 			it('should transform all filter params for ' + c + ' to snake_case', () => {
-				const transformed = transformFilterStringFromApi(fieldCases[c] + ' = ipsum', nullTitleToIdResolver, nullTitleToIdResolver)
+				const transformed = transformFilterStringFromApi(snakeCase + ' = ipsum', nullTitleToIdResolver, nullTitleToIdResolver)
 
 				expect(transformed).toBe(c + ' = ipsum')
 			})

frontend/src/helpers/saveCollapsedBucketState.ts

@@ -18,7 +18,7 @@ export const saveCollapsedBucketState = (
 ) => {
 	const state = getAllState()
 	state[projectId] = collapsedBuckets
-	for (const bucketId in state[projectId]) {
+	for (const bucketId of Object.keys(state[projectId])) {
 		if (!state[projectId][bucketId]) {
 			delete state[projectId][bucketId]
 		}

frontend/src/helpers/time/calculateDayInterval.test.ts

@@ -12,13 +12,13 @@ const days = {
     sunday:    0,
 } as Record<string, number>
 
-for (const n in days) {
+for (const n of Object.keys(days)) {
     test(`today on a ${n}`, () => {
         expect(calculateDayInterval('today', days[n])).toBe(0)
     })
 }
 
-for (const n in days) {
+for (const n of Object.keys(days)) {
     test(`tomorrow on a ${n}`, () => {
         expect(calculateDayInterval('tomorrow', days[n])).toBe(1)
     })
@@ -34,7 +34,7 @@ const nextMonday = {
     sunday:    1,
 } as Record<string, number>
 
-for (const n in nextMonday) {
+for (const n of Object.keys(nextMonday)) {
     test(`next monday on a ${n}`, () => {
         expect(calculateDayInterval('nextMonday', days[n])).toBe(nextMonday[n])
     })
@@ -50,7 +50,7 @@ const thisWeekend = {
     sunday:    0,
 } as Record<string, number>
 
-for (const n in thisWeekend) {
+for (const n of Object.keys(thisWeekend)) {
     test(`this weekend on a ${n}`, () => {
         expect(calculateDayInterval('thisWeekend', days[n])).toBe(thisWeekend[n])
     })
@@ -66,7 +66,7 @@ const laterThisWeek = {
     sunday:    0,
 } as Record<string, number>
 
-for (const n in laterThisWeek) {
+for (const n of Object.keys(laterThisWeek)) {
     test(`later this week on a ${n}`, () => {
         expect(calculateDayInterval('laterThisWeek', days[n])).toBe(laterThisWeek[n])
     })
@@ -82,13 +82,13 @@ const laterNextWeek = {
     sunday:    7 + 0,
 } as Record<string, number>
 
-for (const n in laterNextWeek) {
+for (const n of Object.keys(laterNextWeek)) {
     test(`later next week on a ${n} (this week)`, () => {
         expect(calculateDayInterval('laterNextWeek', days[n])).toBe(laterNextWeek[n])
     })
 }
 
-for (const n in days) {
+for (const n of Object.keys(days)) {
     test(`next week on a ${n}`, () => {
         expect(calculateDayInterval('nextWeek', days[n])).toBe(7)
     })

frontend/src/modules/quickAddMagic/quickAddMagic.test.ts

@@ -165,7 +165,7 @@ describe('Parse Task Text', () => {
 				'at 12:00 am': '0:0',
 			} as const
 			
-			for (const c in cases) {
+			for (const c of Object.keys(cases)) {
 				it(`should recognize today with a time ${c}`, () => {
 					const result = parseTaskText(`Lorem Ipsum today ${c}`)
 
@@ -526,7 +526,7 @@ describe('Parse Task Text', () => {
 			]
 
 			prefix.forEach(p => {
-				for (const d in days) {
+				for (const d of Object.keys(days)) {
 					it(`should recognize ${p}${d}`, () => {
 						const result = parseTaskText(`Lorem Ipsum ${p}${d}`)
 
@@ -556,7 +556,7 @@ describe('Parse Task Text', () => {
 
 			// This tests only standalone days are recognized and not things like "github", "monitor" or "renewed".
 			// We're not using real words here to generate tests for all days on the fly.
-			for (const d in days) {
+			for (const d of Object.keys(days)) {
 				it(`should not recognize ${d} with a space before it but none after it`, () => {
 					const text = `Lorem Ipsum ${d}ipsum`
 					const result = parseTaskText(text)
@@ -660,7 +660,7 @@ describe('Parse Task Text', () => {
 				'01.02.25': '2025-2-1',
 			} as Record<string, string | null>
 
-			for (const c in cases) {
+			for (const c of Object.keys(cases)) {
 				const assertResult = ({date, text}: ParsedTaskText) => {
 					if (date === null && cases[c] === null) {
 						expect(date).toBeNull()
@@ -722,7 +722,7 @@ describe('Parse Task Text', () => {
 				'5th Mar at 3pm': '2021-3-5 15:0',
 			} as Record<string, string>
 
-			for (const c in cases) {
+			for (const c of Object.keys(cases)) {
 				it(`should parse '${c}' as '${cases[c]}'`, () => {
 					const {date} = parseDate(c, now)
 					if (date === null && cases[c] === null) {
@@ -846,7 +846,7 @@ describe('Parse Task Text', () => {
 	})
 
 	describe('Priority', () => {
-		for (const p in PRIORITIES) {
+		for (const p of Object.keys(PRIORITIES) as Array<keyof typeof PRIORITIES>) {
 			it(`should parse priority ${p}`, () => {
 				const result = parseTaskText(`Lorem Ipsum !${PRIORITIES[p]}`)
 
@@ -969,7 +969,7 @@ describe('Parse Task Text', () => {
 			'yearly': {type: 'years', amount: 1},
 		} as Record<string, IRepeatAfter>
 
-		for (const c in cases) {
+		for (const c of Object.keys(cases)) {
 			it(`should parse ${c} as recurring date every ${cases[c].amount} ${cases[c].type}`, () => {
 				const result = parseTaskText(`Lorem Ipsum ${c}`)
 

frontend/src/services/abstractService.ts

@@ -15,7 +15,7 @@ interface Paths {
 	reset?: string
 }
 
-function convertObject(o: Record<string, unknown>) {
+function convertObject(o: unknown) {
 	if (o instanceof Date) {
 		return o.toISOString()
 	}
@@ -28,13 +28,14 @@ function prepareParams(params: Record<string, unknown | unknown[]>) {
 		return params
 	}
 
-	for (const p in params) {
-		if (Array.isArray(params[p])) {
-			params[p] = params[p].map(convertObject)
+	for (const p of Object.keys(params)) {
+		const val = params[p]
+		if (Array.isArray(val)) {
+			params[p] = val.map(convertObject)
 			continue
 		}
 
-		params[p] = convertObject(params[p])
+		params[p] = convertObject(val)
 	}
 
 	return objectToSnakeCase(params)

frontend/src/services/task.ts

@@ -62,7 +62,7 @@ export default class TaskService extends AbstractService<ITask> {
 
 		model.reminderDates = null
 		// remove all nulls, these would create empty reminders
-		for (const index in model.reminders) {
+		for (let index = model.reminders.length - 1; index >= 0; index--) {
 			if (model.reminders[index] === null) {
 				model.reminders.splice(index, 1)
 			}

frontend/src/stores/kanban.ts

@@ -124,9 +124,9 @@ export const useKanbanStore = defineStore('kanban', () => {
 
 		let found = false
 
-		const findAndUpdate = b => {
-			for (const t in buckets.value[b].tasks) {
-				if (buckets.value[b].tasks[t].id === task.id) {
+		const findAndUpdate = (b: number) => {
+			for (const [t, taskInBucket] of buckets.value[b].tasks.entries()) {
+				if (taskInBucket.id === task.id) {
 					const bucket = buckets.value[b]
 					bucket.tasks[t] = task
 
@@ -138,7 +138,7 @@ export const useKanbanStore = defineStore('kanban', () => {
 			}
 		}
 
-		for (const b in buckets.value) {
+		for (let b = 0; b < buckets.value.length; b++) {
 			findAndUpdate(b)
 			if (found) {
 				return

frontend/src/views/tasks/ShowTasks.vue

@@ -287,7 +287,7 @@ async function loadPendingTasks(from: Date|string, to: Date|string, filterId: nu
 
 // FIXME: this modification should happen in the store
 function updateTasks(updatedTask: ITask) {
-	for (const t in tasks.value) {
+	for (let t = 0; t < tasks.value.length; t++) {
 		if (tasks.value[t].id === updatedTask.id) {
 			tasks.value[t] = updatedTask
 			// Move the task to the end of the done tasks if it is now done

frontend/src/views/tasks/TaskDetailView.vue

@@ -1174,9 +1174,12 @@ function setRelatedTasksActive() {
 
 	// If the related tasks are already available, show the form again
 	const el = activeFieldElements['relatedTasks']
-	for (const child in el?.children) {
-		if (el?.children[child]?.id === 'showRelatedTasksFormButton') {
-			el?.children[child]?.click()
+	if (!el) {
+		return
+	}
+	for (const child of Array.from(el.children)) {
+		if ((child as HTMLElement).id === 'showRelatedTasksFormButton') {
+			(child as HTMLElement).click()
 			break
 		}
 	}

frontend/src/views/teams/EditTeam.vue

@@ -349,9 +349,9 @@ async function toggleUserType(member: ITeamMember) {
 	member.admin = !member.admin
 	member.teamId = teamId.value
 	const r = await teamMemberService.value.update(member)
-	for (const tm in team.value.members) {
-		if (team.value.members[tm].id === member.id) {
-			team.value.members[tm].admin = r.admin
+	for (const tm of team.value.members) {
+		if (tm.id === member.id) {
+			tm.admin = r.admin
 			break
 		}
 	}