fix(grammar): Don't consider SELECT_EXPRESSION as a valid expression …

…for all cases, this is necessary to avoid parsing errors like '1 + select 1 from dual' that was being considered valid
felipebz · Mar 10, 2024 · 7c1ee44 · 7c1ee44
1 parent 5df2de6
commit 7c1ee44
Show file tree

Hide file tree

Showing 12 changed files with 11 additions and 52 deletions.
diff --git a/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/EmptyBlockCheck.json b/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/EmptyBlockCheck.json
@@ -54,11 +54,6 @@
     37,
     43
   ],
-  "source/ut_plsql.pkb": [
-    1214,
-    1224,
-    1233
-  ],
   "source/ut_result2.pkb": [
     37
   ],

diff --git a/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/InequalityUsageCheck.json b/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/InequalityUsageCheck.json
@@ -9,10 +9,6 @@
   "source/ut_pipe.pkb": [
     52
   ],
-  "source/ut_plsql.pkb": [
-    489,
-    495
-  ],
   "source/ut_plsql_util.pkb": [
     1223,
     1232,

diff --git a/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/ParsingErrorCheck.json b/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/ParsingErrorCheck.json
@@ -29,6 +29,9 @@
   "source/ut_package.tab": [
     21
   ],
+  "source/ut_plsql.pkb": [
+    778
+  ],
   "source/ut_receq.tab": [
     12
   ],

diff --git a/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/SameBranchCheck.json b/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/SameBranchCheck.json
@@ -2,9 +2,6 @@
   "source/ut_outputreporter.pkb": [
     90
   ],
-  "source/ut_plsql.pkb": [
-    295
-  ],
   "source/ut_plsql_util.pkb": [
     233
   ]

diff --git a/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/SelectAllColumnsCheck.json b/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/SelectAllColumnsCheck.json
@@ -9,9 +9,6 @@
   "examples/ut_te_employee.pkb": [
     179
   ],
-  "source/ut_plsql.pkb": [
-    1140
-  ],
   "source/ut_plsql2.pkb": [
     313,
     413

diff --git a/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/UnnecessaryElseCheck.json b/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/UnnecessaryElseCheck.json
@@ -16,14 +16,6 @@
   "source/ut_outputreporter.pkb": [
     123
   ],
-  "source/ut_plsql.pkb": [
-    124,
-    204,
-    1314,
-    1330,
-    1346,
-    1362
-  ],
   "source/ut_plsql_util.pkb": [
     1130,
     1149

diff --git a/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/UnusedParameterCheck.json b/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/UnusedParameterCheck.json
@@ -50,17 +50,6 @@
     265,
     281
   ],
-  "source/ut_plsql.pkb": [
-    645,
-    722,
-    723,
-    1211,
-    1218,
-    1219,
-    1220,
-    1228,
-    1229
-  ],
   "source/ut_plsql2.pkb": [
     151,
     308

diff --git a/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/UnusedVariableCheck.json b/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/UnusedVariableCheck.json
@@ -54,10 +54,6 @@
   "source/ut_filereporter.pkb": [
     114
   ],
-  "source/ut_plsql.pkb": [
-    882,
-    1269
-  ],
   "source/ut_plsql2.pkb": [
     155,
     405,

diff --git a/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/VariableHidingCheck.json b/zpa-checks/src/integrationTest/resources/expected/utPLSQL2/VariableHidingCheck.json
@@ -1,7 +1,4 @@
 {
-  "source/ut_plsql.pkb": [
-    1279
-  ],
   "source/ut_plsql.pks": [
     44
   ]

diff --git a/...egrationTest/resources/expected/utPLSQL2/VariableInitializationWithFunctionCallCheck.json b/...egrationTest/resources/expected/utPLSQL2/VariableInitializationWithFunctionCallCheck.json
@@ -34,15 +34,6 @@
     135,
     139
   ],
-  "source/ut_plsql.pkb": [
-    356,
-    521,
-    730,
-    926,
-    929,
-    931,
-    1122
-  ],
   "source/ut_plsql2.pkb": [
     248,
     266

diff --git a/zpa-core/src/main/kotlin/org/sonar/plugins/plsqlopen/api/PlSqlGrammar.kt b/zpa-core/src/main/kotlin/org/sonar/plugins/plsqlopen/api/PlSqlGrammar.kt
@@ -604,7 +604,7 @@ enum class PlSqlGrammar : GrammarRuleKey {
 
             b.rule(OPEN_FOR_STATEMENT).define(
                     b.optional(LABEL),
-                    OPEN, MEMBER_EXPRESSION, FOR, EXPRESSION,
+                    OPEN, MEMBER_EXPRESSION, FOR, b.firstOf(SELECT_EXPRESSION, EXPRESSION),
                     b.optional(USING, UNNAMED_ACTUAL_PAMETER, b.zeroOrMore(COMMA, UNNAMED_ACTUAL_PAMETER)),
                     SEMICOLON)
 
@@ -818,7 +818,7 @@ enum class PlSqlGrammar : GrammarRuleKey {
                     NEW_OBJECT_EXPRESSION,
                     CASE_EXPRESSION,
                     IN_EXPRESSION,
-                    SELECT_EXPRESSION),
+                    b.sequence(LPARENTHESIS, SELECT_EXPRESSION, RPARENTHESIS)),
                     b.optional(AT_TIME_ZONE_EXPRESSION)).skipIfOneChild()
 
             b.rule(EXPONENTIATION_EXPRESSION).define(UNARY_EXPRESSION, b.zeroOrMore(EXPONENTIATION, UNARY_EXPRESSION)).skipIfOneChild()

diff --git a/...core/src/test/kotlin/org/sonar/plugins/plsqlopen/api/expressions/NumericExpressionTest.kt b/...core/src/test/kotlin/org/sonar/plugins/plsqlopen/api/expressions/NumericExpressionTest.kt
@@ -144,4 +144,10 @@ class NumericExpressionTest : RuleTest() {
         assertThat(p).matches("collection.prior(1) + 1")
     }
 
+    @Test
+    fun notMatchesQueries() {
+        assertThat(p).notMatches("1 + select 1 from dual")
+        assertThat(p).notMatches("(select 1 from dual) / select 1 from dual")
+    }
+
 }