From ad02dcac5ef38bba052f8ca2f6726903eed6b3a9 Mon Sep 17 00:00:00 2001 From: Piotr Szymanski Date: Thu, 22 Aug 2019 11:31:50 +0200 Subject: [PATCH 1/4] [plsql] Add test case for multiple ddl statements Refs #2009 --- .../plsql/ast/MultipleDDLStatementsTest.java | 26 +++++++++++++++++++ .../pmd/lang/plsql/ast/DDLCommands.sql | 5 ++++ 2 files changed, 31 insertions(+) create mode 100644 pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java create mode 100644 pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/DDLCommands.sql diff --git a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java new file mode 100644 index 0000000000..37bfe03804 --- /dev/null +++ b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java @@ -0,0 +1,26 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.plsql.ast; + +import java.nio.charset.StandardCharsets; +import java.util.List; + +import org.apache.commons.io.IOUtils; +import org.junit.Assert; +import org.junit.Test; + +import net.sourceforge.pmd.lang.plsql.AbstractPLSQLParserTst; + +public class MultipleDDLStatementsTest extends AbstractPLSQLParserTst { + + @Test + public void parseDDLCommands() throws Exception { + String code = IOUtils.toString(this.getClass().getResourceAsStream("DDLCommands.sql"), + StandardCharsets.UTF_8); + ASTInput input = parsePLSQL(code); + List ddlcommands = input.findDescendantsOfType(ASTDDLCommand.class); + Assert.assertEquals(3, ddlcommands.size()); + } +} diff --git a/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/DDLCommands.sql b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/DDLCommands.sql new file mode 100644 index 0000000000..619aa4aa89 --- /dev/null +++ b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/DDLCommands.sql @@ -0,0 +1,5 @@ +COMMENT ON COLUMN employees.job_id IS 'abbreviated job title'; + +COMMENT ON COLUMN employees.job_id IS 'abbreviated job title'; + +COMMENT ON COLUMN employees.job_id IS 'abbreviated job title'; From e230027f9aa2e01819814e8ec79cb08aae5f68dd Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Tue, 24 Dec 2019 11:17:59 +0100 Subject: [PATCH 2/4] [plsql] Support COMMENT statement Ref #2009 --- pmd-plsql/etc/grammar/PldocAST.jjt | 49 ++++++++++++------- .../plsql/ast/MultipleDDLStatementsTest.java | 3 ++ 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/pmd-plsql/etc/grammar/PldocAST.jjt b/pmd-plsql/etc/grammar/PldocAST.jjt index f01524dc45..f256faa56c 100644 --- a/pmd-plsql/etc/grammar/PldocAST.jjt +++ b/pmd-plsql/etc/grammar/PldocAST.jjt @@ -227,6 +227,9 @@ public class PLSQLParser { /** * Semantic lookahead to check if the next identifier is a * specific keyword. + * + *

+ * Usage: LOOKAHEAD({isKeyword("WAIT")}) KEYWORD("WAIT") */ private boolean isKeyword(String keyword) { return getToken(1).kind == IDENTIFIER && getToken(1).image.equalsIgnoreCase(keyword); @@ -259,12 +262,12 @@ ASTInput Input(String sourcecode) : {} | LOOKAHEAD(6) Directory() | LOOKAHEAD(6) DatabaseLink() | LOOKAHEAD(6) Global() - | LOOKAHEAD(6) DDLCommand() //Ignore any other DDL Event + | (LOOKAHEAD(6) DDLCommand())+ | LOOKAHEAD(2) SqlPlusCommand() | UpdateStatement() | DeleteStatement() | InsertStatement() - | SelectStatement() (";")? + | SelectStatement() [";"] |(|||||) SkipPastNextTokenOccurrence(SQLPLUS_TERMINATOR) //Ignore SQL statements in scripts ) ("/")* @@ -279,8 +282,15 @@ ASTDDLCommand DDLCommand() : } { ( - simpleNode = DDLEvent() - SkipPastNextTokenOccurrence(SQLPLUS_TERMINATOR) + ( + LOOKAHEAD({isKeyword("COMMENT")}) + simpleNode = Comment() + ) + | + ( + simpleNode = DDLEvent() + SkipPastNextTokenOccurrence(SQLPLUS_TERMINATOR) + ) ) { jjtThis.setImage(simpleNode.getImage()) ; return jjtThis ; } } @@ -315,7 +325,7 @@ ASTSqlPlusCommand SqlPlusCommand() : | | // DDL that might be encountered - | + | LOOKAHEAD({isKeyword("COMMENT")}) KEYWORD("COMMENT") | | | @@ -3904,19 +3914,22 @@ ASTViewColumn ViewColumn() : { return jjtThis ; } } +/** + * https://docs.oracle.com/en/database/oracle/oracle-database/18/sqlrf/COMMENT.html#GUID-65F447C4-6914-4823-9691-F15D52DB74D7 + */ ASTComment Comment() : { } { - ( + LOOKAHEAD({isKeyword("COMMENT")}) KEYWORD("COMMENT") + ( ((
| | ) [LOOKAHEAD(2) ID()"."] ID()) | ( [LOOKAHEAD(ID()"."ID()"."ID()) ID()"."] ID() "." ID()) ) - - [";"] - { return jjtThis ; } + StringLiteral() + ";" + { return jjtThis ; } } -// SRT * / @@ -4489,23 +4502,26 @@ ASTNonDMLTrigger NonDMLTrigger() : { return jjtThis ; } } - +/** + * https://docs.oracle.com/en/database/oracle/oracle-database/18/sqlrf/Types-of-SQL-Statements.html#GUID-FD9A8CB4-6B9A-44E5-B114-EFB8DA76FC88 + */ ASTDDLEvent DDLEvent(): {} { ( | | | - | + | LOOKAHEAD({isKeyword("COMMENT")}) KEYWORD("COMMENT") | | | + // | | | + // | | | | - | ) { jjtThis.setImage(token.toString()) ; jjtThis.value = token ; return jjtThis ; } } @@ -4741,7 +4757,6 @@ TOKEN [IGNORE_CASE]: | | | - | | | | @@ -4997,7 +5012,6 @@ TOKEN [IGNORE_CASE]: | //11G Trigger Syntax | //11G Trigger Syntax | //11G Trigger Syntax -| //11G Trigger Syntax | //11G Trigger Syntax | //11G Trigger Syntax | //11G Trigger Syntax @@ -5386,7 +5400,6 @@ ASTKEYWORD_UNRESERVED KEYWORD_UNRESERVED (): {} //| //| //| -| | //| //| @@ -5447,7 +5460,6 @@ ASTKEYWORD_UNRESERVED KEYWORD_UNRESERVED (): {} //| //| | -| //| //| | @@ -6352,7 +6364,7 @@ ASTID ID(): {} | KEYWORD_UNRESERVED() //SRT 2011-04-17 /*KEYWORDS_UNRESERVED | | | | | | | | | - | | | | + | | | */ //20120501 | | | @@ -6615,7 +6627,6 @@ ASTQualifiedID QualifiedID(): {} // //| //20120501 | - //| //| // // diff --git a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java index 37bfe03804..91ac15b17e 100644 --- a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java +++ b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java @@ -22,5 +22,8 @@ public class MultipleDDLStatementsTest extends AbstractPLSQLParserTst { ASTInput input = parsePLSQL(code); List ddlcommands = input.findDescendantsOfType(ASTDDLCommand.class); Assert.assertEquals(3, ddlcommands.size()); + List comments = input.findDescendantsOfType(ASTComment.class); + Assert.assertEquals(3, comments.size()); + Assert.assertEquals("'abbreviated job title'", comments.get(0).getFirstChildOfType(ASTStringLiteral.class).getImage()); } } From e345e811bee1c5260490866612af3ab583863911 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Tue, 24 Dec 2019 11:37:41 +0100 Subject: [PATCH 3/4] [plsql] Use ReadPastNextOccurrence rather than SkipPastNextToken The token SQLPLUS_TERMINATOR is useless, since the token ";" is matched before. So, we always skipped till the end of the file. Improved the image of the skipped nodes, so that it can be of some use. Note: This will fix the problem #2009, but it doesn't parse DDL Commands fully. It just skips past the next ";". --- pmd-plsql/etc/grammar/PldocAST.jjt | 16 ++++++++-------- .../plsql/ast/MultipleDDLStatementsTest.java | 2 +- .../pmd/lang/plsql/ast/DDLCommands.sql | 2 ++ 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/pmd-plsql/etc/grammar/PldocAST.jjt b/pmd-plsql/etc/grammar/PldocAST.jjt index f256faa56c..9e492b7d1c 100644 --- a/pmd-plsql/etc/grammar/PldocAST.jjt +++ b/pmd-plsql/etc/grammar/PldocAST.jjt @@ -268,7 +268,7 @@ ASTInput Input(String sourcecode) : {} | DeleteStatement() | InsertStatement() | SelectStatement() [";"] - |(|||
||) SkipPastNextTokenOccurrence(SQLPLUS_TERMINATOR) //Ignore SQL statements in scripts + |(|||
||) ReadPastNextOccurrence(";") //Ignore SQL statements in scripts ) ("/")* )* @@ -289,7 +289,7 @@ ASTDDLCommand DDLCommand() : | ( simpleNode = DDLEvent() - SkipPastNextTokenOccurrence(SQLPLUS_TERMINATOR) + ReadPastNextOccurrence(";") ) ) { jjtThis.setImage(simpleNode.getImage()) ; return jjtThis ; } @@ -1131,6 +1131,7 @@ ASTRead2NextOccurrence Read2NextOccurrence(String target) : { nextToken = getNextToken(); sb.append(nextToken.image); + sb.append(' '); nextToken = getToken(1); } } @@ -1143,10 +1144,11 @@ ASTRead2NextOccurrence Read2NextOccurrence(String target) : */ ASTReadPastNextOccurrence ReadPastNextOccurrence(String target) : { + ASTRead2NextOccurrence skipped = Read2NextOccurrence(target); + StringBuilder sb = new StringBuilder(); - Token t = null; - sb.append(Read2NextOccurrence(target)) ; - t = getNextToken(); // Chomp this one + sb.append(skipped.getImage()) ; + Token t = getNextToken(); // Chomp this one sb.append(t.image); } { @@ -3921,7 +3923,7 @@ ASTComment Comment() : { } { - LOOKAHEAD({isKeyword("COMMENT")}) KEYWORD("COMMENT") + LOOKAHEAD({isKeyword("COMMENT")}) KEYWORD("COMMENT") { jjtThis.setImage(token.image); } ( ((
| | ) [LOOKAHEAD(2) ID()"."] ID()) | ( [LOOKAHEAD(ID()"."ID()"."ID()) ID()"."] ID() "." ID()) @@ -5157,8 +5159,6 @@ TOKEN : < JAVA_INTERFACE_CLASS: ( "SQLData" | "CustomDatum" | "OraData" ) > //| //< #BOOLEAN_LITERAL: "TRUE" | "FALSE" > -| - } /** diff --git a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java index 91ac15b17e..cbd5a74497 100644 --- a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java +++ b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/MultipleDDLStatementsTest.java @@ -21,7 +21,7 @@ public class MultipleDDLStatementsTest extends AbstractPLSQLParserTst { StandardCharsets.UTF_8); ASTInput input = parsePLSQL(code); List ddlcommands = input.findDescendantsOfType(ASTDDLCommand.class); - Assert.assertEquals(3, ddlcommands.size()); + Assert.assertEquals(4, ddlcommands.size()); List comments = input.findDescendantsOfType(ASTComment.class); Assert.assertEquals(3, comments.size()); Assert.assertEquals("'abbreviated job title'", comments.get(0).getFirstChildOfType(ASTStringLiteral.class).getImage()); diff --git a/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/DDLCommands.sql b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/DDLCommands.sql index 619aa4aa89..5833689339 100644 --- a/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/DDLCommands.sql +++ b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/DDLCommands.sql @@ -2,4 +2,6 @@ COMMENT ON COLUMN employees.job_id IS 'abbreviated job title'; COMMENT ON COLUMN employees.job_id IS 'abbreviated job title'; +DROP TABLE employees; + COMMENT ON COLUMN employees.job_id IS 'abbreviated job title'; From 9a82ce14544ce6bc75df4f5eda97d5b1fafc31c6 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Tue, 24 Dec 2019 11:38:35 +0100 Subject: [PATCH 4/4] [doc] Update release notes, fixes #2009 --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 38e0ad5b77..6504119267 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -55,6 +55,8 @@ the implementation based on your feedback. * [#2140](https://github.com/pmd/pmd/issues/2140): \[java] AvoidLiteralsInIfCondition: false negative for expressions * java-performance * [#2141](https://github.com/pmd/pmd/issues/2141): \[java] StringInstatiation: False negative with String-array access +* plsql + * [#2009](https://github.com/pmd/pmd/issues/2009): \[plsql] Multiple DDL commands are skipped during parsing ### API Changes