Skip to content

Fix five SonarQube issues in Oak components#711

Open
sonarqube-agent[bot] wants to merge 1 commit into
mainfrom
remediate-main-20260609-090220-71bce27f
Open

Fix five SonarQube issues in Oak components#711
sonarqube-agent[bot] wants to merge 1 commit into
mainfrom
remediate-main-20260609-090220-71bce27f

Conversation

@sonarqube-agent

Copy link
Copy Markdown

This PR was automatically created by the Remediation Agent's Scheduled backlog remediation feature.

Why these issues? Prioritized MAJOR severity issues across production code that directly impact functionality and maintainability. Fixes address critical accessibility concerns (interactive elements), logic errors (duplicate cases and dead code), and common React anti-patterns (array index keys), all of which are well-scoped and critical to address in a component library.

This change fixes five MAJOR code quality issues across Oak components: converts non-interactive elements to proper buttons, removes useless variable assignments, stabilizes React list keys with content-based identifiers, simplifies overly complex regex patterns, and eliminates duplicate case blocks in switch statements. These fixes improve code maintainability, accessibility, and prevent potential bugs from DOM recreation and excessive complexity.

View Project in SonarCloud


Fixed Issues

typescript:S6847 - Non-interactive elements should not be assigned mouse or keyboard event listeners. • MAJORView issue

Location: src/components/owa/OakListItem/OakListItem.tsx:151

Why is this an issue?

Attaching event handlers to non-interactive HTML elements can lead to significant accessibility issues. These elements, such as <div> and <span>, are not designed to interact with assistive technologies like screen readers, making it difficult for users with disabilities to navigate and interact with the website. Additionally, these elements may not be focusable or provide visual feedback when interacted with, resulting in a confusing and potentially frustrating user experience. Therefore, to maintain an accessible and user-friendly website, event handlers should be used exclusively with interactive elements.

What changed

Adds imports for ReactNode and useCallback, which are needed by the new renderWrapper function and handleKeyDown callback that replace the non-interactive element's onClick handler with proper interactive elements (a or a with htmlFor).

--- a/src/components/owa/OakListItem/OakListItem.tsx
+++ b/src/components/owa/OakListItem/OakListItem.tsx
@@ -1,1 +1,1 @@
-import React, { useContext } from "react";
+import React, { ReactNode, useCallback, useContext } from "react";

typescript:S1854 - Remove this useless assignment to variable "isCheckingMatches". • MAJORView issue

Location: src/components/owa/OakCodeRenderer/OakCodeRenderer.tsx:107

Why is this an issue?

Dead stores refer to assignments made to local variables that are subsequently never used or immediately overwritten. Such assignments are unnecessary and don’t contribute to the functionality or clarity of the code. They may even negatively impact performance. Removing them enhances code cleanliness and readability. Even if the unnecessary operations do not do any harm in terms of the program’s correctness, they are - at best - a waste of computing resources.

What changed

Removes the isCheckingMatches variable and replaces while (isCheckingMatches) with while (true). This eliminates the dead store to isCheckingMatches that was flagged as a useless assignment, since the variable was assigned false but the loop was actually terminated by a break statement, making the assignment unnecessary.

--- a/src/components/owa/OakCodeRenderer/OakCodeRenderer.tsx
+++ b/src/components/owa/OakCodeRenderer/OakCodeRenderer.tsx
@@ -77,2 +82,1 @@ export const OakCodeRenderer = ({ string, ...rest }: OakCodeRendererProps) => {
-    let isCheckingMatches = true;
-    while (isCheckingMatches) {
+    while (true) {
typescript:S6479 - Do not use Array index in keys • MAJORView issue

Location: src/components/owa/OakCodeRenderer/OakCodeRenderer.tsx:36

Why is this an issue?

To optimize the rendering of React list components, a unique identifier (UID) is required for each list item. This UID lets React identify the item throughout its lifetime. Avoid array indexes since the order of the items may change, which will cause keys to not match up between renders, recreating the DOM. It can negatively impact performance and may cause issues with the component state.

What changed

Changes the React list key from using only the array index (key={index}) to a composite key that includes the content and the index (key={${part}-${index}}). This addresses the code smell about using plain array indexes as keys in React list rendering, which can cause unnecessary DOM recreation when list items are reordered.

--- a/src/components/owa/OakCodeRenderer/OakCodeRenderer.tsx
+++ b/src/components/owa/OakCodeRenderer/OakCodeRenderer.tsx
@@ -36,1 +36,1 @@ export const OakCodeRenderer = ({ string, ...rest }: OakCodeRendererProps) => {
-                key={index}
+                key={`${part}-${index}`}
typescript:S5843 - Simplify this regular expression to reduce its complexity from 31 to the 20 allowed. • MAJORView issue

Location: src/components/owa/OakCodeRenderer/OakCodeRenderer.tsx:62

Why is this an issue?

A complex regular expression is one that exhibits several or all of the following characteristics. It can be quite lengthy, containing multiple nested or repeated groups, numerous alternations, extensive use of backreferences and escape characters, lookaheads, lookbehinds, and other advanced features. Additionally, complex regular expressions may lack proper comments and documentation, making them challenging to comprehend and maintain. Overly complicated regular expressions are hard to read and maintain and can easily cause hard-to-find bugs.

What changed

Splits the overly complex Python keywords regular expression into two smaller regexes. This hunk reduces the first regex to only contain keywords from 'and' through 'from', cutting the number of alternations roughly in half. This brings the complexity of this individual regex below the allowed threshold of 20, addressing the warning about excessive regex complexity.

--- a/src/components/owa/OakCodeRenderer/OakCodeRenderer.tsx
+++ b/src/components/owa/OakCodeRenderer/OakCodeRenderer.tsx
@@ -62,1 +62,1 @@ export const OakCodeRenderer = ({ string, ...rest }: OakCodeRendererProps) => {
-          /\b(and|as|assert|async|await|break|class|continue|def|del|elif|else|except|finally|for|from|global|if|import|in|is|lambda|nonlocal|not|or|pass|raise|return|try|while|with|yield)\b/g,
+          /\b(and|as|assert|async|await|break|class|continue|def|del|elif|else|except|finally|for|from)\b/g,
typescript:S1871 - This case's code block is the same as the block for the case on line 185. • MAJORView issue

Location: src/components/owa/pupil/lesson/OakLessonLayout/OakLessonLayout.tsx:227

Why is this an issue?

When the same code is duplicated in two or more separate branches of a conditional, it can make the code harder to understand, maintain, and can potentially introduce bugs if one instance of the code is changed but others are not.

What changed

This hunk adds case "review": as a fall-through case right after case "overview": (at line 185). By making the "review" case fall through to the "overview" case, both cases share the same code block, eliminating the duplicate code that was previously in a separate "review" case block. This directly addresses the code smell where the "review" case (previously at line 227) had an identical implementation to the "overview" case (at line 185).

--- a/src/components/owa/pupil/lesson/OakLessonLayout/OakLessonLayout.tsx
+++ b/src/components/owa/pupil/lesson/OakLessonLayout/OakLessonLayout.tsx
@@ -185,0 +186,1 @@ function pickSectionColours(
+    case "review":

Have a suggestion or found an issue? Share your feedback here.


SonarQube Remediation Agent uses AI. Check for mistakes.

Fixed issues:
- AZCX_zJRUB9YzXIX1wkp for typescript:S1871 rule
- AZY6A9-rukQXIMm7iRsX for typescript:S6479 rule
- AZY6A9-rukQXIMm7iRsY for typescript:S5843 rule
- AZY6A9-rukQXIMm7iRsZ for typescript:S1854 rule
- AZx19D7mQwuJZdSIM-HZ for typescript:S6847 rule

Generated by SonarQube Agent (task: 6013b01d-3b36-416a-bbf9-350be6aedffb)
@vercel

vercel Bot commented Jun 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
oak-components-storybook Ready Ready Preview, Comment Jun 9, 2026 9:09am

Request Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant