Skip to content

Fix codacy.generic.sql.lookup type without language or apps fnd#23

Merged
DMarinhoCodacy merged 5 commits into
mainfrom
fix-codacy.generic.sql.lookup-type-without-language-or-apps-fnd
May 26, 2026
Merged

Fix codacy.generic.sql.lookup type without language or apps fnd#23
DMarinhoCodacy merged 5 commits into
mainfrom
fix-codacy.generic.sql.lookup-type-without-language-or-apps-fnd

Conversation

@DMarinhoCodacy
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Codacy rules in docs/codacy-rules.yaml by replacing the previous regex patterns for SQL files with a pattern-either block containing more complex regular expressions. A critical catastrophic backtracking (ReDoS) vulnerability was identified in one of the new regex patterns, which could cause the analysis process to hang on large SQL files. A fix was suggested to exclude newlines from the character class.

Comment thread docs/codacy-rules.yaml Outdated
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented May 26, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

Copy link
Copy Markdown

@codacy-production codacy-production Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

While this PR aims to refine SQL lookup rules, there are critical gaps between the implementation and the intended acceptance criteria that should prevent merging in its current state. Specifically, the regex for lookup_type fails to exclude SQL comments, which will lead to false positives. Conversely, the pattern for apps.fnd_lookup_values is overly restrictive and will fail to detect valid code when SQL hints or block comments (e.g., /*+ hint */) precede the keyword.

Additionally, there is a systemic risk regarding the use of the \K escape sequence; this requires a PCRE-compatible engine. If the execution environment does not support PCRE, these rules will fail to execute entirely. No sample SQL files or test cases were provided to verify these patterns against the stated requirements.

About this PR

  • The use of \K (Reset Start of Match) requires a PCRE-compatible regex engine. Please verify that the environment where these rules are executed supports PCRE, otherwise the rules may fail to function or throw errors.
  • The PR lacks sample SQL files or test cases. Given the complexity of the regex patterns being introduced to handle block delimiters and comments, including valid and invalid SQL samples is necessary to ensure the rules behave as expected.

Test suggestions

  • Missing recommended test scenario: SQL statement containing 'lookup_type' without a 'language' clause should trigger a violation.
  • Missing recommended test scenario: SQL statement containing 'lookup_type' with a 'language' clause should not trigger a violation.
  • Missing recommended test scenario: Any usage of 'apps.fnd_lookup_values' should trigger a violation.
  • Missing recommended test scenario: Keywords located within '--' or '/* */' comments should be ignored.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Missing recommended test scenario: SQL statement containing 'lookup_type' without a 'language' clause should trigger a violation.
2. Missing recommended test scenario: SQL statement containing 'lookup_type' with a 'language' clause should not trigger a violation.
3. Missing recommended test scenario: Any usage of 'apps.fnd_lookup_values' should trigger a violation.
4. Missing recommended test scenario: Keywords located within '--' or '/* */' comments should be ignored.

TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback

Comment thread docs/codacy-rules.yaml
Comment thread docs/codacy-rules.yaml Outdated
@DMarinhoCodacy DMarinhoCodacy merged commit c2674fa into main May 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants