Skip to content

Commit

Permalink
fix: Add a special channel to handle quoted identifiers and fix false…
Browse files Browse the repository at this point in the history
… positives related to them on rules UnusedVariable, UnusedCursor, UnusedParameter and VariableHiding

Fixes #186
  • Loading branch information
felipebz committed Aug 13, 2024
1 parent bb996a6 commit 7340759
Show file tree
Hide file tree
Showing 12 changed files with 183 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
5
],
"lnpls/plsql-language-fundamentals-12.sql" : [
3
4
],
"lnpls/plsql-language-fundamentals-26.sql" : [
3,
Expand Down Expand Up @@ -82,9 +82,6 @@
4,
8
],
"lnpls/plsql-language-fundamentals-4.sql" : [
3
],
"lnpls/plsql-language-fundamentals-43.sql" : [
3,
4,
Expand All @@ -101,9 +98,8 @@
"lnpls/plsql-language-fundamentals-52.sql" : [
4
],
"lnpls/plsql-language-fundamentals-8.sql" : [
4,
5
"lnpls/plsql-language-fundamentals-6.sql" : [
3
],
"lnpls/plsql-optimization-and-tuning-106.sql" : [
20,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,5 @@
],
"lnpls/plsql-language-fundamentals-52.sql" : [
7
],
"lnpls/plsql-language-fundamentals-8.sql" : [
3,
4
]
}
4 changes: 2 additions & 2 deletions zpa-checks/src/test/resources/checks/unused_cursor.sql
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ declare
cursor cur is -- Noncompliant {{Remove this unused "CUR" cursor.}}
select 1 from dual;

cursor cur2 is
cursor "cur2" is
select 1 from dual;
begin
open cur2;
open "cur2";
end;

create package pkg is
Expand Down
3 changes: 2 additions & 1 deletion zpa-checks/src/test/resources/checks/unused_parameter.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
create procedure foo(a number, b number) is -- Noncompliant {{Remove this unused "B" parameter.}}
create procedure foo(a number, b number, "c" number) is -- Noncompliant {{Remove this unused "B" parameter.}}
-- ^^^^^^^^
begin
print(a);
print("c");
end;
/

Expand Down
5 changes: 5 additions & 0 deletions zpa-checks/src/test/resources/checks/unused_variable.sql
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,16 @@ end;
create or replace package body test is
package_body_var number; -- Noncompliant {{Remove this unused "PACKAGE_BODY_VAR" local variable.}}
hidden_var number; -- Noncompliant {{Remove this unused "HIDDEN_VAR" local variable.}}
"VAR" number; -- Noncompliant {{Remove this unused "VAR" local variable.}}
"var" number;

procedure proc is
hidden_var number; -- this declaration hides the previous one
var number; -- this declaration hides the previous "VAR"
begin
hidden_var := 0;
var := 0;
"var" := 0;
end;
end;
/
Expand Down
16 changes: 11 additions & 5 deletions zpa-checks/src/test/resources/checks/variable_hiding.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ declare
var2 number;
var3 number;
exc exception;
"QUOTEDVAR" number;

procedure test is
var number; -- Noncompliant {{This variable "VAR" hides the declaration on line 2.}} [[secondary=2]]
Expand All @@ -29,6 +30,8 @@ begin
declare
var3 number; -- Noncompliant {{This variable "VAR3" hides the declaration on line 4.}} [[secondary=4]]
-- ^^^^
quotedvar number; -- Noncompliant {{This variable "QUOTEDVAR" hides the declaration on line 6.}} [[secondary=6]]
"QuotedVar" number; -- this is a different variable
begin
null;
end;
Expand All @@ -39,12 +42,12 @@ create package body test is
var number;

procedure test is
var number; -- Noncompliant {{This variable "VAR" hides the declaration on line 39.}} [[secondary=39]]
var number; -- Noncompliant {{This variable "VAR" hides the declaration on line 42.}} [[secondary=42]]
-- ^^^
begin
for i in 1..10 loop
declare
i number; -- Noncompliant {{This variable "I" hides the declaration on line 45.}} [[secondary=45]]
i number; -- Noncompliant {{This variable "I" hides the declaration on line 48.}} [[secondary=48]]
-- ^
begin
null;
Expand All @@ -60,14 +63,17 @@ create package test2 is
end;
/
create package body test2 is
var number; -- Noncompliant {{This variable "VAR" hides the declaration on line 58.}} [[secondary=58]]
exc exception; -- Noncompliant {{This variable "EXC" hides the declaration on line 59.}} [[secondary=59]]
var number; -- Noncompliant {{This variable "VAR" hides the declaration on line 61.}} [[secondary=61]]
exc exception; -- Noncompliant {{This variable "EXC" hides the declaration on line 62.}} [[secondary=62]]
end;
/

-- correct
declare
outer_var number;
outer_var number;
"VAR2" number; --|
"Var2" number; --| These variables are not the same
"var2" number; --|
begin
declare
var number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class IdentifierChannel(private val channel: IdentifierAndKeywordChannel)

override fun consume(code: CodeReader, output: LexerOutput): Boolean {
val nextChar = code.peek().toChar()
if (!nextChar.isLetter() && nextChar != '"') {
if (!nextChar.isLetter()) {
return false
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,10 @@ object PlSqlLexer {
.withChannel(StringChannel(regexp(PlSqlTokenType.STRING_LITERAL, STRING_LITERAL)))
.withChannel(DateChannel(regexp(PlSqlTokenType.DATE_LITERAL, DATE_LITERAL)))
.withChannel(DateChannel(regexp(PlSqlTokenType.TIMESTAMP_LITERAL, TIMESTAMP_LITERAL)))
.withChannel(IdentifierChannel(IdentifierAndKeywordChannel(or(SIMPLE_IDENTIFIER, QUOTED_IDENTIFIER), false,
.withChannel(IdentifierChannel(IdentifierAndKeywordChannel(SIMPLE_IDENTIFIER, false,
PlSqlKeyword.entries.toTypedArray()
)))
.withChannel(QuotedIdentifierChannel(QUOTED_IDENTIFIER, SIMPLE_IDENTIFIER))
.withChannel(PunctuatorChannel(*PlSqlPunctuator.entries.toTypedArray()))
.withChannel(BlackHoleChannel("(?is)" + or(
"\\s&&?$SIMPLE_IDENTIFIER",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* Z PL/SQL Analyzer
* Copyright (C) 2015-2024 Felipe Zorzo
* mailto:felipe AT felipezorzo DOT com DOT br
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.plsqlopen.lexer

import com.felipebz.flr.api.GenericTokenType
import com.felipebz.flr.api.Token
import com.felipebz.flr.channel.Channel
import com.felipebz.flr.channel.CodeReader
import com.felipebz.flr.impl.LexerOutput
import java.util.regex.Pattern


class QuotedIdentifierChannel(quotedIdentifierRegexp: String, simpleIdentifierRegexp: String) : Channel<LexerOutput> {

private val quotedPattern = Pattern.compile(quotedIdentifierRegexp)
private val quotedSimplePattern = Pattern.compile(""""$simpleIdentifierRegexp"""")

override fun consume(code: CodeReader, output: LexerOutput): Boolean {
val nextChar = code.peek().toChar()
if (nextChar != '"') {
return false
}

val tmpBuilder = StringBuilder()
val matcher = quotedPattern.matcher("")

if (code.popTo(matcher, tmpBuilder) > 0) {
var word = tmpBuilder.toString()
val wordOriginal = word
if (quotedSimplePattern.matcher(word).matches() && word == word.uppercase()) {
word = word.substring(1, word.length - 1)
}

val token = Token.builder()
.setType(GenericTokenType.IDENTIFIER)
.setValueAndOriginalValue(word, wordOriginal)
.setLine(code.previousCursor.line)
.setColumn(code.previousCursor.column)
.build()
output.addToken(token)
return true
}

return false
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,11 @@ open class Symbol(val node: AstNode?,

fun `is`(kind: Kind) = kind == this.kind

fun called(name: String) = name.equals(this.name, ignoreCase = true)
fun called(name: String) = if (name.startsWith('"')) {
name == this.name
} else {
name.equals(this.name, ignoreCase = true)
}

override fun toString() = "Symbol name=$name kind=$kind datatype=$datatype"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Z PL/SQL Analyzer
* Copyright (C) 2015-2024 Felipe Zorzo
* mailto:felipe AT felipezorzo DOT com DOT br
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.plsqlopen.lexer

import com.felipebz.flr.api.GenericTokenType
import com.felipebz.flr.impl.LexerOutput
import com.felipebz.flr.tests.Assertions.assertThat
import org.junit.jupiter.api.Test

class QuotedIdentifierChannelTest {
private val channel = QuotedIdentifierChannel("\".*?\"", "[A-Z]+")
private val output = LexerOutput()

@Test
fun doesNotConsumeUnquotedWord() {
assertThat(channel).doesNotConsume("word", output)
}

@Test
fun consumeQuotedWord() {
assertThat(channel).consume("\"some word\"", output)
assertThat(output.tokens).hasToken("\"some word\"", GenericTokenType.IDENTIFIER)
}

@Test
fun consumeQuotedWordWithSimpleIdentifier() {
assertThat(channel).consume("\"WORD\"", output)
assertThat(output.tokens).hasToken("WORD", GenericTokenType.IDENTIFIER)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,47 @@ end;
assertThat(x.innerScope).isNull()
}

@Test
fun symbolsWithQuotedIdentifiers() {
val symbols = scan(
"""
declare
"VAR" number;
"Var" number;
"var" number;
begin
var := 1;
"VAR" := 1;
"Var" := 1;
"var" := 1;
end;
"""
)
assertThat(symbols).hasSize(3)

val var1 = symbols.find("var", 2, 3)
assertThat(var1.type).isEqualTo(PlSqlType.NUMERIC)
assertThat(var1.references).containsExactly(
tuple(6, 3),
tuple(7, 3),
)
assertThat(var1.innerScope).isNull()

val var2 = symbols.find("\"Var\"", 3, 3)
assertThat(var2.type).isEqualTo(PlSqlType.NUMERIC)
assertThat(var2.references).containsExactly(
tuple(8, 3),
)
assertThat(var2.innerScope).isNull()

val var3 = symbols.find("\"var\"", 4, 3)
assertThat(var3.type).isEqualTo(PlSqlType.NUMERIC)
assertThat(var3.references).containsExactly(
tuple(9, 3),
)
assertThat(var3.innerScope).isNull()
}

private fun scan(contents: String): List<Symbol> {
val file = tempFolder.resolve("test.sql")
file.writeText(contents.trim())
Expand Down

0 comments on commit 7340759

Please sign in to comment.