diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md
index a0f145410e..64bed47ed0 100644
--- a/docs/pages/release_notes.md
+++ b/docs/pages/release_notes.md
@@ -13,16 +13,25 @@ This is a minor release.
### Table Of Contents
* [New and noteworthy](#new-and-noteworthy)
+ * [New Rules](#new-rules)
* [Fixed Issues](#fixed-issues)
* [API Changes](#api-changes)
* [External Contributions](#external-contributions)
### New and noteworthy
+#### New Rules
+
+* The new PL/SQL rule [`ForLoopNaming`](pmd_rules_plsql_codestyle.html#forloopnaming) (`plsql-codestyle`)
+ enforces a naming convention for "for loops". Both "cursor for loops" and "index for loops" are covered.
+ The rule can be customized via patterns. By default, short variable names are reported.
+
### Fixed Issues
* java-errorprone
* [#1078](https://github.com/pmd/pmd/issues/1078): \[java] MissingSerialVersionUID rule does not seem to catch inherited classes
+* plsql
+ * [#681](https://github.com/pmd/pmd/issues/681): \[plsql] Parse error with Cursor For Loop
### API Changes
diff --git a/pmd-core/src/main/resources/rulesets/releases/670.xml b/pmd-core/src/main/resources/rulesets/releases/670.xml
new file mode 100644
index 0000000000..650eb5c783
--- /dev/null
+++ b/pmd-core/src/main/resources/rulesets/releases/670.xml
@@ -0,0 +1,13 @@
+
+
+
+
+This ruleset contains links to rules that are new in PMD v6.7.0
+
+
+
+
+
diff --git a/pmd-plsql/etc/grammar/PldocAST.jjt b/pmd-plsql/etc/grammar/PldocAST.jjt
index 0d162e6bec..f265be2476 100644
--- a/pmd-plsql/etc/grammar/PldocAST.jjt
+++ b/pmd-plsql/etc/grammar/PldocAST.jjt
@@ -28,6 +28,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
/**
* Added more complete support for CREATE TABLE
+ * Added ASTCursorForLoop and ASTSelectStatement
*
* Andreas Dangel 07/2018
*====================================================================
@@ -1176,6 +1177,7 @@ ASTUnlabelledStatement UnlabelledStatement() :
| LOOKAHEAD(3) ContinueStatement() ";" // CONTINUE keyword was added in 11G, so Oracle compilation supports CONTINUE as a variable name
| CaseStatement() ";"
| IfStatement() ";"
+ | LOOKAHEAD( ID() "(" ) CursorForLoopStatement() ";"
| ForStatement() ";"
| ForAllStatement() ";"
| LoopStatement() ";"
@@ -1273,6 +1275,20 @@ ASTLoopStatement LoopStatement() :
{ return jjtThis ; }
}
+ASTCursorForLoopStatement CursorForLoopStatement() :
+{}
+{
+ ForIndex() "(" SelectStatement() ")" (Statement())+ []
+ { return jjtThis ; }
+}
+
+ASTSelectStatement SelectStatement() :
+{}
+{
+ SqlStatement("(", ")")
+ { return jjtThis ; }
+}
+
/** Scope rule: the loop index only exists within the Loop */
ASTForStatement ForStatement() :
{}
@@ -3732,7 +3748,7 @@ TOKEN [IGNORE_CASE]:
*/
TOKEN :
{
-< #GERMAN_SPECIAL_CHARACTERS: "Ä" | "Ö" | "Ü" | "ä" | "ü" | "ö" | "ß" >
+< #GERMAN_SPECIAL_CHARACTERS: "Ä" | "Ö" | "Ü" | "ä" | "ü" | "ö" | "ß" >
|
< #LETTER: ["A"-"Z"] | ["a"-"z"] | >
|
@@ -3745,7 +3761,7 @@ TOKEN :
| "$" | "&" | "_" | "|" | "{" | "}" | "?" | "[" | "]"
| " " | "\t" >
|
-< #SPECIAL_CHARACTERS: "á" | "Ž" | "™" | "š" | "„" | "”" | "ý" | "²" | "€" | "³" | "µ">
+< #SPECIAL_CHARACTERS: "á" | "Ž" | "â„¢" | "Å¡" | "„" | "â€" | "ý" | "²" | "€" | "³" | "µ">
|
< #DELIMITER: "+" | "%" | "'" | "\"" | "." | "/" | "(" | ")" | ":" | "," | "*" | "=" | "<" | ">" | "@" | ";" | "-">
|
diff --git a/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/ast/PLSQLParserVisitorAdapter.java b/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/ast/PLSQLParserVisitorAdapter.java
index 10fc5bc7bb..926da8bd41 100644
--- a/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/ast/PLSQLParserVisitorAdapter.java
+++ b/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/ast/PLSQLParserVisitorAdapter.java
@@ -710,4 +710,14 @@ public class PLSQLParserVisitorAdapter implements PLSQLParserVisitor {
public Object visit(ASTReferencesClause node, Object data) {
return visit((PLSQLNode) node, data);
}
+
+ @Override
+ public Object visit(ASTCursorForLoopStatement node, Object data) {
+ return visit((PLSQLNode) node, data);
+ }
+
+ @Override
+ public Object visit(ASTSelectStatement node, Object data) {
+ return visit((PLSQLNode) node, data);
+ }
}
diff --git a/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/rule/AbstractPLSQLRule.java b/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/rule/AbstractPLSQLRule.java
index afb34fc430..d97ecd8a9e 100644
--- a/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/rule/AbstractPLSQLRule.java
+++ b/pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/rule/AbstractPLSQLRule.java
@@ -805,6 +805,16 @@ public abstract class AbstractPLSQLRule extends AbstractRule implements PLSQLPar
return visit((PLSQLNode) node, data);
}
+ @Override
+ public Object visit(ASTCursorForLoopStatement node, Object data) {
+ return visit((PLSQLNode) node, data);
+ }
+
+ @Override
+ public Object visit(ASTSelectStatement node, Object data) {
+ return visit((PLSQLNode) node, data);
+ }
+
/*
* Treat all Executable Code
*/
diff --git a/pmd-plsql/src/main/resources/category/plsql/codestyle.xml b/pmd-plsql/src/main/resources/category/plsql/codestyle.xml
index db3b49a7c6..afac84faff 100644
--- a/pmd-plsql/src/main/resources/category/plsql/codestyle.xml
+++ b/pmd-plsql/src/main/resources/category/plsql/codestyle.xml
@@ -52,4 +52,64 @@ end inline_pragma_error;
+
+
+In case you have loops please name the loop variables more meaningful.
+
+ 3
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/AbstractPLSQLParserTst.java b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/AbstractPLSQLParserTst.java
index b9d737e89a..99531494f2 100644
--- a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/AbstractPLSQLParserTst.java
+++ b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/AbstractPLSQLParserTst.java
@@ -4,6 +4,7 @@
package net.sourceforge.pmd.lang.plsql;
+import java.io.IOException;
import java.io.StringReader;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.InvocationTargetException;
@@ -15,6 +16,8 @@ import java.util.HashSet;
import java.util.List;
import java.util.Set;
+import org.apache.commons.io.IOUtils;
+
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.LanguageVersion;
import net.sourceforge.pmd.lang.LanguageVersionHandler;
@@ -127,4 +130,12 @@ public abstract class AbstractPLSQLParserTst {
return languageVersionHandler.getParser(languageVersionHandler.getDefaultParserOptions()).parse(null,
new StringReader(code));
}
+
+ public String loadTestResource(String name) {
+ try {
+ return IOUtils.toString(this.getClass().getResourceAsStream(name));
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
}
diff --git a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopTest.java b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopTest.java
new file mode 100644
index 0000000000..e3d0a77fc5
--- /dev/null
+++ b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopTest.java
@@ -0,0 +1,64 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.plsql.ast;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import net.sourceforge.pmd.lang.plsql.AbstractPLSQLParserTst;
+
+public class CursorForLoopTest extends AbstractPLSQLParserTst {
+
+ @Test
+ public void parseCursorForLoopSimple() {
+ String code = loadTestResource("CursorForLoopSimple.pls");
+ ASTInput input = parsePLSQL(code);
+ ASTCursorForLoopStatement forloop = input.getFirstDescendantOfType(ASTCursorForLoopStatement.class);
+ Assert.assertNotNull(forloop);
+ ASTForIndex forindex = forloop.getFirstChildOfType(ASTForIndex.class);
+ Assert.assertNotNull(forindex);
+ Assert.assertEquals("someone", forindex.getImage());
+ }
+
+ @Test
+ public void parseCursorForLoopNested() {
+ String code = loadTestResource("CursorForLoopNested.pls");
+ ASTInput input = parsePLSQL(code);
+ ASTCursorForLoopStatement forloop = input.getFirstDescendantOfType(ASTCursorForLoopStatement.class);
+ Assert.assertNotNull(forloop);
+ ASTForIndex forindex = forloop.getFirstChildOfType(ASTForIndex.class);
+ Assert.assertNotNull(forindex);
+ Assert.assertEquals("c_cmp", forindex.getImage());
+
+ ASTCursorForLoopStatement forloop2 = forloop.getFirstDescendantOfType(ASTCursorForLoopStatement.class);
+ ASTForIndex forindex2 = forloop2.getFirstChildOfType(ASTForIndex.class);
+ Assert.assertEquals("c_con", forindex2.getImage());
+
+ ASTCursorForLoopStatement forloop3 = forloop2.getFirstDescendantOfType(ASTCursorForLoopStatement.class);
+ ASTForIndex forindex3 = forloop3.getFirstChildOfType(ASTForIndex.class);
+ Assert.assertEquals("c_pa", forindex3.getImage());
+ }
+
+ @Test
+ public void parseCursorForLoop1047a() {
+ String code = loadTestResource("CursorForLoop1047a.pls");
+ ASTInput input = parsePLSQL(code);
+ Assert.assertNotNull(input);
+ }
+
+ @Test
+ public void parseCursorForLoop1047b() {
+ String code = loadTestResource("CursorForLoop1047b.pls");
+ ASTInput input = parsePLSQL(code);
+ Assert.assertNotNull(input);
+ }
+
+ @Test
+ public void parseCursorForLoop681() {
+ String code = loadTestResource("CursorForLoop681.pls");
+ ASTInput input = parsePLSQL(code);
+ Assert.assertNotNull(input);
+ }
+}
diff --git a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/rule/codestyle/CodeStyleRulesTest.java b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/rule/codestyle/CodeStyleRulesTest.java
index 47e294b3d7..a1cf646748 100644
--- a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/rule/codestyle/CodeStyleRulesTest.java
+++ b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/rule/codestyle/CodeStyleRulesTest.java
@@ -13,5 +13,6 @@ public class CodeStyleRulesTest extends SimpleAggregatorTst {
@Override
public void setUp() {
addRule(RULESET, "MisplacedPragma");
+ addRule(RULESET, "ForLoopNaming");
}
}
diff --git a/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop1047a.pls b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop1047a.pls
new file mode 100644
index 0000000000..04a27665e4
--- /dev/null
+++ b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop1047a.pls
@@ -0,0 +1,16 @@
+--
+-- BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+--
+
+CREATE OR REPLACE PROCEDURE test2 ( a OUT number, b OUT number )
+ AS
+ BEGIN
+ FOR registro IN ( SELECT SUM(field1),
+ MAX(field2)
+ INTO a,b
+ FROM test_tbl
+ GROUP BY fieldx)
+ LOOP
+ null;
+ END LOOP;
+END test2;
diff --git a/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop1047b.pls b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop1047b.pls
new file mode 100644
index 0000000000..009e39cff9
--- /dev/null
+++ b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop1047b.pls
@@ -0,0 +1,13 @@
+--
+-- BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+--
+
+-- this one was processed without errors
+CREATE OR REPLACE PROCEDURE test2 ( a OUT number, b OUT number )
+ AS
+BEGIN
+ FOR registro IN ( SELECT SUM(field1),MAX(field2) INTO a,b FROM test_tbl GROUP BY fieldx )
+ LOOP
+ null;
+ END LOOP;
+END test2;
diff --git a/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop681.pls b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop681.pls
new file mode 100644
index 0000000000..c98deddc3a
--- /dev/null
+++ b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop681.pls
@@ -0,0 +1,20 @@
+--
+-- BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+--
+
+BEGIN
+ FOR sum_res IN (SELECT SUM(loss_reserve) loss_reserve,
+ SUM(expense_reserve) exp_reserve
+ FROM gicl_clm_reserve a, gicl_item_peril b
+ WHERE a.claim_id = b.claim_id
+ AND a.item_no = b.item_no
+ AND a.peril_cd = b.peril_cd
+ AND a.claim_id = p_claim_id
+ AND (NVL(b.close_flag, 'AP') IN ('AP','CC','CP') OR
+ NVL(b.close_flag2, 'AP') IN ('AP','CC','CP')))
+ LOOP
+ v_loss_res_amt := sum_res.loss_reserve;
+ v_exp_res_amt := sum_res.exp_reserve;
+ END LOOP;
+END;
+/
diff --git a/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopNested.pls b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopNested.pls
new file mode 100644
index 0000000000..8eccf7a602
--- /dev/null
+++ b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopNested.pls
@@ -0,0 +1,14 @@
+--
+-- BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+--
+
+BEGIN
+FOR c_cmp IN (SELECT * FROM companies) LOOP
+ FOR c_con IN (SELECT * FROM contacts) LOOP
+ FOR c_pa IN (SELECT * FROM parties) LOOP
+ NULL;
+ END LOOP;
+ END LOOP;
+END LOOP;
+END;
+/
diff --git a/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopSimple.pls b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopSimple.pls
new file mode 100644
index 0000000000..9c43ab8f7d
--- /dev/null
+++ b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopSimple.pls
@@ -0,0 +1,16 @@
+--
+-- BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+--
+
+BEGIN
+FOR someone IN (
+SELECT * FROM employees
+WHERE employee_id < 120
+ORDER BY employee_id
+)
+LOOP
+DBMS_OUTPUT.PUT_LINE('First name = ' || someone.first_name ||
+', Last name = ' || someone.last_name);
+END LOOP;
+END;
+/
diff --git a/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/rule/codestyle/xml/ForLoopNaming.xml b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/rule/codestyle/xml/ForLoopNaming.xml
new file mode 100644
index 0000000000..5d6bdf5df1
--- /dev/null
+++ b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/rule/codestyle/xml/ForLoopNaming.xml
@@ -0,0 +1,211 @@
+
+
+
+
+ CursorForLoop Simple case - no nesting
+ 1
+
+ plsql
+
+
+
+ CursorForLoop Simple case - no nesting - allowSimpleLoops
+ true
+ 0
+
+ plsql
+
+
+
+ CursorForLoop Nested loops - good example, default pattern
+ 0
+
+ plsql
+
+
+
+ CursorForLoop Nested loops - good example, custom pattern
+ c_[a-z]+
+ 0
+
+ plsql
+
+
+
+ CursorForLoop Nested loops - bad example, default pattern
+ 3
+ 2,3,4
+
+ plsql
+
+
+
+ CursorForLoop Nested loops - bad example, custom pattern
+ c_[a-z]+
+ 3
+ 2,3,4
+
+ plsql
+
+
+
+ ForLoop Simple case - no nesting
+ 1
+ 2
+
+
+
+
+ ForLoop Simple case - no nesting - allowSimpleLoops
+ true
+ 0
+
+
+
+
+ ForLoop Nested loops - good example, default pattern
+ 0
+
+
+
+
+ ForLoop Nested loops - good example, custom pattern
+ i_[a-z]+
+ 0
+
+
+
+
+ ForLoop Nested Loops - bad example, default pattern
+ 3
+ 2,3,4
+
+
+
+
+ ForLoop Nested Loops - bad example, custom pattern
+ i_[a-z]+
+ 3
+ 2,3,4
+
+
+
\ No newline at end of file