refactor(web): replace String.match() with RegExp.exec() for non-global regex#32386
Conversation
Summary of ChangesHello @apoorvdarshan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors regular expression usage across the codebase by standardizing the approach for non-global regex matching. The change improves code clarity and aligns with best practices by directly using Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly refactors String.match() to RegExp.exec() for non-global regular expressions, which is a good practice for explicitness and performance. The changes are applied consistently across all relevant files.
I've added a few suggestions to further improve code maintainability. Specifically, I've pointed out some duplicated logic for extracting the appId from the URL path and suggested creating a custom hook to centralize it. I also noted a duplicated regular expression in a test file that could be extracted into a constant. Overall, this is a solid refactoring.
web/app/components/base/features/new-feature-panel/annotation-reply/index.tsx
Show resolved
Hide resolved
web/app/components/base/features/new-feature-panel/text-to-speech/param-config-content.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR refactors string matching operations across 18 files to replace String.match() with RegExp.exec() for non-global regex patterns, following SonarQube rule S6594. This change eliminates indirection since String.match() internally delegates to RegExp.exec() when the global flag is not used.
Changes:
- Replaced
string.match(regex)withregex.exec(string)in ~22 locations across utility functions, components, and tests - Intentionally preserved
.match()calls that use the global flag (e.g.,/pattern/g) as they have different return semantics
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web/utils/urlValidation.ts | Updated IPv4 address matching in private IP check |
| web/utils/format.ts | Updated scientific notation detection in number formatting |
| web/utils/error-parser.ts | Updated plugin error message extraction |
| web/scripts/gen-doc-paths.ts | Updated language path matching |
| web/scripts/component-analyzer.js | Updated complexity pattern extraction |
| web/app/components/tools/mcp/hooks/use-mcp-modal-form.ts | Updated file ID extraction from URLs |
| web/app/components/datasets/common/tests/credential-icon.spec.tsx | Updated CSS class matching in tests |
| web/app/components/billing/utils/index.ts | Updated vector space unit parsing |
| web/app/components/base/mermaid/utils.ts | Updated mermaid arrow syntax validation |
| web/app/components/base/mermaid/index.tsx | Updated gantt task line parsing |
| web/app/components/base/ga/index.tsx | Updated CSP nonce extraction |
| web/app/components/base/features/new-feature-panel/text-to-speech/param-config-content.tsx | Updated app ID extraction from pathname |
| web/app/components/base/features/new-feature-panel/annotation-reply/index.tsx | Updated app ID extraction from pathname |
| web/app/components/base/date-and-time-picker/utils/dayjs.ts | Updated timezone offset parsing |
| web/app/components/base/chat/chat/answer/human-input-content/content-item.tsx | Updated output variable extraction |
| web/app/components/base/block-input/index.tsx | Updated variable matching in block input |
| web/app/components/app/configuration/index.tsx | Updated app ID extraction from pathname |
| web/tests/check-i18n.test.ts | Updated key line pattern matching |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…al regex Replace string.match(regex) with regex.exec(string) across 18 files per SonarQube rule S6594. When match() is called without the global flag, it internally delegates to exec(), so calling exec() directly is more explicit and avoids the indirection. Global-flag match() calls are intentionally left unchanged as they have different semantics.
44ea999 to
043370a
Compare
Summary
string.match(regex)withregex.exec(string)across 18 files (~22 occurrences) per SonarQube rule S6594String#matchis called without the global flag, it internally delegates toRegExp#exec, so callingexec()directly is more explicit and avoids the indirection.match()calls using global (g) flag regexes are intentionally left unchanged as they have different return semantics (returning all matches)Closes #25199
Test plan
pnpm lint— no new lint errorspnpm type-check:tsgo— no type errorspnpm test— all existing tests pass.match()with global regex (e.g.,block-input/index.tsx:16,mermaid/index.tsx:251)