Skip to content

Commit

Permalink
feat: Review all uses of tokenOriginalValue and change to tokenValue …
Browse files Browse the repository at this point in the history
…when applicable. tokenValue returns the normalized token according to the case-sensitivity of the identifiers.
  • Loading branch information
felipebz committed Aug 13, 2024
1 parent 78635c7 commit bb996a6
Show file tree
Hide file tree
Showing 24 changed files with 57 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import org.sonar.plsqlopen.asTree
import org.sonar.plsqlopen.sslr.RaiseStatement
import org.sonar.plugins.plsqlopen.api.PlSqlGrammar
import org.sonar.plugins.plsqlopen.api.annotations.*
import java.util.*

@Rule(priority = Priority.MAJOR)
@ConstantRemediation("20min")
Expand Down Expand Up @@ -63,9 +62,9 @@ class RaiseStandardExceptionCheck : AbstractBaseCheck() {
val statement = node.asTree<RaiseStatement>()
val exceptionIdentifier = statement.exception
if (exceptionIdentifier != null) {
val exceptionName = exceptionIdentifier.tokenOriginalValue
val exceptionName = exceptionIdentifier.tokenValue

if (standardExceptions.contains(exceptionName.uppercase(Locale.getDefault()))) {
if (standardExceptions.contains(exceptionName)) {
addLineIssue(getLocalizedMessage(), exceptionIdentifier.tokenLine, exceptionName)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class UnhandledUserDefinedExceptionCheck : AbstractBaseCheck() {
val identifier = statement.exception
?: return

val identifierName = identifier.tokenOriginalValue
val identifierName = identifier.tokenValue
val symbols = context.currentScope?.getSymbolsAcessibleInScope(identifierName) ?: ArrayDeque()

if (symbols.isNotEmpty()) {
Expand Down Expand Up @@ -75,7 +75,7 @@ class UnhandledUserDefinedExceptionCheck : AbstractBaseCheck() {
}

for (exceptionName in handler.getChildren(PlSqlGrammar.VARIABLE_NAME)) {
if (exceptionName.tokenOriginalValue.equals(identifierName, ignoreCase = true)) {
if (exceptionName.tokenValue == identifierName) {
return true
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import com.felipebz.flr.api.AstNodeType
import org.sonar.plugins.plsqlopen.api.DmlGrammar
import org.sonar.plugins.plsqlopen.api.PlSqlGrammar
import org.sonar.plugins.plsqlopen.api.annotations.*
import java.util.*

@Rule(priority = Priority.MINOR)
@ConstantRemediation("1min")
Expand Down Expand Up @@ -57,7 +56,7 @@ class UnnecessaryAliasInQueryCheck : AbstractBaseCheck() {


if (table != null) {
tableReferences.getOrPut(table.tokenOriginalValue.lowercase(Locale.getDefault())) { mutableListOf() }
tableReferences.getOrPut(table.tokenValue) { mutableListOf() }
.add(TableReference(table, alias))
}
}
Expand All @@ -73,7 +72,7 @@ class UnnecessaryAliasInQueryCheck : AbstractBaseCheck() {

var alias: String? = null
if (reference.alias != null) {
alias = reference.alias.tokenOriginalValue
alias = reference.alias.tokenValue
}

if (alias != null && reference.alias != null && alias.length < acceptedLength) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class UnusedParameterCheck : AbstractBaseCheck() {

private val ignoreRegex: Pattern? by lazy {
if ("" != ignoreMethods) {
Pattern.compile(ignoreMethods)
Pattern.compile(ignoreMethods, Pattern.CASE_INSENSITIVE)
} else null
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class VariableHidingCheck : AbstractBaseCheck() {

override fun visitNode(node: AstNode) {
val identifier = node.getFirstChild(PlSqlGrammar.IDENTIFIER_NAME)
val name = identifier.tokenOriginalValue
val name = identifier.tokenValue

val scope = identifier.asSemantic().symbol?.scope
if (scope != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
create package pkg is
cursor cur is -- Noncompliant {{Move the body of the cursor "cur" to the package body.}}
cursor cur is -- Noncompliant {{Move the body of the cursor "CUR" to the package body.}}
select dummy from dual;
end;
/
Expand All @@ -12,4 +12,4 @@ create package body pkg is
cursor cur is -- Compliant
select dummy from dual;
end;
/
/
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
declare
e_custom exception;
begin
raise no_data_found; -- Noncompliant {{Avoid raising the standard exception no_data_found.}}
raise too_many_rows; -- Noncompliant {{Avoid raising the standard exception too_many_rows.}}
raise no_data_found; -- Noncompliant {{Avoid raising the standard exception NO_DATA_FOUND.}}
raise too_many_rows; -- Noncompliant {{Avoid raising the standard exception TOO_MANY_ROWS.}}
raise e_custom;
raise;
end;
end;
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ declare
ex exception;
ex_handled exception;
begin
raise ex; -- Noncompliant {{There is no exception handler for "ex" here. This will cause an "user-defined exception" error.}}
raise ex; -- Noncompliant {{There is no exception handler for "EX" here. This will cause an "user-defined exception" error.}}

raise ex_handled; -- ok
raise ex_unknown; -- ok, we don't have any information about the excepton
Expand Down
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
@@ -1,5 +1,5 @@
declare
cursor cur is -- Noncompliant {{Remove this unused "cur" cursor.}}
cursor cur is -- Noncompliant {{Remove this unused "CUR" cursor.}}
select 1 from dual;

cursor cur2 is
Expand All @@ -16,4 +16,4 @@ end;
create package body pkg is
cursor cur return custom_type is -- compliant, let's assume that cursors with return type are declared in package spec
select 1 from dual;
end;
end;
4 changes: 2 additions & 2 deletions zpa-checks/src/test/resources/checks/unused_parameter.sql
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
create procedure foo(a number, b number) is -- Noncompliant {{Remove this unused "b" parameter.}}
create procedure foo(a number, b number) is -- Noncompliant {{Remove this unused "B" parameter.}}
-- ^^^^^^^^
begin
print(a);
Expand All @@ -18,7 +18,7 @@ create package test is

procedure foo(a number, b number) is -- Noncompliant
-- ^^^^^^^^
cursor cur(x number) is -- Noncompliant {{Remove this unused "x" parameter.}}
cursor cur(x number) is -- Noncompliant {{Remove this unused "X" parameter.}}
-- ^^^^^^^^
select 1 from dual;
begin
Expand Down
14 changes: 7 additions & 7 deletions zpa-checks/src/test/resources/checks/unused_variable.sql
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
declare
var number; -- Noncompliant {{Remove this unused "var" local variable.}}
i number; -- Noncompliant {{Remove this unused "i" local variable.}}
var number; -- Noncompliant {{Remove this unused "VAR" local variable.}}
i number; -- Noncompliant {{Remove this unused "I" local variable.}}

procedure proc is
proc_var number; -- Noncompliant {{Remove this unused "proc_var" local variable.}}
proc_var number; -- Noncompliant {{Remove this unused "PROC_VAR" local variable.}}
begin
null;
end;

function func return number is
func_var number; -- Noncompliant {{Remove this unused "func_var" local variable.}}
func_var number; -- Noncompliant {{Remove this unused "FUNC_VAR" local variable.}}
begin
null;
end;
begin
null;

declare
var2 number; -- Noncompliant {{Remove this unused "var2" local variable.}}
var2 number; -- Noncompliant {{Remove this unused "VAR2" local variable.}}
begin
null;
end;
Expand All @@ -29,8 +29,8 @@ 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.}}
package_body_var number; -- Noncompliant {{Remove this unused "PACKAGE_BODY_VAR" local variable.}}
hidden_var number; -- Noncompliant {{Remove this unused "HIDDEN_VAR" local variable.}}

procedure proc is
hidden_var number; -- this declaration hides the previous one
Expand Down
16 changes: 8 additions & 8 deletions zpa-checks/src/test/resources/checks/variable_hiding.sql
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,29 @@ declare
exc exception;

procedure test is
var number; -- Noncompliant {{This variable "var" hides the declaration on line 2.}} [[secondary=2]]
var number; -- Noncompliant {{This variable "VAR" hides the declaration on line 2.}} [[secondary=2]]
-- ^^^
begin
null;
end;

function test return number is
var2 number; -- Noncompliant {{This variable "var2" hides the declaration on line 3.}} [[secondary=3]]
var2 number; -- Noncompliant {{This variable "VAR2" hides the declaration on line 3.}} [[secondary=3]]
-- ^^^^
begin
null;
end;

procedure test2 is
exc exception; -- Noncompliant {{This variable "exc" hides the declaration on line 5.}} [[secondary=5]]
exc exception; -- Noncompliant {{This variable "EXC" hides the declaration on line 5.}} [[secondary=5]]
-- ^^^
begin
null;
end;

begin
declare
var3 number; -- Noncompliant {{This variable "var3" hides the declaration on line 4.}} [[secondary=4]]
var3 number; -- Noncompliant {{This variable "VAR3" hides the declaration on line 4.}} [[secondary=4]]
-- ^^^^
begin
null;
Expand All @@ -39,12 +39,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 39.}} [[secondary=39]]
-- ^^^
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 45.}} [[secondary=45]]
-- ^
begin
null;
Expand All @@ -60,8 +60,8 @@ 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 58.}} [[secondary=58]]
exc exception; -- Noncompliant {{This variable "EXC" hides the declaration on line 59.}} [[secondary=59]]
end;
/

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ open class DefaultTypeSolver {
type = JsonDatatype()
} else {
val datatype = node.firstChild
type = scope?.getSymbol(datatype.tokenOriginalValue, Symbol.Kind.TYPE)?.datatype ?: UnknownDatatype()
type = scope?.getSymbol(datatype.tokenValue, Symbol.Kind.TYPE)?.datatype ?: UnknownDatatype()
}
return type
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class ScopeImpl(override val outer: Scope? = null,

override val identifier: String? =
try {
identifier ?: node?.getFirstChildOrNull(PlSqlGrammar.IDENTIFIER_NAME, PlSqlGrammar.UNIT_NAME)?.tokenOriginalValue
identifier ?: node?.getFirstChildOrNull(PlSqlGrammar.IDENTIFIER_NAME, PlSqlGrammar.UNIT_NAME)?.tokenValue
} catch (e: Exception) {
""
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,17 +222,17 @@ class SymbolVisitor(private val typeSolver: DefaultTypeSolver, private val globa
}

val symbol = createSymbol(identifier, symbolKind, type)
enterScope(node, autonomousTransaction, exceptionHandler, isOverridingMember, identifier.tokenOriginalValue,
enterScope(node, autonomousTransaction, exceptionHandler, isOverridingMember, identifier.tokenValue,
nodeType)
symbol.innerScope = currentScope
}

private fun visitPackage(node: AstNode) {
val identifier = node.getFirstChild(PlSqlGrammar.UNIT_NAME)
val packageScope = symbolTable.scopes.lastOrNull { it.identifier == identifier.tokenOriginalValue && it.type == PlSqlGrammar.CREATE_PACKAGE }
val packageScope = symbolTable.scopes.lastOrNull { it.identifier == identifier.tokenValue && it.type == PlSqlGrammar.CREATE_PACKAGE }
if (packageScope != null) {
currentScope = packageScope
val symbol = currentScope?.getSymbol(identifier.tokenOriginalValue)
val symbol = currentScope?.getSymbol(identifier.tokenValue)
if (symbol != null) {
symbol.addUsage(identifier)
semantic(node).symbol = symbol
Expand Down Expand Up @@ -342,7 +342,7 @@ class SymbolVisitor(private val typeSolver: DefaultTypeSolver, private val globa
private fun visitVariableName(node: AstNode) {
val identifier = node.getFirstChildOrNull(PlSqlGrammar.IDENTIFIER_NAME)
if (identifier != null && currentScope != null) {
val symbol = currentScope?.getSymbol(identifier.tokenOriginalValue)
val symbol = currentScope?.getSymbol(identifier.tokenValue)
if (symbol != null) {
symbol.addUsage(identifier)
semantic(node).symbol = symbol
Expand All @@ -352,11 +352,11 @@ class SymbolVisitor(private val typeSolver: DefaultTypeSolver, private val globa

private fun visitMemberExpression(node: AstNode) {
val parts = node.getChildren(PlSqlGrammar.IDENTIFIER_NAME, PlSqlGrammar.VARIABLE_NAME)
val path = parts.dropLast(1).map { it.tokenOriginalValue }.reversed()
val path = parts.dropLast(1).map { it.tokenValue }.reversed()
val identifier = parts.last()

if (currentScope != null) {
val symbol = currentScope?.getSymbol(identifier.tokenOriginalValue, path)
val symbol = currentScope?.getSymbol(identifier.tokenValue, path)
if (symbol != null) {
symbol.addUsage(identifier)
semantic(node).symbol = symbol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class MethodMatcher private constructor()
val nodes = arrayOfNulls<String>(3)
for (child in node.children) {
if (i < 2 && (child.type === PlSqlGrammar.VARIABLE_NAME || child.type === PlSqlGrammar.IDENTIFIER_NAME)) {
nodes[++i] = child.tokenOriginalValue
nodes[++i] = child.tokenValue
}
}

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

val declaration by lazy { node ?: throw IllegalStateException("Symbol must have a declaration") }

val name: String = node?.tokenOriginalValue ?: name
val name: String = node?.tokenValue ?: name

val type: PlSqlType = datatype?.type ?: PlSqlType.UNKNOWN

Expand All @@ -68,7 +68,7 @@ open class Symbol(val node: AstNode?,

fun hasModifier(modifier: String): Boolean {
for (syntaxToken in modifiers) {
if (syntaxToken.tokenOriginalValue.equals(modifier, ignoreCase = true)) {
if (syntaxToken.tokenValue.equals(modifier, ignoreCase = true)) {
return true
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class AssociativeArrayDatatype(node: AstNode? = null, currentScope: Scope?, val
if (it.identifier != null && it.type == PlSqlGrammar.CREATE_PACKAGE)
it.identifier + "."
else "" } +
node?.getFirstChild(PlSqlGrammar.IDENTIFIER_NAME)?.tokenOriginalValue
node?.getFirstChild(PlSqlGrammar.IDENTIFIER_NAME)?.tokenValue

override fun toString(): String {
return "AssociativeArray{nestedType=$nestedType}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class RecordDatatype(node: AstNode? = null, currentScope: Scope?, val fields: Li
if (it.identifier != null && it.type == PlSqlGrammar.CREATE_PACKAGE)
it.identifier + "."
else "" } +
node?.getFirstChild(PlSqlGrammar.IDENTIFIER_NAME)?.tokenOriginalValue
node?.getFirstChild(PlSqlGrammar.IDENTIFIER_NAME)?.tokenValue

override fun toString(): String {
return "Record"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class DefaultTypeSolverTest {

private fun createSymbol(name: String, kind: Symbol.Kind, type: PlSqlDatatype): Symbol {
val node = mockAstNode()
whenever(node.tokenOriginalValue).thenReturn(name)
whenever(node.tokenValue).thenReturn(name)
return Symbol(node, kind, scope, type)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ package org.sonar.plsqlopen.symbols
import com.felipebz.flr.api.AstNode
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.mockito.Mockito.`when`
import org.mockito.Mockito.mock
import org.mockito.Mockito.`when`
import org.sonar.plugins.plsqlopen.api.symbols.Scope
import org.sonar.plugins.plsqlopen.api.symbols.Symbol
import org.sonar.plugins.plsqlopen.api.symbols.Symbol.Kind
Expand Down Expand Up @@ -132,7 +132,7 @@ class ScopeImplTest {

private fun createSymbol(scope: Scope, name: String, kind: Kind): Symbol {
val node = mockAstNode()
`when`(node.tokenOriginalValue).thenReturn(name)
`when`(node.tokenValue).thenReturn(name)
return Symbol(node, kind, scope, null)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class SymbolTableImplTest {

private fun newAstNodeForTest(value: String): AstNode {
val node = mock(AstNode::class.java)
`when`(node.tokenOriginalValue).thenReturn(value)
`when`(node.tokenValue).thenReturn(value)
return node
}

Expand Down
Loading

0 comments on commit bb996a6

Please sign in to comment.